On Sat, Sep 29, 2018 at 04:04:48PM +0200, Andreas Färber wrote: > Hi Phil and Greg, > > Am 12.09.18 um 16:31 schrieb Phil Elwell: > > The SC16IS752 is a dual-channel device. The two channels are largely > > independent, but the IRQ signals are wired together as an open-drain, > > active low signal which will be driven low while either of the > > channels requires attention, which can be for significant periods of > > time until operations complete and the interrupt can be acknowledged. > > In that respect it is should be treated as a true level-sensitive IRQ. > > > > The kernel, however, needs to be able to exit interrupt context in > > order to use I2C or SPI to access the device registers (which may > > involve sleeping). Therefore the interrupt needs to be masked out or > > paused in some way. > > > > The usual way to manage sleeping from within an interrupt handler > > is to use a threaded interrupt handler - a regular interrupt routine > > does the minimum amount of work needed to triage the interrupt before > > waking the interrupt service thread. If the threaded IRQ is marked as > > IRQF_ONESHOT the kernel will automatically mask out the interrupt > > until the thread runs to completion. The sc16is7xx driver used to > > use a threaded IRQ, but a patch switched to using a kthread_worker > > in order to set realtime priorities on the handler thread and for > > other optimisations. The end result is non-threaded IRQ that > > schedules some work then returns IRQ_HANDLED, making the kernel > > think that all IRQ processing has completed. > > > > The work-around to prevent a constant stream of interrupts is to > > mark the interrupt as edge-sensitive rather than level-sensitive, > > but interpreting an active-low source as a falling-edge source > > requires care to prevent a total cessation of interrupts. Whereas > > an edge-triggering source will generate a new edge for every interrupt > > condition a level-triggering source will keep the signal at the > > interrupting level until it no longer requires attention; in other > > words, the host won't see another edge until all interrupt conditions > > are cleared. It is therefore vital that the interrupt handler does not > > exit with an outstanding interrupt condition, otherwise the kernel > > will not receive another interrupt unless some other operation causes > > the interrupt state on the device to be cleared. > > > > The existing sc16is7xx driver has a very simple interrupt "thread" > > (kthread_work job) that processes interrupts on each channel in turn > > until there are no more. If both channels are active and the first > > channel starts interrupting while the handler for the second channel > > is running then it will not be detected and an IRQ stall ensues. This > > could be handled easily if there was a shared IRQ status register, or > > a convenient way to determine if the IRQ had been deasserted for any > > length of time, but both appear to be lacking. > > > > Avoid this problem (or at least make it much less likely to happen) > > by reducing the granularity of per-channel interrupt processing > > to one condition per iteration, only exiting the overall loop when > > both channels are no longer interrupting. > > > > Signed-off-by: Phil Elwell <phil@xxxxxxxxxxxxxxx> > > --- > > drivers/tty/serial/sc16is7xx.c | 19 +++++++++++++------ > > 1 file changed, 13 insertions(+), 6 deletions(-) > > These two patches seem to be applied in linux-next tree, but are lacking > a Fixes: header for backporting to affected stable trees. > > openSUSE Tumbleweed's 4.18 appears to be affected, and I didn't see it > in linux.git for upcoming 4.19 either. > > Can the commit message still be updated to get this fixed everywhere? I can't change the tree, sorry, I can not rebase. If you want these applied to the stable tree, please email stable@xxxxxxxxxxxxxxx when they hit Linus's tree with the git commit ids and I will be glad to queue them up then. thanks, greg k-h
