genirq: Fix handle_nested_irq() for IRQF_SHARED
PATCH 1/1 genirq: Fix handle_nested_irq() for IRQF_SHARED
Shared interrupts can be registered with handle_nested_irq(), but currently only one of the registered handlers gets called.
The use of shared interrupts with handle_nested_irq() is probably rare, but at least a case of a shared VBUS interrupt between USB PHY and battery charger drivers for a PMIC makes sense.
Typically for_each_action_of_desc() gets called via handle_irq_event() at _handle_irq_event_percpu(), but handle_nested_irq() is different because of it's dummy irq_nested_primary_handler().
Let's fix the issue by calling the handler for all the entries in the action list.
--- kernel/irq/chip.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
kernel/irq/chip.c +5 -3
@@ -348,9 +348,11 @@ void handle_nested_irq(unsigned int irq)irqd_set (&desc->irq_data,IRQD_IRQ_INPROGRESS );raw_spin_unlock_irq (&desc->lock); - action_ret = action->thread_fn (action->irq, action->dev_id); -if (!noirqdebug) -note_interrupt (desc, action_ret); +for_each_action_of_desc (desc, action) { + action_ret = action->thread_fn (action->irq, action->dev_id); +if (!noirqdebug) +note_interrupt (desc, action_ret); + }raw_spin_lock_irq (&desc->lock);irqd_clear (&desc->irq_data,IRQD_IRQ_INPROGRESS );
Signature · 1 line
2.11.1
Quoted 7 lines · Tony Lindgren
Shared interrupts can be registered with
handle_nested_irq(), but currently only one of the registered handlers gets called.The use of shared interrupts with
handle_nested_irq()is probably rare, but at least a case of a shared VBUS interrupt between USB PHY and battery charger drivers for a PMIC makes sense.
Quoted 8 lines
Typically
for_each_action_of_desc()gets called viahandle_irq_event()at_handle_irq_event_percpu(), buthandle_nested_irq()is different because of it's dummyirq_nested_primary_handler().Let's fix the issue by calling the handler for all the entries in the action list.
Fixes 399b5da29b9f ("genirq: Support nested threaded irq handling")
It's not a fix, it's an extension. The nested facility was not meant to handle shared interrupts in the first place. I really hope that hardware folks finally understand that irq sharing is crap and broken...
Quoted 9 lines
Signed-off-by Tony Lindgren <[email protected]>--- kernel/irq/chip.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
kernel/irq/chip.c void handle_nested_irq(unsigned int irq) · line 354
Quoted 10 lines
kernel/irq/chip.c +4
-3@@ -348,9 +348,11 @@ void handle_nested_irq(unsigned int irq)irqd_set (&desc->irq_data,IRQD_IRQ_INPROGRESS );raw_spin_unlock_irq (&desc->lock); - action_ret = action->thread_fn (action->irq, action->dev_id); -if (!noirqdebug) -note_interrupt (desc, action_ret); +for_each_action_of_desc (desc, action) { + action_ret = action->thread_fn (action->irq, action->dev_id); +if (!noirqdebug) +note_interrupt (desc, action_ret);
This is wrong. See how __handle_irq_event_percpu() does this. But no need to resend. See:
http://lkml.kernel.org/r/1488904098-5350-1-git-send-email-ckeepax@opensource.wolfsonmicro.com
Thanks,
tglx
Quoted 13 lines · Thomas Gleixner
Quoted 11 lines · Tony Lindgren
Shared interrupts can be registered with
handle_nested_irq(), but currently only one of the registered handlers gets called.The use of shared interrupts with
handle_nested_irq()is probably rare, but at least a case of a shared VBUS interrupt between USB PHY and battery charger drivers for a PMIC makes sense.Typically
for_each_action_of_desc()gets called viahandle_irq_event()at_handle_irq_event_percpu(), buthandle_nested_irq()is different because of it's dummyirq_nested_primary_handler().
Quoted 8 lines
Quoted 4 lines
Let's fix the issue by calling the handler for all the entries in the action list.
Fixes 399b5da29b9f ("genirq: Support nested threaded irq handling")It's not a fix, it's an extension. The nested facility was not meant to handle shared interrupts in the first place. I really hope that hardware folks finally understand that irq sharing is crap and broken...
Yes totally.
Quoted 9 lines
kernel/irq/chip.c void handle_nested_irq(unsigned int irq) · line 354
Quoted 4 lines
kernel/irq/chip.c +4
-0+for_each_action_of_desc (desc, action) { + action_ret = action->thread_fn (action->irq, action->dev_id); +if (!noirqdebug) +note_interrupt (desc, action_ret);This is wrong. See how
__handle_irq_event_percpu()does this. But no need to resend. See:http://lkml.kernel.org/r/1488904098-5350-1-git-send-email-ckeepax@opensource.wolfsonmicro.com
Oh OK yeah makes sense, my patch is overwriting action_ret..
Thanks,
Tony