serial: st-asc: don't get/put GPIOs in atomic context
authorBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tue, 20 Feb 2024 11:34:10 +0000 (12:34 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 2 Mar 2024 21:04:12 +0000 (22:04 +0100)
Since commit 1f2bcb8c8ccd ("gpio: protect the descriptor label with
SRCU") gpiod_set_consumer_name() calls synchronize_srcu() which led to
a "sleeping in atomic context" smatch warning.

This function (along with gpiod_get/put() and all other GPIO APIs apart
from gpiod_get/set_value() and gpiod_direction_input/output()) should
have never been called with a spinlock taken. We're only fixing this now
as GPIOLIB has been rebuilt to use SRCU for access serialization which
uncovered this problem.

Move the calls to gpiod_get/put() outside the spinlock critical section.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-gpio/deee1438-efc1-47c4-8d80-0ab2cf01d60a@moroto.mountain/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Link: https://lore.kernel.org/r/20240220113410.16613-1-brgl@bgdev.pl
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/tty/serial/st-asc.c

index bbb5595d7e24cc22a195e7520366ac0e43e94e3c..a23e5955184885fa51dbdf302b9b39968561aaa2 100644 (file)
@@ -465,6 +465,7 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
                            const struct ktermios *old)
 {
        struct asc_port *ascport = to_asc_port(port);
+       bool manual_rts, toggle_rts = false;
        struct gpio_desc *gpiod;
        unsigned int baud;
        u32 ctrl_val;
@@ -518,25 +519,13 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
 
                /* If flow-control selected, stop handling RTS manually */
                if (ascport->rts) {
-                       devm_gpiod_put(port->dev, ascport->rts);
-                       ascport->rts = NULL;
-
-                       pinctrl_select_state(ascport->pinctrl,
-                                            ascport->states[DEFAULT]);
+                       toggle_rts = true;
+                       manual_rts = false;
                }
        } else {
                /* If flow-control disabled, it's safe to handle RTS manually */
-               if (!ascport->rts && ascport->states[NO_HW_FLOWCTRL]) {
-                       pinctrl_select_state(ascport->pinctrl,
-                                            ascport->states[NO_HW_FLOWCTRL]);
-
-                       gpiod = devm_gpiod_get(port->dev, "rts", GPIOD_OUT_LOW);
-                       if (!IS_ERR(gpiod)) {
-                               gpiod_set_consumer_name(gpiod,
-                                               port->dev->of_node->name);
-                               ascport->rts = gpiod;
-                       }
-               }
+               if (!ascport->rts && ascport->states[NO_HW_FLOWCTRL])
+                       manual_rts = toggle_rts = true;
        }
 
        if ((baud < 19200) && !ascport->force_m1) {
@@ -595,6 +584,25 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
        asc_out(port, ASC_CTL, (ctrl_val | ASC_CTL_RUN));
 
        uart_port_unlock_irqrestore(port, flags);
+
+       if (toggle_rts) {
+               if (manual_rts) {
+                       pinctrl_select_state(ascport->pinctrl,
+                                            ascport->states[NO_HW_FLOWCTRL]);
+
+                       gpiod = devm_gpiod_get(port->dev, "rts", GPIOD_OUT_LOW);
+                       if (!IS_ERR(gpiod)) {
+                               gpiod_set_consumer_name(gpiod,
+                                                       port->dev->of_node->name);
+                               ascport->rts = gpiod;
+                       }
+               } else {
+                               devm_gpiod_put(port->dev, ascport->rts);
+                               ascport->rts = NULL;
+                               pinctrl_select_state(ascport->pinctrl,
+                                                    ascport->states[DEFAULT]);
+               }
+       }
 }
 
 static const char *asc_type(struct uart_port *port)