]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
serial: mctrl_gpio: split disable_ms into sync and no_sync APIs
authorAlexis Lothoré <alexis.lothore@bootlin.com>
Mon, 17 Feb 2025 06:21:53 +0000 (07:21 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 19 Feb 2025 14:08:36 +0000 (15:08 +0100)
The following splat has been observed on a SAMA5D27 platform using
atmel_serial:

BUG: sleeping function called from invalid context at kernel/irq/manage.c:738
in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 27, name: kworker/u5:0
preempt_count: 1, expected: 0
INFO: lockdep is turned off.
irq event stamp: 0
hardirqs last  enabled at (0): [<00000000>] 0x0
hardirqs last disabled at (0): [<c01588f0>] copy_process+0x1c4c/0x7bec
softirqs last  enabled at (0): [<c0158944>] copy_process+0x1ca0/0x7bec
softirqs last disabled at (0): [<00000000>] 0x0
CPU: 0 UID: 0 PID: 27 Comm: kworker/u5:0 Not tainted 6.13.0-rc7+ #74
Hardware name: Atmel SAMA5
Workqueue: hci0 hci_power_on [bluetooth]
Call trace:
  unwind_backtrace from show_stack+0x18/0x1c
  show_stack from dump_stack_lvl+0x44/0x70
  dump_stack_lvl from __might_resched+0x38c/0x598
  __might_resched from disable_irq+0x1c/0x48
  disable_irq from mctrl_gpio_disable_ms+0x74/0xc0
  mctrl_gpio_disable_ms from atmel_disable_ms.part.0+0x80/0x1f4
  atmel_disable_ms.part.0 from atmel_set_termios+0x764/0x11e8
  atmel_set_termios from uart_change_line_settings+0x15c/0x994
  uart_change_line_settings from uart_set_termios+0x2b0/0x668
  uart_set_termios from tty_set_termios+0x600/0x8ec
  tty_set_termios from ttyport_set_flow_control+0x188/0x1e0
  ttyport_set_flow_control from wilc_setup+0xd0/0x524 [hci_wilc]
  wilc_setup [hci_wilc] from hci_dev_open_sync+0x330/0x203c [bluetooth]
  hci_dev_open_sync [bluetooth] from hci_dev_do_open+0x40/0xb0 [bluetooth]
  hci_dev_do_open [bluetooth] from hci_power_on+0x12c/0x664 [bluetooth]
  hci_power_on [bluetooth] from process_one_work+0x998/0x1a38
  process_one_work from worker_thread+0x6e0/0xfb4
  worker_thread from kthread+0x3d4/0x484
  kthread from ret_from_fork+0x14/0x28

This warning is emitted when trying to toggle, at the highest level,
some flow control (with serdev_device_set_flow_control) in a device
driver. At the lowest level, the atmel_serial driver is using
serial_mctrl_gpio lib to enable/disable the corresponding IRQs
accordingly.  The warning emitted by CONFIG_DEBUG_ATOMIC_SLEEP is due to
disable_irq (called in mctrl_gpio_disable_ms) being possibly called in
some atomic context (some tty drivers perform modem lines configuration
in regions protected by port lock).

Split mctrl_gpio_disable_ms into two differents APIs, a non-blocking one
and a blocking one. Replace mctrl_gpio_disable_ms calls with the
relevant version depending on whether the call is protected by some port
lock.

Suggested-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
Acked-by: Richard Genoud <richard.genoud@bootlin.com>
Link: https://lore.kernel.org/r/20250217-atomic_sleep_mctrl_serial_gpio-v3-1-59324b313eef@bootlin.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Documentation/driver-api/serial/driver.rst
drivers/tty/serial/8250/8250_port.c
drivers/tty/serial/atmel_serial.c
drivers/tty/serial/imx.c
drivers/tty/serial/serial_mctrl_gpio.c
drivers/tty/serial/serial_mctrl_gpio.h
drivers/tty/serial/sh-sci.c
drivers/tty/serial/stm32-usart.c

index df353211fc6ba50e0f5ed5db4a54a7817be9a756..fa1ebfcd4472e99caa425bd4225f5a1061f90c68 100644 (file)
@@ -103,4 +103,4 @@ Some helpers are provided in order to set/get modem control lines via GPIO.
 .. kernel-doc:: drivers/tty/serial/serial_mctrl_gpio.c
    :identifiers: mctrl_gpio_init mctrl_gpio_to_gpiod
            mctrl_gpio_set mctrl_gpio_get mctrl_gpio_enable_ms
-           mctrl_gpio_disable_ms
+           mctrl_gpio_disable_ms_sync mctrl_gpio_disable_ms_no_sync
index 442967a6cd52dcb611e312c9a62f88075c0a8ae1..886e40f680d4517f4e0bfe325429ce5629a8a6e3 100644 (file)
@@ -1680,7 +1680,7 @@ static void serial8250_disable_ms(struct uart_port *port)
        if (up->bugs & UART_BUG_NOMSR)
                return;
 
-       mctrl_gpio_disable_ms(up->gpios);
+       mctrl_gpio_disable_ms_no_sync(up->gpios);
 
        up->ier &= ~UART_IER_MSI;
        serial_port_out(port, UART_IER, up->ier);
index f44f9d20a97440c9aea41e9ebe34c34d4dfa0a1c..8918fbd4bddd5dfe7705a75d4132dda59fe9e5e6 100644 (file)
@@ -700,7 +700,7 @@ static void atmel_disable_ms(struct uart_port *port)
 
        atmel_port->ms_irq_enabled = false;
 
-       mctrl_gpio_disable_ms(atmel_port->gpios);
+       mctrl_gpio_disable_ms_no_sync(atmel_port->gpios);
 
        if (!mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS))
                idr |= ATMEL_US_CTSIC;
