| Message ID | 20230905185309.131295-16-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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 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: > 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
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 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 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
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.