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

[Paul Mbewe]

Hi David, Maarten,

I rechecked this with ftrace, and I think the right explanation is
narrower than my v1 commit message.

The underrun trace shows the SC16IS7xx refill path running with the
hardware TX FIFO already empty while the tty xmit FIFO still had data
queued, for example:

  sc16is7xx delayed refill: line=0 txlvl=64 pending=172

So the conclusions for me are:

  - The driver can catch up if the FIFO is only partially drained when
    the THRI path runs. TXLVL will report more than the nominal trigger
    amount and the driver can write more bytes.

  - Once TXLVL is 64, the FIFO has already emptied and the TX gap has
    already occurred.

  - The v1 explanation was wrong to describe a 32-free-space trigger as
    increasing the per-interrupt refill window. It does not. With the
    default trigger, the FIFO has about 56 character times left when THRI
    fires; with a 32-free-space trigger, it has about 32 character times
    left.

  - As Maarten pointed out, the benefit is the other side of the
    trade-off: the refill event rate is reduced by 4x, from roughly
    1 refill per 8 transmitted bytes to 1 refill per 32 transmitted bytes.
    
  - On the tested SPI-backed system, that lower refill rate reduces the
    IRQ/SPI load enough to avoid the observed underruns.

The change is limited to SPI-attached parts so the I2C case is
unaffected.

For v2 I will:

  - rework the commit message around Maarten's trade-off calculation:
    4x fewer TX refill events, at the cost of reducing the time-to-empty
    margin from 56 to 32 character times;
  - limit the TX trigger change to SPI-backed SC16IS7xx devices;
  - leave the RX trigger level unchanged.

If maintainers would prefer this to be board-configurable instead, I can
look at a follow-up DT binding/property, but I would prefer to keep v2
focused on the TX/SPI case.

Thanks,
Paul