[PATCH v2] serial: st-asc: don't get/put GPIOs in atomic context

From: Bartosz Golaszewski
Date: Mon Feb 19 2024 - 03:01:35 EST


From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>

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@xxxxxxxxxx>
Closes: https://lore.kernel.org/linux-gpio/deee1438-efc1-47c4-8d80-0ab2cf01d60a@moroto.mountain/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
---
v1 -> v2:
- initialize the 'manual_rts' variable to false as we don't always get to
the place where it's set

drivers/tty/serial/st-asc.c | 40 ++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index bbb5595d7e24..f184de6af313 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -466,6 +466,7 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
{
struct asc_port *ascport = to_asc_port(port);
struct gpio_desc *gpiod;
+ bool manual_rts = false;
unsigned int baud;
u32 ctrl_val;
tcflag_t cflag;
@@ -517,26 +518,12 @@ static void asc_set_termios(struct uart_port *port, struct ktermios *termios,
ctrl_val |= ASC_CTL_CTSENABLE;

/* 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]);
- }
+ if (ascport->rts)
+ 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 = true;
}

if ((baud < 19200) && !ascport->force_m1) {
@@ -595,6 +582,23 @@ 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 (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)
--
2.40.1