[PATCH v2] serial: fsl_lpuart: add support for GPIO-controlled modem lines

[PATCH v2] serial: fsl_lpuart: add support for GPIO-controlled modem lines

[João Loureiro]

LPUART has no register bit to drive RTS to a chosen level: UARTMODIR only
offers receiver-driven auto-RTS (RXRTSE) and RTS-during-transmit (TXRTSE).
Consequently lpuart_set_mctrl() and lpuart32_set_mctrl() ignore TIOCM_RTS,
and their get_mctrl() counterparts report a hardcoded
TIOCM_CAR | TIOCM_DSR | TIOCM_CTS.

That makes it impossible to talk to a peripheral which is driven by a
software controlled RTS strobe and answers on CTS.  TIOCMBIS/TIOCMBIC
silently do nothing, and because CTS always reads back as asserted, a
userspace handshake that waits for CTS to follow RTS can never complete:
de-asserting RTS and waiting for CTS to drop times out every time.  Boards
that route the two pins to plain GPIOs cannot work around it either, since
the driver never looks at rts-gpios/cts-gpios.

Wire the driver up to the serial_mctrl_gpio helpers, as imx.c and
atmel_serial.c already do.  set_mctrl() forwards the state to
mctrl_gpio_set(), and get_mctrl() runs the flags through mctrl_gpio_get()
so that a described cts-gpios overrides the assumed-asserted default.
Boards without such a description keep the previous behaviour.  Modem
status interrupts are enabled from startup() and disabled from shutdown(),
with a .enable_ms callback for the serial core.

The same limitation was reported for an i.MX 8XLite board that needs RS-485
with a GPIO RTS [1].  This change was developed and tested on an i.MX95
board whose barcode scanner is driven over LPUART with a manual RTS strobe;
with it, RTS toggles as requested and CTS is reported from the real pin.

[1] https://lore.kernel.org/all/20260210-rearview-hungrily-536a95fc3385@thorsis.com/

Signed-off-by: João Loureiro <joaofl@gmail.com>
---

Notes:
    Changes in v2:
    - Resend: v1 (Message-ID <20260819144458.253967-1-joaofl@gmail.com>)
      never reached the mailing lists, so it could not go through CI.
    - Rebased onto tty-next; no functional changes.

 drivers/tty/serial/Kconfig      |  1 +
 drivers/tty/serial/fsl_lpuart.c | 35 +++++++++++++++++++++++++++++++--
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index bf680d22199b..cbf6b44d8a3e 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1316,6 +1316,7 @@ config SERIAL_FSL_LPUART
 	tristate "Freescale lpuart serial port support"
 	depends on HAS_DMA
 	select SERIAL_CORE
+	select SERIAL_MCTRL_GPIO if GPIOLIB
 	help
 	  Support for the on-chip lpuart on some Freescale SOCs.
 
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c8575c965203..909253aab212 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -27,6 +27,8 @@
 #include <linux/slab.h>
 #include <linux/tty_flip.h>
 
+#include "serial_mctrl_gpio.h"
+
 /* All registers are 8-bit width */
 #define UARTBDH			0x00
 #define UARTBDL			0x01
