nmk-gpio: use request_irq instead of chained handler

PATCH New Rabin Vincent 5 participants 8 messages Feb 23, 2011 – Feb 25, 2011 2 messages not in the archive

Subsystems (2)

Arm (32-bit) Arm sub-architectures

lore

Share & tools

lore

Rabin Vincent

author

2011-02-23 12:54 UTC #0 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

__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 <[email protected]>

---
 arch/arm/plat-nomadik/gpio.c |   54 ++++++++++++++++++++---------------------
 1 files changed, 26 insertions(+), 28 deletions(-)

arch/arm/plat-nomadik/gpio.c +26 ~~-28~~

index 45b1cf9..efa6737 100644
@@ -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 */

Signature · 1 line

1.7.2.dirty

Russell King - ARM Linux

2011-02-23 13:18 UTC #1 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

Quoted 3 lines · 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.

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.

Rabin Vincent

2011-02-23 15:22 UTC #2 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

Quoted 6 lines · Russell King - ARM Linux

Quoted 3 lines · 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.
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.

Quoted 2 lines

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.

Quoted 5 lines

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).

Will Deacon

2011-02-23 17:17 UTC #3 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

In reply to Rabin Vincent

Rabin,

Quoted 5 lines

__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 <[email protected]>

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

Will

A message that is not in the archive (sent off-list or not yet received): -6148324460357957969@unknownmsgid

Rabin Vincent

2011-02-23 17:25 UTC #5 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

Quoted 8 lines · Will Deacon

Quoted 5 lines
__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 <[email protected]>
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.

Will Deacon

2011-02-23 17:36 UTC #6 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

Quoted 11 lines · Will Deacon

Quoted 8 lines
Quoted 5 lines
__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 <[email protected]>
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

A message that is not in the archive (sent off-list or not yet received): 5890487119819559395@unknownmsgid

Linus Walleij

2011-02-25 16:59 UTC #8 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

2011/2/23 Will Deacon <[email protected]>:

Quoted 7 lines

Quoted 7 lines · Will Deacon
Quoted 6 lines
Quoted 5 lines
__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 <[email protected]>

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
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

Will Deacon

2011-02-25 17:16 UTC #9 lore raw

PATCH 1/1 nmk-gpio: use request_irq instead of chained handler

Hi Linus,

Quoted 16 lines

2011/2/23 Will Deacon <[email protected]>:
Quoted 7 lines
Quoted 7 lines · Will Deacon
Quoted 6 lines
Quoted 5 lines
__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 <[email protected]>
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
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.

Quoted 6 lines

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