[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<DU0PR04MB9496B8F91EB56D222BDD8C2A90172@DU0PR04MB9496.eurprd04.prod.outlook.com>
Date: Thu, 25 Apr 2024 08:49:39 +0000
From: Bough Chen <haibo.chen@....com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: "brgl@...ev.pl" <brgl@...ev.pl>, "linux-gpio@...r.kernel.org"
<linux-gpio@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.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月24日 19:01
> To: 'Linus Walleij' <linus.walleij@...aro.org>
> Cc: 'brgl@...ev.pl' <brgl@...ev.pl>; 'linux-gpio@...r.kernel.org'
> <linux-gpio@...r.kernel.org>; 'linux-kernel@...r.kernel.org'
> <linux-kernel@...r.kernel.org>; 'imx@...ts.linux.dev' <imx@...ts.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@...aro.org>
> > Cc: brgl@...ev.pl; linux-gpio@...r.kernel.org;
> > linux-kernel@...r.kernel.org; imx@...ts.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@...aro.org>
> > > Sent: 2024年4月23日 19:41
> > > To: Bough Chen <haibo.chen@....com>
> > > Cc: brgl@...ev.pl; linux-gpio@...r.kernel.org;
> > > linux-kernel@...r.kernel.org; imx@...ts.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@....com> wrote:
> > >
> > > > From: Haibo Chen <haibo.chen@....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@....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
Powered by blists - more mailing lists