[PATCH] genirq: handle shared irqs in handle_nested_irq()

[PATCH] genirq: handle shared irqs in handle_nested_irq()

[Rabin Vincent]

Allow shared irqs to work when nested threads are used for handling
threaded interrupts.

Signed-off-by: Rabin Vincent <rabin@rab.in>
---
 kernel/irq/chip.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index baa5c4a..94a45a9 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -418,8 +418,8 @@ static inline void unmask_irq(struct irq_desc *desc)
 void handle_nested_irq(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
+	irqreturn_t action_ret = IRQ_NONE;
 	struct irqaction *action;
-	irqreturn_t action_ret;
 
 	might_sleep();
 
@@ -434,7 +434,10 @@ void handle_nested_irq(unsigned int irq)
 	desc->status |= IRQ_INPROGRESS;
 	raw_spin_unlock_irq(&desc->lock);
 
-	action_ret = action->thread_fn(action->irq, action->dev_id);
+	do {
+		action_ret |= action->thread_fn(action->irq, action->dev_id);
+		action = action->next;
+	} while (action);
 	if (!noirqdebug)
 		note_interrupt(irq, desc, action_ret);
 
-- 
1.7.2.3

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Thomas Gleixner]

On Thu, 28 Oct 2010, Rabin Vincent wrote:

> Allow shared irqs to work when nested threads are used for handling
> threaded interrupts.

Why do we want this ? handle_nested_irq() is called from a interrupt
demultiplexer, so why would we need shared interrupts for this ?

Thanks,

	tglx
 
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
>  kernel/irq/chip.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index baa5c4a..94a45a9 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -418,8 +418,8 @@ static inline void unmask_irq(struct irq_desc *desc)
>  void handle_nested_irq(unsigned int irq)
>  {
>  	struct irq_desc *desc = irq_to_desc(irq);
> +	irqreturn_t action_ret = IRQ_NONE;
>  	struct irqaction *action;
> -	irqreturn_t action_ret;
>  
>  	might_sleep();
>  
> @@ -434,7 +434,10 @@ void handle_nested_irq(unsigned int irq)
>  	desc->status |= IRQ_INPROGRESS;
>  	raw_spin_unlock_irq(&desc->lock);
>  
> -	action_ret = action->thread_fn(action->irq, action->dev_id);
> +	do {
> +		action_ret |= action->thread_fn(action->irq, action->dev_id);
> +		action = action->next;
> +	} while (action);
>  	if (!noirqdebug)
>  		note_interrupt(irq, desc, action_ret);
>  
> -- 
> 1.7.2.3
> 

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Rabin Vincent]

On Fri, Oct 29, 2010 at 12:24 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 28 Oct 2010, Rabin Vincent wrote:
>> Allow shared irqs to work when nested threads are used for handling
>> threaded interrupts.
>
> Why do we want this ? handle_nested_irq() is called from a interrupt
> demultiplexer, so why would we need shared interrupts for this ?

Threaded IRQ + handle_nested_irq() is used by I2C GPIO expander drivers
like chained hander + generic_handle_irq() is used by on-chip GPIO
drivers.  If two irq handlers are registered on an on-chip GPIO
interrupt (because two devices are externally connected), it works.  If
the handlers are instead registered on one of the expander's pins'
interrupt, without this patch, only the handler registered first is
called.

Rabin

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Thomas Gleixner]

On Fri, 29 Oct 2010, Rabin Vincent wrote:

> On Fri, Oct 29, 2010 at 12:24 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 28 Oct 2010, Rabin Vincent wrote:
> >> Allow shared irqs to work when nested threads are used for handling
> >> threaded interrupts.
> >
> > Why do we want this ? handle_nested_irq() is called from a interrupt
> > demultiplexer, so why would we need shared interrupts for this ?
> 
> Threaded IRQ + handle_nested_irq() is used by I2C GPIO expander drivers
> like chained hander + generic_handle_irq() is used by on-chip GPIO
> drivers.  If two irq handlers are registered on an on-chip GPIO
> interrupt (because two devices are externally connected), it works.  If
> the handlers are instead registered on one of the expander's pins'
> interrupt, without this patch, only the handler registered first is
> called.

Have the hardware dudes not yet understood that shared interrupts are
a horrible idea ?

Sigh,

	tglx

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Rabin Vincent]

