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

[sashiko-bot@kernel.org]

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