[PATCH v2 1/2] serial: sc16is7xx: refill TX FIFO below trigger using fresh TXLVL

[Paul Mbewe]

sc16is7xx_handle_tx() reads TXLVL once and sizes the hardware TX FIFO
write from that value. TXLVL reports free space in the hardware TX FIFO.

On this SPI-backed path, hardirq/softirq activity, RT scheduling, and
waiting for synchronous SPI transfers can delay the refill while the UART
continues draining. The TXLVL value can therefore become stale before the
hardware TX FIFO write completes.

One failing ftrace with the default 8-free-space trigger showed:

  tx_start       txlvl=9  txlvl_read_us=580
  tx_pre_write   sent=9   pre_write_us=16
  tx_segment     sent=9   seg_us=364
  tx_post_write  txlvl_before=9 sent=9 txlvl_after=12 pending_after=29
                 post_gap_us=12 post_txlvl_us=130

The driver read 9 free spaces and wrote 9 bytes to the hardware TX FIFO,
but the post-write TXLVL read still reported 12 free spaces while 29 bytes
remained queued in the xmit kfifo. Even allowing for the post-write read
window, the hardware TX FIFO had not been filled below the 8-free-space
trigger, so no new threshold crossing was expected.

The captured failing samples had the same pattern: data remained queued
in the xmit kfifo while post-write TXLVL remained above the hardware
trigger. The hardware TX FIFO then drained empty before another refill
was requested, producing an unintended gap on the wire.

Fix this by re-reading TXLVL after each hardware TX FIFO write while data
remains queued in the xmit kfifo. If TXLVL is still at or above the
trigger, top up the hardware TX FIFO again. Stop when the xmit kfifo is
empty or a TXLVL read confirms that hardware TX FIFO free space is
strictly below the trigger.

Stopping when TXLVL was equal to the trigger still allowed TX gaps in the
tested workload. Continuing until TXLVL was strictly below the trigger
eliminated the observed gaps caused by stale-TXLVL under-fill.

Program the hardware TX trigger explicitly through TLR using the same
constant as the refill-loop threshold. This prevents the software refill
condition from diverging from the programmed hardware trigger.

Tested on SC16IS752 over 1 MHz SPI on an i.MX6ULL single-core
PREEMPT_RT system, transmitting RS-485 at 115200 baud 8N1 under
continuous Modbus RTU load.

Fixes: dfeae619d781 ("serial: sc16is7xx")
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: Tobias Gannert <tobias.gannert@xxxxxxxxxxxxxx>
Link: https://lore.kernel.org/linux-serial/20260623112225.82386-3-paultyson.mbewe@xxxxxxxxxxxxxx/
Signed-off-by: Paul Mbewe <paultyson.mbewe@xxxxxxxxxxxxxx>
---
Changes in v2:
  - New patch arising from review of v1 patch 2
  - Added a fresh TXLVL read after each hardware TX FIFO write
  - Continue refilling until TXLVL is strictly below the trigger
  - Explicitly program the hardware trigger from the same constant used
    by the refill loop
  - Corrected the root cause from trigger-level margin to stale-TXLVL
    under-fill
  - Removed the max310x comparison

 drivers/tty/serial/sc16is7xx.c | 38 +++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index fa7805d2cde2..1f7e98c2b570 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -216,6 +216,8 @@
 #define SC16IS7XX_TLR_TX_TRIGGER(words)	((((words) / 4) & 0x0f) << 0)
 #define SC16IS7XX_TLR_RX_TRIGGER(words)	((((words) / 4) & 0x0f) << 4)
 
+#define SC16IS7XX_TX_TRIGGER_LEVEL	8
+
 /* IOControl register bits (Only 75x/76x) */
 #define SC16IS7XX_IOCONTROL_LATCH_BIT	BIT(0)   /* Enable input latching */
 #define SC16IS7XX_IOCONTROL_MODEM_A_BIT	BIT(1)   /* Enable GPIO[7:4] as modem A pins */
@@ -647,6 +649,21 @@ static void sc16is7xx_handle_rx(struct uart_port *port, unsigned int rxlen,
 	tty_flip_buffer_push(&port->state->port);
 }
 
+static unsigned int sc16is7xx_txlvl(struct uart_port *port)
+{
+	unsigned int txlvl;
+
+	txlvl = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
+	if (txlvl > SC16IS7XX_FIFO_SIZE) {
+		dev_err_ratelimited(port->dev,
+				    "chip reports %u free bytes in TX FIFO, but it only has %u\n",
+				    txlvl, SC16IS7XX_FIFO_SIZE);
+		return 0;
+	}
+
+	return txlvl;
+}
+
 static void sc16is7xx_handle_tx(struct uart_port *port)
 {
 	struct tty_port *tport = &port->state->port;
@@ -668,13 +685,7 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
 	}
 
 	/* Limit to space available in TX FIFO */
-	txlen = sc16is7xx_port_read(port, SC16IS7XX_TXLVL_REG);
-	if (txlen > SC16IS7XX_FIFO_SIZE) {
-		dev_err_ratelimited(port->dev,
-			"chip reports %d free bytes in TX fifo, but it only has %d",
-			txlen, SC16IS7XX_FIFO_SIZE);
-		txlen = 0;
-	}
+	txlen = sc16is7xx_txlvl(port);
 
 	/* Handle circular buffer wrap-around by sending multiple segments */
 	while (txlen > 0 && !kfifo_is_empty(&tport->xmit_fifo)) {
@@ -687,7 +698,14 @@ static void sc16is7xx_handle_tx(struct uart_port *port)
 
 		sc16is7xx_fifo_write(port, tail, to_send);
 		uart_xmit_advance(port, to_send);
-		txlen -= to_send;
+
+		if (kfifo_is_empty(&tport->xmit_fifo))
+			break;
+
+		/* Refill below the trigger to enable the next THRI crossing. */
+		txlen = sc16is7xx_txlvl(port);
+		if (txlen < SC16IS7XX_TX_TRIGGER_LEVEL)
+			break;
 	}
 
 	uart_port_lock_irqsave(port, &flags);
@@ -1139,6 +1157,10 @@ static int sc16is7xx_startup(struct uart_port *port)
 			     SC16IS7XX_TCR_RX_RESUME(24) |
 			     SC16IS7XX_TCR_RX_HALT(48));
 
+	/* Sync hardware and software TX trigger levels */
+	sc16is7xx_port_write(port, SC16IS7XX_TLR_REG,
+			     SC16IS7XX_TLR_TX_TRIGGER(SC16IS7XX_TX_TRIGGER_LEVEL));
+
 	/* Disable TCR/TLR access */
 	sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, SC16IS7XX_MCR_TCRTLR_BIT, 0);
 
-- 
2.43.0