Date: Sat, 13 Feb 2016 06:53:25 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Marius Strobl <marius@freebsd.org> Cc: Michal Meloun <mmel@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r295557 - head/sys/dev/uart Message-ID: <20160213041246.V1974@besplex.bde.org> In-Reply-To: <20160212164755.GC4980@alchemy.franken.de> References: <201602120514.u1C5EwWt053622@repo.freebsd.org> <20160212164755.GC4980@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160213041246.V1974>