[PATCH] nmk-gpio: use request_irq instead of chained handler

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Rabin Vincent]

__nmk_gpio_irq_handler doesn't do anything more that what a normal handler
does, so it can become one.  This is also needed for changing the GIC
implementation to a different flow handler.

Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/plat-nomadik/gpio.c |   54 ++++++++++++++++++++---------------------
 1 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/arch/arm/plat-nomadik/gpio.c b/arch/arm/plat-nomadik/gpio.c
index 45b1cf9..efa6737 100644
--- a/arch/arm/plat-nomadik/gpio.c
+++ b/arch/arm/plat-nomadik/gpio.c
@@ -651,22 +651,14 @@ static struct irq_chip nmk_gpio_irq_chip = {
 	.irq_set_wake	= nmk_gpio_irq_set_wake,
 };
 
-static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
-				   u32 status)
+static irqreturn_t
+__nmk_gpio_irq_handler(struct nmk_gpio_chip *nmk_chip, u32 status)
 {
-	struct nmk_gpio_chip *nmk_chip;
-	struct irq_chip *host_chip = get_irq_chip(irq);
 	unsigned int first_irq;
 
-	if (host_chip->irq_mask_ack)
-		host_chip->irq_mask_ack(&desc->irq_data);
-	else {
-		host_chip->irq_mask(&desc->irq_data);
-		if (host_chip->irq_ack)
-			host_chip->irq_ack(&desc->irq_data);
-	}
+	if (!status)
+		return IRQ_NONE;
 
-	nmk_chip = get_irq_data(irq);
 	first_irq = NOMADIK_GPIO_TO_IRQ(nmk_chip->chip.base);
 	while (status) {
 		int bit = __ffs(status);
@@ -675,29 +667,30 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 		status &= ~BIT(bit);
 	}
 
-	host_chip->irq_unmask(&desc->irq_data);
+	return IRQ_HANDLED;
 }
 
-static void nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
+static irqreturn_t nmk_gpio_irq_handler(int irq, void *dev_id)
 {
-	struct nmk_gpio_chip *nmk_chip = get_irq_data(irq);
+	struct nmk_gpio_chip *nmk_chip = dev_id;
 	u32 status = readl(nmk_chip->addr + NMK_GPIO_IS);
 
-	__nmk_gpio_irq_handler(irq, desc, status);
+	return __nmk_gpio_irq_handler(nmk_chip, status);
 }
 
-static void nmk_gpio_secondary_irq_handler(unsigned int irq,
-					   struct irq_desc *desc)
+static irqreturn_t nmk_gpio_secondary_irq_handler(int irq, void *dev_id)
 {
-	struct nmk_gpio_chip *nmk_chip = get_irq_data(irq);
+	struct nmk_gpio_chip *nmk_chip = dev_id;
 	u32 status = nmk_chip->get_secondary_status(nmk_chip->bank);
 
-	__nmk_gpio_irq_handler(irq, desc, status);
+	return __nmk_gpio_irq_handler(nmk_chip, status);
 }
 
-static int nmk_gpio_init_irq(struct nmk_gpio_chip *nmk_chip)
+static void nmk_gpio_init_irq(struct nmk_gpio_chip *nmk_chip)
 {
+	struct gpio_chip *gpio_chip = &nmk_chip->chip;
 	unsigned int first_irq;
+	int ret;
 	int i;
 
 	first_irq = NOMADIK_GPIO_TO_IRQ(nmk_chip->chip.base);
@@ -709,16 +702,21 @@ static int nmk_gpio_init_irq(struct nmk_gpio_chip *nmk_chip)
 		set_irq_type(i, IRQ_TYPE_EDGE_FALLING);
 	}
 
-	set_irq_chained_handler(nmk_chip->parent_irq, nmk_gpio_irq_handler);
-	set_irq_data(nmk_chip->parent_irq, nmk_chip);
+	ret = request_irq(nmk_chip->parent_irq, nmk_gpio_irq_handler,
+			  IRQF_NO_SUSPEND, gpio_chip->label, nmk_chip);
+	if (ret)
+		dev_err(gpio_chip->dev, "Unable to request IRQ%d: %d\n",
+			nmk_chip->parent_irq, ret);
 
 	if (nmk_chip->secondary_parent_irq >= 0) {
-		set_irq_chained_handler(nmk_chip->secondary_parent_irq,
-					nmk_gpio_secondary_irq_handler);
-		set_irq_data(nmk_chip->secondary_parent_irq, nmk_chip);
-	}
+		ret = request_irq(nmk_chip->secondary_parent_irq,
+				  nmk_gpio_secondary_irq_handler,
+				  IRQF_NO_SUSPEND, gpio_chip->label, nmk_chip);
 
-	return 0;
+		if (ret)
+			dev_err(gpio_chip->dev, "Unable to request secondary IRQ%d: %d\n",
+				nmk_chip->secondary_parent_irq, ret);
+	}
 }
 
 /* I/O Functions */
-- 
1.7.2.dirty

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Russell King - ARM Linux]

On Wed, Feb 23, 2011 at 06:24:50PM +0530, Rabin Vincent wrote:
> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
> does, so it can become one.  This is also needed for changing the GIC
> implementation to a different flow handler.

So you want to allow other people to request the GPIO interrupt?

