Re: [PATCH v3 1/5] gpio: Allow per-parent interrupt data

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Top-posting because I need a nod from Bartosz that I can
merge this patch with the rest in the pinctrl tree.

Bartosz: I can also offer this one patch in an immutable branch
as well so you can pull it in as well.

Yours,
Linus Walleij

On Sat, Oct 16, 2021 at 4:19 PM Joey Gouly <joey.gouly@xxxxxxx> wrote:

> From: Marc Zyngier <maz@xxxxxxxxxx>
>
> The core gpiolib code is able to deal with multiple interrupt parents
> for a single gpio irqchip. It however only allows a single piece
> of data to be conveyed to all flow handlers (either the gpio_chip
> or some other, driver-specific data).
>
> This means that drivers have to go through some interesting dance
> to find the correct context, something that isn't great in interrupt
> context (see aebdc8abc9db86e2bd33070fc2f961012fff74b4 for a prime
> example).
>
> Instead, offer an optional way for a pinctrl/gpio driver to provide
> an array of pointers which gets used to provide the correct context
> to the flow handler.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> Signed-off-by: Joey Gouly <joey.gouly@xxxxxxx>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
>  drivers/gpio/gpiolib.c      |  9 +++++++--
>  include/linux/gpio/driver.h | 19 +++++++++++++++++--
>  2 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index d1b9b721218f..abfbf546d159 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1534,9 +1534,14 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>         }
>
>         if (gc->irq.parent_handler) {
> -               void *data = gc->irq.parent_handler_data ?: gc;
> -
>                 for (i = 0; i < gc->irq.num_parents; i++) {
> +                       void *data;
> +
> +                       if (gc->irq.per_parent_data)
> +                               data = gc->irq.parent_handler_data_array[i];
> +                       else
> +                               data = gc->irq.parent_handler_data ?: gc;
> +
>                         /*
>                          * The parent IRQ chip is already using the chip_data
>                          * for this IRQ chip, so our callbacks simply use the
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index a0f9901dcae6..a673a359e20b 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -168,11 +168,18 @@ struct gpio_irq_chip {
>
>         /**
>          * @parent_handler_data:
> +        * @parent_handler_data_array:
>          *
>          * Data associated, and passed to, the handler for the parent
> -        * interrupt.
> +        * interrupt. Can either be a single pointer if @per_parent_data
> +        * is false, or an array of @num_parents pointers otherwise.  If
> +        * @per_parent_data is true, @parent_handler_data_array cannot be
> +        * NULL.
>          */
> -       void *parent_handler_data;
> +       union {
> +               void *parent_handler_data;
> +               void **parent_handler_data_array;
> +       };
>
>         /**
>          * @num_parents:
> @@ -203,6 +210,14 @@ struct gpio_irq_chip {
>          */
>         bool threaded;
>
> +       /**
> +        * @per_parent_data:
> +        *
> +        * True if parent_handler_data_array describes a @num_parents
> +        * sized array to be used as parent data.
> +        */
> +       bool per_parent_data;
> +
>         /**
>          * @init_hw: optional routine to initialize hardware before
>          * an IRQ chip will be added. This is quite useful when
> --
> 2.17.1
>