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

[Paul Mbewe]

Hi David,

On the test system, the relevant threads already run with RT priority:

  irq/134-spi2.0     SCHED_FIFO priority 50
  sc16is7xx          SCHED_FIFO priority 50

So this is not caused by the IRQ thread running as a normal SCHED_OTHER
task. That does not remove all latency sources, of course; on this
single-core SPI system the threaded IRQ can still be affected by other
RT/kernel activity, IRQ/preemption-disabled sections, SPI transfer time,
or lock contention.

I agree, changing the TX trigger from 8 to 32 free spaces reduces the
per-interrupt time-to-empty margin. With an 8-space trigger the FIFO
still contains 56 bytes when THRI asserts, while with a 32-space trigger
it contains 32 bytes. So the v1 commit message is misleading when it
describes this as increasing the refill window.

The observed effect is instead that the 8-space trigger causes many
small TX refill events. Each event has roughly the same cost, as you
said. If the handler runs in time, it can catch up by seeing more than
8 free spaces and writing more data. The failure happens when one event
is delayed long enough for the FIFO to drain.

Using a 32-space trigger reduces the number of refill events and the
associated IRQ/SPI load. It also reduces the chance that one delayed
event lets the FIFO drain completely. On the tested setup this reduced
irq/134-spi2.0 CPU usage from about 15-17% to about 5%, sys CPU from
about 51-61% to about 19-28%, and load average from about 2.0-2.2 to
about 0.65-1.3. With that change, the observed TX gaps disappeared.

So I agree the commit message should be reworked to describe this as
reducing TX refill events and IRQ/SPI load, not as increasing the
per-interrupt latency margin.

If changing the default trigger globally is considered too broad, I can
also look at making the TX trigger configurable or limiting the change to
SPI-backed devices.

Thanks
Paul