Re: [PATCH] gpio: Revert regression in sysfs-gpio (gpiolib.c)

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Mar 14, 2022 at 4:55 PM Michael Walle <michael@xxxxxxxx> wrote:
>
> Hi,
>
> > Some GPIO lines have stopped working after the patch
> > commit 2ab73c6d8323f ("gpio: Support GPIO controllers without pin-ranges")
> >
> > And this has supposedly been fixed in the following patches
> > commit 89ad556b7f96a ("gpio: Avoid using pin ranges with !PINCTRL")
> > commit 6dbbf84603961 ("gpiolib: Don't free if pin ranges are not defined")
> >
> > But an erratic behavior where some GPIO lines work while others do not work
> > has been introduced.
> >
> > This patch reverts those changes so that the sysfs-gpio interface works
> > properly again.
> >
> > Signed-off-by: Marcelo Roberto Jimenez <marcelo.jimenez@xxxxxxxxx>
>
> This breaks the pinctrl-microchip-sgpio driver as far as I can see.
>
> I tried to debug it and this is what I have discovered so far:
>  (1) the sgpio driver will use the gpio_stub_drv for its child nodes.
>      Looks like a workaround, see [1].
>  (2) these will have an empty gpio range
>  (3) with the changes of this patch, pinctrl_gpio_request() will now
>      be called and will fail with -EPROBE_DEFER.
>
> I'm not exactly sure what to do here. Saravana Kannan once suggested
> to use devm_of_platform_populate() to probe the child nodes [2]. But
> I haven't found any other driver doing that.
>

TI AEMIF driver (drivers/memory/ti-aemif.c) does something like this:

406         if (np) {
407                 for_each_available_child_of_node(np, child_np) {
408                         ret = of_platform_populate(child_np, NULL,
409                                                    dev_lookup, dev);
410                         if (ret < 0) {
411                                 of_node_put(child_np);
412                                 goto error;
413                         }
414                 }
415         } else if (pdata) {
416                 for (i = 0; i < pdata->num_sub_devices; i++) {
417                         pdata->sub_devices[i].dev.parent = dev;
418                         ret =
platform_device_register(&pdata->sub_devices[i]);
419                         if (ret) {
420                                 dev_warn(dev, "Error register sub
device %s\n",
421                                          pdata->sub_devices[i].name);
422                         }
423                 }
424         }

A bunch of different devices (like NAND) get instantiated this way.
Would this work?

Bart

> Also, I'm not sure if there are any other other driver which get
> broken by this. I.e. ones falling into the gpio_stub_drv category.
>
> [1] https://lore.kernel.org/lkml/20210122193600.1415639-1-saravanak@xxxxxxxxxx/
> [2] https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@xxxxxxxxxxxxxx/
>
> -michael
>
> NB. this patch doesn't contain a Fixes tag. Was this on purpose?