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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[+ Saravana ]

Am 2022-03-15 16:32, schrieb Bartosz Golaszewski:
On Mon, Mar 14, 2022 at 4:55 PM Michael Walle <michael@xxxxxxxx> wrote:
> 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.

Oh I meant gpio/pinctrl drivers.

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?

I started to try this out, but then I was wondering if there weren't
other gpio/pinctrl drivers with the same problem. And judging by the
reports [1], I'd say there are. Then I wasn't sure if this is actually
the correct fix here - or if that old workaround [2] doesn't work
anymore because it might have that empty ranges "feature".

To answer your question: I don't know. But I don't know if that is
actually the correct way of fixing this either.

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?

-michael

[1] https://lore.kernel.org/lkml/20220314192522.GA3031157@xxxxxxxxxxxx/
[2] https://lore.kernel.org/lkml/20210122193600.1415639-1-saravanak@xxxxxxxxxx/



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux