On Fri, Feb 26, 2016 at 09:34:57AM -0500, Kuba Kicinski wrote: > On 26 February 2016 06:48:09 GMT-05:00, Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote: > >This ONESHOT + workqueue combo is something that is not required > >because > >we have infrastrucure for this kind of things: threaded interrupts. > > > >This is compile tested only due to -ENODEV. > >Now that we that sc16is7xx_irq() is an actual interrupt handler > >sc16is7xx_port_irq() could be improved so the former can return > >IRQ_NONE > >if nothing has been done. > > > >Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> > > The reason why this driver is not using threaded irqs is that it > already has a kthread to handle other things. The core problem here is still the use of IRQF_ONESHOT. The semantics of IRQF_ONESHOT are only defined w.r.t. the irq core's threaded interrupt handling. If the driver isn't going to make use of the existing irqthread functionality, then it _also_ should not be making use of IRQF_ONESHOT. Instead, the driver needs to implement it's own oneshot-like handling at the device-level: in the registered irq handler, capture triggered interrupt state, squelch/mask, and enqueue the kthread_work. In the tail-end of the kthread_work, re-enable interrupts at the device level. Now, with forced_irqthreads, this still isn't ideal, as we'd have two threads, but perhaps the right way forward there is to be more clever at the kthread_worker() layer to make it's work list management amenable to being done under a raw spinlock. Josh
Attachment:
signature.asc
Description: PGP signature
