On 26 February 2016 11:52:28 GMT-05:00, Josh Cartwright [off-list ref] wrote:

On Fri, Feb 26, 2016 at 09:34:57AM -0500, Kuba Kicinski wrote:

quoted

On 26 February 2016 06:48:09 GMT-05:00, Sebastian Andrzej Siewior

[off-list ref] wrote:

quoted

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.

Yes, I definitely dropped the ball there.

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.

The problem there being IIRC that i2c doesn't provide async writes so we can't mask from irq callback. The only option would be disable_irq/enable_irq, right?