From: Bough Chen <haibo.chen@nxp.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Linus Walleij <linus.walleij@linaro.org>,
"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
Date: Sun, 28 Apr 2024 02:31:00 +0000 [thread overview]
Message-ID: <DU0PR04MB94965CE7D3BFBA35887A5C4190142@DU0PR04MB9496.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <CAMRc=McQtohbuUSrhH8LiF6EWUzRKyHSObBv4D6VeE87NmdZQQ@mail.gmail.com>
> -----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
prev parent reply other threads:[~2024-04-28 2:31 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-23 2:28 [PATCH] gpio: vf610: always set GPIO to input mode when used as interrupt source haibo.chen
2024-04-23 10:56 ` Bartosz Golaszewski
2024-04-23 11:40 ` Linus Walleij
2024-04-23 12:20 ` Bough Chen
2024-04-24 11:01 ` Bough Chen
2024-04-25 8:49 ` Bough Chen
2024-04-25 13:06 ` Bartosz Golaszewski
2024-04-28 2:31 ` Bough Chen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DU0PR04MB94965CE7D3BFBA35887A5C4190142@DU0PR04MB9496.eurprd04.prod.outlook.com \
--to=haibo.chen@nxp.com \
--cc=brgl@bgdev.pl \
--cc=imx@lists.linux.dev \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).