diff mbox series

gpio: vf610: always set GPIO to input mode when used as interrupt source

Message ID 20240423022814.3951048-1-haibo.chen@nxp.com (mailing list archive)
State New, archived
Headers show
Series gpio: vf610: always set GPIO to input mode when used as interrupt source | expand

Commit Message

Bough Chen April 23, 2024, 2:28 a.m. UTC 
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(-)

Comments

Bartosz Golaszewski April 23, 2024, 10:56 a.m. UTC | #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);
>  }
>
>  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
Linus Walleij April 23, 2024, 11:40 a.m. UTC | #2 
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
Bough Chen April 23, 2024, 12:20 p.m. UTC | #3 
> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: 2024年4月23日 19:41
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org; linux-kernel@vger.kernel.org;
> imx@lists.linux.dev
> Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> interrupt source
> 
> 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.

Thanks for your quick review, I will do that in V2.

> 
> 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.

Yes, will implement gc->get_direction(), if we did this before, then for this case we meet, framework will print out error log, save much debug time. 

Best Regards
Haibo Chen 
> 
> Yours,
> Linus Walleij
Bough Chen April 24, 2024, 11:01 a.m. UTC | #4 
> -----Original Message-----
> From: Bough Chen
> Sent: 2024年4月23日 20:21
> To: Linus Walleij <linus.walleij@linaro.org>
> Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org; linux-kernel@vger.kernel.org;
> imx@lists.linux.dev
> Subject: RE: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> interrupt source
> 
> > -----Original Message-----
> > From: Linus Walleij <linus.walleij@linaro.org>
> > Sent: 2024年4月23日 19:41
> > To: Bough Chen <haibo.chen@nxp.com>
> > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when
> > used as interrupt source
> >
> > 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.
> 
> Thanks for your quick review, I will do that in V2.
> 
> >
> > 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.
> 
> Yes, will implement gc->get_direction(), if we did this before, then for this case
> we meet, framework will print out error log, save much debug time.

Hi Linus,

I implement gc->get_direction() today, for the case to request one gpio as irq, gpio architecture will first
call gpiochip_reqres_irq(), if the ROM or Uboot already config this gpio pin as OUTPUT mode, will get
the following log:

[    2.714889] gpio gpiochip3: (43850000.gpio): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
[    2.724816] gpio gpiochip3: (43850000.gpio): unable to lock HW IRQ 11 for IRQ
[    2.731972] genirq: Failed to request resources for 2-0050 (irq 211) on irqchip gpio-vf610

Any suggested method to avoid this case? My previous method works because driver lack of gc->get_direction().

Best Regards
Haibo Chen

> 
> Best Regards
> Haibo Chen
> >
> > Yours,
> > Linus Walleij
Bough Chen April 25, 2024, 8:49 a.m. UTC | #5 
> -----Original Message-----
> From: Bough Chen
> Sent: 2024年4月24日 19:01
> To: 'Linus Walleij' <linus.walleij@linaro.org>
> Cc: 'brgl@bgdev.pl' <brgl@bgdev.pl>; 'linux-gpio@vger.kernel.org'
> <linux-gpio@vger.kernel.org>; 'linux-kernel@vger.kernel.org'
> <linux-kernel@vger.kernel.org>; 'imx@lists.linux.dev' <imx@lists.linux.dev>
> Subject: RE: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> interrupt source
> 
> > -----Original Message-----
> > From: Bough Chen
> > Sent: 2024年4月23日 20:21
> > To: Linus Walleij <linus.walleij@linaro.org>
> > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > Subject: RE: [PATCH] gpio: vf610: always set GPIO to input mode when
> > used as interrupt source
> >
> > > -----Original Message-----
> > > From: Linus Walleij <linus.walleij@linaro.org>
> > > Sent: 2024年4月23日 19:41
> > > To: Bough Chen <haibo.chen@nxp.com>
> > > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > > Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when
> > > used as interrupt source
> > >
> > > 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.
> >
> > Thanks for your quick review, I will do that in V2.
> >
> > >
> > > 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.
> >
> > Yes, will implement gc->get_direction(), if we did this before, then
> > for this case we meet, framework will print out error log, save much debug
> time.
> 
> Hi Linus,
> 
> I implement gc->get_direction() today, for the case to request one gpio as irq,
> gpio architecture will first call gpiochip_reqres_irq(), if the ROM or Uboot
> already config this gpio pin as OUTPUT mode, will get the following log:
> 
> [    2.714889] gpio gpiochip3: (43850000.gpio): gpiochip_lock_as_irq: tried to
> flag a GPIO set as output for IRQ
> [    2.724816] gpio gpiochip3: (43850000.gpio): unable to lock HW IRQ 11 for
> IRQ
> [    2.731972] genirq: Failed to request resources for 2-0050 (irq 211) on
> irqchip gpio-vf610
> 
> Any suggested method to avoid this case? My previous method works because
> driver lack of gc->get_direction().

