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 [off-list ref] wrote:

quoted

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@linutronix.de>

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