Re: [PATCH 1/3] gpio: zynq: Take bank offset into account when reporting a IRQ

[Alexandre Courbot <gnurou@xxxxxxxxx>]

On Fri, Jul 18, 2014 at 6:52 PM, Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
> When looking up the IRQ the bank offset needs to be taken into account.
> Otherwise interrupts for banks other than bank 0 get incorrectly reported as
> interrupts for bank 0.
>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> ---
>  drivers/gpio/gpio-zynq.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c
> index c0c53fd..8e6a32f 100644
> --- a/drivers/gpio/gpio-zynq.c
> +++ b/drivers/gpio/gpio-zynq.c
> @@ -136,6 +136,29 @@ static inline void zynq_gpio_get_bank_pin(unsigned int pin_num,
>  }
>
>  /**
> + * zynq_gpio_get_bank_pin - Gets the pin number of the first pin in a bank
> + * @bank: The bank for which to return the first pin number
> + *
> + * Returns the pin number of the first pin in the specified bank
> + */
> +static int zynq_gpio_get_bank_offset(unsigned int bank)
> +{
> +       switch (bank) {
> +       case 0:
> +               return ZYNQ_GPIO_BANK0_PIN_MIN;
> +       case 1:
> +               return ZYNQ_GPIO_BANK1_PIN_MIN;
> +       case 2:
> +               return ZYNQ_GPIO_BANK2_PIN_MIN;
> +       case 3:
> +               return ZYNQ_GPIO_BANK3_PIN_MIN;
> +       default:
> +               /* We'll never get here */
> +               return -1;
> +       }
> +}

Wouldn't this be handled better by a simple, static array? I.e.

static int zynq_gpio_bank_offset[] = {
        ZYNQ_GPIO_BANK0_PIN_MIN,
        ZYNQ_GPIO_BANK1_PIN_MIN,
        ZYNQ_GPIO_BANK2_PIN_MIN,
        ZYNQ_GPIO_BANK3_PIN_MIN
};

...

        int bank offset = zynq_gpio_bank_offset(bank_num);

> +
> +/**
>   * zynq_gpio_get_value - Get the state of the specified pin of GPIO device
>   * @chip:      gpio_chip instance to be worked on
>   * @pin:       gpio pin number within the device
> @@ -419,11 +442,12 @@ static void zynq_gpio_irqhandler(unsigned int irq, struct irq_desc *desc)
>                 if (int_sts) {
>                         int offset;
>                         unsigned long pending = int_sts;
> +                       int bank_offset = zynq_gpio_get_bank_offset(bank_num);
>
>                         for_each_set_bit(offset, &pending, 32) {
>                                 unsigned int gpio_irq =
>                                         irq_find_mapping(gpio->chip.irqdomain,
> -                                                       offset);
> +                                                        offset + bank_offset);
>                                 generic_handle_irq(gpio_irq);
>                         }
>
> --
> 1.8.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html