* [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
@ 2026-03-16 13:52 Bartosz Golaszewski
2026-03-17 8:47 ` Linus Walleij
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-16 13:52 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-kernel, Jon Hunter, Bartosz Golaszewski
OF-based GPIO controller drivers may provide a translation function that
calculates the real chip offset from whatever devicetree sources
provide. We need to take this into account in the shared GPIO management
and call of_xlate() if it's provided and adjust the entry->offset we
initially set when scanning the tree.
To that end: modify the shared GPIO API to take the GPIO chip as
argument on setup (to avoid having to rcu_dereference() it from the GPIO
device) and protect the access to entry->offset with the existing lock.
Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Closes: https://lore.kernel.org/all/921ba8ce-b18e-4a99-966d-c763d22081e2@nvidia.com/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-shared.c | 27 ++++++++++++++++++++++++++-
drivers/gpio/gpiolib-shared.h | 4 ++--
drivers/gpio/gpiolib.c | 2 +-
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
index 17a7128b6bd9bf6023deccee50b2453caebe3d9a..3a8db9bf456daaf021d3c691677a90fc6da15889 100644
--- a/drivers/gpio/gpiolib-shared.c
+++ b/drivers/gpio/gpiolib-shared.c
@@ -506,8 +506,9 @@ static void gpio_shared_remove_adev(struct auxiliary_device *adev)
auxiliary_device_uninit(adev);
}
-int gpio_device_setup_shared(struct gpio_device *gdev)
+int gpiochip_setup_shared(struct gpio_chip *gc)
{
+ struct gpio_device *gdev = gc->gpiodev;
struct gpio_shared_entry *entry;
struct gpio_shared_ref *ref;
struct gpio_desc *desc;
@@ -532,12 +533,34 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
* exposing shared pins. Find them and create the proxy devices.
*/
list_for_each_entry(entry, &gpio_shared_list, list) {
+ guard(mutex)(&entry->lock);
+
if (!device_match_fwnode(&gdev->dev, entry->fwnode))
continue;
if (list_count_nodes(&entry->refs) <= 1)
continue;
+#if IS_ENABLED(CONFIG_OF)
+ if (is_of_node(entry->fwnode) && gc->of_xlate) {
+ /*
+ * This is the earliest that we can tranlate the
+ * devicetree offset to the chip offset.
+ */
+ struct of_phandle_args gpiospec = { };
+
+ gpiospec.np = to_of_node(entry->fwnode);
+ gpiospec.args_count = 2;
+ gpiospec.args[0] = entry->offset;
+
+ ret = gc->of_xlate(gc, &gpiospec, NULL);
+ if (ret < 0)
+ return ret;
+
+ entry->offset = ret;
+ }
+#endif /* CONFIG_OF */
+
desc = &gdev->descs[entry->offset];
__set_bit(GPIOD_FLAG_SHARED, &desc->flags);
@@ -575,6 +598,8 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
struct gpio_shared_ref *ref;
list_for_each_entry(entry, &gpio_shared_list, list) {
+ guard(mutex)(&entry->lock);
+
if (!device_match_fwnode(&gdev->dev, entry->fwnode))
continue;
diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
index 40568ef7364ccbf08b7f583e279a7d5b572af477..e11e260e1f590c46c5e575d3bb8f3b5a2240892d 100644
--- a/drivers/gpio/gpiolib-shared.h
+++ b/drivers/gpio/gpiolib-shared.h
@@ -14,14 +14,14 @@ struct device;
#if IS_ENABLED(CONFIG_GPIO_SHARED)
-int gpio_device_setup_shared(struct gpio_device *gdev);
+int gpiochip_setup_shared(struct gpio_chip *gc);
void gpio_device_teardown_shared(struct gpio_device *gdev);
int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id,
unsigned long lflags);
#else
-static inline int gpio_device_setup_shared(struct gpio_device *gdev)
+static inline int gpiochip_setup_shared(struct gpio_chip *gc)
{
return 0;
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6e000ad58a11f7e3de85d8a0630150368acc53ce..1777efe1a986c941da464da92255c261f27a5a6b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1223,7 +1223,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
if (ret)
goto err_remove_irqchip_mask;
- ret = gpio_device_setup_shared(gdev);
+ ret = gpiochip_setup_shared(gc);
if (ret)
goto err_remove_irqchip;
---
base-commit: eadf2995ab7c8158bf694304d41e49cada263277
change-id: 20260316-gpio-shared-xlate-708c651cac5f
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-16 13:52 [PATCH] gpio: shared: call gpio_chip::of_xlate() if set Bartosz Golaszewski
@ 2026-03-17 8:47 ` Linus Walleij
2026-03-17 10:12 ` Jon Hunter
2026-03-17 12:53 ` Jon Hunter
2 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2026-03-17 8:47 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, Jon Hunter
On Mon, Mar 16, 2026 at 2:52 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
> OF-based GPIO controller drivers may provide a translation function that
> calculates the real chip offset from whatever devicetree sources
> provide. We need to take this into account in the shared GPIO management
> and call of_xlate() if it's provided and adjust the entry->offset we
> initially set when scanning the tree.
>
> To that end: modify the shared GPIO API to take the GPIO chip as
> argument on setup (to avoid having to rcu_dereference() it from the GPIO
> device) and protect the access to entry->offset with the existing lock.
>
> Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/all/921ba8ce-b18e-4a99-966d-c763d22081e2@nvidia.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
That was a tricky one!
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-16 13:52 [PATCH] gpio: shared: call gpio_chip::of_xlate() if set Bartosz Golaszewski
2026-03-17 8:47 ` Linus Walleij
@ 2026-03-17 10:12 ` Jon Hunter
2026-03-17 11:44 ` Bartosz Golaszewski
2026-03-17 12:53 ` Jon Hunter
2 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-17 10:12 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-kernel, linux-tegra@vger.kernel.org
Hi Bartosz,
On 16/03/2026 13:52, Bartosz Golaszewski wrote:
> OF-based GPIO controller drivers may provide a translation function that
> calculates the real chip offset from whatever devicetree sources
> provide. We need to take this into account in the shared GPIO management
> and call of_xlate() if it's provided and adjust the entry->offset we
> initially set when scanning the tree.
>
> To that end: modify the shared GPIO API to take the GPIO chip as
> argument on setup (to avoid having to rcu_dereference() it from the GPIO
> device) and protect the access to entry->offset with the existing lock.
>
> Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/all/921ba8ce-b18e-4a99-966d-c763d22081e2@nvidia.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> drivers/gpio/gpiolib-shared.c | 27 ++++++++++++++++++++++++++-
> drivers/gpio/gpiolib-shared.h | 4 ++--
> drivers/gpio/gpiolib.c | 2 +-
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 17a7128b6bd9bf6023deccee50b2453caebe3d9a..3a8db9bf456daaf021d3c691677a90fc6da15889 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
> @@ -506,8 +506,9 @@ static void gpio_shared_remove_adev(struct auxiliary_device *adev)
> auxiliary_device_uninit(adev);
> }
>
> -int gpio_device_setup_shared(struct gpio_device *gdev)
> +int gpiochip_setup_shared(struct gpio_chip *gc)
> {
> + struct gpio_device *gdev = gc->gpiodev;
> struct gpio_shared_entry *entry;
> struct gpio_shared_ref *ref;
> struct gpio_desc *desc;
> @@ -532,12 +533,34 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
> * exposing shared pins. Find them and create the proxy devices.
> */
> list_for_each_entry(entry, &gpio_shared_list, list) {
> + guard(mutex)(&entry->lock);
> +
> if (!device_match_fwnode(&gdev->dev, entry->fwnode))
> continue;
>
> if (list_count_nodes(&entry->refs) <= 1)
> continue;
>
> +#if IS_ENABLED(CONFIG_OF)
> + if (is_of_node(entry->fwnode) && gc->of_xlate) {
> + /*
> + * This is the earliest that we can tranlate the
> + * devicetree offset to the chip offset.
> + */
> + struct of_phandle_args gpiospec = { };
> +
> + gpiospec.np = to_of_node(entry->fwnode);
> + gpiospec.args_count = 2;
> + gpiospec.args[0] = entry->offset;
> +
> + ret = gc->of_xlate(gc, &gpiospec, NULL);
> + if (ret < 0)
> + return ret;
> +
> + entry->offset = ret;
> + }
> +#endif /* CONFIG_OF */
> +
> desc = &gdev->descs[entry->offset];
>
> __set_bit(GPIOD_FLAG_SHARED, &desc->flags);
> @@ -575,6 +598,8 @@ void gpio_device_teardown_shared(struct gpio_device *gdev)
> struct gpio_shared_ref *ref;
>
> list_for_each_entry(entry, &gpio_shared_list, list) {
> + guard(mutex)(&entry->lock);
> +
> if (!device_match_fwnode(&gdev->dev, entry->fwnode))
> continue;
>
> diff --git a/drivers/gpio/gpiolib-shared.h b/drivers/gpio/gpiolib-shared.h
> index 40568ef7364ccbf08b7f583e279a7d5b572af477..e11e260e1f590c46c5e575d3bb8f3b5a2240892d 100644
> --- a/drivers/gpio/gpiolib-shared.h
> +++ b/drivers/gpio/gpiolib-shared.h
> @@ -14,14 +14,14 @@ struct device;
>
> #if IS_ENABLED(CONFIG_GPIO_SHARED)
>
> -int gpio_device_setup_shared(struct gpio_device *gdev);
> +int gpiochip_setup_shared(struct gpio_chip *gc);
> void gpio_device_teardown_shared(struct gpio_device *gdev);
> int gpio_shared_add_proxy_lookup(struct device *consumer, const char *con_id,
> unsigned long lflags);
>
> #else
>
> -static inline int gpio_device_setup_shared(struct gpio_device *gdev)
> +static inline int gpiochip_setup_shared(struct gpio_chip *gc)
> {
> return 0;
> }
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 6e000ad58a11f7e3de85d8a0630150368acc53ce..1777efe1a986c941da464da92255c261f27a5a6b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1223,7 +1223,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> if (ret)
> goto err_remove_irqchip_mask;
>
> - ret = gpio_device_setup_shared(gdev);
> + ret = gpiochip_setup_shared(gc);
> if (ret)
> goto err_remove_irqchip;
>
>
Thanks for sending this. However, I am seeing a different issue now ...
------------[ cut here ]------------
WARNING: drivers/gpio/gpiolib-shared.c:499 at gpio_shared_add_proxy_lookup+0x118/0x1d8, CPU#8: swapper/0/1
Modules linked in:
CPU: 8 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-rc3-next-20260309-00005-g02826fefa46f #14 PREEMPT
Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-42974706 11/20/2025
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : gpio_shared_add_proxy_lookup+0x118/0x1d8
lr : gpio_shared_add_proxy_lookup+0xfc/0x1d8
sp : ffff8000832bba30
x29: ffff8000832bba30 x28: ffff000080d01010 x27: ffffffffffffefff
x26: 0000000000000001 x25: ffff800082df0538 x24: ffff800082df0528
x23: 0000000000000000 x22: ffff00008012c158 x21: ffff000081455010
usb 1-3: new full-speed USB device number 2 using tegra-xusb
x20: ffff000080d5d430 x19: ffff00008012c158 x18: 00000000ffffffff
x17: ffff8000830786a8 x16: ffff800083078718 x15: ffff8000832bb880
x14: ffffffffffffffff x13: 0000000000000008 x12: 0101010101010101
x11: 7f7f7f7f7f7f7f7f x10: ffff8000827f20d0 x9 : 0000000000000003
x8 : 0101010101010101 x7 : 0080808080808000 x6 : 15151a0a59460209
x5 : 000000000000003c x4 : ffff8000832bb990 x3 : ffff0000800fe800
x2 : ffff0000801c2f40 x1 : ffff0000801c2f40 x0 : ffff800082df0538
Call trace:
gpio_shared_add_proxy_lookup+0x118/0x1d8 (P)
gpiod_find_and_request+0x1bc/0x548
devm_fwnode_gpiod_get_index+0x1c/0x6c
gpio_keys_probe+0x494/0x9fc
platform_probe+0x5c/0x98
really_probe+0xbc/0x2a8
__driver_probe_device+0x78/0x12c
driver_probe_device+0x3c/0x15c
__driver_attach+0x90/0x19c
bus_for_each_dev+0x78/0xd4
driver_attach+0x24/0x30
bus_add_driver+0xe4/0x208
driver_register+0x5c/0x124
__platform_driver_register+0x24/0x30
gpio_keys_init+0x1c/0x28
do_one_initcall+0x7c/0x1c0
kernel_init_freeable+0x204/0x2ec
kernel_init+0x24/0x1e0
ret_from_fork+0x10/0x20
---[ end trace 0000000000000000 ]---
I see the comment says ...
/* We warn here because this can only happen if the programmer borked. */
WARN_ON(1);
I will take a closer look, but let me know if you have any thoughts?
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 10:12 ` Jon Hunter
@ 2026-03-17 11:44 ` Bartosz Golaszewski
2026-03-17 12:53 ` Jon Hunter
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-17 11:44 UTC (permalink / raw)
To: Jon Hunter
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On Tue, Mar 17, 2026 at 11:12 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> Thanks for sending this. However, I am seeing a different issue now ...
>
> ------------[ cut here ]------------
> WARNING: drivers/gpio/gpiolib-shared.c:499 at gpio_shared_add_proxy_lookup+0x118/0x1d8, CPU#8: swapper/0/1
> Modules linked in:
> CPU: 8 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-rc3-next-20260309-00005-g02826fefa46f #14 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-42974706 11/20/2025
> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : gpio_shared_add_proxy_lookup+0x118/0x1d8
> lr : gpio_shared_add_proxy_lookup+0xfc/0x1d8
> sp : ffff8000832bba30
> x29: ffff8000832bba30 x28: ffff000080d01010 x27: ffffffffffffefff
> x26: 0000000000000001 x25: ffff800082df0538 x24: ffff800082df0528
> x23: 0000000000000000 x22: ffff00008012c158 x21: ffff000081455010
> usb 1-3: new full-speed USB device number 2 using tegra-xusb
> x20: ffff000080d5d430 x19: ffff00008012c158 x18: 00000000ffffffff
> x17: ffff8000830786a8 x16: ffff800083078718 x15: ffff8000832bb880
> x14: ffffffffffffffff x13: 0000000000000008 x12: 0101010101010101
> x11: 7f7f7f7f7f7f7f7f x10: ffff8000827f20d0 x9 : 0000000000000003
> x8 : 0101010101010101 x7 : 0080808080808000 x6 : 15151a0a59460209
> x5 : 000000000000003c x4 : ffff8000832bb990 x3 : ffff0000800fe800
> x2 : ffff0000801c2f40 x1 : ffff0000801c2f40 x0 : ffff800082df0538
> Call trace:
> gpio_shared_add_proxy_lookup+0x118/0x1d8 (P)
> gpiod_find_and_request+0x1bc/0x548
> devm_fwnode_gpiod_get_index+0x1c/0x6c
> gpio_keys_probe+0x494/0x9fc
> platform_probe+0x5c/0x98
> really_probe+0xbc/0x2a8
> __driver_probe_device+0x78/0x12c
> driver_probe_device+0x3c/0x15c
> __driver_attach+0x90/0x19c
> bus_for_each_dev+0x78/0xd4
> driver_attach+0x24/0x30
> bus_add_driver+0xe4/0x208
> driver_register+0x5c/0x124
> __platform_driver_register+0x24/0x30
> gpio_keys_init+0x1c/0x28
> do_one_initcall+0x7c/0x1c0
> kernel_init_freeable+0x204/0x2ec
> kernel_init+0x24/0x1e0
> ret_from_fork+0x10/0x20
> ---[ end trace 0000000000000000 ]---
>
> I see the comment says ...
>
> /* We warn here because this can only happen if the programmer borked. */
> WARN_ON(1);
>
> I will take a closer look, but let me know if you have any thoughts?
>
I suppose this is not a reset-gpio-like use-case? Could you point me
to the DTS you're using? I've reproduced the bug with a dummy dts and
xlate function in gpio-sim and this patch fixes it but maybe I'm
missing something.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 11:44 ` Bartosz Golaszewski
@ 2026-03-17 12:53 ` Jon Hunter
2026-03-17 13:43 ` Bartosz Golaszewski
0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-17 12:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On 17/03/2026 11:44, Bartosz Golaszewski wrote:
> On Tue, Mar 17, 2026 at 11:12 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>> Thanks for sending this. However, I am seeing a different issue now ...
>>
>> ------------[ cut here ]------------
>> WARNING: drivers/gpio/gpiolib-shared.c:499 at gpio_shared_add_proxy_lookup+0x118/0x1d8, CPU#8: swapper/0/1
>> Modules linked in:
>> CPU: 8 UID: 0 PID: 1 Comm: swapper/0 Not tainted 7.0.0-rc3-next-20260309-00005-g02826fefa46f #14 PREEMPT
>> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-42974706 11/20/2025
>> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : gpio_shared_add_proxy_lookup+0x118/0x1d8
>> lr : gpio_shared_add_proxy_lookup+0xfc/0x1d8
>> sp : ffff8000832bba30
>> x29: ffff8000832bba30 x28: ffff000080d01010 x27: ffffffffffffefff
>> x26: 0000000000000001 x25: ffff800082df0538 x24: ffff800082df0528
>> x23: 0000000000000000 x22: ffff00008012c158 x21: ffff000081455010
>> usb 1-3: new full-speed USB device number 2 using tegra-xusb
>> x20: ffff000080d5d430 x19: ffff00008012c158 x18: 00000000ffffffff
>> x17: ffff8000830786a8 x16: ffff800083078718 x15: ffff8000832bb880
>> x14: ffffffffffffffff x13: 0000000000000008 x12: 0101010101010101
>> x11: 7f7f7f7f7f7f7f7f x10: ffff8000827f20d0 x9 : 0000000000000003
>> x8 : 0101010101010101 x7 : 0080808080808000 x6 : 15151a0a59460209
>> x5 : 000000000000003c x4 : ffff8000832bb990 x3 : ffff0000800fe800
>> x2 : ffff0000801c2f40 x1 : ffff0000801c2f40 x0 : ffff800082df0538
>> Call trace:
>> gpio_shared_add_proxy_lookup+0x118/0x1d8 (P)
>> gpiod_find_and_request+0x1bc/0x548
>> devm_fwnode_gpiod_get_index+0x1c/0x6c
>> gpio_keys_probe+0x494/0x9fc
>> platform_probe+0x5c/0x98
>> really_probe+0xbc/0x2a8
>> __driver_probe_device+0x78/0x12c
>> driver_probe_device+0x3c/0x15c
>> __driver_attach+0x90/0x19c
>> bus_for_each_dev+0x78/0xd4
>> driver_attach+0x24/0x30
>> bus_add_driver+0xe4/0x208
>> driver_register+0x5c/0x124
>> __platform_driver_register+0x24/0x30
>> gpio_keys_init+0x1c/0x28
>> do_one_initcall+0x7c/0x1c0
>> kernel_init_freeable+0x204/0x2ec
>> kernel_init+0x24/0x1e0
>> ret_from_fork+0x10/0x20
>> ---[ end trace 0000000000000000 ]---
>>
>> I see the comment says ...
>>
>> /* We warn here because this can only happen if the programmer borked. */
>> WARN_ON(1);
>>
>> I will take a closer look, but let me know if you have any thoughts?
>>
>
> I suppose this is not a reset-gpio-like use-case? Could you point me
> to the DTS you're using? I've reproduced the bug with a dummy dts and
> xlate function in gpio-sim and this patch fixes it but maybe I'm
> missing something.
So the board I originally observed this on is a farm board with camera
hardware I don't have. So like you I started off reproducing locally
with a dummy dts by making the following change ...
diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
index f6cad29355e6..5e62ffb425f4 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
@@ -389,14 +389,14 @@ gpio-keys {
key-force-recovery {
label = "Force Recovery";
- gpios = <&gpio TEGRA234_MAIN_GPIO(G, 0) GPIO_ACTIVE_LOW>;
+ gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
linux,input-type = <EV_KEY>;
linux,code = <BTN_1>;
};
key-power {
label = "Power";
- gpios = <&gpio_aon TEGRA234_AON_GPIO(EE, 4) GPIO_ACTIVE_LOW>;
+ gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
linux,input-type = <EV_KEY>;
linux,code = <KEY_POWER>;
wakeup-event-action = <EV_ACT_ASSERTED>;
With this I see ...
gpiolib_shared: GPIO 154 owned by tegra234-gpio is shared by multiple consumers
gpiolib_shared: Setting up a shared GPIO entry for key-force-recovery (con_id: '(none)')
gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.3 for GPIO device tegra234-gpio
gpiolib_shared: Setting up a shared GPIO entry for key-power (con_id: '(none)')
gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.4 for GPIO device tegra234-gpio
...
gpio-keys gpio-keys: error -ENXIO: Unable to determine IRQ# for button #0
I am not sure if this is because these are child nodes of gpio-keys?
Obviously this is not a proper example, but something quick and dirty
for local testing :-)
Jon
--
nvpublic
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-16 13:52 [PATCH] gpio: shared: call gpio_chip::of_xlate() if set Bartosz Golaszewski
2026-03-17 8:47 ` Linus Walleij
2026-03-17 10:12 ` Jon Hunter
@ 2026-03-17 12:53 ` Jon Hunter
2026-03-17 13:44 ` Bartosz Golaszewski
2 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-17 12:53 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski
Cc: linux-gpio, linux-kernel
On 16/03/2026 13:52, Bartosz Golaszewski wrote:
> OF-based GPIO controller drivers may provide a translation function that
> calculates the real chip offset from whatever devicetree sources
> provide. We need to take this into account in the shared GPIO management
> and call of_xlate() if it's provided and adjust the entry->offset we
> initially set when scanning the tree.
>
> To that end: modify the shared GPIO API to take the GPIO chip as
> argument on setup (to avoid having to rcu_dereference() it from the GPIO
> device) and protect the access to entry->offset with the existing lock.
>
> Fixes: a060b8c511ab ("gpiolib: implement low-level, shared GPIO support")
> Reported-by: Jon Hunter <jonathanh@nvidia.com>
> Closes: https://lore.kernel.org/all/921ba8ce-b18e-4a99-966d-c763d22081e2@nvidia.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> drivers/gpio/gpiolib-shared.c | 27 ++++++++++++++++++++++++++-
> drivers/gpio/gpiolib-shared.h | 4 ++--
> drivers/gpio/gpiolib.c | 2 +-
> 3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c
> index 17a7128b6bd9bf6023deccee50b2453caebe3d9a..3a8db9bf456daaf021d3c691677a90fc6da15889 100644
> --- a/drivers/gpio/gpiolib-shared.c
> +++ b/drivers/gpio/gpiolib-shared.c
> @@ -506,8 +506,9 @@ static void gpio_shared_remove_adev(struct auxiliary_device *adev)
> auxiliary_device_uninit(adev);
> }
>
> -int gpio_device_setup_shared(struct gpio_device *gdev)
> +int gpiochip_setup_shared(struct gpio_chip *gc)
> {
> + struct gpio_device *gdev = gc->gpiodev;
> struct gpio_shared_entry *entry;
> struct gpio_shared_ref *ref;
> struct gpio_desc *desc;
> @@ -532,12 +533,34 @@ int gpio_device_setup_shared(struct gpio_device *gdev)
> * exposing shared pins. Find them and create the proxy devices.
> */
> list_for_each_entry(entry, &gpio_shared_list, list) {
> + guard(mutex)(&entry->lock);
> +
> if (!device_match_fwnode(&gdev->dev, entry->fwnode))
> continue;
>
> if (list_count_nodes(&entry->refs) <= 1)
> continue;
>
> +#if IS_ENABLED(CONFIG_OF)
> + if (is_of_node(entry->fwnode) && gc->of_xlate) {
> + /*
> + * This is the earliest that we can tranlate the
> + * devicetree offset to the chip offset.
> + */
> + struct of_phandle_args gpiospec = { };
> +
> + gpiospec.np = to_of_node(entry->fwnode);
> + gpiospec.args_count = 2;
> + gpiospec.args[0] = entry->offset;
> +
> + ret = gc->of_xlate(gc, &gpiospec, NULL);
> + if (ret < 0)
> + return ret;
> +
> + entry->offset = ret;
> + }
> +#endif /* CONFIG_OF */
Do we need something similar to this in the gpio_device_teardown_shared
function?
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 12:53 ` Jon Hunter
@ 2026-03-17 13:43 ` Bartosz Golaszewski
2026-03-17 13:46 ` Jon Hunter
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-17 13:43 UTC (permalink / raw)
To: Jon Hunter
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On Tue, Mar 17, 2026 at 1:53 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> >
> > I suppose this is not a reset-gpio-like use-case? Could you point me
> > to the DTS you're using? I've reproduced the bug with a dummy dts and
> > xlate function in gpio-sim and this patch fixes it but maybe I'm
> > missing something.
>
> So the board I originally observed this on is a farm board with camera
> hardware I don't have. So like you I started off reproducing locally
> with a dummy dts by making the following change ...
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
> index f6cad29355e6..5e62ffb425f4 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
> @@ -389,14 +389,14 @@ gpio-keys {
>
> key-force-recovery {
> label = "Force Recovery";
> - gpios = <&gpio TEGRA234_MAIN_GPIO(G, 0) GPIO_ACTIVE_LOW>;
> + gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
The difference between my test and this is that I used foo-gpios vs
gpios here. I was thinking this would be the culprit but when I
changed my setup to replicate it, it works fine...
> linux,input-type = <EV_KEY>;
> linux,code = <BTN_1>;
> };
>
> key-power {
> label = "Power";
> - gpios = <&gpio_aon TEGRA234_AON_GPIO(EE, 4) GPIO_ACTIVE_LOW>;
> + gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
> linux,input-type = <EV_KEY>;
> linux,code = <KEY_POWER>;
> wakeup-event-action = <EV_ACT_ASSERTED>;
>
>
> With this I see ...
>
> gpiolib_shared: GPIO 154 owned by tegra234-gpio is shared by multiple consumers
> gpiolib_shared: Setting up a shared GPIO entry for key-force-recovery (con_id: '(none)')
> gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.3 for GPIO device tegra234-gpio
> gpiolib_shared: Setting up a shared GPIO entry for key-power (con_id: '(none)')
> gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.4 for GPIO device tegra234-gpio
This looks correct.
> ...
> gpio-keys gpio-keys: error -ENXIO: Unable to determine IRQ# for button #0
>
Is the above error linked to the issue?
> I am not sure if this is because these are child nodes of gpio-keys?
> Obviously this is not a proper example, but something quick and dirty
> for local testing :-)
>
Let me create a setup with GPIO keys then.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 12:53 ` Jon Hunter
@ 2026-03-17 13:44 ` Bartosz Golaszewski
0 siblings, 0 replies; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-17 13:44 UTC (permalink / raw)
To: Jon Hunter; +Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel
On Tue, Mar 17, 2026 at 1:54 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
> >
> > +#if IS_ENABLED(CONFIG_OF)
> > + if (is_of_node(entry->fwnode) && gc->of_xlate) {
> > + /*
> > + * This is the earliest that we can tranlate the
> > + * devicetree offset to the chip offset.
> > + */
> > + struct of_phandle_args gpiospec = { };
> > +
> > + gpiospec.np = to_of_node(entry->fwnode);
> > + gpiospec.args_count = 2;
> > + gpiospec.args[0] = entry->offset;
> > +
> > + ret = gc->of_xlate(gc, &gpiospec, NULL);
> > + if (ret < 0)
> > + return ret;
> > +
> > + entry->offset = ret;
> > + }
> > +#endif /* CONFIG_OF */
>
> Do we need something similar to this in the gpio_device_teardown_shared
> function?
>
No, we adjust the offset here so it'll be correct later.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 13:43 ` Bartosz Golaszewski
@ 2026-03-17 13:46 ` Jon Hunter
2026-03-17 14:05 ` Bartosz Golaszewski
0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-17 13:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On 17/03/2026 13:43, Bartosz Golaszewski wrote:
> On Tue, Mar 17, 2026 at 1:53 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>>
>>> I suppose this is not a reset-gpio-like use-case? Could you point me
>>> to the DTS you're using? I've reproduced the bug with a dummy dts and
>>> xlate function in gpio-sim and this patch fixes it but maybe I'm
>>> missing something.
>>
>> So the board I originally observed this on is a farm board with camera
>> hardware I don't have. So like you I started off reproducing locally
>> with a dummy dts by making the following change ...
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
>> index f6cad29355e6..5e62ffb425f4 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701.dtsi
>> @@ -389,14 +389,14 @@ gpio-keys {
>>
>> key-force-recovery {
>> label = "Force Recovery";
>> - gpios = <&gpio TEGRA234_MAIN_GPIO(G, 0) GPIO_ACTIVE_LOW>;
>> + gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
>
> The difference between my test and this is that I used foo-gpios vs
> gpios here. I was thinking this would be the culprit but when I
> changed my setup to replicate it, it works fine...
>
>> linux,input-type = <EV_KEY>;
>> linux,code = <BTN_1>;
>> };
>>
>> key-power {
>> label = "Power";
>> - gpios = <&gpio_aon TEGRA234_AON_GPIO(EE, 4) GPIO_ACTIVE_LOW>;
>> + gpios = <&gpio TEGRA234_MAIN_GPIO(AF, 2) GPIO_ACTIVE_LOW>;
>> linux,input-type = <EV_KEY>;
>> linux,code = <KEY_POWER>;
>> wakeup-event-action = <EV_ACT_ASSERTED>;
>>
>>
>> With this I see ...
>>
>> gpiolib_shared: GPIO 154 owned by tegra234-gpio is shared by multiple consumers
>> gpiolib_shared: Setting up a shared GPIO entry for key-force-recovery (con_id: '(none)')
>> gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.3 for GPIO device tegra234-gpio
>> gpiolib_shared: Setting up a shared GPIO entry for key-power (con_id: '(none)')
>> gpiolib_shared: Created an auxiliary GPIO proxy gpiolib_shared.proxy.4 for GPIO device tegra234-gpio
>
> This looks correct.
>
>> ...
>> gpio-keys gpio-keys: error -ENXIO: Unable to determine IRQ# for button #0
>>
>
> Is the above error linked to the issue?
Yes, that the final error message I see after the WARNING splat, hence,
I was thinking that this is somehow linked to the above.
>> I am not sure if this is because these are child nodes of gpio-keys?
>> Obviously this is not a proper example, but something quick and dirty
>> for local testing :-)
>>
>
> Let me create a setup with GPIO keys then.
Thanks!
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 13:46 ` Jon Hunter
@ 2026-03-17 14:05 ` Bartosz Golaszewski
2026-03-17 15:19 ` Bartosz Golaszewski
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-17 14:05 UTC (permalink / raw)
To: Jon Hunter
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On Tue, Mar 17, 2026 at 2:47 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> >> I am not sure if this is because these are child nodes of gpio-keys?
> >> Obviously this is not a proper example, but something quick and dirty
> >> for local testing :-)
> >>
> >
> > Let me create a setup with GPIO keys then.
>
> Thanks!
> Jon
>
I can reproduce this with a gpio-keys setup. I think you hit an
interesting corner-case where the consumer device is the same for two
shared pins assigned to its child fwnodes. The setup doesn't make
sense really but I guess this shouldn't just fail like that.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 14:05 ` Bartosz Golaszewski
@ 2026-03-17 15:19 ` Bartosz Golaszewski
2026-03-17 22:46 ` Jon Hunter
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-17 15:19 UTC (permalink / raw)
To: Jon Hunter
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On Tue, Mar 17, 2026 at 3:05 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Tue, Mar 17, 2026 at 2:47 PM Jon Hunter <jonathanh@nvidia.com> wrote:
> >
> >
> > >> I am not sure if this is because these are child nodes of gpio-keys?
> > >> Obviously this is not a proper example, but something quick and dirty
> > >> for local testing :-)
> > >>
> > >
> > > Let me create a setup with GPIO keys then.
> >
> > Thanks!
> > Jon
> >
>
> I can reproduce this with a gpio-keys setup. I think you hit an
> interesting corner-case where the consumer device is the same for two
> shared pins assigned to its child fwnodes. The setup doesn't make
> sense really but I guess this shouldn't just fail like that.
>
So the problem goes like this: we're using lookup tables for shared
GPIOs but they are not capable of dealing with two fwnodes that are
children of the same device that share the same pin but are themselves
not attached to a device bound to a driver. While we could extend
lookup tables to take that into account, I think that the setup here
is so hypothetical, it doesn't really make sense to spend time on it.
Does this patch fix the real problem on the tegra board that you
reported initially? I doubt two separate GPIO keys, share the same pin
in real life.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 15:19 ` Bartosz Golaszewski
@ 2026-03-17 22:46 ` Jon Hunter
2026-03-18 8:09 ` Bartosz Golaszewski
0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-17 22:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On 17/03/2026 15:19, Bartosz Golaszewski wrote:
> On Tue, Mar 17, 2026 at 3:05 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>>
>> On Tue, Mar 17, 2026 at 2:47 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>
>>>
>>>>> I am not sure if this is because these are child nodes of gpio-keys?
>>>>> Obviously this is not a proper example, but something quick and dirty
>>>>> for local testing :-)
>>>>>
>>>>
>>>> Let me create a setup with GPIO keys then.
>>>
>>> Thanks!
>>> Jon
>>>
>>
>> I can reproduce this with a gpio-keys setup. I think you hit an
>> interesting corner-case where the consumer device is the same for two
>> shared pins assigned to its child fwnodes. The setup doesn't make
>> sense really but I guess this shouldn't just fail like that.
>>
>
> So the problem goes like this: we're using lookup tables for shared
> GPIOs but they are not capable of dealing with two fwnodes that are
> children of the same device that share the same pin but are themselves
> not attached to a device bound to a driver. While we could extend
> lookup tables to take that into account, I think that the setup here
> is so hypothetical, it doesn't really make sense to spend time on it.
Makes sense.
> Does this patch fix the real problem on the tegra board that you
> reported initially? I doubt two separate GPIO keys, share the same pin
> in real life.
Yes it fixes the initial issue. However, now I am seeing a different
error on the actual platform that is having the issue to begin with ...
------------[ cut here ]------------
WARNING: kernel/rcu/srcutree.c:757 at cleanup_srcu_struct+0xc0/0x1e0, CPU#2: kworker/u49:1/114
Modules linked in:
CPU: 2 UID: 0 PID: 114 Comm: kworker/u49:1 Not tainted 6.19.0-tegra #1 PREEMPT
Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-44496888 03/15/2026
Workqueue: events_unbound deferred_probe_work_func
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : cleanup_srcu_struct+0xc0/0x1e0
lr : cleanup_srcu_struct+0xb4/0x1e0
sp : ffff800081cbb930
x29: ffff800081cbb930 x28: ffffd79ff96d0c40 x27: ffff000086059000
x26: 00000000fffffff0 x25: ffff000086571200 x24: ffffd79ff94adb10
x23: ffffd79ff86400c0 x22: ffff000086059390 x21: ffffd79ff94aa040
x20: 0000000000000000 x19: fffffdffbf669d40 x18: 00000000ffffffff
x17: 0000000000000000 x16: ffffd79ff62dc8a0 x15: 0081cf5fe0409838
x14: 0000000000000000 x13: 0000000000000272 x12: 0000000000000000
x11: 00000000000000c0 x10: f7c5d06d757a4b3a x9 : 15ccf89dfeffb5e1
x8 : ffff800081cbb8c8 x7 : 0000000000000000 x6 : 000000000151e960
x5 : 0800000000000000 x4 : 0000000000000000 x3 : 0000000000000000
x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000004
Call trace:
cleanup_srcu_struct+0xc0/0x1e0 (P)
gpiochip_add_data_with_key+0x3dc/0xf68
devm_gpiochip_add_data_with_key+0x30/0x84
tegra186_gpio_probe+0x5e4/0x808
platform_probe+0x5c/0xb0
really_probe+0xbc/0x2b4
__driver_probe_device+0x78/0x134
driver_probe_device+0x3c/0x164
__device_attach_driver+0xc8/0x15c
bus_for_each_drv+0x88/0x100
__device_attach+0xa0/0x198
device_initial_probe+0x58/0x5c
bus_probe_device+0x38/0xbc
deferred_probe_work_func+0x88/0xc8
process_one_work+0x16c/0x3fc
worker_thread+0x2d8/0x3ec
kthread+0x144/0x22c
ret_from_fork+0x10/0x20
---[ end trace 0000000000000000 ]---
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
Note this is on top of a Linux v6.19 kernel I was using to track down the
original problem.
Looking at the above there appears to be two other issues; one the warning from
SRCU and the other an -EBUSY failure when registering the GPIO controller. I am
guessing the warning is triggered when gpiochip_add_data_with_key fails. I need
to look at this closer.
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-17 22:46 ` Jon Hunter
@ 2026-03-18 8:09 ` Bartosz Golaszewski
2026-03-18 19:09 ` Jon Hunter
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-18 8:09 UTC (permalink / raw)
To: Jon Hunter
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org, Bartosz Golaszewski
On Tue, 17 Mar 2026 23:46:55 +0100, Jon Hunter <jonathanh@nvidia.com> said:
>
> On 17/03/2026 15:19, Bartosz Golaszewski wrote:
>> On Tue, Mar 17, 2026 at 3:05 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>>>
>>> On Tue, Mar 17, 2026 at 2:47 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>
>>>>
>>>>>> I am not sure if this is because these are child nodes of gpio-keys?
>>>>>> Obviously this is not a proper example, but something quick and dirty
>>>>>> for local testing :-)
>>>>>>
>>>>>
>>>>> Let me create a setup with GPIO keys then.
>>>>
>>>> Thanks!
>>>> Jon
>>>>
>>>
>>> I can reproduce this with a gpio-keys setup. I think you hit an
>>> interesting corner-case where the consumer device is the same for two
>>> shared pins assigned to its child fwnodes. The setup doesn't make
>>> sense really but I guess this shouldn't just fail like that.
>>>
>>
>> So the problem goes like this: we're using lookup tables for shared
>> GPIOs but they are not capable of dealing with two fwnodes that are
>> children of the same device that share the same pin but are themselves
>> not attached to a device bound to a driver. While we could extend
>> lookup tables to take that into account, I think that the setup here
>> is so hypothetical, it doesn't really make sense to spend time on it.
>
> Makes sense.
>
Just to be clear, this:
gpio-keys {
compatible = "gpio-keys";
key-one {
label = "foo";
gpios = <&gpio_sim0 10 0>;
linux,input-type = <EV_KEY>;
linux,code = <KEY_POWER>;
};
key-two {
label = "bar";
gpios = <&gpio_sim0 10 0>;
linux,input-type = <EV_KEY>;
linux,code = <KEY_POWER>;
};
};
doesn't work, but this:
gpio-keys-1 {
compatible = "gpio-keys";
key-one {
label = "foo";
gpios = <&gpio_sim0 10 0>;
linux,input-type = <EV_KEY>;
linux,code = <KEY_POWER>;
};
};
gpio-keys-2 {
compatible = "gpio-keys";
key-two {
label = "bar";
gpios = <&gpio_sim0 10 0>;
linux,input-type = <EV_KEY>;
linux,code = <KEY_POWER>;
};
};
does. I don't think making the former work is worth the effort.
>> Does this patch fix the real problem on the tegra board that you
>> reported initially? I doubt two separate GPIO keys, share the same pin
>> in real life.
>
> Yes it fixes the initial issue. However, now I am seeing a different
> error on the actual platform that is having the issue to begin with ...
>
This is *with* the fix?
> ------------[ cut here ]------------
> WARNING: kernel/rcu/srcutree.c:757 at cleanup_srcu_struct+0xc0/0x1e0, CPU#2: kworker/u49:1/114
> Modules linked in:
> CPU: 2 UID: 0 PID: 114 Comm: kworker/u49:1 Not tainted 6.19.0-tegra #1 PREEMPT
> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-44496888 03/15/2026
> Workqueue: events_unbound deferred_probe_work_func
> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : cleanup_srcu_struct+0xc0/0x1e0
> lr : cleanup_srcu_struct+0xb4/0x1e0
> sp : ffff800081cbb930
> x29: ffff800081cbb930 x28: ffffd79ff96d0c40 x27: ffff000086059000
> x26: 00000000fffffff0 x25: ffff000086571200 x24: ffffd79ff94adb10
> x23: ffffd79ff86400c0 x22: ffff000086059390 x21: ffffd79ff94aa040
> x20: 0000000000000000 x19: fffffdffbf669d40 x18: 00000000ffffffff
> x17: 0000000000000000 x16: ffffd79ff62dc8a0 x15: 0081cf5fe0409838
> x14: 0000000000000000 x13: 0000000000000272 x12: 0000000000000000
> x11: 00000000000000c0 x10: f7c5d06d757a4b3a x9 : 15ccf89dfeffb5e1
> x8 : ffff800081cbb8c8 x7 : 0000000000000000 x6 : 000000000151e960
> x5 : 0800000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000004
> Call trace:
> cleanup_srcu_struct+0xc0/0x1e0 (P)
> gpiochip_add_data_with_key+0x3dc/0xf68
> devm_gpiochip_add_data_with_key+0x30/0x84
> tegra186_gpio_probe+0x5e4/0x808
> platform_probe+0x5c/0xb0
> really_probe+0xbc/0x2b4
> __driver_probe_device+0x78/0x134
> driver_probe_device+0x3c/0x164
> __device_attach_driver+0xc8/0x15c
> bus_for_each_drv+0x88/0x100
> __device_attach+0xa0/0x198
> device_initial_probe+0x58/0x5c
> bus_probe_device+0x38/0xbc
> deferred_probe_work_func+0x88/0xc8
> process_one_work+0x16c/0x3fc
> worker_thread+0x2d8/0x3ec
> kthread+0x144/0x22c
> ret_from_fork+0x10/0x20
> ---[ end trace 0000000000000000 ]---
> 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
>
> Note this is on top of a Linux v6.19 kernel I was using to track down the
> original problem.
>
There's a change to how gpiochip_add_data_with_key() error path works in
linux-next at the moment but it's not in any stable branch yet.
> Looking at the above there appears to be two other issues; one the warning from
> SRCU and the other an -EBUSY failure when registering the GPIO controller. I am
> guessing the warning is triggered when gpiochip_add_data_with_key fails. I need
> to look at this closer.
>
-EBUSY can typically only happen if gpiod_request_commit() is called twice on
the same descriptor. Is that the case here?
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-18 8:09 ` Bartosz Golaszewski
@ 2026-03-18 19:09 ` Jon Hunter
2026-03-19 9:41 ` Bartosz Golaszewski
0 siblings, 1 reply; 17+ messages in thread
From: Jon Hunter @ 2026-03-18 19:09 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
On 18/03/2026 08:09, Bartosz Golaszewski wrote:
...
>>>> I can reproduce this with a gpio-keys setup. I think you hit an
>>>> interesting corner-case where the consumer device is the same for two
>>>> shared pins assigned to its child fwnodes. The setup doesn't make
>>>> sense really but I guess this shouldn't just fail like that.
>>>>
>>>
>>> So the problem goes like this: we're using lookup tables for shared
>>> GPIOs but they are not capable of dealing with two fwnodes that are
>>> children of the same device that share the same pin but are themselves
>>> not attached to a device bound to a driver. While we could extend
>>> lookup tables to take that into account, I think that the setup here
>>> is so hypothetical, it doesn't really make sense to spend time on it.
>>
>> Makes sense.
>>
>
> Just to be clear, this:
>
> gpio-keys {
> compatible = "gpio-keys";
>
> key-one {
> label = "foo";
> gpios = <&gpio_sim0 10 0>;
> linux,input-type = <EV_KEY>;
> linux,code = <KEY_POWER>;
> };
>
> key-two {
> label = "bar";
> gpios = <&gpio_sim0 10 0>;
> linux,input-type = <EV_KEY>;
> linux,code = <KEY_POWER>;
> };
> };
>
> doesn't work, but this:
>
> gpio-keys-1 {
> compatible = "gpio-keys";
>
> key-one {
> label = "foo";
> gpios = <&gpio_sim0 10 0>;
> linux,input-type = <EV_KEY>;
> linux,code = <KEY_POWER>;
> };
> };
>
> gpio-keys-2 {
> compatible = "gpio-keys";
>
> key-two {
> label = "bar";
> gpios = <&gpio_sim0 10 0>;
> linux,input-type = <EV_KEY>;
> linux,code = <KEY_POWER>;
> };
> };
>
> does. I don't think making the former work is worth the effort.
That's fine with me.
>>> Does this patch fix the real problem on the tegra board that you
>>> reported initially? I doubt two separate GPIO keys, share the same pin
>>> in real life.
>>
>> Yes it fixes the initial issue. However, now I am seeing a different
>> error on the actual platform that is having the issue to begin with ...
>>
>
> This is *with* the fix?
Yes.
>> ------------[ cut here ]------------
>> WARNING: kernel/rcu/srcutree.c:757 at cleanup_srcu_struct+0xc0/0x1e0, CPU#2: kworker/u49:1/114
>> Modules linked in:
>> CPU: 2 UID: 0 PID: 114 Comm: kworker/u49:1 Not tainted 6.19.0-tegra #1 PREEMPT
>> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-44496888 03/15/2026
>> Workqueue: events_unbound deferred_probe_work_func
>> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : cleanup_srcu_struct+0xc0/0x1e0
>> lr : cleanup_srcu_struct+0xb4/0x1e0
>> sp : ffff800081cbb930
>> x29: ffff800081cbb930 x28: ffffd79ff96d0c40 x27: ffff000086059000
>> x26: 00000000fffffff0 x25: ffff000086571200 x24: ffffd79ff94adb10
>> x23: ffffd79ff86400c0 x22: ffff000086059390 x21: ffffd79ff94aa040
>> x20: 0000000000000000 x19: fffffdffbf669d40 x18: 00000000ffffffff
>> x17: 0000000000000000 x16: ffffd79ff62dc8a0 x15: 0081cf5fe0409838
>> x14: 0000000000000000 x13: 0000000000000272 x12: 0000000000000000
>> x11: 00000000000000c0 x10: f7c5d06d757a4b3a x9 : 15ccf89dfeffb5e1
>> x8 : ffff800081cbb8c8 x7 : 0000000000000000 x6 : 000000000151e960
>> x5 : 0800000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>> x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000004
>> Call trace:
>> cleanup_srcu_struct+0xc0/0x1e0 (P)
>> gpiochip_add_data_with_key+0x3dc/0xf68
>> devm_gpiochip_add_data_with_key+0x30/0x84
>> tegra186_gpio_probe+0x5e4/0x808
>> platform_probe+0x5c/0xb0
>> really_probe+0xbc/0x2b4
>> __driver_probe_device+0x78/0x134
>> driver_probe_device+0x3c/0x164
>> __device_attach_driver+0xc8/0x15c
>> bus_for_each_drv+0x88/0x100
>> __device_attach+0xa0/0x198
>> device_initial_probe+0x58/0x5c
>> bus_probe_device+0x38/0xbc
>> deferred_probe_work_func+0x88/0xc8
>> process_one_work+0x16c/0x3fc
>> worker_thread+0x2d8/0x3ec
>> kthread+0x144/0x22c
>> ret_from_fork+0x10/0x20
>> ---[ end trace 0000000000000000 ]---
It seems that when the gpiochip_add_data_with_key(), then to avoid the
above warning I needed to ...
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 27ea5bc9ed8a..3130acfeeb66 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1277,6 +1277,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
goto err_print_message;
}
err_cleanup_desc_srcu:
+ synchronize_srcu(&gdev->desc_srcu);
cleanup_srcu_struct(&gdev->desc_srcu);
err_cleanup_gdev_srcu:
cleanup_srcu_struct(&gdev->srcu);
>> 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
Which leaves the above.
> There's a change to how gpiochip_add_data_with_key() error path works in
> linux-next at the moment but it's not in any stable branch yet.
>
This commit?
16fdabe143fc ("gpio: Fix resource leaks on errors in gpiochip_add_data_with_key()")
> -EBUSY can typically only happen if gpiod_request_commit() is called twice on
> the same descriptor. Is that the case here?
I have been looking at this today and now I can see that we have a
'gpio-hog' set for the same pins that are shared and hence it is
getting request twice. If I drop the hog it goes away. This is a
produce device-tree, not upstream, for some camera modules so I am
wondering if we are doing something here we should not be. I am
taking a closer look.
Thanks!
Jon
--
nvpublic
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-18 19:09 ` Jon Hunter
@ 2026-03-19 9:41 ` Bartosz Golaszewski
2026-03-20 4:49 ` Tzung-Bi Shih
0 siblings, 1 reply; 17+ messages in thread
From: Bartosz Golaszewski @ 2026-03-19 9:41 UTC (permalink / raw)
To: Jon Hunter, Tzung-Bi Shih
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org, Bartosz Golaszewski
On Wed, 18 Mar 2026 20:09:08 +0100, Jon Hunter <jonathanh@nvidia.com> said:
>
>>>> Does this patch fix the real problem on the tegra board that you
>>>> reported initially? I doubt two separate GPIO keys, share the same pin
>>>> in real life.
>>>
>>> Yes it fixes the initial issue. However, now I am seeing a different
>>> error on the actual platform that is having the issue to begin with ...
>>>
>>
>> This is *with* the fix?
>
> Yes.
>
>>> ------------[ cut here ]------------
>>> WARNING: kernel/rcu/srcutree.c:757 at cleanup_srcu_struct+0xc0/0x1e0, CPU#2: kworker/u49:1/114
>>> Modules linked in:
>>> CPU: 2 UID: 0 PID: 114 Comm: kworker/u49:1 Not tainted 6.19.0-tegra #1 PREEMPT
>>> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-44496888 03/15/2026
>>> Workqueue: events_unbound deferred_probe_work_func
>>> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> pc : cleanup_srcu_struct+0xc0/0x1e0
>>> lr : cleanup_srcu_struct+0xb4/0x1e0
>>> sp : ffff800081cbb930
>>> x29: ffff800081cbb930 x28: ffffd79ff96d0c40 x27: ffff000086059000
>>> x26: 00000000fffffff0 x25: ffff000086571200 x24: ffffd79ff94adb10
>>> x23: ffffd79ff86400c0 x22: ffff000086059390 x21: ffffd79ff94aa040
>>> x20: 0000000000000000 x19: fffffdffbf669d40 x18: 00000000ffffffff
>>> x17: 0000000000000000 x16: ffffd79ff62dc8a0 x15: 0081cf5fe0409838
>>> x14: 0000000000000000 x13: 0000000000000272 x12: 0000000000000000
>>> x11: 00000000000000c0 x10: f7c5d06d757a4b3a x9 : 15ccf89dfeffb5e1
>>> x8 : ffff800081cbb8c8 x7 : 0000000000000000 x6 : 000000000151e960
>>> x5 : 0800000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>>> x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000004
>>> Call trace:
>>> cleanup_srcu_struct+0xc0/0x1e0 (P)
>>> gpiochip_add_data_with_key+0x3dc/0xf68
>>> devm_gpiochip_add_data_with_key+0x30/0x84
>>> tegra186_gpio_probe+0x5e4/0x808
>>> platform_probe+0x5c/0xb0
>>> really_probe+0xbc/0x2b4
>>> __driver_probe_device+0x78/0x134
>>> driver_probe_device+0x3c/0x164
>>> __device_attach_driver+0xc8/0x15c
>>> bus_for_each_drv+0x88/0x100
>>> __device_attach+0xa0/0x198
>>> device_initial_probe+0x58/0x5c
>>> bus_probe_device+0x38/0xbc
>>> deferred_probe_work_func+0x88/0xc8
>>> process_one_work+0x16c/0x3fc
>>> worker_thread+0x2d8/0x3ec
>>> kthread+0x144/0x22c
>>> ret_from_fork+0x10/0x20
>>> ---[ end trace 0000000000000000 ]---
>
> It seems that when the gpiochip_add_data_with_key(), then to avoid the
> above warning I needed to ...
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 27ea5bc9ed8a..3130acfeeb66 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1277,6 +1277,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> goto err_print_message;
> }
> 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?
>
>>> 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
>
> Which leaves the above.
>
>> There's a change to how gpiochip_add_data_with_key() error path works in
>> linux-next at the moment but it's not in any stable branch yet.
>>
>
> This commit?
>
> 16fdabe143fc ("gpio: Fix resource leaks on errors in gpiochip_add_data_with_key()")
>
Yes, I Cc'ed the author above.
>
>> -EBUSY can typically only happen if gpiod_request_commit() is called twice on
>> the same descriptor. Is that the case here?
>
> I have been looking at this today and now I can see that we have a
> 'gpio-hog' set for the same pins that are shared and hence it is
> getting request twice. If I drop the hog it goes away. This is a
> produce device-tree, not upstream, for some camera modules so I am
> wondering if we are doing something here we should not be. I am
> taking a closer look.
>
Ah, yes that definitely will not work. Hogs are taken first during the chip's
bringup and hogged lines will not be available to users. The error returned is
-EBUSY so it makes perfect sense and is expected.
Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-19 9:41 ` Bartosz Golaszewski
@ 2026-03-20 4:49 ` Tzung-Bi Shih
2026-03-20 11:46 ` Jon Hunter
0 siblings, 1 reply; 17+ messages in thread
From: Tzung-Bi Shih @ 2026-03-20 4:49 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Jon Hunter, Bartosz Golaszewski, Linus Walleij, linux-gpio,
linux-kernel, linux-tegra@vger.kernel.org
On Thu, Mar 19, 2026 at 02:41:07AM -0700, Bartosz Golaszewski wrote:
> On Wed, 18 Mar 2026 20:09:08 +0100, Jon Hunter <jonathanh@nvidia.com> said:
> >
> >>>> Does this patch fix the real problem on the tegra board that you
> >>>> reported initially? I doubt two separate GPIO keys, share the same pin
> >>>> in real life.
> >>>
> >>> Yes it fixes the initial issue. However, now I am seeing a different
> >>> error on the actual platform that is having the issue to begin with ...
> >>>
> >>
> >> This is *with* the fix?
> >
> > Yes.
> >
> >>> ------------[ cut here ]------------
> >>> WARNING: kernel/rcu/srcutree.c:757 at cleanup_srcu_struct+0xc0/0x1e0, CPU#2: kworker/u49:1/114
> >>> Modules linked in:
> >>> CPU: 2 UID: 0 PID: 114 Comm: kworker/u49:1 Not tainted 6.19.0-tegra #1 PREEMPT
> >>> Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS buildbrain-gcid-44496888 03/15/2026
> >>> Workqueue: events_unbound deferred_probe_work_func
> >>> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >>> pc : cleanup_srcu_struct+0xc0/0x1e0
> >>> lr : cleanup_srcu_struct+0xb4/0x1e0
> >>> sp : ffff800081cbb930
> >>> x29: ffff800081cbb930 x28: ffffd79ff96d0c40 x27: ffff000086059000
> >>> x26: 00000000fffffff0 x25: ffff000086571200 x24: ffffd79ff94adb10
> >>> x23: ffffd79ff86400c0 x22: ffff000086059390 x21: ffffd79ff94aa040
> >>> x20: 0000000000000000 x19: fffffdffbf669d40 x18: 00000000ffffffff
> >>> x17: 0000000000000000 x16: ffffd79ff62dc8a0 x15: 0081cf5fe0409838
> >>> x14: 0000000000000000 x13: 0000000000000272 x12: 0000000000000000
> >>> x11: 00000000000000c0 x10: f7c5d06d757a4b3a x9 : 15ccf89dfeffb5e1
> >>> x8 : ffff800081cbb8c8 x7 : 0000000000000000 x6 : 000000000151e960
> >>> x5 : 0800000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> >>> x2 : 0000000000000001 x1 : 0000000000000000 x0 : 0000000000000004
> >>> Call trace:
> >>> cleanup_srcu_struct+0xc0/0x1e0 (P)
> >>> gpiochip_add_data_with_key+0x3dc/0xf68
> >>> devm_gpiochip_add_data_with_key+0x30/0x84
> >>> tegra186_gpio_probe+0x5e4/0x808
> >>> platform_probe+0x5c/0xb0
> >>> really_probe+0xbc/0x2b4
> >>> __driver_probe_device+0x78/0x134
> >>> driver_probe_device+0x3c/0x164
> >>> __device_attach_driver+0xc8/0x15c
> >>> bus_for_each_drv+0x88/0x100
> >>> __device_attach+0xa0/0x198
> >>> device_initial_probe+0x58/0x5c
> >>> bus_probe_device+0x38/0xbc
> >>> deferred_probe_work_func+0x88/0xc8
> >>> process_one_work+0x16c/0x3fc
> >>> worker_thread+0x2d8/0x3ec
> >>> kthread+0x144/0x22c
> >>> ret_from_fork+0x10/0x20
> >>> ---[ end trace 0000000000000000 ]---
> >
> > It seems that when the gpiochip_add_data_with_key(), then to avoid the
> > above warning I needed to ...
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 27ea5bc9ed8a..3130acfeeb66 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1277,6 +1277,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> > goto err_print_message;
> > }
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.
> > 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`?
> >
> > Which leaves the above.
> >
> >> There's a change to how gpiochip_add_data_with_key() error path works in
> >> linux-next at the moment but it's not in any stable branch yet.
> >>
> >
> > This commit?
> >
> > 16fdabe143fc ("gpio: Fix resource leaks on errors in gpiochip_add_data_with_key()")
> >
>
> Yes, I Cc'ed the author above.
>
> >
> >> -EBUSY can typically only happen if gpiod_request_commit() is called twice on
> >> the same descriptor. Is that the case here?
> >
> > I have been looking at this today and now I can see that we have a
> > 'gpio-hog' set for the same pins that are shared and hence it is
> > getting request twice. If I drop the hog it goes away. This is a
> > produce device-tree, not upstream, for some camera modules so I am
> > wondering if we are doing something here we should not be. I am
> > taking a closer look.
> >
>
> Ah, yes that definitely will not work. Hogs are taken first during the chip's
> bringup and hogged lines will not be available to users. The error returned is
> -EBUSY so it makes perfect sense and is expected.
>
> Bart
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] gpio: shared: call gpio_chip::of_xlate() if set
2026-03-20 4:49 ` Tzung-Bi Shih
@ 2026-03-20 11:46 ` Jon Hunter
0 siblings, 0 replies; 17+ messages in thread
From: Jon Hunter @ 2026-03-20 11:46 UTC (permalink / raw)
To: Tzung-Bi Shih, Bartosz Golaszewski
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel,
linux-tegra@vger.kernel.org
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
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-03-20 11:46 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 13:52 [PATCH] gpio: shared: call gpio_chip::of_xlate() if set Bartosz Golaszewski
2026-03-17 8:47 ` Linus Walleij
2026-03-17 10:12 ` Jon Hunter
2026-03-17 11:44 ` Bartosz Golaszewski
2026-03-17 12:53 ` Jon Hunter
2026-03-17 13:43 ` Bartosz Golaszewski
2026-03-17 13:46 ` Jon Hunter
2026-03-17 14:05 ` Bartosz Golaszewski
2026-03-17 15:19 ` Bartosz Golaszewski
2026-03-17 22:46 ` Jon Hunter
2026-03-18 8:09 ` Bartosz Golaszewski
2026-03-18 19:09 ` Jon Hunter
2026-03-19 9:41 ` Bartosz Golaszewski
2026-03-20 4:49 ` Tzung-Bi Shih
2026-03-20 11:46 ` Jon Hunter
2026-03-17 12:53 ` Jon Hunter
2026-03-17 13:44 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox