Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set

[Jon Hunter]

On 20/03/2026 04:49, Tzung-Bi Shih wrote:

...

> The context '}' here suggests that commit 16fdabe143fc ("gpio: Fix resource
> leaks on errors in gpiochip_add_data_with_key()") might not be applied in
> your code base.  After applying that commit, this code should look like:
>
>      err_put_device:
>          gpio_device_put(gdev);
>          goto err_print_message;
>
>      err_cleanup_desc_srcu:
>          cleanup_srcu_struct(&gdev->desc_srcu);
>
> I'll use v6.19 (i.e., without the commit) for the following examples.

Yes that's correct that commit is not present.

> > >    err_cleanup_desc_srcu:
> > > +       synchronize_srcu(&gdev->desc_srcu);
> > >           cleanup_srcu_struct(&gdev->desc_srcu);
> > >    err_cleanup_gdev_srcu:
> > >           cleanup_srcu_struct(&gdev->srcu);
> >
> > Hi Tzung-Bi, allow me to Cc you. It looks like someone takes the SRCU lock
> > during the call to gpiochip_add_data_with_key() and this is why the cleanup
> > path complains. Does it make sense to add this synchronize_srcu() call here?
>
> No, I think this is very unusual: `gdev` is still initializing in
> gpiochip_add_data_with_key(), but someone else already starts to access
> members of `gdev`.
>
> > > > >     gpiochip_add_data_with_key: GPIOs 512..675 (tegra234-gpio) failed to register, -16
> > > > >     tegra186-gpio 2200000.gpio: probe with driver tegra186-gpio failed with error -16
>
> Along with the above patch, the -16 (-EBUSY) should be from
> gpiodev_add_to_list_unlocked()[1].
>
>      scoped_guard(mutex, &gpio_devices_lock) {
>          ...
>
>          ret = gpiodev_add_to_list_unlocked(gdev);
>          if (ret) {
>              gpiochip_err(gc, "GPIO integer space overlap, cannot add chip\n");
>              goto err_cleanup_desc_srcu;
>          }
>      }
>
> [1] https://elixir.bootlin.com/linux/v6.19/source/drivers/gpio/gpiolib.c#L1151
>
> My understanding is that within this function, there appear to be no other
> users of `gdev->desc_srcu` between the calls to init_srcu_struct() and
> gpiodev_add_to_list_unlocked().
>
> At the point gpiodev_add_to_list_unlocked() is called, `gc->gpiodev` and
> `gdev->descs` have also been set.
>
> Jon: My main concern is about potential races from other threads.  Is it
> possible that another thread could start accessing struct gpio_desc elements
> (e.g., via gpiochip_request_own_desc() and desc_set_label()) before
> gpiochip_add_data_with_key() has fully completed the initialization of `gdev`?


So today I tried to reproduce this WARNING with next-20260319 by making 
the erroneous change in device-tree (combining gpio-hogs with shared 
gpios) but after doing so I don't see this. So for now I don't think 
that we should worry about this.

Jon

--
nvpublic