* [PATCH 0/2] gpio: shared: another set of small fixes @ 2026-01-05 15:52 ` Bartosz Golaszewski 2026-01-05 15:52 ` [PATCH 1/2] gpio: shared: assign the correct firmware node for reset-gpio use-case Bartosz Golaszewski ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-05 15:52 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, Marek Szyprowski Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- Bartosz Golaszewski (2): gpio: shared: assign the correct firmware node for reset-gpio use-case gpio: shared: fix a race condition drivers/gpio/gpiolib-shared.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) --- base-commit: 19fb766a1e5ed5943a62fc671c09d45352a81b1d change-id: 20260105-gpio-shared-fixes-40a8ec3b6b25 Best regards, -- Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] gpio: shared: assign the correct firmware node for reset-gpio use-case 2026-01-05 15:52 ` [PATCH 0/2] gpio: shared: another set of small fixes Bartosz Golaszewski @ 2026-01-05 15:52 ` Bartosz Golaszewski 2026-01-05 15:52 ` [PATCH 2/2] gpio: shared: fix a race condition Bartosz Golaszewski 2026-01-05 16:48 ` [PATCH 0/2] gpio: shared: another set of small fixes Marek Szyprowski 2 siblings, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-05 15:52 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, Marek Szyprowski When we defer probe due to unlucky timing of adding the lookup table, we assign the matching firmware node to the shared reference for the future probing. However, the fwnode we assign is wrong so fix it and assign the one associated with the reset-gpio device. Fixes: 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib") Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/ Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- drivers/gpio/gpiolib-shared.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index baf7e07a3bb887dab8155078666a15779e304409..a68af06a6cc4e1e33946d7f200cecd4d3dc066af 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -417,7 +417,7 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, * Reuse the fwnode of the real device, next time we'll use it * in the normal path. */ - ref->fwnode = fwnode_handle_get(real_ref->fwnode); + ref->fwnode = fwnode_handle_get(reset_fwnode); return true; } -- 2.47.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] gpio: shared: fix a race condition 2026-01-05 15:52 ` [PATCH 0/2] gpio: shared: another set of small fixes Bartosz Golaszewski 2026-01-05 15:52 ` [PATCH 1/2] gpio: shared: assign the correct firmware node for reset-gpio use-case Bartosz Golaszewski @ 2026-01-05 15:52 ` Bartosz Golaszewski 2026-01-05 16:48 ` [PATCH 0/2] gpio: shared: another set of small fixes Marek Szyprowski 2 siblings, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-05 15:52 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, Marek Szyprowski When matching the reset-gpio reference with the actual firmware node consuming the GPIO, we also need to lock the structure associated with the latter as it can change while we're doing it. Fixes: 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib") Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Closes: https://lore.kernel.org/all/00107523-7737-4b92-a785-14ce4e93b8cb@samsung.com/ Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- drivers/gpio/gpiolib-shared.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index a68af06a6cc4e1e33946d7f200cecd4d3dc066af..198951c4c80bc0965c4b45a97f4f41ecc1f517b3 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -378,6 +378,8 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, * arguments match the ones from this consumer's node. */ list_for_each_entry(real_ref, &entry->refs, list) { + guard(mutex)(&real_ref->lock); + if (!real_ref->fwnode) continue; -- 2.47.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] gpio: shared: another set of small fixes 2026-01-05 15:52 ` [PATCH 0/2] gpio: shared: another set of small fixes Bartosz Golaszewski 2026-01-05 15:52 ` [PATCH 1/2] gpio: shared: assign the correct firmware node for reset-gpio use-case Bartosz Golaszewski 2026-01-05 15:52 ` [PATCH 2/2] gpio: shared: fix a race condition Bartosz Golaszewski @ 2026-01-05 16:48 ` Marek Szyprowski 2026-01-05 16:53 ` Bartosz Golaszewski 2 siblings, 1 reply; 7+ messages in thread From: Marek Szyprowski @ 2026-01-05 16:48 UTC (permalink / raw) To: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-kernel On 05.01.2026 16:52, Bartosz Golaszewski wrote: > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > --- > Bartosz Golaszewski (2): > gpio: shared: assign the correct firmware node for reset-gpio use-case > gpio: shared: fix a race condition > > drivers/gpio/gpiolib-shared.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > --- > base-commit: 19fb766a1e5ed5943a62fc671c09d45352a81b1d > change-id: 20260105-gpio-shared-fixes-40a8ec3b6b25 Those patches indeed fixes some timing issues with the commit 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between reset-gpio and gpiolib"), but they also reveals another one. I've initially thought that this is a false positive and needs only a proper lockdep annotation, but some boards hang just after similar lockdep splat: ============================================ WARNING: possible recursive locking detected 6.19.0-rc4-next-20260105+ #11974 Not tainted -------------------------------------------- (udev-worker)/192 is trying to acquire lock: ffff000004c15098 (&ref->lock){+.+.}-{4:4}, at: gpio_shared_dev_is_reset_gpio+0xcc/0x234 but task is already holding lock: ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: gpio_shared_add_proxy_lookup+0x98/0x228 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&ref->lock); lock(&ref->lock); *** DEADLOCK *** May be due to missing lock nesting notation 3 locks held by (udev-worker)/192: #0: ffff00000b3ad8e8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x90/0x1ac #1: ffff8000830f2600 (gpio_devices_srcu){.+.+}-{0:0}, at: gpiod_find_and_request+0x0/0x574 #2: ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: gpio_shared_add_proxy_lookup+0x98/0x228 stack backtrace: CPU: 3 UID: 0 PID: 192 Comm: (udev-worker) Not tainted 6.19.0-rc4-next-20260105+ #11974 PREEMPT Hardware name: Raspberry Pi 3 Model B (DT) Call trace: show_stack+0x18/0x24 (C) dump_stack_lvl+0x90/0xd0 dump_stack+0x18/0x24 print_deadlock_bug+0x260/0x350 __lock_acquire+0x11b0/0x2254 lock_acquire+0x1c8/0x354 __mutex_lock+0xa8/0x894 mutex_lock_nested+0x24/0x30 gpio_shared_dev_is_reset_gpio+0xcc/0x234 gpio_shared_add_proxy_lookup+0x1a0/0x228 gpiod_find_and_request+0x200/0x574 gpiod_get_index+0x58/0x84 devm_gpiod_get_index+0x20/0xb4 devm_gpiod_get+0x18/0x24 reset_gpio_probe+0x4c/0x14c [reset_gpio] auxiliary_bus_probe+0x44/0x90 really_probe+0xbc/0x298 __driver_probe_device+0x78/0x12c driver_probe_device+0xdc/0x164 __driver_attach+0x9c/0x1ac bus_for_each_dev+0x74/0xd0 driver_attach+0x24/0x30 bus_add_driver+0xe4/0x208 driver_register+0x60/0x128 __auxiliary_driver_register+0x68/0xe4 reset_gpio_driver_init+0x28/0x1000 [reset_gpio] do_one_initcall+0x64/0x308 do_init_module+0x58/0x23c load_module+0x1b48/0x1dc4 init_module_from_file+0xd4/0xec idempotent_init_module+0x188/0x280 __arm64_sys_finit_module+0x68/0xac invoke_syscall+0x48/0x10c el0_svc_common.constprop.0+0xc8/0xe8 do_el0_svc+0x20/0x2c el0_svc+0x50/0x2e8 el0t_64_sync_handler+0xa0/0xe4 el0t_64_sync+0x198/0x19c Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] gpio: shared: another set of small fixes 2026-01-05 16:48 ` [PATCH 0/2] gpio: shared: another set of small fixes Marek Szyprowski @ 2026-01-05 16:53 ` Bartosz Golaszewski 2026-01-05 17:22 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-05 16:53 UTC (permalink / raw) To: Marek Szyprowski Cc: Bartosz Golaszewski, Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel On Mon, 5 Jan 2026 17:48:05 +0100, Marek Szyprowski <m.szyprowski@samsung.com> said: > On 05.01.2026 16:52, Bartosz Golaszewski wrote: >> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >> --- >> Bartosz Golaszewski (2): >> gpio: shared: assign the correct firmware node for reset-gpio use-case >> gpio: shared: fix a race condition >> >> drivers/gpio/gpiolib-shared.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> --- >> base-commit: 19fb766a1e5ed5943a62fc671c09d45352a81b1d >> change-id: 20260105-gpio-shared-fixes-40a8ec3b6b25 > > Those patches indeed fixes some timing issues with the commit > 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between > reset-gpio and gpiolib"), but they also reveals another one. I've > initially thought that this is a false positive and needs only a proper > lockdep annotation, but some boards hang just after similar lockdep splat: > > ============================================ > WARNING: possible recursive locking detected > 6.19.0-rc4-next-20260105+ #11974 Not tainted > -------------------------------------------- > (udev-worker)/192 is trying to acquire lock: > ffff000004c15098 (&ref->lock){+.+.}-{4:4}, at: > gpio_shared_dev_is_reset_gpio+0xcc/0x234 > > but task is already holding lock: > ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: > gpio_shared_add_proxy_lookup+0x98/0x228 > > other info that might help us debug this: > Possible unsafe locking scenario: > > CPU0 > ---- > lock(&ref->lock); > lock(&ref->lock); > > *** DEADLOCK *** > > May be due to missing lock nesting notation > > 3 locks held by (udev-worker)/192: > #0: ffff00000b3ad8e8 (&dev->mutex){....}-{4:4}, at: > __driver_attach+0x90/0x1ac > #1: ffff8000830f2600 (gpio_devices_srcu){.+.+}-{0:0}, at: > gpiod_find_and_request+0x0/0x574 > #2: ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: > gpio_shared_add_proxy_lookup+0x98/0x228 > Ah this must be due to also trying to compare the ref to the base ref... Could you try to add the following on top: diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c index 198951c4c80b..5f3b8bc4a4fc 100644 --- a/drivers/gpio/gpiolib-shared.c +++ b/drivers/gpio/gpiolib-shared.c @@ -378,6 +378,9 @@ static bool gpio_shared_dev_is_reset_gpio(struct device *consumer, * arguments match the ones from this consumer's node. */ list_for_each_entry(real_ref, &entry->refs, list) { + if (real_ref == ref) + continue; + guard(mutex)(&real_ref->lock); if (!real_ref->fwnode) If that works, I'll send a v2. Bart ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] gpio: shared: another set of small fixes 2026-01-05 16:53 ` Bartosz Golaszewski @ 2026-01-05 17:22 ` Marek Szyprowski 2026-01-05 17:42 ` Bartosz Golaszewski 0 siblings, 1 reply; 7+ messages in thread From: Marek Szyprowski @ 2026-01-05 17:22 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel On 05.01.2026 17:53, Bartosz Golaszewski wrote: > On Mon, 5 Jan 2026 17:48:05 +0100, Marek Szyprowski > <m.szyprowski@samsung.com> said: >> On 05.01.2026 16:52, Bartosz Golaszewski wrote: >>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >>> --- >>> Bartosz Golaszewski (2): >>> gpio: shared: assign the correct firmware node for reset-gpio use-case >>> gpio: shared: fix a race condition >>> >>> drivers/gpio/gpiolib-shared.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> --- >>> base-commit: 19fb766a1e5ed5943a62fc671c09d45352a81b1d >>> change-id: 20260105-gpio-shared-fixes-40a8ec3b6b25 >> Those patches indeed fixes some timing issues with the commit >> 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between >> reset-gpio and gpiolib"), but they also reveals another one. I've >> initially thought that this is a false positive and needs only a proper >> lockdep annotation, but some boards hang just after similar lockdep splat: >> >> ============================================ >> WARNING: possible recursive locking detected >> 6.19.0-rc4-next-20260105+ #11974 Not tainted >> -------------------------------------------- >> (udev-worker)/192 is trying to acquire lock: >> ffff000004c15098 (&ref->lock){+.+.}-{4:4}, at: >> gpio_shared_dev_is_reset_gpio+0xcc/0x234 >> >> but task is already holding lock: >> ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: >> gpio_shared_add_proxy_lookup+0x98/0x228 >> >> other info that might help us debug this: >> Possible unsafe locking scenario: >> >> CPU0 >> ---- >> lock(&ref->lock); >> lock(&ref->lock); >> >> *** DEADLOCK *** >> >> May be due to missing lock nesting notation >> >> 3 locks held by (udev-worker)/192: >> #0: ffff00000b3ad8e8 (&dev->mutex){....}-{4:4}, at: >> __driver_attach+0x90/0x1ac >> #1: ffff8000830f2600 (gpio_devices_srcu){.+.+}-{0:0}, at: >> gpiod_find_and_request+0x0/0x574 >> #2: ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: >> gpio_shared_add_proxy_lookup+0x98/0x228 >> > Ah this must be due to also trying to compare the ref to the base ref... > > Could you try to add the following on top: > > diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c > index 198951c4c80b..5f3b8bc4a4fc 100644 > --- a/drivers/gpio/gpiolib-shared.c > +++ b/drivers/gpio/gpiolib-shared.c > @@ -378,6 +378,9 @@ static bool gpio_shared_dev_is_reset_gpio(struct > device *consumer, > * arguments match the ones from this consumer's node. > */ > list_for_each_entry(real_ref, &entry->refs, list) { > + if (real_ref == ref) > + continue; > + > guard(mutex)(&real_ref->lock); > > if (!real_ref->fwnode) > > > If that works, I'll send a v2. This fixed the hangs, but the lockdep whining is still there. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] gpio: shared: another set of small fixes 2026-01-05 17:22 ` Marek Szyprowski @ 2026-01-05 17:42 ` Bartosz Golaszewski 0 siblings, 0 replies; 7+ messages in thread From: Bartosz Golaszewski @ 2026-01-05 17:42 UTC (permalink / raw) To: Marek Szyprowski Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel, Bartosz Golaszewski On Mon, 5 Jan 2026 18:22:31 +0100, Marek Szyprowski <m.szyprowski@samsung.com> said: > On 05.01.2026 17:53, Bartosz Golaszewski wrote: >> On Mon, 5 Jan 2026 17:48:05 +0100, Marek Szyprowski >> <m.szyprowski@samsung.com> said: >>> On 05.01.2026 16:52, Bartosz Golaszewski wrote: >>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >>>> --- >>>> Bartosz Golaszewski (2): >>>> gpio: shared: assign the correct firmware node for reset-gpio use-case >>>> gpio: shared: fix a race condition >>>> >>>> drivers/gpio/gpiolib-shared.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> --- >>>> base-commit: 19fb766a1e5ed5943a62fc671c09d45352a81b1d >>>> change-id: 20260105-gpio-shared-fixes-40a8ec3b6b25 >>> Those patches indeed fixes some timing issues with the commit >>> 49416483a953 ("gpio: shared: allow sharing a reset-gpios pin between >>> reset-gpio and gpiolib"), but they also reveals another one. I've >>> initially thought that this is a false positive and needs only a proper >>> lockdep annotation, but some boards hang just after similar lockdep splat: >>> >>> ============================================ >>> WARNING: possible recursive locking detected >>> 6.19.0-rc4-next-20260105+ #11974 Not tainted >>> -------------------------------------------- >>> (udev-worker)/192 is trying to acquire lock: >>> ffff000004c15098 (&ref->lock){+.+.}-{4:4}, at: >>> gpio_shared_dev_is_reset_gpio+0xcc/0x234 >>> >>> but task is already holding lock: >>> ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: >>> gpio_shared_add_proxy_lookup+0x98/0x228 >>> >>> other info that might help us debug this: >>> Possible unsafe locking scenario: >>> >>> CPU0 >>> ---- >>> lock(&ref->lock); >>> lock(&ref->lock); >>> >>> *** DEADLOCK *** >>> >>> May be due to missing lock nesting notation >>> >>> 3 locks held by (udev-worker)/192: >>> #0: ffff00000b3ad8e8 (&dev->mutex){....}-{4:4}, at: >>> __driver_attach+0x90/0x1ac >>> #1: ffff8000830f2600 (gpio_devices_srcu){.+.+}-{0:0}, at: >>> gpiod_find_and_request+0x0/0x574 >>> #2: ffff000004c15898 (&ref->lock){+.+.}-{4:4}, at: >>> gpio_shared_add_proxy_lookup+0x98/0x228 >>> >> Ah this must be due to also trying to compare the ref to the base ref... >> >> Could you try to add the following on top: >> >> diff --git a/drivers/gpio/gpiolib-shared.c b/drivers/gpio/gpiolib-shared.c >> index 198951c4c80b..5f3b8bc4a4fc 100644 >> --- a/drivers/gpio/gpiolib-shared.c >> +++ b/drivers/gpio/gpiolib-shared.c >> @@ -378,6 +378,9 @@ static bool gpio_shared_dev_is_reset_gpio(struct >> device *consumer, >> * arguments match the ones from this consumer's node. >> */ >> list_for_each_entry(real_ref, &entry->refs, list) { >> + if (real_ref == ref) >> + continue; >> + >> guard(mutex)(&real_ref->lock); >> >> if (!real_ref->fwnode) >> >> >> If that works, I'll send a v2. > > This fixed the hangs, but the lockdep whining is still there. > Then it's a false-positive. I will try to play with lockdep_set_class() tomorrow. Bart ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-01-05 17:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20260105155232eucas1p1a0d0d603716267eacef8f57f7aa5e34f@eucas1p1.samsung.com>
2026-01-05 15:52 ` [PATCH 0/2] gpio: shared: another set of small fixes Bartosz Golaszewski
2026-01-05 15:52 ` [PATCH 1/2] gpio: shared: assign the correct firmware node for reset-gpio use-case Bartosz Golaszewski
2026-01-05 15:52 ` [PATCH 2/2] gpio: shared: fix a race condition Bartosz Golaszewski
2026-01-05 16:48 ` [PATCH 0/2] gpio: shared: another set of small fixes Marek Szyprowski
2026-01-05 16:53 ` Bartosz Golaszewski
2026-01-05 17:22 ` Marek Szyprowski
2026-01-05 17:42 ` Bartosz Golaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®