Re: [PATCH 2/2] serial: sc16is7xx: set TX FIFO trigger level to half FIFO to prevent underruns

[David Laight <david.laight.linux@gmail.com>]

On Fri, 3 Jul 2026 20:19:39 +0200
Paul Mbewe <paultyson.mbewe@ziehl-abegg.de> wrote:

> Hi David, Maarten,
> 
> short status update before v2.
> 
> David, your ethernet-rx double-check analogy reinforced the direction I
> had started investigating from the ftrace data. With ftrace plus a scope
> on the INT line, I now see two refill-latency modes:
> 
>   1. Under-fill, common case.
> 
>      sc16is7xx reads TXLVL once and writes that many bytes. On a slow SPI
>      write, the wire can drain about as fast as the driver fills. The FIFO
>      free space then stays at or above the trigger level, no new threshold
>      crossing occurs, and no further THRI is generated until the FIFO is
>      empty.

That doesn't sound very good at all.
You need the refill path to be much faster then the drain path.
It might be worth testing at a much lower baud rate to see how it
'should' operate.
(I've run serial code at 'single digit' baud rates in the past, needed
to see exactly how the zilog scc handled sending crc at the end of
hdlc frames and just how broken its reporting of received aborts was.) 
That path also sounds like it doesn't depend on the fifo trigger level.

One thing I did notice is that the initial write doesn't loop and only
puts one block of data into the fifo.
The isr isn't enabled until the end (and by a diffrent thread).
The block of data also isn't guaranteed to fill the hardware fifo,
it only contains data to the end of the software ring buffer - so
might only be 1 byte.
After a partial write to the fifo, the code ought to try to get more
data from the ring buffer before doing anything else.

> 
>      The scope confirms that the INT line stays deasserted in this case,

Are you sure?
I'd expect it to be active low and asserted throughout.
I think the 'threaded ISR' code masks the interrupt on the interrupt
controller until the ISR thread finishes.
(I've NFI how this actually works, especially if PCIe MSI(-X) interrupts
are involved.)

>      so this is not a lost SoC edge. max310x avoids this pattern by
>      re-reading the FIFO level each refill iteration and filling until the
>      xmit FIFO is empty or the hardware FIFO is full. I am testing the same
>      refill pattern for sc16is7xx.

I'd look up what causes the IIR to report THRI.
It might be based in the fifo level (and whether the ISR is enabled),
but might be cleared by reads and set when the fifo crosses the level.
It seems odd that you are seeing THRI read multiple times but the IRQ
line is inactive, perhaps the IRQ line gets pulsed - that will make life hard.

There is a timing window between the read of the IIR that return 'nothing to do'
and the system re-enabling the hardware interrupt when the threaded ISR finishes.
The hardware interrupt has to fire at that point.

I also noticed that the code seems to be doing slow RMW cycles at the end of
every tx to enable/disable the tx interrupt.
The read shouldn't be needed (the driver knows the current value) and the write
is almost certainly not needed unless the value has changed.
The whole operation is also deferred to a different worker - that can't help.


> 
>   2. Preempted refill, rarer case.
> 
>      Some traces show the IRQ thread being delayed during the refill path.
>      In those cases the FIFO does appear to be pushed below the trigger,
>      but the next THRI still does not arrive until the FIFO is empty. That
>      looks closer to your "threshold reached while the threaded ISR is
>      still running and the re-arm is lost" case.
> 
>      I want to see whether this still survives the TXLVL re-read fix before
>      adding a TX watchdog.
> 
> On your specific questions:
> 
>   - The trailing sc16is7xx_ier_set(THRI) is a 1->1 update mid-message. The
>     bit is already set, so it does not create a fresh event. The driver
>     relies on the FIFO being refilled far enough for the chip to produce a
>     new threshold crossing.

So you mean the the chip is generating 'threshold crossing' interrupt
rather than a 'threshold level' one.
That relies on the driver filling the fifo enough.
Given it only writes the bytes that are contiguous in the ring buffer
it might only write one byte.

>   - trigger=32 is not a wider refill window. It gives about 4x fewer
>     refills, lowering IRQ/SPI load and reducing exposure to these latency
>     outliers. I will keep it as a separate load-reduction patch with the
>     corrected wording, no longer as the root fix.

That was my original thought.
Something subtle makes it happen less often, but whatever it is can
still happen.
OTOH if the interrupt latency is 'good enough' for trigger=32 the reduced
system/isr load is probably a good idea.

	David

> 
> Maarten, I will drop the SPI-only guard. I also agree that configurable
> RX/TX trigger levels belong with Crescent's rx_trig_bytes work as a
> follow-up, rather than blocking this underrun fix.
> 
> I will follow up with the A/B numbers on Monday.
> 
> Thanks,
> Paul
> 
> _______________________________________ 
> 
> ZIEHL-ABEGG 
> 
> Executive Board: Joachim Ley (Chairman), Marco Altherr, Wolfgang Mayer
> Supervisory Board: Dennis Ziehl (Chairman) 
> 
> Court of Registry: District Court Stuttgart HRB 746188
> Company Seat: Künzelsau, Germany 
> 
> Der Inhalt dieser E-Mail und/oder jegliche Anhänge können vertrauliche Mitteilungen enthalten und sind ausschließlich für den bezeichneten Adressaten bestimmt.
> Wenn Sie nicht der vorgesehene Adressat dieser E-Mail oder dessen Vertreter sein sollten, so beachten Sie bitte, dass jede Form der Kenntnisnahme, Veröffentlichung,
> Vervielfältigung oder Weitergabe des Inhalts dieser E-Mail einschließlich Anhängen unzulässig ist.
> Wir bitten Sie, sich in diesem Fall mit dem Absender der E-Mail in Verbindung zu setzen. 
> 
> The content of this e-mail and/or attachments may contain confidential information and is intended solely for the named recipient.
> If you are not the intended recipient of this e-mail or on its distribution list, please note that any type of disclosure, publication,
> copying or distribution of the content of this e-mail including attachments is strictly forbidden.
> In this case, we would kindly ask you to notify the sender of the e-mail.