Message ID | 20230113205922.2312951-1-andreas@kemnade.info |
---|---|
State | Accepted |
Commit | 92bf78b33b0b463b00c6b0203b49aea845daecc8 |
Headers | show |
Series | gpio: omap: use dynamic allocation of base | expand |
On Mon, Jan 16, 2023 at 9:54 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Fri, Jan 13, 2023 at 9:59 PM Andreas Kemnade <andreas@kemnade.info> wrote: > > > > Static allocatin is deprecated and may cause probe mess, > > if probe order is unusual. > > > > like this example > > [ 2.553833] twl4030_gpio twl4030-gpio: gpio (irq 145) chaining IRQs 161..178 > > [ 2.561401] gpiochip_find_base: found new base at 160 > > [ 2.564392] gpio gpiochip5: (twl4030): added GPIO chardev (254:5) > > [ 2.564544] gpio gpiochip5: registered GPIOs 160 to 177 on twl4030 > > [...] > > [ 2.692169] omap-gpmc 6e000000.gpmc: GPMC revision 5.0 > > [ 2.697357] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 > > [ 2.703643] gpiochip_find_base: found new base at 178 > > [ 2.704376] gpio gpiochip6: (omap-gpmc): added GPIO chardev (254:6) > > [ 2.704589] gpio gpiochip6: registered GPIOs 178 to 181 on omap-gpmc > > [...] > > [ 2.840393] gpio gpiochip7: Static allocation of GPIO base is deprecated, use dynamic allocation. > > [ 2.849365] gpio gpiochip7: (gpio-160-191): GPIO integer space overlap, cannot add chip > > [ 2.857513] gpiochip_add_data_with_key: GPIOs 160..191 (gpio-160-191) failed to register, -16 > > [ 2.866149] omap_gpio 48310000.gpio: error -EBUSY: Could not register gpio chip > > > > So probing was done in an unusual order, causing mess > > and chips not getting their gpio in the end. > > > > Signed-off-by: Andreas Kemnade <andreas@kemnade.info> > > --- > > maybe CC stable? not sure about good fixes tag. > > > > drivers/gpio/gpio-omap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > > index 80ddc43fd875..f5f3d4b22452 100644 > > --- a/drivers/gpio/gpio-omap.c > > +++ b/drivers/gpio/gpio-omap.c > > @@ -1020,7 +1020,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc, > > if (!label) > > return -ENOMEM; > > bank->chip.label = label; > > - bank->chip.base = gpio; > > + bank->chip.base = -1; > > } > > bank->chip.ngpio = bank->width; > > > > -- > > 2.30.2 > > > > This could potentially break some legacy user-space programs using > sysfs but whatever, let's apply it and see if anyone complains. FWIW, this broke users pace on my side. Not a super big deal, I'll just revert this patch for now. Maybe the application in question can get rewritten to find the gpio by label.
On Thu, Aug 29, 2024 at 10:52 AM Richard Weinberger <richard.weinberger@gmail.com> wrote: > > This could potentially break some legacy user-space programs using > > sysfs but whatever, let's apply it and see if anyone complains. > > FWIW, this broke users pace on my side. > Not a super big deal, I'll just revert this patch for now. > Maybe the application in question can get rewritten to find the gpio by label. Ugh we might need to back this out if the userspace is critical and you need it. Ideally userspace should use libgpiod for GPIO access, but I understand if it's a higher bar. Yours, Linus Walleij
Linus, On Sat, Aug 31, 2024 at 12:47 AM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Thu, Aug 29, 2024 at 10:52 AM Richard Weinberger > <richard.weinberger@gmail.com> wrote: > > > > This could potentially break some legacy user-space programs using > > > sysfs but whatever, let's apply it and see if anyone complains. > > > > FWIW, this broke users pace on my side. > > Not a super big deal, I'll just revert this patch for now. > > Maybe the application in question can get rewritten to find the gpio by label. > > Ugh we might need to back this out if the userspace is critical > and you need it. > > Ideally userspace should use libgpiod for GPIO access, but I understand > if it's a higher bar. In the meanwhile I've explained to everyone involved on the project that gpiod is the way to go and the application will get changed. So, no worries. :-) But I'm not sure about other applications in the wild, writing a number to /sys/class/gpio/export is just too easy and people assume the numbers are stable.
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 80ddc43fd875..f5f3d4b22452 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1020,7 +1020,7 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc, if (!label) return -ENOMEM; bank->chip.label = label; - bank->chip.base = gpio; + bank->chip.base = -1; } bank->chip.ngpio = bank->width;
Static allocatin is deprecated and may cause probe mess, if probe order is unusual. like this example [ 2.553833] twl4030_gpio twl4030-gpio: gpio (irq 145) chaining IRQs 161..178 [ 2.561401] gpiochip_find_base: found new base at 160 [ 2.564392] gpio gpiochip5: (twl4030): added GPIO chardev (254:5) [ 2.564544] gpio gpiochip5: registered GPIOs 160 to 177 on twl4030 [...] [ 2.692169] omap-gpmc 6e000000.gpmc: GPMC revision 5.0 [ 2.697357] gpmc_mem_init: disabling cs 0 mapped at 0x0-0x1000000 [ 2.703643] gpiochip_find_base: found new base at 178 [ 2.704376] gpio gpiochip6: (omap-gpmc): added GPIO chardev (254:6) [ 2.704589] gpio gpiochip6: registered GPIOs 178 to 181 on omap-gpmc [...] [ 2.840393] gpio gpiochip7: Static allocation of GPIO base is deprecated, use dynamic allocation. [ 2.849365] gpio gpiochip7: (gpio-160-191): GPIO integer space overlap, cannot add chip [ 2.857513] gpiochip_add_data_with_key: GPIOs 160..191 (gpio-160-191) failed to register, -16 [ 2.866149] omap_gpio 48310000.gpio: error -EBUSY: Could not register gpio chip So probing was done in an unusual order, causing mess and chips not getting their gpio in the end. Signed-off-by: Andreas Kemnade <andreas@kemnade.info> --- maybe CC stable? not sure about good fixes tag. drivers/gpio/gpio-omap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)