[PATCH 0/2] tty: serial: lpuart: keep RTS deasserted while the port is being configured

[PATCH 0/2] tty: serial: lpuart: keep RTS deasserted while the port is being configured

[Sherry Sun (OSS)]

When hardware flow control is enabled, the RXRTSE bit in UARTMODIR makes
RTS follow the receiver FIFO level as long as the receiver is enabled.
In the lpuart32 code there are still two windows where RXRTSE is set
while the receiver is enabled but the port is not fully configured yet.
In both cases RTS can be asserted, so the remote side may keep sending,
or start sending, right before set_termios() disables the receiver and
reprograms the baud rate. The characters in flight are then lost or
corrupted, and a short RTS pulse can be observed on the line.

Patch 1 fixes set_termios(). Since commit e98ab45ec518 ("tty: serial:
lpuart: only disable CTS instead of overwriting the whole UARTMODIR
register"), the UARTMODIR write before waiting for transmission complete
only masks TXCTSE, so RXRTSE stays set, or is even newly set when
CRTSCTS is requested, while the receiver is still enabled. Mask RXRTSE
in that write as well; the final UARTMODIR write still restores the
flow control setting requested by termios.

Patch 2 fixes startup(). RXRTSE/TXCTSE keep their value from the
previous session, and lpuart32_startup() enables the receiver before
set_termios() is called, so RTS is asserted immediately on open and on
resume. Clear both bits at the beginning of lpuart32_startup() and leave
the flow control configuration to set_termios().

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
Sherry Sun (2):
      tty: serial: lpuart: deassert RTS while reconfiguring in set_termios()
      tty: serial: lpuart: disable hardware flow control in lpuart32_startup()

 drivers/tty/serial/fsl_lpuart.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
---
base-commit: 8f7df005a7564bd7ffa75da9c3085d5b996f1e16
change-id: 20261022-b4-lpuart_rts_fix-730498ccb73d

Best regards,
-- 
Sherry Sun <sherry.sun@nxp.com>

[PATCH 1/2] tty: serial: lpuart: deassert RTS while reconfiguring in set_termios()

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

Commit e98ab45ec518 ("tty: serial: lpuart: only disable CTS instead of
overwriting the whole UARTMODIR register") replaced the write of 0 to
UARTMODIR before waiting for the transmission complete flag with a
write that only masks TXCTSE. As a side effect RXRTSE is no longer
cleared there, and when CRTSCTS is requested that write even sets it
while the receiver is still enabled.

RTS therefore keeps following the receiver FIFO level during the whole
reconfiguration, so the remote side may keep sending, or start sending,
right before set_termios() disables the receiver and reprograms the
baud rate. The characters in flight are then lost or corrupted, and a
short RTS pulse can be observed on the line.

Mask RXRTSE in that first write as well, so that RTS is deasserted and
the remote side is asked to stop sending before the receiver is
disabled. The final write of the modem value restores both RXRTSE and
TXCTSE as requested by termios, and the other UARTMODIR fields are still
preserved.

Fixes: e98ab45ec518 ("tty: serial: lpuart: only disable CTS instead of overwriting the whole UARTMODIR register")
Cc: stable@vger.kernel.org
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c8575c965203..70b69870aaba 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -2360,9 +2360,12 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
 
 	/*
 	 * disable CTS to ensure the transmit engine is not blocked by the flow
-	 * control when there is dirty data in TX FIFO
+	 * control when there is dirty data in TX FIFO, and deassert RTS to ask
+	 * the remote to stop sending before the receiver is disabled and the
+	 * baud rate is changed below
 	 */
-	lpuart32_write(port, modem & ~UARTMODIR_TXCTSE, UARTMODIR);
+	lpuart32_write(port, modem & ~(UARTMODIR_TXCTSE | UARTMODIR_RXRTSE),
+		       UARTMODIR);
 
 	/*
 	 * LPUART Transmission Complete Flag may never be set while queuing a break
@@ -2380,7 +2383,7 @@ lpuart32_set_termios(struct uart_port *port, struct ktermios *termios,
 	lpuart32_serial_setbrg(sport, baud);
 	/* restore control register */
 	lpuart32_write(port, ctrl, UARTCTRL);
-	/* re-enable the CTS if needed */
+	/* re-enable the CTS/RTS if needed */
 	lpuart32_write(port, modem, UARTMODIR);
 
 	if ((ctrl & (UARTCTRL_PE | UARTCTRL_M)) == UARTCTRL_PE)

-- 
2.50.1

Re: [PATCH 1/2] tty: serial: lpuart: deassert RTS while reconfiguring in set_termios()

[sashiko-bot]

> From: Sherry Sun <sherry.sun@nxp.com>
> 
> Commit e98ab45ec518 ("tty: serial: lpuart: only disable CTS instead of
> overwriting the whole UARTMODIR register") replaced the write of 0 to
> UARTMODIR before waiting for the transmission complete flag with a
> write that only masks TXCTSE. As a side effect RXRTSE is no longer
> cleared there, and when CRTSCTS is requested that write even sets it
> while the receiver is still enabled.
> 
> RTS therefore keeps following the receiver FIFO level during the whole
> reconfiguration, so the remote side may keep sending, or start sending,
> right before set_termios() disables the receiver and reprograms the
> baud rate. The characters in flight are then lost or corrupted, and a
> short RTS pulse can be observed on the line.
> 
> Mask RXRTSE in that first write as well, so that RTS is deasserted and
> the remote side is asked to stop sending before the receiver is
> disabled. The final write of the modem value restores both RXRTSE and
> TXCTSE as requested by termios, and the other UARTMODIR fields are still
> preserved.
> 
> Fixes: e98ab45ec518 ("tty: serial: lpuart: only disable CTS instead of overwriting the whole UARTMODIR register")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-b4-lpuart_rts_fix-v1-0-207f91580dee@nxp.com?part=1

[PATCH 2/2] tty: serial: lpuart: disable hardware flow control in lpuart32_startup()

[Sherry Sun (OSS)]

From: Sherry Sun <sherry.sun@nxp.com>

The RXRTSE/TXCTSE bits in UARTMODIR are configured by set_termios()
according to CRTSCTS and are left untouched by shutdown(), so they keep
their value from the previous session when the port is started again.

On open, and on resume when a tty is attached, the serial core calls
ops->startup() before set_termios(). lpuart32_startup() enables the
receiver (UARTCTRL_RE), and with RXRTSE still set from the previous
session RTS is asserted immediately. The remote side may then start
sending before set_termios() deasserts RTS again, disables TE/RE and
reprograms the baud rate, so the characters in flight are lost or
corrupted.

Clear RXRTSE/TXCTSE at the beginning of lpuart32_startup() instead.
Together with set_termios() keeping both bits masked until its last
UARTMODIR write, RTS now stays deasserted from startup() until the port
is fully configured. The flow control setting itself is still owned by
set_termios().

Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support")
Cc: stable@vger.kernel.org
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/tty/serial/fsl_lpuart.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 70b69870aaba..87737f0e0f21 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1889,7 +1889,19 @@ static void lpuart32_hw_setup(struct lpuart_port *sport)
 static int lpuart32_startup(struct uart_port *port)
 {
 	struct lpuart_port *sport = container_of(port, struct lpuart_port, port);
-	u32 fifo;
+	u32 fifo, modem;
+
+	/*
+	 * The hardware flow control is configured by set_termios(), which the
+	 * serial core calls after startup() to apply the termios settings. Keep
+	 * it disabled until then, otherwise the RXRTSE left over from the
+	 * previous session asserts RTS as soon as the receiver is enabled
+	 * below, and the remote side may start sending while set_termios()
+	 * still reconfigures the receiver.
+	 */
+	modem = lpuart32_read(port, UARTMODIR);
+	modem &= ~(UARTMODIR_RXRTSE | UARTMODIR_TXCTSE);
+	lpuart32_write(port, modem, UARTMODIR);
 
 	/* determine FIFO size */
 	fifo = lpuart32_read(port, UARTFIFO);

-- 
2.50.1

Re: [PATCH 2/2] tty: serial: lpuart: disable hardware flow control in lpuart32_startup()

[sashiko-bot]

> From: Sherry Sun <sherry.sun@nxp.com>
> 
> The RXRTSE/TXCTSE bits in UARTMODIR are configured by set_termios()
> according to CRTSCTS and are left untouched by shutdown(), so they keep
> their value from the previous session when the port is started again.
> 
> On open, and on resume when a tty is attached, the serial core calls
> ops->startup() before set_termios(). lpuart32_startup() enables the
> receiver (UARTCTRL_RE), and with RXRTSE still set from the previous
> session RTS is asserted immediately. The remote side may then start
> sending before set_termios() deasserts RTS again, disables TE/RE and
> reprograms the baud rate, so the characters in flight are lost or
> corrupted.
> 
> Clear RXRTSE/TXCTSE at the beginning of lpuart32_startup() instead.
> Together with set_termios() keeping both bits masked until its last
> UARTMODIR write, RTS now stays deasserted from startup() until the port
> is fully configured. The flow control setting itself is still owned by
> set_termios().
> 
> Fixes: 380c966c093e ("tty: serial: fsl_lpuart: add 32-bit register interface support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-b4-lpuart_rts_fix-v1-0-207f91580dee@nxp.com?part=2