index 9c59ec128bb4fc0ff54cb9a1a66eabbc9e391a9a..cfeb3f8cf45eaeea8afaa767a4ad849bb8d19f69 100644 (file)
@@ -1608,7 +1608,7 @@ static void imx_uart_shutdown(struct uart_port *port)
                imx_uart_dma_exit(sport);
        }
 
-       mctrl_gpio_disable_ms(sport->gpios);
+       mctrl_gpio_disable_ms_sync(sport->gpios);
 
        uart_port_lock_irqsave(&sport->port, &flags);
        ucr2 = imx_uart_readl(sport, UCR2);
index 85c172ad1de90d8aded0f8578abc1f721d46471a..7b02c5ca4afd5c7cf21528b0b9496d7055456e55 100644 (file)
@@ -296,11 +296,7 @@ void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
 }
 EXPORT_SYMBOL_GPL(mctrl_gpio_enable_ms);
 
-/**
- * mctrl_gpio_disable_ms - disable irqs and handling of changes to the ms lines
- * @gpios: gpios to disable
- */
-void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
+static void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios, bool sync)
 {
        enum mctrl_gpio_idx i;
 
@@ -316,10 +312,34 @@ void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
                if (!gpios->irq[i])
                        continue;
 
-               disable_irq(gpios->irq[i]);
+               if (sync)
+                       disable_irq(gpios->irq[i]);
+               else
+                       disable_irq_nosync(gpios->irq[i]);
        }
 }
-EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms);
+
+/**
+ * mctrl_gpio_disable_ms_sync - disable irqs and handling of changes to the ms
+ * lines, and wait for any pending IRQ to be processed
+ * @gpios: gpios to disable
+ */
+void mctrl_gpio_disable_ms_sync(struct mctrl_gpios *gpios)
+{
+       mctrl_gpio_disable_ms(gpios, true);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms_sync);
+
+/**
+ * mctrl_gpio_disable_ms_no_sync - disable irqs and handling of changes to the
+ * ms lines, and return immediately
+ * @gpios: gpios to disable
+ */
+void mctrl_gpio_disable_ms_no_sync(struct mctrl_gpios *gpios)
+{
+       mctrl_gpio_disable_ms(gpios, false);
+}
+EXPORT_SYMBOL_GPL(mctrl_gpio_disable_ms_no_sync);
 
 void mctrl_gpio_enable_irq_wake(struct mctrl_gpios *gpios)
 {
index ec4170084766f5fa7fa3a6f79e947f85a305d49d..961e4ba0c6f80965f4818a5ac884dae6b9bbe265 100644 (file)
@@ -80,9 +80,16 @@ struct mctrl_gpios *mctrl_gpio_init_noauto(struct device *dev,
 void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios);
 
 /*
- * Disable gpio interrupts to report status line changes.
+ * Disable gpio interrupts to report status line changes, and block until
+ * any corresponding IRQ is processed
  */
-void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios);
+void mctrl_gpio_disable_ms_sync(struct mctrl_gpios *gpios);
+
+/*
+ * Disable gpio interrupts to report status line changes, and return
+ * immediately
+ */
+void mctrl_gpio_disable_ms_no_sync(struct mctrl_gpios *gpios);
 
 /*
  * Enable gpio wakeup interrupts to enable wake up source.
@@ -136,7 +143,11 @@ static inline void mctrl_gpio_enable_ms(struct mctrl_gpios *gpios)
 {
 }
 
-static inline void mctrl_gpio_disable_ms(struct mctrl_gpios *gpios)
+static inline void mctrl_gpio_disable_ms_sync(struct mctrl_gpios *gpios)
+{
+}
+
+static inline void mctrl_gpio_disable_ms_no_sync(struct mctrl_gpios *gpios)
 {
 }
 
index 4bc637f9d6499ee154e5e2161930322ce6c1e71d..e0ead0147bfe080ebbe492d58cd9cacd289bc56a 100644 (file)
@@ -2310,7 +2310,7 @@ static void sci_shutdown(struct uart_port *port)
        dev_dbg(port->dev, "%s(%d)\n", __func__, port->line);
 
        s->autorts = false;
-       mctrl_gpio_disable_ms(to_sci_port(port)->gpios);
+       mctrl_gpio_disable_ms_sync(to_sci_port(port)->gpios);
 
        uart_port_lock_irqsave(port, &flags);
        sci_stop_rx(port);
index 1ec5d8c3aef8ddbca615a149c2fe81c90c83a22b..4c97965ec43b30113a255661e6a64a6b476d7a9c 100644 (file)
@@ -944,7 +944,7 @@ static void stm32_usart_enable_ms(struct uart_port *port)
 
 static void stm32_usart_disable_ms(struct uart_port *port)
 {
-       mctrl_gpio_disable_ms(to_stm32_port(port)->gpios);
+       mctrl_gpio_disable_ms_sync(to_stm32_port(port)->gpios);
 }
 
 /* Transmit stop */