[v2,1/6] arm: davinci: Fix low level gpio irq handlers' argument
diff mbox

Message ID 1310303679-17936-2-git-send-email-ido@wizery.com
State New, archived
Headers show

Commit Message

Ido Yariv July 10, 2011, 1:14 p.m. UTC
Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip
conversion") introduced a bug, causing low level interrupt handlers to
get a bogus irq number as an argument. The gpio irq handler falsely
assumes that the handler data is the irq base number and that is no
longer true.

Fix this by converting gpio_irq_handler's bank_irq argument to the
corresponding irq base number.

Signed-off-by: Ido Yariv <ido@wizery.com>
CC: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-davinci/gpio.c |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

Comments

Sekhar Nori July 11, 2011, 11:58 a.m. UTC | #1
Hi Ido,

On Sun, Jul 10, 2011 at 18:44:34, Ido Yariv wrote:
> Commit 7416401 ("arm: davinci: Fix fallout from generic irq chip
> conversion") introduced a bug, causing low level interrupt handlers to
> get a bogus irq number as an argument. The gpio irq handler falsely
> assumes that the handler data is the irq base number and that is no
> longer true.
> 
> Fix this by converting gpio_irq_handler's bank_irq argument to the
> corresponding irq base number.
> 
> Signed-off-by: Ido Yariv <ido@wizery.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/arm/mach-davinci/gpio.c |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/gpio.c b/arch/arm/mach-davinci/gpio.c
> index e722139..ff43e2a 100644
> --- a/arch/arm/mach-davinci/gpio.c
> +++ b/arch/arm/mach-davinci/gpio.c
> @@ -249,16 +249,40 @@ static struct irq_chip gpio_irqchip = {
>  	.flags		= IRQCHIP_SET_TYPE_MASKED,
>  };
>  
> +static inline int bankirq_to_irqbase(unsigned int bank_irq)
> +{
> +	int gpio;
> +	int index;
> +
> +	/* Each irq bank consists of up to 16 irqs */
> +	gpio = 16 * (bank_irq - davinci_soc_info.gpio_irq);
> +
> +	/* Each controller controls 32 GPIOs */
> +	index = gpio / 32;
> +
> +	if (unlikely(!davinci_soc_info.gpio_ctlrs))
> +		return -EINVAL;
> +
> +	if (unlikely(index >= davinci_soc_info.gpio_ctlrs_num))
> +		return -EINVAL;
> +
> +	return davinci_soc_info.gpio_ctlrs[index].irq_base;
> +}
> +
>  static void
> -gpio_irq_handler(unsigned irq, struct irq_desc *desc)
> +gpio_irq_handler(unsigned bank_irq, struct irq_desc *desc)
>  {
>  	struct davinci_gpio_regs __iomem *g;
>  	u32 mask = 0xffff;
> +	int irqbase = bankirq_to_irqbase(bank_irq);
> +
> +	if (unlikely(irqbase < 0))
> +		return;
>  
>  	g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc);
>  
>  	/* we only care about one bank */
> -	if (irq & 1)
> +	if (bank_irq & 1)
>  		mask <<= 16;
>  
>  	/* temporarily mask (level sensitive) parent IRQ */
> @@ -274,11 +298,11 @@ gpio_irq_handler(unsigned irq, struct irq_desc *desc)
>  		if (!status)
>  			break;
>  		__raw_writel(status, &g->intstat);
> -		if (irq & 1)
> +		if (bank_irq & 1)
>  			status >>= 16;
>  
>  		/* now demux them to the right lowlevel handler */
> -		n = (int)irq_get_handler_data(irq);
> +		n = irqbase;
>  		while (status) {
>  			res = ffs(status);
>  			n += res;

Thanks for the bug fix.

How about setting the handler data for bank IRQ in
davinci_gpio_irq_setup() to &chips[bank]?

chips[bank].regs should give you the register base address
(g) and chips[bank].irq_base should give you 'n'.

Also please drop the rename of irq to bank_irq as it is not
central to the bug fix.

If you spin in soon enough, we may make it to v3.0

Thanks,
Sekhar
Ido Yariv July 11, 2011, 8:33 p.m. UTC | #2
Hi Sekhar,

On Mon, Jul 11, 2011 at 05:28:44PM +0530, Nori, Sekhar wrote:
> Thanks for the bug fix.
> 
> How about setting the handler data for bank IRQ in
> davinci_gpio_irq_setup() to &chips[bank]?
> 
> chips[bank].regs should give you the register base address
> (g) and chips[bank].irq_base should give you 'n'.
> 
> Also please drop the rename of irq to bank_irq as it is not
> central to the bug fix.
> 
> If you spin in soon enough, we may make it to v3.0

Sure, I'll send a v3 in a few.

Thanks for the review,
Ido.

Patch
diff mbox

diff --git a/arch/arm/mach-davinci/gpio.c b/arch/arm/mach-davinci/gpio.c
index e722139..ff43e2a 100644
--- a/arch/arm/mach-davinci/gpio.c
+++ b/arch/arm/mach-davinci/gpio.c
@@ -249,16 +249,40 @@  static struct irq_chip gpio_irqchip = {
 	.flags		= IRQCHIP_SET_TYPE_MASKED,
 };
 
+static inline int bankirq_to_irqbase(unsigned int bank_irq)
+{
+	int gpio;
+	int index;
+
+	/* Each irq bank consists of up to 16 irqs */
+	gpio = 16 * (bank_irq - davinci_soc_info.gpio_irq);
+
+	/* Each controller controls 32 GPIOs */
+	index = gpio / 32;
+
+	if (unlikely(!davinci_soc_info.gpio_ctlrs))
+		return -EINVAL;
+
+	if (unlikely(index >= davinci_soc_info.gpio_ctlrs_num))
+		return -EINVAL;
+
+	return davinci_soc_info.gpio_ctlrs[index].irq_base;
+}
+
 static void
-gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+gpio_irq_handler(unsigned bank_irq, struct irq_desc *desc)
 {
 	struct davinci_gpio_regs __iomem *g;
 	u32 mask = 0xffff;
+	int irqbase = bankirq_to_irqbase(bank_irq);
+
+	if (unlikely(irqbase < 0))
+		return;
 
 	g = (__force struct davinci_gpio_regs __iomem *) irq_desc_get_handler_data(desc);
 
 	/* we only care about one bank */
-	if (irq & 1)
+	if (bank_irq & 1)
 		mask <<= 16;
 
 	/* temporarily mask (level sensitive) parent IRQ */
@@ -274,11 +298,11 @@  gpio_irq_handler(unsigned irq, struct irq_desc *desc)
 		if (!status)
 			break;
 		__raw_writel(status, &g->intstat);
-		if (irq & 1)
+		if (bank_irq & 1)
 			status >>= 16;
 
 		/* now demux them to the right lowlevel handler */
-		n = (int)irq_get_handler_data(irq);
+		n = irqbase;
 		while (status) {
 			res = ffs(status);
 			n += res;