@@ -291,6 +293,7 @@ struct lpuart_port {
 	bool			is_cs7; /* Set to true when character size is 7 */
 					/* and the parity is enabled		*/
 	bool			dma_idle_int;
+	struct mctrl_gpios	*gpios;
 };
 
 struct lpuart_soc_data {
@@ -1528,6 +1531,7 @@ static int lpuart32_config_rs485(struct uart_port *port, struct ktermios *termio
 
 static unsigned int lpuart_get_mctrl(struct uart_port *port)
 {
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
 	unsigned int mctrl = 0;
 	u8 cr1;
 
@@ -1535,11 +1539,12 @@ static unsigned int lpuart_get_mctrl(struct uart_port *port)
 	if (cr1 & UARTCR1_LOOPS)
 		mctrl |= TIOCM_LOOP;
 
-	return mctrl;
+	return mctrl_gpio_get(sport->gpios, &mctrl);
 }
 
 static unsigned int lpuart32_get_mctrl(struct uart_port *port)
 {
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
 	unsigned int mctrl = TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
 	u32 ctrl;
 
@@ -1547,11 +1552,13 @@ static unsigned int lpuart32_get_mctrl(struct uart_port *port)
 	if (ctrl & UARTCTRL_LOOPS)
 		mctrl |= TIOCM_LOOP;
 
-	return mctrl;
+	/* A cts-gpio, when present, overrides the assumed-asserted CTS above. */
+	return mctrl_gpio_get(sport->gpios, &mctrl);
 }
 
 static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
 	u8 cr1;
 
 	cr1 = readb(port->membase + UARTCR1);
@@ -1562,10 +1569,13 @@ static void lpuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		cr1 |= UARTCR1_LOOPS;
 
 	writeb(cr1, port->membase + UARTCR1);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
 }
 
 static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
 	u32 ctrl;
 
 	ctrl = lpuart32_read(port, UARTCTRL);
@@ -1576,6 +1586,15 @@ static void lpuart32_set_mctrl(struct uart_port *port, unsigned int mctrl)
 		ctrl |= UARTCTRL_LOOPS;
 
 	lpuart32_write(port, ctrl, UARTCTRL);
+
+	mctrl_gpio_set(sport->gpios, mctrl);
+}
+
+static void lpuart_enable_ms(struct uart_port *port)
+{
+	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
+
+	mctrl_gpio_enable_ms(sport->gpios);
 }
 
 static void lpuart_break_ctl(struct uart_port *port, int break_state)
@@ -1844,6 +1863,8 @@ static int lpuart_startup(struct uart_port *port)
 	lpuart_request_dma(sport);
 	lpuart_hw_setup(sport);
 
+	mctrl_gpio_enable_ms(sport->gpios);
+
 	return 0;
 }
 
@@ -1915,6 +1936,8 @@ static int lpuart32_startup(struct uart_port *port)
 	lpuart_request_dma(sport);
 	lpuart32_hw_setup(sport);
 
+	mctrl_gpio_enable_ms(sport->gpios);
+
 	return 0;
 }
 
@@ -1956,6 +1979,7 @@ static void lpuart_shutdown(struct uart_port *port)
 
 	uart_port_unlock_irqrestore(port, flags);
 
+	mctrl_gpio_disable_ms_sync(sport->gpios);
 	lpuart_dma_shutdown(sport);
 }
 
@@ -1990,6 +2014,7 @@ static void lpuart32_shutdown(struct uart_port *port)
 
 	uart_port_unlock_irqrestore(port, flags);
 
+	mctrl_gpio_disable_ms_sync(sport->gpios);
 	lpuart_dma_shutdown(sport);
 }
 