Hi Linus,

I find gpio-hog can work around this case.

I add the following change, config this gpio as input in gpio-hog, so that when this gpio controller probe, gpiolib will setting the pin to input mode.

+&gpio5 {
+       ptn5110-int-hog {
+               gpio-hog;
+               gpios = <11 GPIO_ACTIVE_LOW>;
+               input;
        };

Not sure whether this is an acceptable method. If yes, what I need to do in the gpio-vf610 driver is to implement gc->get_direction() call back.

Best Regards
Haibo Chen
> 
> Best Regards
> Haibo Chen
> 
> >
> > Best Regards
> > Haibo Chen
> > >
> > > Yours,
> > > Linus Walleij
Bartosz Golaszewski April 25, 2024, 1:06 p.m. UTC | #6 
On Wed, Apr 24, 2024 at 1:01 PM Bough Chen <haibo.chen@nxp.com> wrote:
>
> > -----Original Message-----
> > From: Bough Chen
> > Sent: 2024年4月23日 20:21
> > To: Linus Walleij <linus.walleij@linaro.org>
> > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org; linux-kernel@vger.kernel.org;
> > imx@lists.linux.dev
> > Subject: RE: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> > interrupt source
> >
> > > -----Original Message-----
> > > From: Linus Walleij <linus.walleij@linaro.org>
> > > Sent: 2024年4月23日 19:41
> > > To: Bough Chen <haibo.chen@nxp.com>
> > > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > > Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when
> > > used as interrupt source
> > >
> > > 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.
> >
> > Thanks for your quick review, I will do that in V2.
> >
> > >
> > > 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.
> >
> > Yes, will implement gc->get_direction(), if we did this before, then for this case
> > we meet, framework will print out error log, save much debug time.
>
> Hi Linus,
>
> I implement gc->get_direction() today, for the case to request one gpio as irq, gpio architecture will first
> call gpiochip_reqres_irq(), if the ROM or Uboot already config this gpio pin as OUTPUT mode, will get
> the following log:
>
> [    2.714889] gpio gpiochip3: (43850000.gpio): gpiochip_lock_as_irq: tried to flag a GPIO set as output for IRQ
> [    2.724816] gpio gpiochip3: (43850000.gpio): unable to lock HW IRQ 11 for IRQ
> [    2.731972] genirq: Failed to request resources for 2-0050 (irq 211) on irqchip gpio-vf610
>
> Any suggested method to avoid this case? My previous method works because driver lack of gc->get_direction().
>

Can you make the driver default all lines to input when the device is
being registered? Possibly also revert to input when the line is being
freed?

Bartosz
Bough Chen April 28, 2024, 2:31 a.m. UTC | #7 
> -----Original Message-----
> From: Bartosz Golaszewski <brgl@bgdev.pl>
> Sent: 2024年4月25日 21:07
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org;
> linux-kernel@vger.kernel.org; imx@lists.linux.dev
> Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> interrupt source
> 
> On Wed, Apr 24, 2024 at 1:01 PM Bough Chen <haibo.chen@nxp.com> wrote:
> >
> > > -----Original Message-----
> > > From: Bough Chen
> > > Sent: 2024年4月23日 20:21
> > > To: Linus Walleij <linus.walleij@linaro.org>
> > > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > > Subject: RE: [PATCH] gpio: vf610: always set GPIO to input mode when
> > > used as interrupt source
> > >
> > > > -----Original Message-----
> > > > From: Linus Walleij <linus.walleij@linaro.org>
> > > > Sent: 2024年4月23日 19:41
> > > > To: Bough Chen <haibo.chen@nxp.com>
> > > > Cc: brgl@bgdev.pl; linux-gpio@vger.kernel.org;
> > > > linux-kernel@vger.kernel.org; imx@lists.linux.dev
> > > > Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode
> > > > when used as interrupt source
> > > >
> > > > 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.
> > >
> > > Thanks for your quick review, I will do that in V2.
> > >
> > > >
> > > > 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.
> > >
> > > Yes, will implement gc->get_direction(), if we did this before, then
> > > for this case we meet, framework will print out error log, save much debug
> time.
> >
> > Hi Linus,
> >
> > I implement gc->get_direction() today, for the case to request one
> > gpio as irq, gpio architecture will first call gpiochip_reqres_irq(),
> > if the ROM or Uboot already config this gpio pin as OUTPUT mode, will get the
> following log:
> >
> > [    2.714889] gpio gpiochip3: (43850000.gpio): gpiochip_lock_as_irq: tried
> to flag a GPIO set as output for IRQ
> > [    2.724816] gpio gpiochip3: (43850000.gpio): unable to lock HW IRQ 11
> for IRQ
> > [    2.731972] genirq: Failed to request resources for 2-0050 (irq 211) on
> irqchip gpio-vf610
> >
> > Any suggested method to avoid this case? My previous method works because
> driver lack of gc->get_direction().
> >
> 
> Can you make the driver default all lines to input when the device is being
> registered? Possibly also revert to input when the line is being freed?

Yes, this should be a better method, I will have a try.

Thanks
Haibo Chen
> 
> Bartosz
Stefan Wahren July 18, 2024, 5:08 p.m. UTC | #8 
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
Bough Chen July 19, 2024, 1:46 a.m. UTC | #9 
> -----Original Message-----
> From: Stefan Wahren <wahrenst@gmx.net>
> Sent: 2024年7月19日 1:09
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Bartosz Golaszewski <brgl@bgdev.pl>; Linus Walleij
> <linus.walleij@linaro.org>; open list:GPIO SUBSYSTEM
> <linux-gpio@vger.kernel.org>; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>; imx@lists.linux.dev
> Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when used as
> interrupt source
> 
> Hi,
> 
> I found this thread about the vf610 driver, because I noticed today that the
> i.MX93 is missing the GPIO get_direction callback.

I find the patch was wrong, we can't call gc.direction_input in vf610_gpio_irq_set_type(), because gc.direction_input will call pinctrl_gpio_direction_input(), this API can't be used in irq context. 

For get_direction callback, yes, I will send a patch this week.

Best Regards
Haibo Chen
> 
> Are there any plans to implement this?
> 
> Best regards
Stefan Wahren July 19, 2024, 3:53 a.m. UTC | #10 
Am 19.07.24 um 03:46 schrieb Bough Chen:
>> -----Original Message-----
>> From: Stefan Wahren <wahrenst@gmx.net>
>> Sent: 2024年7月19日 1:09
>> To: Bough Chen <haibo.chen@nxp.com>
>> Cc: Bartosz Golaszewski <brgl@bgdev.pl>; Linus Walleij
>> <linus.walleij@linaro.org>; open list:GPIO SUBSYSTEM
>> <linux-gpio@vger.kernel.org>; Linux Kernel Mailing List
>> <linux-kernel@vger.kernel.org>; imx@lists.linux.dev
>> Subject: Re: [PATCH] gpio: vf610: always set GPIO to input mode when used as
>> interrupt source
>>
>> Hi,
>>
>> I found this thread about the vf610 driver, because I noticed today that the
>> i.MX93 is missing the GPIO get_direction callback.
> I find the patch was wrong, we can't call gc.direction_input in vf610_gpio_irq_set_type(), because gc.direction_input will call pinctrl_gpio_direction_input(), this API can't be used in irq context.
>
> For get_direction callback, yes, I will send a patch this week.
Sounds great. You can add me to CC for it, because i'm usually
subscribed to linux-arm-kernel instead of linux-kernel.

Regards
diff mbox series

Patch

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)