[PATCH 0/2] gpio: shared: another set of small fixes

Bartosz Golaszewski posted 2 patches 1 month ago
There is a newer version of this series
drivers/gpio/gpiolib-shared.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH 0/2] gpio: shared: another set of small fixes
Posted by Bartosz Golaszewski 1 month ago
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>
Re: [PATCH 0/2] gpio: shared: another set of small fixes
Posted by Marek Szyprowski 1 month ago
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

Re: [PATCH 0/2] gpio: shared: another set of small fixes
Posted by Bartosz Golaszewski 1 month ago
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
Re: [PATCH 0/2] gpio: shared: another set of small fixes
Posted by Marek Szyprowski 1 month ago
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

Re: [PATCH 0/2] gpio: shared: another set of small fixes
Posted by Bartosz Golaszewski 1 month ago
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