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

[Paul Mbewe]

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.

     The scope confirms that the INT line stays deasserted in this case,
     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.

  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.

  - 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.

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