| 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:50PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > gpiochip_find() is wrong and its kernel doc is misleading as the > function doesn't return a reference to the gpio_chip but just a raw > pointer. The chip itself is not guaranteed to stay alive, in fact it can > be deleted at any point. Also: other than GPIO drivers themselves, > nobody else has any business accessing gpio_chip structs. > > Provide a new gpio_device_find() function that returns a real reference > to the opaque gpio_device structure that is guaranteed to stay alive for > as long as there are active users of it. ... > +/** > + * gpio_device_find() - find a specific GPIO device > + * @data: data to pass to match function > + * @match: Callback function to check gpio_chip > + * Returns: > + * New reference to struct gpio_device. I believe this is wrong location of the Return section. AFAIU how kernel doc uses section markers, this entire description becomes a Return(s) section. Have you tried to render man/html/pdf and see this? > + * Similar to bus_find_device(). It returns a reference to a gpio_device as > + * determined by a user supplied @match callback. The callback should return > + * 0 if the device doesn't match and non-zero if it does. If the callback > + * returns non-zero, this function will return to the caller and not iterate > + * over any more gpio_devices. > + * > + * The callback takes the GPIO chip structure as argument. During the execution > + * of the callback function the chip is protected from being freed. TODO: This > + * actually has yet to be implemented. > + * > + * If the function returns non-NULL, the returned reference must be freed by > + * the caller using gpio_device_put(). > + */ > +struct gpio_device *gpio_device_find(void *data, > + int (*match)(struct gpio_chip *gc, > + void *data)) One line? Or maybe a type for it? (gpio_match_fn, for example) > +{ > + struct gpio_device *gdev; > + > + guard(spinlock_irqsave)(&gpio_lock); > + > + list_for_each_entry(gdev, &gpio_devices, list) { > + if (gdev->chip && match(gdev->chip, data)) > + return gpio_device_get(gdev); > + } > + > + return NULL; > +} ... > +struct gpio_device *gpio_device_find(void *data, > + int (*match)(struct gpio_chip *gc, > + void *data)); Ditto.
On Tue, Sep 05, 2023 at 08:52:52PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Getting the GPIO descriptor directly from the gpio_chip struct is > dangerous as we don't take the reference to the underlying GPIO device. > In order to start working towards removing gpiochip_get_desc(), let's > provide a safer variant that works with an existing reference to struct > gpio_device. ... > +/** > + * gpio_device_get_desc() - get the GPIO descriptor corresponding to the given > + * hardware number for this GPIO device > + * @gdev: GPIO device to get the descriptor from > + * @hwnum: hardware number of the GPIO for this chip > + * > + * Returns: > + * A pointer to the GPIO descriptor or ``ERR_PTR(-EINVAL)`` if no GPIO exists The known constants can be referenced as %EINVAL. > + * in the given chip for the specified hardware number or ``ERR_PTR(-ENODEV)`` Ditto. > + * if the underlying chip already vanished. > + * > + * The reference count of struct gpio_device is *NOT* increased like when the > + * GPIO is being requested for exclusive usage. It's up to the caller to make > + * sure the GPIO device will stay alive together with the descriptor returned > + * by this function. > + */
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:09PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > With all users of gpiochip_find() converted to using gpio_device_find(), > we can now remove this function from the kernel. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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> > > In order to start migrating away from accessing struct gpio_chip by > users other than their owners, let's first make the reference management > functions for the opaque struct gpio_device public in the driver.h > header. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> 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> > > gpiochip_find() is wrong and its kernel doc is misleading as the > function doesn't return a reference to the gpio_chip but just a raw > pointer. The chip itself is not guaranteed to stay alive, in fact it can > be deleted at any point. Also: other than GPIO drivers themselves, > nobody else has any business accessing gpio_chip structs. > > Provide a new gpio_device_find() function that returns a real reference > to the opaque gpio_device structure that is guaranteed to stay alive for > as long as there are active users of it. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> 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> > > Getting the GPIO descriptor directly from the gpio_chip struct is > dangerous as we don't take the reference to the underlying GPIO device. > In order to start working towards removing gpiochip_get_desc(), let's > provide a safer variant that works with an existing reference to struct > gpio_device. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Andy had some good doc comments, with these 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> > > 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> > > 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 have removed all callers of gpiochip_find() so don't mention it in > gpiolib-sysfs.c. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> 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> > > With all users of gpiochip_find() converted to using gpio_device_find(), > we can now remove this function from the kernel. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> This is so much cleaner, and introducing some scoped guards along the way it's a clear win. 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>
On Wed, Sep 6, 2023 at 4:10 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Sep 05, 2023 at 08:52:50PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > gpiochip_find() is wrong and its kernel doc is misleading as the > > function doesn't return a reference to the gpio_chip but just a raw > > pointer. The chip itself is not guaranteed to stay alive, in fact it can > > be deleted at any point. Also: other than GPIO drivers themselves, > > nobody else has any business accessing gpio_chip structs. > > > > Provide a new gpio_device_find() function that returns a real reference > > to the opaque gpio_device structure that is guaranteed to stay alive for > > as long as there are active users of it. > > ... > > > +/** > > + * gpio_device_find() - find a specific GPIO device > > + * @data: data to pass to match function > > + * @match: Callback function to check gpio_chip > > > + * Returns: > > + * New reference to struct gpio_device. > > I believe this is wrong location of the Return section. > AFAIU how kernel doc uses section markers, this entire description becomes > a Return(s) section. Have you tried to render man/html/pdf and see this? > Yes, it works just fine. Try for yourself: scripts/kernel-doc -rst drivers/gpio/gpiolib.c Bart > > + * Similar to bus_find_device(). It returns a reference to a gpio_device as > > + * determined by a user supplied @match callback. The callback should return > > + * 0 if the device doesn't match and non-zero if it does. If the callback > > + * returns non-zero, this function will return to the caller and not iterate > > + * over any more gpio_devices. > > + * > > + * The callback takes the GPIO chip structure as argument. During the execution > > + * of the callback function the chip is protected from being freed. TODO: This > > + * actually has yet to be implemented. > > + * > > + * If the function returns non-NULL, the returned reference must be freed by > > + * the caller using gpio_device_put(). > > + */ > > +struct gpio_device *gpio_device_find(void *data, > > > + int (*match)(struct gpio_chip *gc, > > + void *data)) > > One line? > Or maybe a type for it? (gpio_match_fn, for example) > > > +{ > > + struct gpio_device *gdev; > > + > > + guard(spinlock_irqsave)(&gpio_lock); > > + > > + list_for_each_entry(gdev, &gpio_devices, list) { > > + if (gdev->chip && match(gdev->chip, data)) > > + return gpio_device_get(gdev); > > + } > > + > > + return NULL; > > +} > > ... > > > +struct gpio_device *gpio_device_find(void *data, > > + int (*match)(struct gpio_chip *gc, > > + void *data)); > > Ditto. > > > -- > With Best Regards, > Andy Shevchenko > >
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 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> > --- > arch/arm/mach-omap1/board-ams-delta.c | 36 +++++++++++++-------------- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c > index 9808cd27e2cf..a28ea6ac1eba 100644 > --- a/arch/arm/mach-omap1/board-ams-delta.c > +++ b/arch/arm/mach-omap1/board-ams-delta.c > @@ -560,22 +560,6 @@ static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = { > &ams_delta_nand_gpio_table, > }; > > -/* > - * Some drivers may not use GPIO lookup tables but need to be provided > - * with GPIO numbers. The same applies to GPIO based IRQ lines - some > - * drivers may even not use GPIO layer but expect just IRQ numbers. > - * We could either define GPIO lookup tables then use them on behalf > - * of those devices, or we can use GPIO driver level methods for > - * identification of GPIO and IRQ numbers. For the purpose of the latter, > - * defina a helper function which identifies GPIO chips by their labels. > - */ > -static int gpiochip_match_by_label(struct gpio_chip *chip, void *data) > -{ > - char *label = data; > - > - return !strcmp(label, chip->label); > -} > - > static struct gpiod_hog ams_delta_gpio_hogs[] = { > GPIO_HOG(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT, "keybrd_dataout", > GPIO_ACTIVE_HIGH, GPIOD_OUT_LOW), > @@ -615,14 +599,28 @@ static void __init modem_assign_irq(struct gpio_chip *chip) > */ > static void __init omap_gpio_deps_init(void) > { > + struct gpio_device *gdev; > struct gpio_chip *chip; > > - chip = gpiochip_find(OMAP_GPIO_LABEL, gpiochip_match_by_label); > - if (!chip) { > - pr_err("%s: OMAP GPIO chip not found\n", __func__); > + /* > + * Some drivers may not use GPIO lookup tables but need to be provided > + * with GPIO numbers. The same applies to GPIO based IRQ lines - some > + * drivers may even not use GPIO layer but expect just IRQ numbers. > + * We could either define GPIO lookup tables then use them on behalf > + * of those devices, or we can use GPIO driver level methods for > + * identification of GPIO and IRQ numbers. > + * > + * This reference will be leaked but that's alright as this device > + * never goes down. > + */ > + gdev = gpio_device_find_by_label(OMAP_GPIO_LABEL); > + if (!gdev) { > + pr_err("%s: OMAP GPIO device not found\n", __func__); > return; > } > > + chip = gpio_device_get_chip(gdev); > + > /* > * Start with FIQ initialization as it may have to request > * and release successfully each OMAP GPIO pin in turn. > -- > 2.39.2 > Patch applied, thanks! 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/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(-)