[PATCH] serial: tegra: don't clear the Tx FIFO on an Rx-only reset

[PATCH] serial: tegra: don't clear the Tx FIFO on an Rx-only reset

[Simon Gassner]

tegra_uart_fifo_reset() applies the Tegra30 workaround for
"cannot clear the Tx FIFO while FIFO mode is enabled"
unconditionally: it leaves FIFO mode, writes the requested FCR
clear bits, and re-enters FIFO mode. Leaving FIFO mode empties
both FIFOs, so an Rx-only reset discards queued Tx data as well.

The break handler in tegra_uart_decode_rx_error() calls it with
UART_FCR_CLEAR_RCVR only. On a half-duplex RS485 board whose Rx
line is pulled low while the transceiver drives the bus, every
transmission raises a spurious break, and all but the first
character of the frame is lost.

Only take the FIFO-mode path when the caller actually asked for
CLEAR_XMIT. Likewise only wait for TEMT in that case: with the Tx
FIFO deliberately left intact, that loop would otherwise spin for
a full frame time in hard IRQ context with the port lock held.

Tested on a Colibri T30 (Tegra30) with Rx DMA:

 - Break during transmission: the reset still fires from
   tegra_uart_decode_rx_error(), and the complete frame now
   reaches the peer. Before this change only the character in
   the shift register was sent.
 - Internal loopback with a generated break: the Rx FIFO is
   correctly cleared by the plain FCR write, and subsequent
   receive works with no frame, parity or overrun errors.

Fixes: e9ea096dd225 ("serial: tegra: add serial driver")
Cc: stable@vger.kernel.org
Signed-off-by: Simon Gassner <simon.gassner@noxsystems.com>
---
 drivers/tty/serial/serial-tegra.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index 8004fc00fb9c..b0fc4ae3686f 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -306,11 +306,19 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 {
 	unsigned long fcr = tup->fcr_shadow;
 	unsigned int lsr, tmout = 10000;
+	bool clear_tx = !!(fcr_bits & UART_FCR_CLEAR_XMIT);
 
 	if (tup->rts_active)
 		set_rts(tup, false);
 
-	if (tup->cdata->allow_txfifo_reset_fifo_mode) {
+	/*
+	 * Leaving FIFO mode below is a workaround for a Tegra30
+	 * restriction on clearing the Tx FIFO while FIFO mode is
+	 * enabled. It empties both FIFOs, so applying it to an
+	 * Rx-only reset would destroy an in-flight transmission.
+	 * Only take that path when the caller asked for CLEAR_XMIT.
+	 */
+	if (tup->cdata->allow_txfifo_reset_fifo_mode || !clear_tx) {
 		fcr |= fcr_bits & (UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
 		tegra_uart_write(tup, fcr, UART_FCR);
 	} else {
@@ -335,9 +343,15 @@ static void tegra_uart_fifo_reset(struct tegra_uart_port *tup, u8 fcr_bits)
 	 */
 	tegra_uart_wait_cycle_time(tup, 32);
 
+	/*
+	 * Only wait for the transmitter to drain when the Tx FIFO was
+	 * part of the reset. For an Rx-only reset it is left intact,
+	 * and waiting for TEMT here would spin for a full frame time.
+	 */
 	do {
 		lsr = tegra_uart_read(tup, UART_LSR);
-		if ((lsr & UART_LSR_TEMT) && !(lsr & UART_LSR_DR))
+		if ((!clear_tx || (lsr & UART_LSR_TEMT)) &&
+		    !(lsr & UART_LSR_DR))
 			break;
 		udelay(1);
 	} while (--tmout);
-- 
2.43.0

Re: [PATCH] serial: tegra: don't clear the Tx FIFO on an Rx-only reset

[sashiko-bot]

> tegra_uart_fifo_reset() applies the Tegra30 workaround for
> "cannot clear the Tx FIFO while FIFO mode is enabled"
> unconditionally: it leaves FIFO mode, writes the requested FCR
> clear bits, and re-enters FIFO mode. Leaving FIFO mode empties
> both FIFOs, so an Rx-only reset discards queued Tx data as well.
> 
> The break handler in tegra_uart_decode_rx_error() calls it with
> UART_FCR_CLEAR_RCVR only. On a half-duplex RS485 board whose Rx
> line is pulled low while the transceiver drives the bus, every
> transmission raises a spurious break, and all but the first
> character of the frame is lost.
> 
> Only take the FIFO-mode path when the caller actually asked for
> CLEAR_XMIT. Likewise only wait for TEMT in that case: with the Tx
> FIFO deliberately left intact, that loop would otherwise spin for
> a full frame time in hard IRQ context with the port lock held.
> 
> Tested on a Colibri T30 (Tegra30) with Rx DMA:
> 
>  - Break during transmission: the reset still fires from
>    tegra_uart_decode_rx_error(), and the complete frame now
>    reaches the peer. Before this change only the character in
>    the shift register was sent.
>  - Internal loopback with a generated break: the Rx FIFO is
>    correctly cleared by the plain FCR write, and subsequent
>    receive works with no frame, parity or overrun errors.
> 
> Fixes: e9ea096dd225 ("serial: tegra: add serial driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Simon Gassner <simon.gassner@noxsystems.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001060539.32659-1-simon.gassner@noxsystems.com?part=1