From: Haibo Chen <haibo.chen@nxp.com>

Though the default pin configuration is INPUT, but if the prior stage does
configure the pins as OUTPUT, then Linux will not reconfigure the pin as
INPUT.

e.g. When use one pin as interrupt source, and set as low level trigger,
if prior stage already set this pin as OUTPUT low, then will meet interrupt
storm.

So always set GPIO to input mode when used as interrupt source to fix above
case.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/gpio/gpio-vf610.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index 07e5e6323e86..305b0bcdee6f 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -214,7 +214,7 @@ static int vf610_gpio_irq_set_type(struct irq_data *d, u32 type)
 	else
 		irq_set_handler_locked(d, handle_edge_irq);
 
-	return 0;
+	return port->gc.direction_input(&port->gc, d->hwirq);
 }
 
 static void vf610_gpio_irq_mask(struct irq_data *d)
-- 
2.34.1

On Tue, Apr 23, 2024 at 4:28 AM <haibo.chen@nxp.com> wrote:

> From: Haibo Chen <haibo.chen@nxp.com>
>
> Though the default pin configuration is INPUT, but if the prior stage does
> configure the pins as OUTPUT, then Linux will not reconfigure the pin as
> INPUT.
>
> e.g. When use one pin as interrupt source, and set as low level trigger,
> if prior stage already set this pin as OUTPUT low, then will meet interrupt
> storm.
>
> So always set GPIO to input mode when used as interrupt source to fix above
> case.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/gpio/gpio-vf610.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> index 07e5e6323e86..305b0bcdee6f 100644
> --- a/drivers/gpio/gpio-vf610.c
> +++ b/drivers/gpio/gpio-vf610.c
> @@ -214,7 +214,7 @@ static int vf610_gpio_irq_set_type(struct irq_data *d, u32 type)
>         else
>                 irq_set_handler_locked(d, handle_edge_irq);
>
> -       return 0;
> +       return port->gc.direction_input(&port->gc, d->hwirq);

Just call vf610_gpio_direction_input() instead of indirecting through
gc->direction_input(), no need to jump through the vtable and as
Bartosz says: it just makes that struct vulnerable.

Second:

In this patch also implement gc->get_direction() which is currently
unimplemented. If you are going to change the direction of a
GPIO randomly at runtime then the framework really likes to
have a chance to know the current direction for obvious reasons.

Yours,
Linus Walleij

Hi,

I found this thread about the vf610 driver, because I noticed today that
the i.MX93 is missing the GPIO get_direction callback.

Are there any plans to implement this?

Best regards

On Tue, Apr 23, 2024 at 4:28 AM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> Though the default pin configuration is INPUT, but if the prior stage does
> configure the pins as OUTPUT, then Linux will not reconfigure the pin as
> INPUT.
>
> e.g. When use one pin as interrupt source, and set as low level trigger,
> if prior stage already set this pin as OUTPUT low, then will meet interrupt
> storm.
>
> So always set GPIO to input mode when used as interrupt source to fix above
> case.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/gpio/gpio-vf610.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> index 07e5e6323e86..305b0bcdee6f 100644
> --- a/drivers/gpio/gpio-vf610.c
> +++ b/drivers/gpio/gpio-vf610.c
> @@ -214,7 +214,7 @@ static int vf610_gpio_irq_set_type(struct irq_data *d, u32 type)
>         else
>                 irq_set_handler_locked(d, handle_edge_irq);
>
> -       return 0;
> +       return port->gc.direction_input(&port->gc, d->hwirq);
>  }
>
>  static void vf610_gpio_irq_mask(struct irq_data *d)
> --
> 2.34.1
>

Can you use gpiod_direction_output()? Otherwise the flags of the
descriptor will tell a different story.

Also: this doesn't matter here as it's a built-in driver but irq
callbacks accessing gpio_chip is a thing that still needs addressing
as it doesn't use SRCU. :(

Bart