| Message ID | 20230905185309.131295-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | gpio: convert users to gpio_device_find() and remove gpiochip_find() | expand |
Hi, On Tue, Sep 05, 2023 at 08:52:56PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We're moving away from public functions that take struct gpio_chip as > argument as the chip - unlike struct gpio_device - is owned by its > provider and tied to its lifetime. > > Provide an alternative to acpi_gpiochip_free_interrupts(). Looks good to me, few minor comments below. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/gpio/gpiolib-acpi.c | 29 +++++++++++++++++++++++------ > include/linux/gpio/driver.h | 12 ++++++++++++ > 2 files changed, 35 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c > index fbda452fb4d6..5633e39396bc 100644 > --- a/drivers/gpio/gpiolib-acpi.c > +++ b/drivers/gpio/gpiolib-acpi.c > @@ -558,12 +558,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip) > } > EXPORT_SYMBOL_GPL(acpi_gpiochip_request_interrupts); > > -/** > - * acpi_gpiochip_free_interrupts() - Free GPIO ACPI event interrupts. > - * @chip: GPIO chip > - * > - * Free interrupts associated with GPIO ACPI event method for the given > - * GPIO chip. > +/* > + * This function is deprecated, use acpi_gpio_device_free_interrupts() > + * instead. > */ > void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) > { > @@ -604,6 +601,26 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) > } > EXPORT_SYMBOL_GPL(acpi_gpiochip_free_interrupts); > > +/** > + * acpi_gpio_device_free_interrupts() - Free GPIO ACPI event interrupts. > + * @chip GPIO device Should be: @chip: GPIO device > + * > + * Free interrupts associated with GPIO ACPI event method for the given > + * GPIO device. > + */ > +void acpi_gpio_device_free_interrupts(struct gpio_device *gdev) > +{ > + struct gpio_chip *gc; > + > + /* TODO: protect gdev->chip once SRCU is in place in GPIOLIB. */ > + gc = gdev->chip; > + if (!gc) > + return; > + > + acpi_gpiochip_free_interrupts(gc); > +} > +EXPORT_SYMBOL_GPL(acpi_gpio_device_free_interrupts); > + > int acpi_dev_add_driver_gpios(struct acpi_device *adev, > const struct acpi_gpio_mapping *gpios) > { > diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h > index b68b3493b29d..47906bc56b3d 100644 > --- a/include/linux/gpio/driver.h > +++ b/include/linux/gpio/driver.h > @@ -835,4 +835,16 @@ static inline struct fwnode_handle *gpiochip_node_get_first(struct device *dev) > return NULL; > } > > +/* > + * FIXME: Remove this once the only driver that uses it - android tablets - > + * becomes a good citizen and stops abusing GPIOLIB. There are a acouple of more when grepping for acpi_gpiochip_free_interrupts(). I'm not entirely sure why these functions are exposed to the drivers in the first place. IMHO GPIOLIB should deal with these but perhaps there is some good reason these drivers do it... > + */ > +#ifdef CONFIG_ACPI > +void acpi_gpio_device_free_interrupts(struct gpio_device *gdev); > +#else > +static inline void acpi_gpio_device_free_interrupts(struct gpio_device *gdev) > +{ > +} I would put these {} to the same line: static inline void acpi_gpio_device_free_interrupts(struct gpio_device *gdev) { } > +#endif > + > #endif /* __LINUX_GPIO_DRIVER_H */ > -- > 2.39.2
On Tue, Sep 05, 2023 at 08:52:51PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > By far the most common way of looking up GPIO devices is using their > label. Provide a helpers for that to avoid every user implementing their > own matching function. ... > +/** > + * gpio_device_find_by_label() - wrapper around gpio_device_find() finding the > + * GPIO device by its backing chip's label > + * @label: Label to lookup > + * > + * Returns: > + * Reference to the GPIO device or NULL. Reference must be released with > + * ``gpio_device_put``. Out of a sudden different format to refer the function. Should be gpio_device_put(), so kernel-doc makes a hyperlink. > + */ So, please, render all your kernel docs and look at the end result. ... > + return gpio_device_find((void *)label, gpio_chip_match_by_label); This casting is a bit awkward... But I have no good proposal for a change.
On Tue, Sep 05, 2023 at 08:52:58PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Remove all remaining uses of find_chip_by_name() (and subsequently: > gpiochip_find()) from gpiolib.c and use the new > gpio_device_find_by_label() instead. ... > - desc = gpiochip_get_desc(gc, p->chip_hwnum); > + desc = gpiochip_get_desc(gdev->chip, p->chip_hwnum); Why not gpio_device_get_desc()?
On Tue, Sep 05, 2023 at 08:53:02PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Using struct gpio_chip is not safe as it will disappear if the > underlying driver is unbound for any reason. Switch to using reference > counted struct gpio_device and its dedicated accessors. ... > + struct gpio_device *gdev __free(gpio_device_put) = NULL; Using this requires cleanup.h to be included. Does any of the included GPIO headers guarantee that inclusion implicitly? Even though, it's a good practice to include headers of what we are using independently if other (library) headers include them. I.o.w. we can rely only on our headers (here HTE framework related) to guarantee any inclusions implicitly. This also applies to other users of the same construct. ... > static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > { > return chip->fwnode == of_node_to_fwnode(data); > } Not sure how many users of this kind of match, but it might be useful to have it by GPIO library gpio_device_find_by_fwnode()
On Tue, Sep 05, 2023 at 08:53:03PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > is not affected by any of the related problems as this GPIO controller > cannot really go away but in order to finally remove this function, we > need to convert it to using gpio_device_find() as well. Side question, have you used --patience when preparing this series?
On Tue, Sep 05, 2023 at 08:53:06PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We're porting all users of gpiochip_find() to using gpio_device_find(). > Update the ACPI GPIO code. ... > - return gpiochip_get_desc(chip, pin); > + return gpiochip_get_desc(gdev->chip, pin); gpio_device_get_desc() ?
On Wed, Sep 6, 2023 at 4:50 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Sep 05, 2023 at 08:53:03PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > is not affected by any of the related problems as this GPIO controller > > cannot really go away but in order to finally remove this function, we > > need to convert it to using gpio_device_find() as well. > > Side question, have you used --patience when preparing this series? > Yes! Thanks for bringing it to my attention. Bart
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > The GPIO subsystem does not handle hot-unplug events very well. Yeah :/ it was never designed for this, and I've seen the discussions. The person who made the biggest effort to make this sort-of work was actually Johan Hovold so I added him to the mail so you can include him in upcoming iterations. I think he was working with GPIO on greybus at the time. Maybe he want to take a look! > Before we can even get to fixing the locking, we need to address a serious > abuse of the GPIO driver API - accessing struct gpio_chip by anyone who isn't > the driver owning this object. This structure is owned by the GPIO provider > and its lifetime is tied to that of that provider. It is destroyed when the > device is unregistered and this may happen at any moment. struct gpio_device > is the opaque, reference counted interface to struct gpio_chip (which is the > low-level implementation) and all access should pass through it. Thanks for looking into this. As I remember I have tried to bring down this abuse over the years and IIRC it used to be even worse, it came from the fact that all GPIO drivers used to be under some arch/* tree and often loosely using the kernel GPIO API but in addition providing a custom API... > The end-goal is to make all gpio_device manipulators check the existence of > gdev->chip and then lock it for the duration of any of the calls using SRCU. Excellent! > This series starts the process by replacing gpiochip_find() with > gpio_device_find(). This is in line with other device_find type interfaces and > returns a reference to the GPIO device that is guaranteed to remain valid > until it is put. I agree with the direction and I see no major problem with the patches other than some testing and cosmetics. The kernel sure as hell looks better *after* this than *before* so once you have rough confidence in the patches I think they should be merged and any issuse fixed up in-tree so we get wider audience and can continue the planned refactorings. So: Acked-by: Linus Walleij <linus.walleij@linaro.org> I'll try to provide some detailed reviews if something stands out. Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > By far the most common way of looking up GPIO devices is using their > label. Provide a helpers for that to avoid every user implementing their > own matching function. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Yeah this is what everyone and their dog reimplements. Good to centralize this. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > As the few users that need to get the reference to the GPIO device often > release it right after inspecting its properties, let's add support for > the automatic reference release to struct gpio_device. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Needless to say, I'm a strong advocate for scoped resource management. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Let's start adding getters for the opaque struct gpio_device. Start with > a function allowing to retrieve the base GPIO number. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> I guess you have a solid usecase for drivers needing to do this crazy thing, because I suppose you feel as much as me that this should rather be gpiolib-internal and confined to drivers/gpio/gpiolib.h? If you add a valid reason for making this globally visible outside of drivers/[gpio|pinctrl] to the commit message I guess I can live with it because we need to think of the bigger picture: Acked-by: Linus Walleij <linus.walleij@linaro.org> It brings to mind the now confusing "base" inside of struct gpio_chip. We all know it should go away, but since it is never used during the lifetime of the gpio_chip - or SHOULD never be used - it should rather be an argument to [devm_]gpiochip_add_data( .... int base);... Maybe something we should add to our TODO file. Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The process of converting all unauthorized users of struct gpio_chip to > using dedicated struct gpio_device function will be long so in the > meantime we must provide a way of retrieving the pointer to struct > gpio_chip from a GPIO device. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Yeah, sigh. Maybe add a notice to drivers/gpio/TODO that this needs to happen long time. Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Remove all remaining uses of find_chip_by_name() (and subsequently: > gpiochip_find()) from gpiolib.c and use the new > gpio_device_find_by_label() instead. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> With Andy's comment addressed: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Allow building all HTE modules with COMPILE_TEST Kconfig option enabled. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> This should be a separate patch should it not? Just send it separately to Dipen so he can merge it. Acked-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Using struct gpio_chip is not safe as it will disappear if the > underlying driver is unbound for any reason. Switch to using reference > counted struct gpio_device and its dedicated accessors. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> As Andy points out add <linux/cleanup.h>, with that fixed: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> I think this can be merged into the gpio tree after leaving some slack for the HTE maintainer to look at it, things look so much better after this. Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > is not affected by any of the related problems as this GPIO controller > cannot really go away but in order to finally remove this function, we > need to convert it to using gpio_device_find() as well. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> I was cleaning this one just some merge cycle ago, now it looks even better! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, Sep 7, 2023 at 9:22 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Allow building all HTE modules with COMPILE_TEST Kconfig option enabled. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > This should be a separate patch should it not? > Just send it separately to Dipen so he can merge it. > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > Yours, > Linus Walleij Dipen, Can you just pick this up and the other patch addressing a comment in a HTE driver separately? Would spare a resend to the list and I'd drop it from the series. Bart
Oops one more note: On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > is not affected by any of the related problems as this GPIO controller > cannot really go away but in order to finally remove this function, we > need to convert it to using gpio_device_find() as well. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> (...) > + struct gpio_device *gdev; (...) > + gdev = gpio_device_find_by_label(OMAP_GPIO_LABEL); This leaves a reference to the gdev right? No scoped guard? If you leave a dangling reference intentionally I think it warrants a comment ("leaving a ref here so the device will never be free:ed"). Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We're porting all users of gpiochip_find() to using gpio_device_find(). > Update the OF GPIO code. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
(Sorry for fatfinger reply) On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We're porting all users of gpiochip_find() to using gpio_device_find(). > Update the OF GPIO code. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Scoped guards, nice! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We're porting all users of gpiochip_find() to using gpio_device_find(). > Update the ACPI GPIO code. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> With Andy's comment addressed: Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, Sep 7, 2023 at 9:17 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Let's start adding getters for the opaque struct gpio_device. Start with > > a function allowing to retrieve the base GPIO number. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > I guess you have a solid usecase for drivers needing to do this > crazy thing, because I suppose you feel as much as me that > this should rather be gpiolib-internal and confined to > drivers/gpio/gpiolib.h? > > If you add a valid reason for making this globally visible outside > of drivers/[gpio|pinctrl] to the commit message I guess I can live > with it because we need to think of the bigger picture: > Acked-by: Linus Walleij <linus.walleij@linaro.org> > > It brings to mind the now confusing "base" inside of > struct gpio_chip. We all know it should go away, but since it > is never used during the lifetime of the gpio_chip - or SHOULD > never be used - it should rather be an argument to > [devm_]gpiochip_add_data( .... int base);... > > Maybe something we should add to our TODO file. > > Yours, > Linus Walleij For this series it's the HTE driver that uses it and I don't have a good idea about how to change it. Dipen? I would also love to make pinctrl not use the internal GPIOLIB header so it'll be another user, unless you can figure out a way to not use gc->base? :) I think we're stuck with it for now. Bart
On Thu, Sep 7, 2023 at 9:35 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > Oops one more note: > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > is not affected by any of the related problems as this GPIO controller > > cannot really go away but in order to finally remove this function, we > > need to convert it to using gpio_device_find() as well. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > (...) > > + struct gpio_device *gdev; > (...) > > + gdev = gpio_device_find_by_label(OMAP_GPIO_LABEL); > > This leaves a reference to the gdev right? No scoped guard? > > If you leave a dangling reference intentionally I think it warrants > a comment ("leaving a ref here so the device will never be > free:ed"). > It's right there in the comment. :) Bart
Dnia czwartek, 7 września 2023 09:31:01 CEST Linus Walleij pisze: > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > is not affected by any of the related problems as this GPIO controller > > cannot really go away but in order to finally remove this function, we > > need to convert it to using gpio_device_find() as well. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > I was cleaning this one just some merge cycle ago, now it > looks even better! > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> Thanks, Janusz > > Yours, > Linus Walleij >
On Fri, Sep 8, 2023 at 8:07 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote: > > Dnia czwartek, 7 września 2023 09:31:01 CEST Linus Walleij pisze: > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > > is not affected by any of the related problems as this GPIO controller > > > cannot really go away but in order to finally remove this function, we > > > need to convert it to using gpio_device_find() as well. > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > I was cleaning this one just some merge cycle ago, now it > > looks even better! > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > Janusz, Is it fine if I take it through the GPIO tree? Bartosz > Thanks, > Janusz > > > > > Yours, > > Linus Walleij > > > > > >
* Bartosz Golaszewski <brgl@bgdev.pl> [230911 11:10]: > On Fri, Sep 8, 2023 at 8:07 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote: > > > > Dnia czwartek, 7 września 2023 09:31:01 CEST Linus Walleij pisze: > > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > > > is not affected by any of the related problems as this GPIO controller > > > > cannot really go away but in order to finally remove this function, we > > > > need to convert it to using gpio_device_find() as well. > > > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > I was cleaning this one just some merge cycle ago, now it > > > looks even better! > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > > Janusz, > > Is it fine if I take it through the GPIO tree? Works for me at least: Acked-by: Tony Lindgren <tony@atomide.com>
Hi Bartosz, Dnia poniedziałek, 11 września 2023 13:09:56 CEST Bartosz Golaszewski pisze: > On Fri, Sep 8, 2023 at 8:07 PM Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote: > > > > Dnia czwartek, 7 września 2023 09:31:01 CEST Linus Walleij pisze: > > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > > > gpiochip_find() is going away as it's not hot-unplug safe. This platform > > > > is not affected by any of the related problems as this GPIO controller > > > > cannot really go away but in order to finally remove this function, we > > > > need to convert it to using gpio_device_find() as well. > > > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > I was cleaning this one just some merge cycle ago, now it > > > looks even better! > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > Acked-by: Janusz Krzysztofik <jmkrzyszt@gmail.com> > > > > Janusz, > > Is it fine if I take it through the GPIO tree? Yes, should be fine, I believe. Tony, Aaro, any doubts? Thanks, Janusz > > Bartosz > > > Thanks, > > Janusz > > > > > > > > Yours, > > > Linus Walleij > > > > > > > > > > > >
On 9/7/23 12:57 AM, Bartosz Golaszewski wrote: > On Thu, Sep 7, 2023 at 9:17 AM Linus Walleij <linus.walleij@linaro.org> wrote: >> >> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> Let's start adding getters for the opaque struct gpio_device. Start with >>> a function allowing to retrieve the base GPIO number. >>> >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> >> I guess you have a solid usecase for drivers needing to do this >> crazy thing, because I suppose you feel as much as me that >> this should rather be gpiolib-internal and confined to >> drivers/gpio/gpiolib.h? >> >> If you add a valid reason for making this globally visible outside >> of drivers/[gpio|pinctrl] to the commit message I guess I can live >> with it because we need to think of the bigger picture: >> Acked-by: Linus Walleij <linus.walleij@linaro.org> >> >> It brings to mind the now confusing "base" inside of >> struct gpio_chip. We all know it should go away, but since it >> is never used during the lifetime of the gpio_chip - or SHOULD >> never be used - it should rather be an argument to >> [devm_]gpiochip_add_data( .... int base);... >> >> Maybe something we should add to our TODO file. >> >> Yours, >> Linus Walleij > > For this series it's the HTE driver that uses it and I don't have a > good idea about how to change it. Dipen? Missed responding to this, apologies. I believe we are having similar discussion in the hte only patch. We can continue discussing there. > > I would also love to make pinctrl not use the internal GPIOLIB header > so it'll be another user, unless you can figure out a way to not use > gc->base? :) > > I think we're stuck with it for now. > > Bart
On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Using struct gpio_chip is not safe as it will disappear if the > > underlying driver is unbound for any reason. Switch to using reference > > counted struct gpio_device and its dedicated accessors. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > As Andy points out add <linux/cleanup.h>, with that fixed: > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > I think this can be merged into the gpio tree after leaving some > slack for the HTE maintainer to look at it, things look so much > better after this. > > Yours, > Linus Walleij Dipen, if you could give this patch a test and possibly ack it for me to take it through the GPIO tree (or go the immutable tag from HTE route) then it would be great. This is the last user of gpiochip_find() treewide, so with it we could remove it entirely for v6.7. Bart
On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: > On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >> >> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> Using struct gpio_chip is not safe as it will disappear if the >>> underlying driver is unbound for any reason. Switch to using reference >>> counted struct gpio_device and its dedicated accessors. >>> >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> >> As Andy points out add <linux/cleanup.h>, with that fixed: >> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >> >> I think this can be merged into the gpio tree after leaving some >> slack for the HTE maintainer to look at it, things look so much >> better after this. >> >> Yours, >> Linus Walleij > > Dipen, > > if you could give this patch a test and possibly ack it for me to take > it through the GPIO tree (or go the immutable tag from HTE route) then > it would be great. This is the last user of gpiochip_find() treewide, > so with it we could remove it entirely for v6.7. Progress so far for the RFT... I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, some patches I needed to manually apply and correct. With all this, it failed compilation at some spi/spi-bcm2835 driver. I disabled that and was able to compile. I thought I should let you know this part. Now, I tried to test the hte and it seems to fail finding the gpio device, roughly around this place [1]. I thought it would be your patch series so tried to just use 6.6rc1 without your patches and it still failed at the same place. I have to trace back now from which kernel version it broke. > > Bart
On 10/4/23 1:30 PM, Dipen Patel wrote: > On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>> >>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>> >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> Using struct gpio_chip is not safe as it will disappear if the >>>> underlying driver is unbound for any reason. Switch to using reference >>>> counted struct gpio_device and its dedicated accessors. >>>> >>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> As Andy points out add <linux/cleanup.h>, with that fixed: >>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>> >>> I think this can be merged into the gpio tree after leaving some >>> slack for the HTE maintainer to look at it, things look so much >>> better after this. >>> >>> Yours, >>> Linus Walleij >> >> Dipen, >> >> if you could give this patch a test and possibly ack it for me to take >> it through the GPIO tree (or go the immutable tag from HTE route) then >> it would be great. This is the last user of gpiochip_find() treewide, >> so with it we could remove it entirely for v6.7. > > Progress so far for the RFT... > > I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, > some patches I needed to manually apply and correct. With all this, it failed > compilation at some spi/spi-bcm2835 driver. I disabled that and was able to > compile. I thought I should let you know this part. > > Now, I tried to test the hte and it seems to fail finding the gpio device, > roughly around this place [1]. I thought it would be your patch series so > tried to just use 6.6rc1 without your patches and it still failed at the > same place. I have to trace back now from which kernel version it broke. [1]. https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 of course with your patches it would fail for the gdev instead of the chip. > >> >> Bart >
On 10/4/23 1:33 PM, Dipen Patel wrote: > On 10/4/23 1:30 PM, Dipen Patel wrote: >> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>> >>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>> >>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>> >>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>> underlying driver is unbound for any reason. Switch to using reference >>>>> counted struct gpio_device and its dedicated accessors. >>>>> >>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>> >>>> I think this can be merged into the gpio tree after leaving some >>>> slack for the HTE maintainer to look at it, things look so much >>>> better after this. >>>> >>>> Yours, >>>> Linus Walleij >>> >>> Dipen, >>> >>> if you could give this patch a test and possibly ack it for me to take >>> it through the GPIO tree (or go the immutable tag from HTE route) then >>> it would be great. This is the last user of gpiochip_find() treewide, >>> so with it we could remove it entirely for v6.7. >> >> Progress so far for the RFT... >> >> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >> some patches I needed to manually apply and correct. With all this, it failed >> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >> compile. I thought I should let you know this part. >> >> Now, I tried to test the hte and it seems to fail finding the gpio device, >> roughly around this place [1]. I thought it would be your patch series so >> tried to just use 6.6rc1 without your patches and it still failed at the >> same place. I have to trace back now from which kernel version it broke. > > [1]. > https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 > > of course with your patches it would fail for the gdev instead of the chip. Small update: I put some debugging prints in the gpio match function in the hte-tegra194.c as below: static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) { + struct device_node *node = data; + struct fwnode_handle *fw = of_node_to_fwnode(data); + if (!fw || !chip->fwnode) + pr_err("dipen patel: fw is null\n"); - pr_err("%s:%d\n", __func__, __LINE__); + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == fw), fw->dev->init_name); return chip->fwnode == of_node_to_fwnode(data); } The output of the printfs looks like below: [ 3.955194] dipen patel: fw is null -----> this message started appearing when I added !chip->fwnode test in the if condition line. [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, gpio@c2f0000, match?:0, fwnode name:(null) I conclude that chip->fwnode is empty. Any idea in which conditions that node would be empty? >> >>> >>> Bart >> >
On 10/4/23 3:54 PM, Dipen Patel wrote: > On 10/4/23 1:33 PM, Dipen Patel wrote: >> On 10/4/23 1:30 PM, Dipen Patel wrote: >>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>>> >>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>> >>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>> >>>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>>> underlying driver is unbound for any reason. Switch to using reference >>>>>> counted struct gpio_device and its dedicated accessors. >>>>>> >>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>> >>>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>> >>>>> I think this can be merged into the gpio tree after leaving some >>>>> slack for the HTE maintainer to look at it, things look so much >>>>> better after this. >>>>> >>>>> Yours, >>>>> Linus Walleij >>>> >>>> Dipen, >>>> >>>> if you could give this patch a test and possibly ack it for me to take >>>> it through the GPIO tree (or go the immutable tag from HTE route) then >>>> it would be great. This is the last user of gpiochip_find() treewide, >>>> so with it we could remove it entirely for v6.7. >>> >>> Progress so far for the RFT... >>> >>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >>> some patches I needed to manually apply and correct. With all this, it failed >>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >>> compile. I thought I should let you know this part. >>> >>> Now, I tried to test the hte and it seems to fail finding the gpio device, >>> roughly around this place [1]. I thought it would be your patch series so >>> tried to just use 6.6rc1 without your patches and it still failed at the >>> same place. I have to trace back now from which kernel version it broke. >> >> [1]. >> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 >> >> of course with your patches it would fail for the gdev instead of the chip. > > Small update: > > I put some debugging prints in the gpio match function in the hte-tegra194.c as > below: > > static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > { > + struct device_node *node = data; > + struct fwnode_handle *fw = of_node_to_fwnode(data); > + if (!fw || !chip->fwnode) > + pr_err("dipen patel: fw is null\n"); > > - pr_err("%s:%d\n", __func__, __LINE__); > + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", > __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == > fw), fw->dev->init_name); > return chip->fwnode == of_node_to_fwnode(data); > } > > The output of the printfs looks like below: > [ 3.955194] dipen patel: fw is null -----> this message started appearing > when I added !chip->fwnode test in the if condition line. > > [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, > gpio@c2f0000, match?:0, fwnode name:(null) > > I conclude that chip->fwnode is empty. Any idea in which conditions that node > would be empty? sorry for spamming, one last message before I sign off for the day.... Seems, adding below in the tegra gpio driver resolved the issue I am facing, I was able to verify your patch series. diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c index d87dd06db40d..a56c159d7136 100644 --- a/drivers/gpio/gpio-tegra186.c +++ b/drivers/gpio/gpio-tegra186.c @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) offset += port->pins; } + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); + return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); } Now, few follow up questions: 1) is this the correct way of setting the chip fwnode in the gpio driver? 2) Or should I use something else in hte matching function instead of fwnode so to avoid adding above line in the gpio driver? > >>> >>>> >>>> Bart >>> >> >
On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: > > On 10/4/23 3:54 PM, Dipen Patel wrote: > > On 10/4/23 1:33 PM, Dipen Patel wrote: > >> On 10/4/23 1:30 PM, Dipen Patel wrote: > >>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: > >>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > >>>>> > >>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >>>>> > >>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>> > >>>>>> Using struct gpio_chip is not safe as it will disappear if the > >>>>>> underlying driver is unbound for any reason. Switch to using reference > >>>>>> counted struct gpio_device and its dedicated accessors. > >>>>>> > >>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>> > >>>>> As Andy points out add <linux/cleanup.h>, with that fixed: > >>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>>>> > >>>>> I think this can be merged into the gpio tree after leaving some > >>>>> slack for the HTE maintainer to look at it, things look so much > >>>>> better after this. > >>>>> > >>>>> Yours, > >>>>> Linus Walleij > >>>> > >>>> Dipen, > >>>> > >>>> if you could give this patch a test and possibly ack it for me to take > >>>> it through the GPIO tree (or go the immutable tag from HTE route) then > >>>> it would be great. This is the last user of gpiochip_find() treewide, > >>>> so with it we could remove it entirely for v6.7. > >>> > >>> Progress so far for the RFT... > >>> > >>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, > >>> some patches I needed to manually apply and correct. With all this, it failed > >>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to > >>> compile. I thought I should let you know this part. > >>> > >>> Now, I tried to test the hte and it seems to fail finding the gpio device, > >>> roughly around this place [1]. I thought it would be your patch series so > >>> tried to just use 6.6rc1 without your patches and it still failed at the > >>> same place. I have to trace back now from which kernel version it broke. > >> > >> [1]. > >> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 > >> > >> of course with your patches it would fail for the gdev instead of the chip. > > > > Small update: > > > > I put some debugging prints in the gpio match function in the hte-tegra194.c as > > below: > > > > static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > > { > > + struct device_node *node = data; > > + struct fwnode_handle *fw = of_node_to_fwnode(data); > > + if (!fw || !chip->fwnode) > > + pr_err("dipen patel: fw is null\n"); > > > > - pr_err("%s:%d\n", __func__, __LINE__); > > + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", > > __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == > > fw), fw->dev->init_name); > > return chip->fwnode == of_node_to_fwnode(data); > > } > > > > The output of the printfs looks like below: > > [ 3.955194] dipen patel: fw is null -----> this message started appearing > > when I added !chip->fwnode test in the if condition line. > > > > [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, > > gpio@c2f0000, match?:0, fwnode name:(null) > > > > I conclude that chip->fwnode is empty. Any idea in which conditions that node > > would be empty? > > sorry for spamming, one last message before I sign off for the day.... > > Seems, adding below in the tegra gpio driver resolved the issue I am facing, I > was able to verify your patch series. > > diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > index d87dd06db40d..a56c159d7136 100644 > --- a/drivers/gpio/gpio-tegra186.c > +++ b/drivers/gpio/gpio-tegra186.c > @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) > offset += port->pins; > } > > + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); > + > return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); > } > > Now, few follow up questions: > 1) is this the correct way of setting the chip fwnode in the gpio driver? You shouldn't need this. This driver already does: gpio->gpio.parent = &pdev->dev; so fwnode should be assigned in gpiochip_add_data_with_key(). Can you check why this doesn't happen? Bart > 2) Or should I use something else in hte matching function instead of fwnode so > to avoid adding above line in the gpio driver? > > > > >>> > >>>> > >>>> Bart > >>> > >> > > >
On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: > On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: >> >> On 10/4/23 3:54 PM, Dipen Patel wrote: >>> On 10/4/23 1:33 PM, Dipen Patel wrote: >>>> On 10/4/23 1:30 PM, Dipen Patel wrote: >>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>>>>> >>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>> >>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>> >>>>>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>>>>> underlying driver is unbound for any reason. Switch to using reference >>>>>>>> counted struct gpio_device and its dedicated accessors. >>>>>>>> >>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>> >>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>>>> >>>>>>> I think this can be merged into the gpio tree after leaving some >>>>>>> slack for the HTE maintainer to look at it, things look so much >>>>>>> better after this. >>>>>>> >>>>>>> Yours, >>>>>>> Linus Walleij >>>>>> >>>>>> Dipen, >>>>>> >>>>>> if you could give this patch a test and possibly ack it for me to take >>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then >>>>>> it would be great. This is the last user of gpiochip_find() treewide, >>>>>> so with it we could remove it entirely for v6.7. >>>>> >>>>> Progress so far for the RFT... >>>>> >>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >>>>> some patches I needed to manually apply and correct. With all this, it failed >>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >>>>> compile. I thought I should let you know this part. >>>>> >>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, >>>>> roughly around this place [1]. I thought it would be your patch series so >>>>> tried to just use 6.6rc1 without your patches and it still failed at the >>>>> same place. I have to trace back now from which kernel version it broke. >>>> >>>> [1]. >>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 >>>> >>>> of course with your patches it would fail for the gdev instead of the chip. >>> >>> Small update: >>> >>> I put some debugging prints in the gpio match function in the hte-tegra194.c as >>> below: >>> >>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) >>> { >>> + struct device_node *node = data; >>> + struct fwnode_handle *fw = of_node_to_fwnode(data); >>> + if (!fw || !chip->fwnode) >>> + pr_err("dipen patel: fw is null\n"); >>> >>> - pr_err("%s:%d\n", __func__, __LINE__); >>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", >>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == >>> fw), fw->dev->init_name); >>> return chip->fwnode == of_node_to_fwnode(data); >>> } >>> >>> The output of the printfs looks like below: >>> [ 3.955194] dipen patel: fw is null -----> this message started appearing >>> when I added !chip->fwnode test in the if condition line. >>> >>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, >>> gpio@c2f0000, match?:0, fwnode name:(null) >>> >>> I conclude that chip->fwnode is empty. Any idea in which conditions that node >>> would be empty? >> >> sorry for spamming, one last message before I sign off for the day.... >> >> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I >> was able to verify your patch series. >> >> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c >> index d87dd06db40d..a56c159d7136 100644 >> --- a/drivers/gpio/gpio-tegra186.c >> +++ b/drivers/gpio/gpio-tegra186.c >> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) >> offset += port->pins; >> } >> >> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); >> + >> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); >> } >> >> Now, few follow up questions: >> 1) is this the correct way of setting the chip fwnode in the gpio driver? > > You shouldn't need this. This driver already does: > > gpio->gpio.parent = &pdev->dev; > > so fwnode should be assigned in gpiochip_add_data_with_key(). Can you > check why this doesn't happen? I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. The only reference I see is here [1]. Does it mean I need to change my match function from: chip->fwnode == of_node_to_fwnode(data) to: dev_fwnode(chip->parent) == of_node_to_fwnode(data)? [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 > > Bart > >> 2) Or should I use something else in hte matching function instead of fwnode so >> to avoid adding above line in the gpio driver? >> >>> >>>>> >>>>>> >>>>>> Bart >>>>> >>>> >>> >>
On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: > > On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: > > On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: > >> > >> On 10/4/23 3:54 PM, Dipen Patel wrote: > >>> On 10/4/23 1:33 PM, Dipen Patel wrote: > >>>> On 10/4/23 1:30 PM, Dipen Patel wrote: > >>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: > >>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > >>>>>>> > >>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >>>>>>> > >>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>>> > >>>>>>>> Using struct gpio_chip is not safe as it will disappear if the > >>>>>>>> underlying driver is unbound for any reason. Switch to using reference > >>>>>>>> counted struct gpio_device and its dedicated accessors. > >>>>>>>> > >>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>> > >>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: > >>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>>>>>> > >>>>>>> I think this can be merged into the gpio tree after leaving some > >>>>>>> slack for the HTE maintainer to look at it, things look so much > >>>>>>> better after this. > >>>>>>> > >>>>>>> Yours, > >>>>>>> Linus Walleij > >>>>>> > >>>>>> Dipen, > >>>>>> > >>>>>> if you could give this patch a test and possibly ack it for me to take > >>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then > >>>>>> it would be great. This is the last user of gpiochip_find() treewide, > >>>>>> so with it we could remove it entirely for v6.7. > >>>>> > >>>>> Progress so far for the RFT... > >>>>> > >>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, > >>>>> some patches I needed to manually apply and correct. With all this, it failed > >>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to > >>>>> compile. I thought I should let you know this part. > >>>>> > >>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, > >>>>> roughly around this place [1]. I thought it would be your patch series so > >>>>> tried to just use 6.6rc1 without your patches and it still failed at the > >>>>> same place. I have to trace back now from which kernel version it broke. > >>>> > >>>> [1]. > >>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 > >>>> > >>>> of course with your patches it would fail for the gdev instead of the chip. > >>> > >>> Small update: > >>> > >>> I put some debugging prints in the gpio match function in the hte-tegra194.c as > >>> below: > >>> > >>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > >>> { > >>> + struct device_node *node = data; > >>> + struct fwnode_handle *fw = of_node_to_fwnode(data); > >>> + if (!fw || !chip->fwnode) > >>> + pr_err("dipen patel: fw is null\n"); > >>> > >>> - pr_err("%s:%d\n", __func__, __LINE__); > >>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", > >>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == > >>> fw), fw->dev->init_name); > >>> return chip->fwnode == of_node_to_fwnode(data); > >>> } > >>> > >>> The output of the printfs looks like below: > >>> [ 3.955194] dipen patel: fw is null -----> this message started appearing > >>> when I added !chip->fwnode test in the if condition line. > >>> > >>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, > >>> gpio@c2f0000, match?:0, fwnode name:(null) > >>> > >>> I conclude that chip->fwnode is empty. Any idea in which conditions that node > >>> would be empty? > >> > >> sorry for spamming, one last message before I sign off for the day.... > >> > >> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I > >> was able to verify your patch series. > >> > >> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > >> index d87dd06db40d..a56c159d7136 100644 > >> --- a/drivers/gpio/gpio-tegra186.c > >> +++ b/drivers/gpio/gpio-tegra186.c > >> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) > >> offset += port->pins; > >> } > >> > >> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); > >> + > >> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); > >> } > >> > >> Now, few follow up questions: > >> 1) is this the correct way of setting the chip fwnode in the gpio driver? > > > > You shouldn't need this. This driver already does: > > > > gpio->gpio.parent = &pdev->dev; > > > > so fwnode should be assigned in gpiochip_add_data_with_key(). Can you > > check why this doesn't happen? > > I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. > The only reference I see is here [1]. Does it mean I need to change my match > function from: > > chip->fwnode == of_node_to_fwnode(data) > > to: > dev_fwnode(chip->parent) == of_node_to_fwnode(data)? No! chip->fwnode is only used to let GPIOLIB know which fwnode to assign to the GPIO device (struct gpio_device). Bart > > [1]: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 > > > > > Bart > > > >> 2) Or should I use something else in hte matching function instead of fwnode so > >> to avoid adding above line in the gpio driver? > >> > >>> > >>>>> > >>>>>> > >>>>>> Bart > >>>>> > >>>> > >>> > >> >
On 10/5/23 12:05 PM, Bartosz Golaszewski wrote: > On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: >> >> On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: >>> On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: >>>> >>>> On 10/4/23 3:54 PM, Dipen Patel wrote: >>>>> On 10/4/23 1:33 PM, Dipen Patel wrote: >>>>>> On 10/4/23 1:30 PM, Dipen Patel wrote: >>>>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>>>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>>>>>>> >>>>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>>>> >>>>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>> >>>>>>>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>>>>>>> underlying driver is unbound for any reason. Switch to using reference >>>>>>>>>> counted struct gpio_device and its dedicated accessors. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>> >>>>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>>>>>> >>>>>>>>> I think this can be merged into the gpio tree after leaving some >>>>>>>>> slack for the HTE maintainer to look at it, things look so much >>>>>>>>> better after this. >>>>>>>>> >>>>>>>>> Yours, >>>>>>>>> Linus Walleij >>>>>>>> >>>>>>>> Dipen, >>>>>>>> >>>>>>>> if you could give this patch a test and possibly ack it for me to take >>>>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then >>>>>>>> it would be great. This is the last user of gpiochip_find() treewide, >>>>>>>> so with it we could remove it entirely for v6.7. >>>>>>> >>>>>>> Progress so far for the RFT... >>>>>>> >>>>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >>>>>>> some patches I needed to manually apply and correct. With all this, it failed >>>>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >>>>>>> compile. I thought I should let you know this part. >>>>>>> >>>>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, >>>>>>> roughly around this place [1]. I thought it would be your patch series so >>>>>>> tried to just use 6.6rc1 without your patches and it still failed at the >>>>>>> same place. I have to trace back now from which kernel version it broke. >>>>>> >>>>>> [1]. >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 >>>>>> >>>>>> of course with your patches it would fail for the gdev instead of the chip. >>>>> >>>>> Small update: >>>>> >>>>> I put some debugging prints in the gpio match function in the hte-tegra194.c as >>>>> below: >>>>> >>>>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) >>>>> { >>>>> + struct device_node *node = data; >>>>> + struct fwnode_handle *fw = of_node_to_fwnode(data); >>>>> + if (!fw || !chip->fwnode) >>>>> + pr_err("dipen patel: fw is null\n"); >>>>> >>>>> - pr_err("%s:%d\n", __func__, __LINE__); >>>>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", >>>>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == >>>>> fw), fw->dev->init_name); >>>>> return chip->fwnode == of_node_to_fwnode(data); >>>>> } >>>>> >>>>> The output of the printfs looks like below: >>>>> [ 3.955194] dipen patel: fw is null -----> this message started appearing >>>>> when I added !chip->fwnode test in the if condition line. >>>>> >>>>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, >>>>> gpio@c2f0000, match?:0, fwnode name:(null) >>>>> >>>>> I conclude that chip->fwnode is empty. Any idea in which conditions that node >>>>> would be empty? >>>> >>>> sorry for spamming, one last message before I sign off for the day.... >>>> >>>> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I >>>> was able to verify your patch series. >>>> >>>> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c >>>> index d87dd06db40d..a56c159d7136 100644 >>>> --- a/drivers/gpio/gpio-tegra186.c >>>> +++ b/drivers/gpio/gpio-tegra186.c >>>> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) >>>> offset += port->pins; >>>> } >>>> >>>> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); >>>> + >>>> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); >>>> } >>>> >>>> Now, few follow up questions: >>>> 1) is this the correct way of setting the chip fwnode in the gpio driver? >>> >>> You shouldn't need this. This driver already does: >>> >>> gpio->gpio.parent = &pdev->dev; >>> >>> so fwnode should be assigned in gpiochip_add_data_with_key(). Can you >>> check why this doesn't happen? >> >> I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. >> The only reference I see is here [1]. Does it mean I need to change my match >> function from: >> >> chip->fwnode == of_node_to_fwnode(data) >> >> to: >> dev_fwnode(chip->parent) == of_node_to_fwnode(data)? > > No! chip->fwnode is only used to let GPIOLIB know which fwnode to > assign to the GPIO device (struct gpio_device). What do you suggest I should use for the match as I do not see chip->fwnode being set? > > Bart > >> >> [1]: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 >> >>> >>> Bart >>> >>>> 2) Or should I use something else in hte matching function instead of fwnode so >>>> to avoid adding above line in the gpio driver? >>>> >>>>> >>>>>>> >>>>>>>> >>>>>>>> Bart >>>>>>> >>>>>> >>>>> >>>> >>
On Thu, Oct 5, 2023 at 9:43 PM Dipen Patel <dipenp@nvidia.com> wrote: > > On 10/5/23 12:05 PM, Bartosz Golaszewski wrote: > > On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: > >> > >> On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: > >>> On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: > >>>> > >>>> On 10/4/23 3:54 PM, Dipen Patel wrote: > >>>>> On 10/4/23 1:33 PM, Dipen Patel wrote: > >>>>>> On 10/4/23 1:30 PM, Dipen Patel wrote: > >>>>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: > >>>>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > >>>>>>>>> > >>>>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >>>>>>>>> > >>>>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>>>>> > >>>>>>>>>> Using struct gpio_chip is not safe as it will disappear if the > >>>>>>>>>> underlying driver is unbound for any reason. Switch to using reference > >>>>>>>>>> counted struct gpio_device and its dedicated accessors. > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>>>> > >>>>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: > >>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>>>>>>>> > >>>>>>>>> I think this can be merged into the gpio tree after leaving some > >>>>>>>>> slack for the HTE maintainer to look at it, things look so much > >>>>>>>>> better after this. > >>>>>>>>> > >>>>>>>>> Yours, > >>>>>>>>> Linus Walleij > >>>>>>>> > >>>>>>>> Dipen, > >>>>>>>> > >>>>>>>> if you could give this patch a test and possibly ack it for me to take > >>>>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then > >>>>>>>> it would be great. This is the last user of gpiochip_find() treewide, > >>>>>>>> so with it we could remove it entirely for v6.7. > >>>>>>> > >>>>>>> Progress so far for the RFT... > >>>>>>> > >>>>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, > >>>>>>> some patches I needed to manually apply and correct. With all this, it failed > >>>>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to > >>>>>>> compile. I thought I should let you know this part. > >>>>>>> > >>>>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, > >>>>>>> roughly around this place [1]. I thought it would be your patch series so > >>>>>>> tried to just use 6.6rc1 without your patches and it still failed at the > >>>>>>> same place. I have to trace back now from which kernel version it broke. > >>>>>> > >>>>>> [1]. > >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 > >>>>>> > >>>>>> of course with your patches it would fail for the gdev instead of the chip. > >>>>> > >>>>> Small update: > >>>>> > >>>>> I put some debugging prints in the gpio match function in the hte-tegra194.c as > >>>>> below: > >>>>> > >>>>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > >>>>> { > >>>>> + struct device_node *node = data; > >>>>> + struct fwnode_handle *fw = of_node_to_fwnode(data); > >>>>> + if (!fw || !chip->fwnode) > >>>>> + pr_err("dipen patel: fw is null\n"); > >>>>> > >>>>> - pr_err("%s:%d\n", __func__, __LINE__); > >>>>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", > >>>>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == > >>>>> fw), fw->dev->init_name); > >>>>> return chip->fwnode == of_node_to_fwnode(data); > >>>>> } > >>>>> > >>>>> The output of the printfs looks like below: > >>>>> [ 3.955194] dipen patel: fw is null -----> this message started appearing > >>>>> when I added !chip->fwnode test in the if condition line. > >>>>> > >>>>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, > >>>>> gpio@c2f0000, match?:0, fwnode name:(null) > >>>>> > >>>>> I conclude that chip->fwnode is empty. Any idea in which conditions that node > >>>>> would be empty? > >>>> > >>>> sorry for spamming, one last message before I sign off for the day.... > >>>> > >>>> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I > >>>> was able to verify your patch series. > >>>> > >>>> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > >>>> index d87dd06db40d..a56c159d7136 100644 > >>>> --- a/drivers/gpio/gpio-tegra186.c > >>>> +++ b/drivers/gpio/gpio-tegra186.c > >>>> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) > >>>> offset += port->pins; > >>>> } > >>>> > >>>> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); > >>>> + > >>>> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); > >>>> } > >>>> > >>>> Now, few follow up questions: > >>>> 1) is this the correct way of setting the chip fwnode in the gpio driver? > >>> > >>> You shouldn't need this. This driver already does: > >>> > >>> gpio->gpio.parent = &pdev->dev; > >>> > >>> so fwnode should be assigned in gpiochip_add_data_with_key(). Can you > >>> check why this doesn't happen? > >> > >> I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. > >> The only reference I see is here [1]. Does it mean I need to change my match > >> function from: > >> > >> chip->fwnode == of_node_to_fwnode(data) > >> > >> to: > >> dev_fwnode(chip->parent) == of_node_to_fwnode(data)? > > > > No! chip->fwnode is only used to let GPIOLIB know which fwnode to > > assign to the GPIO device (struct gpio_device). > What do you suggest I should use for the match as I do not see chip->fwnode > being set? > Andy, Linus, Do you think it makes sense to make gpiochip_add_data_with_key() assign the chip's fwnode if it's not set by the caller (and instead taken from the parent device) for this particular use-case? I think it's fine but wanted to run it by you. Bart > > > > Bart > > > >> > >> [1]: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 > >> > >>> > >>> Bart > >>> > >>>> 2) Or should I use something else in hte matching function instead of fwnode so > >>>> to avoid adding above line in the gpio driver? > >>>> > >>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> Bart > >>>>>>> > >>>>>> > >>>>> > >>>> > >> >
On Thu, Oct 5, 2023 at 9:43 PM Dipen Patel <dipenp@nvidia.com> wrote: > > On 10/5/23 12:05 PM, Bartosz Golaszewski wrote: > > On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: > >> > >> On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: > >>> On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: > >>>> > >>>> On 10/4/23 3:54 PM, Dipen Patel wrote: > >>>>> On 10/4/23 1:33 PM, Dipen Patel wrote: > >>>>>> On 10/4/23 1:30 PM, Dipen Patel wrote: > >>>>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: > >>>>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: > >>>>>>>>> > >>>>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > >>>>>>>>> > >>>>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>>>>> > >>>>>>>>>> Using struct gpio_chip is not safe as it will disappear if the > >>>>>>>>>> underlying driver is unbound for any reason. Switch to using reference > >>>>>>>>>> counted struct gpio_device and its dedicated accessors. > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>>>>>>>> > >>>>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: > >>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > >>>>>>>>> > >>>>>>>>> I think this can be merged into the gpio tree after leaving some > >>>>>>>>> slack for the HTE maintainer to look at it, things look so much > >>>>>>>>> better after this. > >>>>>>>>> > >>>>>>>>> Yours, > >>>>>>>>> Linus Walleij > >>>>>>>> > >>>>>>>> Dipen, > >>>>>>>> > >>>>>>>> if you could give this patch a test and possibly ack it for me to take > >>>>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then > >>>>>>>> it would be great. This is the last user of gpiochip_find() treewide, > >>>>>>>> so with it we could remove it entirely for v6.7. > >>>>>>> > >>>>>>> Progress so far for the RFT... > >>>>>>> > >>>>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, > >>>>>>> some patches I needed to manually apply and correct. With all this, it failed > >>>>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to > >>>>>>> compile. I thought I should let you know this part. > >>>>>>> > >>>>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, > >>>>>>> roughly around this place [1]. I thought it would be your patch series so > >>>>>>> tried to just use 6.6rc1 without your patches and it still failed at the > >>>>>>> same place. I have to trace back now from which kernel version it broke. > >>>>>> > >>>>>> [1]. > >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 > >>>>>> > >>>>>> of course with your patches it would fail for the gdev instead of the chip. > >>>>> > >>>>> Small update: > >>>>> > >>>>> I put some debugging prints in the gpio match function in the hte-tegra194.c as > >>>>> below: > >>>>> > >>>>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) > >>>>> { > >>>>> + struct device_node *node = data; > >>>>> + struct fwnode_handle *fw = of_node_to_fwnode(data); > >>>>> + if (!fw || !chip->fwnode) > >>>>> + pr_err("dipen patel: fw is null\n"); > >>>>> > >>>>> - pr_err("%s:%d\n", __func__, __LINE__); > >>>>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", > >>>>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == > >>>>> fw), fw->dev->init_name); > >>>>> return chip->fwnode == of_node_to_fwnode(data); > >>>>> } > >>>>> > >>>>> The output of the printfs looks like below: > >>>>> [ 3.955194] dipen patel: fw is null -----> this message started appearing > >>>>> when I added !chip->fwnode test in the if condition line. > >>>>> > >>>>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, > >>>>> gpio@c2f0000, match?:0, fwnode name:(null) > >>>>> > >>>>> I conclude that chip->fwnode is empty. Any idea in which conditions that node > >>>>> would be empty? > >>>> > >>>> sorry for spamming, one last message before I sign off for the day.... > >>>> > >>>> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I > >>>> was able to verify your patch series. > >>>> > >>>> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > >>>> index d87dd06db40d..a56c159d7136 100644 > >>>> --- a/drivers/gpio/gpio-tegra186.c > >>>> +++ b/drivers/gpio/gpio-tegra186.c > >>>> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) > >>>> offset += port->pins; > >>>> } > >>>> > >>>> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); > >>>> + > >>>> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); > >>>> } > >>>> > >>>> Now, few follow up questions: > >>>> 1) is this the correct way of setting the chip fwnode in the gpio driver? > >>> > >>> You shouldn't need this. This driver already does: > >>> > >>> gpio->gpio.parent = &pdev->dev; > >>> > >>> so fwnode should be assigned in gpiochip_add_data_with_key(). Can you > >>> check why this doesn't happen? > >> > >> I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. > >> The only reference I see is here [1]. Does it mean I need to change my match > >> function from: > >> > >> chip->fwnode == of_node_to_fwnode(data) > >> > >> to: > >> dev_fwnode(chip->parent) == of_node_to_fwnode(data)? > > > > No! chip->fwnode is only used to let GPIOLIB know which fwnode to > > assign to the GPIO device (struct gpio_device). > What do you suggest I should use for the match as I do not see chip->fwnode > being set? > This is most likely going to be a longer discussion. I suggest that in the meantime you just assign the gc->fwnode pointer explicitly from the platform device in the tegra GPIO driver and use it in the lookup function. Note that this is NOT wrong or a hack. It's just that most devices don't need to be looked up using gpio_device_find(). Bart > > > > Bart > > > >> > >> [1]: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 > >> > >>> > >>> Bart > >>> > >>>> 2) Or should I use something else in hte matching function instead of fwnode so > >>>> to avoid adding above line in the gpio driver? > >>>> > >>>>> > >>>>>>> > >>>>>>>> > >>>>>>>> Bart > >>>>>>> > >>>>>> > >>>>> > >>>> > >> >
On 10/8/23 11:48 PM, Bartosz Golaszewski wrote: > On Thu, Oct 5, 2023 at 9:43 PM Dipen Patel <dipenp@nvidia.com> wrote: >> >> On 10/5/23 12:05 PM, Bartosz Golaszewski wrote: >>> On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: >>>> >>>> On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: >>>>> On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: >>>>>> >>>>>> On 10/4/23 3:54 PM, Dipen Patel wrote: >>>>>>> On 10/4/23 1:33 PM, Dipen Patel wrote: >>>>>>>> On 10/4/23 1:30 PM, Dipen Patel wrote: >>>>>>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>>>>>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>>>>>>>>> >>>>>>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>>>>>> >>>>>>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>>>> >>>>>>>>>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>>>>>>>>> underlying driver is unbound for any reason. Switch to using reference >>>>>>>>>>>> counted struct gpio_device and its dedicated accessors. >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>>> >>>>>>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>>>>>>>> >>>>>>>>>>> I think this can be merged into the gpio tree after leaving some >>>>>>>>>>> slack for the HTE maintainer to look at it, things look so much >>>>>>>>>>> better after this. >>>>>>>>>>> >>>>>>>>>>> Yours, >>>>>>>>>>> Linus Walleij >>>>>>>>>> >>>>>>>>>> Dipen, >>>>>>>>>> >>>>>>>>>> if you could give this patch a test and possibly ack it for me to take >>>>>>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then >>>>>>>>>> it would be great. This is the last user of gpiochip_find() treewide, >>>>>>>>>> so with it we could remove it entirely for v6.7. >>>>>>>>> >>>>>>>>> Progress so far for the RFT... >>>>>>>>> >>>>>>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >>>>>>>>> some patches I needed to manually apply and correct. With all this, it failed >>>>>>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >>>>>>>>> compile. I thought I should let you know this part. >>>>>>>>> >>>>>>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, >>>>>>>>> roughly around this place [1]. I thought it would be your patch series so >>>>>>>>> tried to just use 6.6rc1 without your patches and it still failed at the >>>>>>>>> same place. I have to trace back now from which kernel version it broke. >>>>>>>> >>>>>>>> [1]. >>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 >>>>>>>> >>>>>>>> of course with your patches it would fail for the gdev instead of the chip. >>>>>>> >>>>>>> Small update: >>>>>>> >>>>>>> I put some debugging prints in the gpio match function in the hte-tegra194.c as >>>>>>> below: >>>>>>> >>>>>>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) >>>>>>> { >>>>>>> + struct device_node *node = data; >>>>>>> + struct fwnode_handle *fw = of_node_to_fwnode(data); >>>>>>> + if (!fw || !chip->fwnode) >>>>>>> + pr_err("dipen patel: fw is null\n"); >>>>>>> >>>>>>> - pr_err("%s:%d\n", __func__, __LINE__); >>>>>>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", >>>>>>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == >>>>>>> fw), fw->dev->init_name); >>>>>>> return chip->fwnode == of_node_to_fwnode(data); >>>>>>> } >>>>>>> >>>>>>> The output of the printfs looks like below: >>>>>>> [ 3.955194] dipen patel: fw is null -----> this message started appearing >>>>>>> when I added !chip->fwnode test in the if condition line. >>>>>>> >>>>>>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, >>>>>>> gpio@c2f0000, match?:0, fwnode name:(null) >>>>>>> >>>>>>> I conclude that chip->fwnode is empty. Any idea in which conditions that node >>>>>>> would be empty? >>>>>> >>>>>> sorry for spamming, one last message before I sign off for the day.... >>>>>> >>>>>> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I >>>>>> was able to verify your patch series. >>>>>> >>>>>> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c >>>>>> index d87dd06db40d..a56c159d7136 100644 >>>>>> --- a/drivers/gpio/gpio-tegra186.c >>>>>> +++ b/drivers/gpio/gpio-tegra186.c >>>>>> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) >>>>>> offset += port->pins; >>>>>> } >>>>>> >>>>>> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); >>>>>> + >>>>>> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); >>>>>> } >>>>>> >>>>>> Now, few follow up questions: >>>>>> 1) is this the correct way of setting the chip fwnode in the gpio driver? >>>>> >>>>> You shouldn't need this. This driver already does: >>>>> >>>>> gpio->gpio.parent = &pdev->dev; >>>>> >>>>> so fwnode should be assigned in gpiochip_add_data_with_key(). Can you >>>>> check why this doesn't happen? >>>> >>>> I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. >>>> The only reference I see is here [1]. Does it mean I need to change my match >>>> function from: >>>> >>>> chip->fwnode == of_node_to_fwnode(data) >>>> >>>> to: >>>> dev_fwnode(chip->parent) == of_node_to_fwnode(data)? >>> >>> No! chip->fwnode is only used to let GPIOLIB know which fwnode to >>> assign to the GPIO device (struct gpio_device). >> What do you suggest I should use for the match as I do not see chip->fwnode >> being set? >> > > This is most likely going to be a longer discussion. I suggest that in > the meantime you just assign the gc->fwnode pointer explicitly from > the platform device in the tegra GPIO driver and use it in the lookup > function. Note that this is NOT wrong or a hack. It's just that most > devices don't need to be looked up using gpio_device_find(). Sure, at the same time, I am also find to use any other method/s. > > Bart > >>> >>> Bart >>> >>>> >>>> [1]: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 >>>> >>>>> >>>>> Bart >>>>> >>>>>> 2) Or should I use something else in hte matching function instead of fwnode so >>>>>> to avoid adding above line in the gpio driver? >>>>>> >>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Bart >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >>
On 10/9/23 9:34 AM, Dipen Patel wrote: > On 10/8/23 11:48 PM, Bartosz Golaszewski wrote: >> On Thu, Oct 5, 2023 at 9:43 PM Dipen Patel <dipenp@nvidia.com> wrote: >>> >>> On 10/5/23 12:05 PM, Bartosz Golaszewski wrote: >>>> On Thu, Oct 5, 2023 at 8:12 PM Dipen Patel <dipenp@nvidia.com> wrote: >>>>> >>>>> On 10/5/23 6:48 AM, Bartosz Golaszewski wrote: >>>>>> On Thu, Oct 5, 2023 at 1:52 AM Dipen Patel <dipenp@nvidia.com> wrote: >>>>>>> >>>>>>> On 10/4/23 3:54 PM, Dipen Patel wrote: >>>>>>>> On 10/4/23 1:33 PM, Dipen Patel wrote: >>>>>>>>> On 10/4/23 1:30 PM, Dipen Patel wrote: >>>>>>>>>> On 10/4/23 5:00 AM, Bartosz Golaszewski wrote: >>>>>>>>>>> On Thu, Sep 7, 2023 at 9:28 AM Linus Walleij <linus.walleij@linaro.org> wrote: >>>>>>>>>>>> >>>>>>>>>>>> On Tue, Sep 5, 2023 at 8:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>>>>> >>>>>>>>>>>>> Using struct gpio_chip is not safe as it will disappear if the >>>>>>>>>>>>> underlying driver is unbound for any reason. Switch to using reference >>>>>>>>>>>>> counted struct gpio_device and its dedicated accessors. >>>>>>>>>>>>> >>>>>>>>>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>>>>>>>>> >>>>>>>>>>>> As Andy points out add <linux/cleanup.h>, with that fixed: >>>>>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> >>>>>>>>>>>> >>>>>>>>>>>> I think this can be merged into the gpio tree after leaving some >>>>>>>>>>>> slack for the HTE maintainer to look at it, things look so much >>>>>>>>>>>> better after this. >>>>>>>>>>>> >>>>>>>>>>>> Yours, >>>>>>>>>>>> Linus Walleij >>>>>>>>>>> >>>>>>>>>>> Dipen, >>>>>>>>>>> >>>>>>>>>>> if you could give this patch a test and possibly ack it for me to take >>>>>>>>>>> it through the GPIO tree (or go the immutable tag from HTE route) then >>>>>>>>>>> it would be great. This is the last user of gpiochip_find() treewide, >>>>>>>>>>> so with it we could remove it entirely for v6.7. >>>>>>>>>> >>>>>>>>>> Progress so far for the RFT... >>>>>>>>>> >>>>>>>>>> I tried applying the patch series on 6.6-rc1 and it did not apply cleanly, >>>>>>>>>> some patches I needed to manually apply and correct. With all this, it failed >>>>>>>>>> compilation at some spi/spi-bcm2835 driver. I disabled that and was able to >>>>>>>>>> compile. I thought I should let you know this part. >>>>>>>>>> >>>>>>>>>> Now, I tried to test the hte and it seems to fail finding the gpio device, >>>>>>>>>> roughly around this place [1]. I thought it would be your patch series so >>>>>>>>>> tried to just use 6.6rc1 without your patches and it still failed at the >>>>>>>>>> same place. I have to trace back now from which kernel version it broke. >>>>>>>>> >>>>>>>>> [1]. >>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/pateldipen1984/linux.git/tree/drivers/hte/hte-tegra194.c?h=for-next#n781 >>>>>>>>> >>>>>>>>> of course with your patches it would fail for the gdev instead of the chip. >>>>>>>> >>>>>>>> Small update: >>>>>>>> >>>>>>>> I put some debugging prints in the gpio match function in the hte-tegra194.c as >>>>>>>> below: >>>>>>>> >>>>>>>> static int tegra_gpiochip_match(struct gpio_chip *chip, void *data) >>>>>>>> { >>>>>>>> + struct device_node *node = data; >>>>>>>> + struct fwnode_handle *fw = of_node_to_fwnode(data); >>>>>>>> + if (!fw || !chip->fwnode) >>>>>>>> + pr_err("dipen patel: fw is null\n"); >>>>>>>> >>>>>>>> - pr_err("%s:%d\n", __func__, __LINE__); >>>>>>>> + pr_err("dipen patel, %s:%d: %s, %s, %s, match?:%d, fwnode name:%s\n", >>>>>>>> __func__, __LINE__, chip->label, node->name, node->full_name, (chip->fwnode == >>>>>>>> fw), fw->dev->init_name); >>>>>>>> return chip->fwnode == of_node_to_fwnode(data); >>>>>>>> } >>>>>>>> >>>>>>>> The output of the printfs looks like below: >>>>>>>> [ 3.955194] dipen patel: fw is null -----> this message started appearing >>>>>>>> when I added !chip->fwnode test in the if condition line. >>>>>>>> >>>>>>>> [ 3.958864] dipen patel, tegra_gpiochip_match:689: tegra234-gpio, gpio, >>>>>>>> gpio@c2f0000, match?:0, fwnode name:(null) >>>>>>>> >>>>>>>> I conclude that chip->fwnode is empty. Any idea in which conditions that node >>>>>>>> would be empty? >>>>>>> >>>>>>> sorry for spamming, one last message before I sign off for the day.... >>>>>>> >>>>>>> Seems, adding below in the tegra gpio driver resolved the issue I am facing, I >>>>>>> was able to verify your patch series. >>>>>>> >>>>>>> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c >>>>>>> index d87dd06db40d..a56c159d7136 100644 >>>>>>> --- a/drivers/gpio/gpio-tegra186.c >>>>>>> +++ b/drivers/gpio/gpio-tegra186.c >>>>>>> @@ -989,6 +989,8 @@ static int tegra186_gpio_probe(struct platform_device *pdev) >>>>>>> offset += port->pins; >>>>>>> } >>>>>>> >>>>>>> + gpio->gpio.fwnode = of_node_to_fwnode(pdev->dev.of_node); >>>>>>> + >>>>>>> return devm_gpiochip_add_data(&pdev->dev, &gpio->gpio, gpio); >>>>>>> } >>>>>>> >>>>>>> Now, few follow up questions: >>>>>>> 1) is this the correct way of setting the chip fwnode in the gpio driver? >>>>>> >>>>>> You shouldn't need this. This driver already does: >>>>>> >>>>>> gpio->gpio.parent = &pdev->dev; >>>>>> >>>>>> so fwnode should be assigned in gpiochip_add_data_with_key(). Can you >>>>>> check why this doesn't happen? >>>>> >>>>> I do not see anywhere chip->fwnode being set in the gpiochip_add_* function. >>>>> The only reference I see is here [1]. Does it mean I need to change my match >>>>> function from: >>>>> >>>>> chip->fwnode == of_node_to_fwnode(data) >>>>> >>>>> to: >>>>> dev_fwnode(chip->parent) == of_node_to_fwnode(data)? >>>> >>>> No! chip->fwnode is only used to let GPIOLIB know which fwnode to >>>> assign to the GPIO device (struct gpio_device). >>> What do you suggest I should use for the match as I do not see chip->fwnode >>> being set? >>> >> >> This is most likely going to be a longer discussion. I suggest that in >> the meantime you just assign the gc->fwnode pointer explicitly from >> the platform device in the tegra GPIO driver and use it in the lookup >> function. Note that this is NOT wrong or a hack. It's just that most >> devices don't need to be looked up using gpio_device_find(). > > Sure, at the same time, I am also find to use any other method/s. (Correction) I am also fine* With patch https://patchwork.ozlabs.org/project/linux-gpio/patch/20231009173858.723686-1-dipenp@nvidia.com/ Tested-by: Dipen Patel <dipenp@nvidia.com> >> >> Bart >> >>>> >>>> Bart >>>> >>>>> >>>>> [1]: >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.6-rc1#n767 >>>>> >>>>>> >>>>>> Bart >>>>>> >>>>>>> 2) Or should I use something else in hte matching function instead of fwnode so >>>>>>> to avoid adding above line in the gpio driver? >>>>>>> >>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Bart >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>> >>> >
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> The GPIO subsystem does not handle hot-unplug events very well. We have recently patched the user-space part of it so that at least a rouge user cannot crash the kernel but in-kernel users are still affected by a lot of issues: from incorrect locking or lack thereof to using structures that are private to GPIO drivers. Since almost all GPIO controllers can be unbound, not to mention that we have USB devices registering GPIO expanders as well as I2C-on-USB HID devices on which I2C GPIO expanders can live, various media gadgets etc., we really need to make GPIO hotplug/unplug friendly. Before we can even get to fixing the locking, we need to address a serious abuse of the GPIO driver API - accessing struct gpio_chip by anyone who isn't the driver owning this object. This structure is owned by the GPIO provider and its lifetime is tied to that of that provider. It is destroyed when the device is unregistered and this may happen at any moment. struct gpio_device is the opaque, reference counted interface to struct gpio_chip (which is the low-level implementation) and all access should pass through it. The end-goal is to make all gpio_device manipulators check the existence of gdev->chip and then lock it for the duration of any of the calls using SRCU. Before we can get there, we need to first provide a set of functions that will replace any gpio_chip functions and convert all in-kernel users. This series starts the process by replacing gpiochip_find() with gpio_device_find(). This is in line with other device_find type interfaces and returns a reference to the GPIO device that is guaranteed to remain valid until it is put. Note that this does not make everything correct just yet. Especially the GPIOLIB internal users release the reference returned by the lookup function after getting the descriptor of interest but before requesting it. This will eventually be addressed. This is not a regression either. First we add a bunch of new APIs that are needed to start replacing calls to gpiochip_find. We then use them first in external users and then locally in GPIOLIB core. Finally we remove gpiochip_find(). Bartosz Golaszewski (21): gpiolib: make gpio_device_get() and gpio_device_put() public gpiolib: provide gpio_device_find() gpiolib: provide gpio_device_find_by_label() gpiolib: provide gpio_device_get_desc() gpiolib: add support for scope-based management to gpio_device gpiolib: provide gpiod_to_device() gpiolib: provide gpio_device_get_base() gpio: acpi: provide acpi_gpio_device_free_interrupts() gpiolib: reluctantly provide gpio_device_get_chip() gpiolib: replace find_chip_by_name() with gpio_device_find_by_label() platform: x86: android-tablets: don't access GPIOLIB private members hte: allow building modules with COMPILE_TEST enabled hte: tegra194: improve the GPIO-related comment hte: tegra194: don't access struct gpio_chip arm: omap1: ams-delta: stop using gpiochip_find() gpio: of: correct notifier return codes gpio: of: replace gpiochip_find_* with gpio_device_find_* gpio: acpi: replace gpiochip_find() with gpio_device_find() gpio: swnode: replace gpiochip_find() with gpio_device_find_by_label() gpio: sysfs: drop the mention of gpiochip_find() from sysfs code gpiolib: remove gpiochip_find() arch/arm/mach-omap1/board-ams-delta.c | 36 ++-- drivers/gpio/gpiolib-acpi.c | 37 +++- drivers/gpio/gpiolib-of.c | 48 ++--- drivers/gpio/gpiolib-swnode.c | 29 ++- drivers/gpio/gpiolib-sysfs.c | 2 +- drivers/gpio/gpiolib.c | 203 +++++++++++++----- drivers/gpio/gpiolib.h | 10 - drivers/hte/Kconfig | 4 +- drivers/hte/hte-tegra194.c | 49 +++-- .../platform/x86/x86-android-tablets/core.c | 38 ++-- include/linux/gpio/driver.h | 30 ++- 11 files changed, 316 insertions(+), 170 deletions(-)