@@ -2441,6 +2466,7 @@ static const struct uart_ops lpuart_pops = {
 	.tx_empty	= lpuart_tx_empty,
 	.set_mctrl	= lpuart_set_mctrl,
 	.get_mctrl	= lpuart_get_mctrl,
+	.enable_ms	= lpuart_enable_ms,
 	.stop_tx	= lpuart_stop_tx,
 	.start_tx	= lpuart_start_tx,
 	.stop_rx	= lpuart_stop_rx,
@@ -2466,6 +2492,7 @@ static const struct uart_ops lpuart32_pops = {
 	.tx_empty	= lpuart32_tx_empty,
 	.set_mctrl	= lpuart32_set_mctrl,
 	.get_mctrl	= lpuart32_get_mctrl,
+	.enable_ms	= lpuart_enable_ms,
 	.stop_tx	= lpuart32_stop_tx,
 	.start_tx	= lpuart32_start_tx,
 	.stop_rx	= lpuart32_stop_rx,
@@ -2912,6 +2939,10 @@ static int lpuart_probe(struct platform_device *pdev)
 		sport->port.rs485_config = lpuart_config_rs485;
 	sport->port.rs485_supported = lpuart_rs485_supported;
 
+	sport->gpios = mctrl_gpio_init(&sport->port, 0);
+	if (IS_ERR(sport->gpios))
+		return PTR_ERR(sport->gpios);
+
 	sport->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
 	if (IS_ERR(sport->ipg_clk)) {
 		ret = PTR_ERR(sport->ipg_clk);

base-commit: c44a4925cdac02af781ebfae96df68a7bf3580b0
-- 
2.55.0

Re: [PATCH v2] serial: fsl_lpuart: add support for GPIO-controlled modem lines

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] tty: fsl_lpuart: native hardware flow control blocks TX with GPIO CTS
- [Medium] tty: fsl_lpuart: missing RTS GPIO toggle during RS-485 transmission
- [Medium] tty: fsl_lpuart: modem status interrupts unconditionally enabled on startup
--

commit 992ffb5df51e6a0dce4af1057b32c3465f75e984
Author: João Loureiro <joaofl@gmail.com>

serial: fsl_lpuart: add support for GPIO-controlled modem lines

This commit adds support for GPIO-controlled modem lines (RTS/CTS) to the
LPUART driver by wiring it up to the serial_mctrl_gpio helpers. It aims to
support configurations where RTS is driven by software and answers on CTS,
or where RS-485 needs a GPIO RTS.

Link: https://lore.kernel.org/all/20260210-rearview-hungrily-536a95fc3385@thorsis.com/

> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index c8575c9652035..909253aab2125 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c

[ ... ]

> @@ -1844,6 +1863,8 @@ static int lpuart_startup(struct uart_port *port)
>  	lpuart_request_dma(sport);
>  	lpuart_hw_setup(sport);
>  
> +	mctrl_gpio_enable_ms(sport->gpios);
> +
>  	return 0;
>  }

[Severity: Medium]
Does unconditionally enabling modem status interrupts here defeat the serial
core's demand-driven .enable_ms callback design? 

This will permanently enable GPIO interrupts whenever the port is open,
which introduces unnecessary interrupt overhead.

[ ... ]

> @@ -2912,6 +2939,10 @@ static int lpuart_probe(struct platform_device *pdev)
>  		sport->port.rs485_config = lpuart_config_rs485;
>  	sport->port.rs485_supported = lpuart_rs485_supported;
>  
> +	sport->gpios = mctrl_gpio_init(&sport->port, 0);
> +	if (IS_ERR(sport->gpios))
> +		return PTR_ERR(sport->gpios);
> +

[Severity: Medium]
When setting up GPIO-controlled modem lines, what happens when hardware flow
control is enabled via standard userspace termios configuration?

Looking at lpuart_set_termios():

	if (termios->c_cflag & CRTSCTS)
		modem |= UARTMODEM_RXRTSE | UARTMODEM_TXCTSE;

And lpuart32_set_termios():

	if (termios->c_cflag & CRTSCTS)
		modem |= UARTMODIR_RXRTSE | UARTMODIR_TXCTSE;

If cts-gpios is being used, native hardware flow control is erroneously
enabled without checking if the CTS/RTS lines are being driven by the GPIO
framework. 

This causes the LPUART hardware to block transmission waiting for the
physical CTS pin, ignoring the newly added GPIO CTS line.

[Severity: Medium]
Additionally, when using RS-485 mode with a board using rts-gpios, does this
implementation miss toggling the RTS GPIO during transmission?

Looking at lpuart_config_rs485(), it only enables the hardware's auto-RTS
feature on the dedicated physical RTS pin, which has no effect on the GPIO
RTS:

		modem |= UARTMODEM_TXRTSE;

		/*
		 * The hardware defaults to RTS logic HIGH while transfer.
		 * Switch polarity in case RTS shall be logic HIGH
		 * after transfer.
		 */
		if (rs485->flags & SER_RS485_RTS_ON_SEND)
			modem |= UARTMODEM_TXRTSPOL;

Then when starting TX in lpuart_start_tx():

	if (sport->lpuart_dma_tx_use) {
		if (!lpuart_stopped_or_empty(port))
			lpuart_dma_tx(sport);
	} else {
		if (readb(port->membase + UARTSR1) & UARTSR1_TDRE)
			lpuart_transmit_buffer(sport);
	}

And similarly in lpuart32_start_tx():

	if (sport->lpuart_dma_tx_use) {
		if (!lpuart_stopped_or_empty(port))
			lpuart_dma_tx(sport);
	} else {
		ctrl = lpuart32_read(port, UARTCTRL);
		lpuart32_write(port, ctrl | UARTCTRL_TIE, UARTCTRL);
	}

Because the driver relies exclusively on the native UARTMODIR_TXRTSE
auto-RTS feature and fails to invoke mctrl_gpio_set(), the GPIO pin is never
toggled and RS-485 transmission over an RTS GPIO fails.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003155022.466971-1-joaofl@gmail.com?part=1