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

[Nori, Sekhar]

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 at wizery.com>
> CC: Thomas Gleixner <tglx at 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