diff mbox series

gpio: omap: use dynamic allocation of base

Message ID 20230113205922.2312951-1-andreas@kemnade.info
State Accepted
Commit 92bf78b33b0b463b00c6b0203b49aea845daecc8
Headers show
Series gpio: omap: use dynamic allocation of base | expand

Commit Message

Andreas Kemnade Jan. 13, 2023, 8:59 p.m. UTC
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(-)

Comments

Richard Weinberger Aug. 29, 2024, 8:52 a.m. UTC | #1
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.
Linus Walleij Aug. 30, 2024, 10:46 p.m. UTC | #2
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
Richard Weinberger Aug. 31, 2024, 7:32 a.m. UTC | #3
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 mbox series

Patch

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;