On Fri, Oct 29, 2010 at 8:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 29 Oct 2010, Rabin Vincent wrote:
>> On Fri, Oct 29, 2010 at 12:24 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 28 Oct 2010, Rabin Vincent wrote:
>> >> Allow shared irqs to work when nested threads are used for handling
>> >> threaded interrupts.
>> >
>> > Why do we want this ? handle_nested_irq() is called from a interrupt
>> > demultiplexer, so why would we need shared interrupts for this ?
>>
>> Threaded IRQ + handle_nested_irq() is used by I2C GPIO expander drivers
>> like chained hander + generic_handle_irq() is used by on-chip GPIO
>> drivers.  If two irq handlers are registered on an on-chip GPIO
>> interrupt (because two devices are externally connected), it works.  If
>> the handlers are instead registered on one of the expander's pins'
>> interrupt, without this patch, only the handler registered first is
>> called.
>
> Have the hardware dudes not yet understood that shared interrupts are
> a horrible idea ?

Note that I haven't actually seen the multiple devices to expander GPIO
(yet?, though I've seen the multiple devices to onchip GPIO).  It seemed
to be something missing from the irq handling though, rather than
something that was left out on purpose.

Rabin

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1481 bytes --]

On Fri, 29 Oct 2010, Rabin Vincent wrote:

> On Fri, Oct 29, 2010 at 8:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 29 Oct 2010, Rabin Vincent wrote:
> >> On Fri, Oct 29, 2010 at 12:24 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > On Thu, 28 Oct 2010, Rabin Vincent wrote:
> >> >> Allow shared irqs to work when nested threads are used for handling
> >> >> threaded interrupts.
> >> >
> >> > Why do we want this ? handle_nested_irq() is called from a interrupt
> >> > demultiplexer, so why would we need shared interrupts for this ?
> >>
> >> Threaded IRQ + handle_nested_irq() is used by I2C GPIO expander drivers
> >> like chained hander + generic_handle_irq() is used by on-chip GPIO
> >> drivers.  If two irq handlers are registered on an on-chip GPIO
> >> interrupt (because two devices are externally connected), it works.  If
> >> the handlers are instead registered on one of the expander's pins'
> >> interrupt, without this patch, only the handler registered first is
> >> called.
> >
> > Have the hardware dudes not yet understood that shared interrupts are
> > a horrible idea ?
> 
> Note that I haven't actually seen the multiple devices to expander GPIO
> (yet?, though I've seen the multiple devices to onchip GPIO).  It seemed
> to be something missing from the irq handling though, rather than
> something that was left out on purpose.

No, I left it out on purpose and I'd rather go w/o it unless there is
a intree user.

Thanks,

	tglx

Re: [PATCH] genirq: handle shared irqs in handle_nested_irq()

[Rabin Vincent]

On Fri, Oct 29, 2010 at 10:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 29 Oct 2010, Rabin Vincent wrote:
>> On Fri, Oct 29, 2010 at 8:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Fri, 29 Oct 2010, Rabin Vincent wrote:
>> >> On Fri, Oct 29, 2010 at 12:24 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> > On Thu, 28 Oct 2010, Rabin Vincent wrote:
>> >> >> Allow shared irqs to work when nested threads are used for handling
>> >> >> threaded interrupts.
>> >> >
>> >> > Why do we want this ? handle_nested_irq() is called from a interrupt
>> >> > demultiplexer, so why would we need shared interrupts for this ?
>> >>
>> >> Threaded IRQ + handle_nested_irq() is used by I2C GPIO expander drivers
>> >> like chained hander + generic_handle_irq() is used by on-chip GPIO
>> >> drivers.  If two irq handlers are registered on an on-chip GPIO
>> >> interrupt (because two devices are externally connected), it works.  If
>> >> the handlers are instead registered on one of the expander's pins'
>> >> interrupt, without this patch, only the handler registered first is
>> >> called.
>> >
>> > Have the hardware dudes not yet understood that shared interrupts are
>> > a horrible idea ?
>>
>> Note that I haven't actually seen the multiple devices to expander GPIO
>> (yet?, though I've seen the multiple devices to onchip GPIO).  It seemed
>> to be something missing from the irq handling though, rather than
>> something that was left out on purpose.
>
> No, I left it out on purpose and I'd rather go w/o it unless there is
> a intree user.

OK, fair enough.

Thanks,
Rabin