Re: [PATCH] tty: serial: sc16is7xx: use threaded interrupts instead of homegrow

[Kuba Kicinski <kubakici@wp.pl>]

On 26 February 2016 11:52:28 GMT-05:00, Josh Cartwright <joshc@ni.com> wrote:
>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@linutronix.de> 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@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?