svn commit: r295557 - head/sys/dev/uart

[Bruce Evans]

On Fri, 12 Feb 2016, Marius Strobl wrote:

> On Fri, Feb 12, 2016 at 05:14:58AM +0000, Michal Meloun wrote:
>> Author: mmel
>> Date: Fri Feb 12 05:14:58 2016
>> New Revision: 295557
>> URL: https://svnweb.freebsd.org/changeset/base/295557
>>
>> Log:
>>   UART: Fix spurious interrupts generated by ns8250 and lpc drivers:
>>    - don't enable transmitter empty interrupt before filling TX FIFO.
>
> Are you sure this doesn't create a race that leads to lost TX ready
> interrupts? For a single character, the TX FIFO very well may be empty
> again at the point in time IER_ETXRDY is enabled now.  With the varying
> behavior of 8250/16x50 chips - some of which is documented in sio(4) -

That is mostly FUD.  More likely driver bugs than chip bugs.

A non-broken xx50 interrupts after you (re)enable tx interrupts, iff
the fifo is already empty.  This gives a "spurious" interrupt.  But
perhaps depending on this is too fragile.  Normal operation is to keep
the tx interrupt enabled and depend on writing to the fifo causing a
tx interrupt later.  But it is a more common chip bug for tx interrupts
later to not go away when they should (normally by reading the IIR),
so some drivers toggle the tx interrupt enable dynamically.

An example of a driver bug is only enabling tx interrupts for this.
It takes a transition of the interrupt enable bit from off to on to
get the interrupt.  Other driver bugs may result in a missing transition
because the bit was supposed to be off but is actually on.

> I'd expect there are many that no longer generate a TX ready at all
> with this change in place. In this case, receiving spurious interrupts
> (which ones? IIR_NOPEND? IIR_TXRDY?) with some devices appears to be
> the lesser evil.

Not many.  Only broken ones.  The "spurious" interrupts are just normal
ones from bon-broken chips:

- uart first does a potentially-unbounded busy-wait before the doing
   anything to ensure that the fifo is empty.  This should be unecessary
   since this function should not be called unless sc_txbusy is 0 and
   sc_txbusy should be nonzero if the fifo is not empty.  If it is called
   when the fifo is not emptu, then the worst-case busy-wait is approx.
   640 seconds for a 128-byte fifo at 2 bps. The 'broken_txfifo case'
   busy-waits for a long time in normal operation.
- enabling the tx interrupt causes one immediately on non-broken uarts
- the interrupt handler is normally called immediately.  Then it always
   blocks on uart_lock()
- then the main code fills the fifo and unlocks
- then the interrupt handler runs.  It normally finds that the fifo is
   not empty (since it has just been filled) and does nothing
- another tx interrupt occurs later and the interrupt handler runs again.
   It normally doesn't hit the lock again, and normally finds the fifo
   empty, so it does something.

But you are probably correct that a 1-byte write to the fifo often
loses the race.  This depends on how fast the hardware moves the byte
from the fifo to the tx register.  Actually, since we didn't wait
for the tx register to become empty, it will often take a full character
time before the move.  After that, I think it might take 1 bit time but
no more.

sio uses a timeout to recover from lost output interrupts in case they
occur because the driver or hardware has unforseen bugs.  This works
too well.  It hides bugs like interrupts not working at all provided
the fifo size is larger enough for the low timeout frequency to give
enough throughput.  On newer systems, the overhead for timeouts is in
the noise compared with the overhead for bus accesses (especially
reads), so a simple timeout method is almost as good as an
interrupt-driven method.  It can probably be even better for output
by calculating times to avoid slow reads of status registers to see
how far the chip got.

Bruce