You also want to have the IRQ tracing code called once for entry into
this handler and once for each GPIO interrupt which is pending?

You want to have the overhead of dealing with the inprogress stuff, etc?

I really don't like the idea of getting rid of the interrupt chaining
and reverting back to the proven-to-be-broken 2.2 kernel way of handing
this kind of thing - which is exactly what you're doing here.

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Rabin Vincent]

On Wed, Feb 23, 2011 at 18:48, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 23, 2011 at 06:24:50PM +0530, Rabin Vincent wrote:
>> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
>> does, so it can become one. ?This is also needed for changing the GIC
>> implementation to a different flow handler.
>
> So you want to allow other people to request the GPIO interrupt?

No, but we request it without IRQF_SHARED so why would other people be
able to do so?  If they had done it before, there will be an error here.

> You also want to have the IRQ tracing code called once for entry into
> this handler and once for each GPIO interrupt which is pending?

Because the GPIO interrupts come via a different parent interrupt (the
secondary handler stuff) when in certain low power states, it may be
useful to see which parent the interrupt comes from.  Not a deal breaker
though.

> You want to have the overhead of dealing with the inprogress stuff, etc?
>
> I really don't like the idea of getting rid of the interrupt chaining
> and reverting back to the proven-to-be-broken 2.2 kernel way of handing
> this kind of thing - which is exactly what you're doing here.

I thought it was the recommendation in the other thread to have chained
interrupts that do nothing but the normal irq handling to just use that.
With the GIC fasteoi change we will anyway not have the interrupt being
masked.  The secondary irq handler chip uses handle_simple_irq so that
should be OK too.

Anyway, I guess there's no strong reason to drop the chained handler,
and we can do without any unnecessary overheads so I will just preserve
the chained handler and do only the eoi here (along with the GIC
conversion).

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Will Deacon]

Rabin,

> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
> does, so it can become one.  This is also needed for changing the GIC
> implementation to a different flow handler.
> 
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>

mach-nomadik only seems to use the vic. Why does this GPIO handler
need updating?

Will

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Rabin Vincent]

On Wed, Feb 23, 2011 at 22:47, Will Deacon <will.deacon@arm.com> wrote:
>> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
>> does, so it can become one. ?This is also needed for changing the GIC
>> implementation to a different flow handler.
>>
>> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
>
> mach-nomadik only seems to use the vic. Why does this GPIO handler
> need updating?

plat-nomadik/gpio.c is also used by mach-ux500, which uses the GIC.

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Will Deacon]

> On Wed, Feb 23, 2011 at 22:47, Will Deacon <will.deacon@arm.com> wrote:
> >> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
> >> does, so it can become one. ?This is also needed for changing the GIC
> >> implementation to a different flow handler.
> >>
> >> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> >
> > mach-nomadik only seems to use the vic. Why does this GPIO handler
> > need updating?
> 
> plat-nomadik/gpio.c is also used by mach-ux500, which uses the GIC.

Ah yes, sorry I missed that one. I'm happy to include any patches from you
in the GIC fasteoi series if it makes merging easier.

Will

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Linus Walleij]

2011/2/23 Will Deacon <will.deacon@arm.com>:
>> On Wed, Feb 23, 2011 at 22:47, Will Deacon <will.deacon@arm.com> wrote:
>> >> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
>> >> does, so it can become one. ?This is also needed for changing the GIC
>> >> implementation to a different flow handler.
>> >>
>> >> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
>> >
>> > mach-nomadik only seems to use the vic. Why does this GPIO handler
>> > need updating?
>>
>> plat-nomadik/gpio.c is also used by mach-ux500, which uses the GIC.
>
> Ah yes, sorry I missed that one. I'm happy to include any patches from you
> in the GIC fasteoi series if it makes merging easier.

OK if this one is OK I sure prefer that you take it in your series Will,
since you have the whole picture of what you want to do with the
GIC.

It'll likely collide with the stuff I have stacked up for the 2.6.39 merge
window though, which will be pull-requested any day now.

Yours,
Linus Walleij

[PATCH] nmk-gpio: use request_irq instead of chained handler

[Will Deacon]

Hi Linus,

> 2011/2/23 Will Deacon <will.deacon@arm.com>:
> >> On Wed, Feb 23, 2011 at 22:47, Will Deacon <will.deacon@arm.com> wrote:
> >> >> __nmk_gpio_irq_handler doesn't do anything more that what a normal handler
> >> >> does, so it can become one. ?This is also needed for changing the GIC
> >> >> implementation to a different flow handler.
> >> >>
> >> >> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> >> >
> >> > mach-nomadik only seems to use the vic. Why does this GPIO handler
> >> > need updating?
> >>
> >> plat-nomadik/gpio.c is also used by mach-ux500, which uses the GIC.
> >
> > Ah yes, sorry I missed that one. I'm happy to include any patches from you
> > in the GIC fasteoi series if it makes merging easier.
> 
> OK if this one is OK I sure prefer that you take it in your series Will,
> since you have the whole picture of what you want to do with the
> GIC.
> 
> It'll likely collide with the stuff I have stacked up for the 2.6.39 merge
> window though, which will be pull-requested any day now.

Sure. I'll add a patch to my series that makes the minimal change required
and post it next week.

Will