[PATCH v2 RESEND 0/2] serial: sc16is7xx: improve TX FIFO refill

[PATCH v2 RESEND 0/2] serial: sc16is7xx: improve TX FIFO refill

[Paul Mbewe]

This series follows the TX inter-frame-gap work submitted as:

  [PATCH 0/2] serial: sc16is7xx: fix TX inter-frame gaps on SPI UARTs

Following review, the kfifo wrap-around fix was resent separately as a
standalone v2 patch:

  https://lore.kernel.org/linux-serial/20260930143207.542930-1-paultyson.mbewe@ziehl-abegg.de/

This series depends on that fix because patch 1/2 extends its
multi-segment TX refill loop.

The original trigger-level patch has been split into:

  1. a stale-TXLVL under-fill correctness fix; and
  2. a separate trigger-level load-reduction change.

Patch 1/2 re-reads TXLVL after each hardware TX FIFO write while data
remains queued. It continues refilling until the xmit kfifo is empty or
the hardware TX FIFO free space is strictly below the programmed
trigger. The hardware trigger and software refill threshold use the
same constant.

Patch 2/2 raises that trigger from 8 to 32 free spaces. This reduces the
TX refill rate by about 4x, but also reduces the time-to-empty after THRI
from 56 to 32 byte times.

The changes were tested with 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. Each transaction used an 8-byte RX
request and a 255-byte TX response.

For the benchmark, "both TX fixes" includes the standalone kfifo
wrap-around fix and patch 1/2. "Fixes + trigger" additionally includes
patch 2/2.

                         before fixes   both TX fixes   fixes + trigger
                                           trigger=8       trigger=32
  SPI IRQ thread CPU          16%            15%             5%
  system CPU                  49%            44%            29%
  idle CPU                    36%            40%            52%
  one-minute load             2.04           2.02           0.99
  samples                     579            547            535

Comparing the final two configurations isolates patch 2/2. Raising the
trigger reduced median SPI IRQ thread CPU usage from 15% to 5% and
median system CPU usage from 44% to 29%, while median idle CPU increased
from 40% to 52%.

RX trigger tuning and generic RX/TX trigger configuration are left for
follow-up work.

Paul Mbewe (2):
  serial: sc16is7xx: refill TX FIFO below trigger using fresh TXLVL
  serial: sc16is7xx: reduce TX refill rate with half-FIFO trigger

 drivers/tty/serial/sc16is7xx.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)
---
Resent after fixing the mail configuration so that all CC'd reviewers receive
unmodified plain-text copies of the patches. No patch changes since v2.

Changes in v2:
  - Submitted the kfifo wrap-around fix separately as a standalone patch
  - Split the original trigger-level patch into a stale-TXLVL correctness
    fix and an independent trigger-level load-reduction change
  - Re-read TXLVL and refill until free space is strictly below the
    programmed trigger; use the same constant for hardware and software
  - Corrected the trigger trade-off explanation and added post-fix A/B
    CPU and load measurements
  - Deferred generic RX/TX trigger configuration

Link: https://lore.kernel.org/linux-serial/20260623112225.82386-1-paultyson.mbewe@ziehl-abegg.de/
prerequisite-message-id: <20260930143207.542930-1-paultyson.mbewe@ziehl-abegg.de>
prerequisite-patch-id: c177f0e42a7122bbb1accf4397c73ccec4eaabb6

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.43.0

[PATCH v2 RESEND 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@vger.kernel.org
Reported-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
Link: https://lore.kernel.org/linux-serial/20260623112225.82386-3-paultyson.mbewe@ziehl-abegg.de/
Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>
---
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

[PATCH v2 RESEND 2/2] serial: sc16is7xx: reduce TX refill rate with half-FIFO trigger

[Paul Mbewe]

With the TX trigger set to 8 free spaces, THRI is generated roughly once
per 8 transmitted bytes. At 115200 baud 8N1, this corresponds to
approximately 0.7 ms between TX refill events.

Set the TX trigger to 32 free spaces via TLR[3:0]. This makes each refill
larger and reduces the refill rate by about 4x. At 115200 baud 8N1, the
refill cadence becomes approximately 2.8 ms.

The trade-off is that the time-to-empty after THRI asserts is reduced
from 56 to 32 byte times. The fresh-TXLVL refill loop fills the hardware
TX FIFO strictly below whichever trigger is selected. This patch changes
only the refill frequency and the associated latency trade-off.

With the two TX gap fixes applied in both configurations, changing the
trigger from 8 to 32 free spaces produced the following median values
from repeated top snapshots under the same continuous Modbus RTU load.
Each transaction used an 8-byte RX request and a 255-byte TX response,
so the workload was dominated by TX traffic:

                         trigger=8     trigger=32
  SPI IRQ thread CPU        15%            5%
  system CPU                44%           29%
  idle CPU                  40%           52%
  one-minute load           2.02          0.99

The datasets contain 547 snapshots with trigger=8 and 535 snapshots with
trigger=32.

Only TLR[3:0] is changed. TLR[7:4] remains zero so the RX trigger retains
its FCR setting. RX trigger tuning may also be useful, but generic RX/TX
trigger configuration is left for follow-up work.

SC16IS7XX_TX_TRIGGER_LEVEL is used for both the programmed TLR value and
the TXLVL refill-loop threshold, keeping the hardware trigger and the
software refill condition synchronized.

TCR/TLR access requires EFR[4] and MCR[2], which are already enabled by
the TCR setup immediately preceding the TLR write.

Reviewed-by: Joachim Knorr <joachim.knorr@ziehl-abegg.de>
Link: https://lore.kernel.org/linux-serial/20260623112225.82386-3-paultyson.mbewe@ziehl-abegg.de/
Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>
---
Changes in v2:
  - Split the trigger-level change from the stale-TXLVL refill fix
  - Reworked the change as a TX load-reduction patch
  - Corrected the trigger-level explanation: trigger=32 reduces the
    refill rate by about 4x but reduces the time-to-empty after THRI
    from 56 to 32 byte times
  - Added post-fix A/B CPU and load measurements
  - Used one constant for the programmed trigger and refill-loop threshold
  - Left the RX trigger unchanged; generic RX/TX trigger configuration
    is deferred to follow-up work

 drivers/tty/serial/sc16is7xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 1f7e98c2b570..296079c16fdb 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -216,6 +216,6 @@
 #define SC16IS7XX_TLR_RX_TRIGGER(words)	((((words) / 4) & 0x0f) << 4)
 
-#define SC16IS7XX_TX_TRIGGER_LEVEL	8
+#define SC16IS7XX_TX_TRIGGER_LEVEL	32
 
 /* IOControl register bits (Only 75x/76x) */
 #define SC16IS7XX_IOCONTROL_LATCH_BIT	BIT(0)   /* Enable input latching */
-- 
2.43.0

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

[David Laight]

On Wed, 30 Sep 2026 16:56:27 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:

> 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@vger.kernel.org
> Reported-by: Tobias Gannert <tobias.gannert@ziehl-abegg.de>
> Link: https://lore.kernel.org/linux-serial/20260623112225.82386-3-paultyson.mbewe@ziehl-abegg.de/
> Signed-off-by: Paul Mbewe <paultyson.mbewe@ziehl-abegg.de>

Since I made him find the real bug...

Reviewed-by: David Laight <david.laight.linux@gmail.com>

> ---
> 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);
>