14 Commits

Author SHA1 Message Date
Bartosz Golaszewski
ea513dd3c0 gpio: shared: make locking more fine-grained
The global gpio_shared_lock has caused some issues when recursively
removing GPIO shared proxies. The thing is: we don't really need it.
Once created from an init-call, the shared GPIO data structures are
never removed, there's no need to protect the linked lists of entries
and references. All we need to protect is the shared GPIO descriptor
(which we already do with a per-entry mutex) and the auxiliary device
data in struct gpio_shared_ref.

Remove the global gpio_shared_lock and use a per-reference mutex to
protect shared references when adding/removing the auxiliary devices and
their GPIO lookup entries.

Link: https://lore.kernel.org/r/20251206-gpio-shared-teardown-fixes-v1-4-35ac458cfce1@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
2025-12-09 07:16:45 +01:00
Bartosz Golaszewski
d382c765d0 gpio: shared: fix auxiliary device cleanup order
Dropping the last reference to the internal struct device should be the
last thing we do so delete the device first and then uninit it which
also involves the final put_device().

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Link: https://lore.kernel.org/r/20251206-gpio-shared-teardown-fixes-v1-3-35ac458cfce1@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
2025-12-09 07:16:45 +01:00
Bartosz Golaszewski
c904a0d852 gpio: shared: check if a reference is populated before cleaning its resources
It's possible that not all proxy entries will be set up when the device
gets removed so check if they are before trying to dereference members
which are still NULL. This can happen if some consumers never requested
their shared GPIOs.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Link: https://lore.kernel.org/r/20251206-gpio-shared-teardown-fixes-v1-2-35ac458cfce1@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
2025-12-09 07:16:45 +01:00
Bartosz Golaszewski
e2c4175b8d gpio: shared: fix NULL-pointer dereference in teardown path
We need to actually store the address of the GPIO lookup table in the
reference struct before we try to free it or - worse - dereference its
members.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Link: https://lore.kernel.org/r/20251206-gpio-shared-teardown-fixes-v1-1-35ac458cfce1@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
2025-12-09 07:16:45 +01:00
Bartosz Golaszewski
9e7a40a284 gpio: shared: ignore disabled nodes when traversing the device-tree
Don't consider disabled devices when traversing the device-tree looking
for shared GPIOs. Even if they do share a phandle to a pin, they can
never be instantiated and request it.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Link: https://lore.kernel.org/r/20251203092309.34737-1-bartosz.golaszewski@oss.qualcomm.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
2025-12-09 07:14:53 +01:00
Bartosz Golaszewski
54a2df5afa gpio: shared: fix a deadlock
It's possible that the auxiliary proxy device we add when setting up the
GPIO controller exposing shared pins, will get matched and probed
immediately. The gpio-proxy-driver will then retrieve the shared
descriptor structure. That will cause a recursive mutex locking and
a deadlock because we're already holding the gpio_shared_lock in
gpio_device_setup_shared() and try to take it again in
devm_gpiod_shared_get() like this:

[    4.298346] gpiolib_shared: GPIO 130 owned by f100000.pinctrl is shared by multiple consumers
[    4.307157] gpiolib_shared: Setting up a shared GPIO entry for speaker@0,3
[    4.314604]
[    4.316146] ============================================
[    4.321600] WARNING: possible recursive locking detected
[    4.327054] 6.18.0-rc7-next-20251125-g3f300d0674f6-dirty #3887 Not tainted
[    4.334115] --------------------------------------------
[    4.339566] kworker/u32:3/71 is trying to acquire lock:
[    4.344931] ffffda019ba71850 (gpio_shared_lock){+.+.}-{4:4}, at: devm_gpiod_shared_get+0x34/0x2e0
[    4.354057]
[    4.354057] but task is already holding lock:
[    4.360041] ffffda019ba71850 (gpio_shared_lock){+.+.}-{4:4}, at: gpio_device_setup_shared+0x30/0x268
[    4.369421]
[    4.369421] other info that might help us debug this:
[    4.376126]  Possible unsafe locking scenario:
[    4.376126]
[    4.382198]        CPU0
[    4.384711]        ----
[    4.387223]   lock(gpio_shared_lock);
[    4.390992]   lock(gpio_shared_lock);
[    4.394761]
[    4.394761]  *** DEADLOCK ***
[    4.394761]
[    4.400832]  May be due to missing lock nesting notation
[    4.400832]
[    4.407802] 5 locks held by kworker/u32:3/71:
[    4.412279]  #0: ffff000080020948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x194/0x64c
[    4.422650]  #1: ffff800080963d60 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1bc/0x64c
[    4.432117]  #2: ffff00008165c8f8 (&dev->mutex){....}-{4:4}, at: __device_attach+0x3c/0x198
[    4.440700]  #3: ffffda019ba71850 (gpio_shared_lock){+.+.}-{4:4}, at: gpio_device_setup_shared+0x30/0x268
[    4.450523]  #4: ffff0000810fe918 (&dev->mutex){....}-{4:4}, at: __device_attach+0x3c/0x198
[    4.459103]
[    4.459103] stack backtrace:
[    4.463581] CPU: 6 UID: 0 PID: 71 Comm: kworker/u32:3 Not tainted 6.18.0-rc7-next-20251125-g3f300d0674f6-dirty #3887 PREEMPT
[    4.463589] Hardware name: Qualcomm Technologies, Inc. Robotics RB5 (DT)
[    4.463593] Workqueue: events_unbound deferred_probe_work_func
[    4.463602] Call trace:
[    4.463604]  show_stack+0x18/0x24 (C)
[    4.463617]  dump_stack_lvl+0x70/0x98
[    4.463627]  dump_stack+0x18/0x24
[    4.463636]  print_deadlock_bug+0x224/0x238
[    4.463643]  __lock_acquire+0xe4c/0x15f0
[    4.463648]  lock_acquire+0x1cc/0x344
[    4.463653]  __mutex_lock+0xb8/0x840
[    4.463661]  mutex_lock_nested+0x24/0x30
[    4.463667]  devm_gpiod_shared_get+0x34/0x2e0
[    4.463674]  gpio_shared_proxy_probe+0x18/0x138
[    4.463682]  auxiliary_bus_probe+0x40/0x78
[    4.463688]  really_probe+0xbc/0x2c0
[    4.463694]  __driver_probe_device+0x78/0x120
[    4.463701]  driver_probe_device+0x3c/0x160
[    4.463708]  __device_attach_driver+0xb8/0x140
[    4.463716]  bus_for_each_drv+0x88/0xe8
[    4.463723]  __device_attach+0xa0/0x198
[    4.463729]  device_initial_probe+0x14/0x20
[    4.463737]  bus_probe_device+0xb4/0xc0
[    4.463743]  device_add+0x578/0x76c
[    4.463747]  __auxiliary_device_add+0x40/0xac
[    4.463752]  gpio_device_setup_shared+0x1f8/0x268
[    4.463758]  gpiochip_add_data_with_key+0xdac/0x10ac
[    4.463763]  devm_gpiochip_add_data_with_key+0x30/0x80
[    4.463768]  msm_pinctrl_probe+0x4b0/0x5e0
[    4.463779]  sm8250_pinctrl_probe+0x18/0x40
[    4.463784]  platform_probe+0x5c/0xa4
[    4.463793]  really_probe+0xbc/0x2c0
[    4.463800]  __driver_probe_device+0x78/0x120
[    4.463807]  driver_probe_device+0x3c/0x160
[    4.463814]  __device_attach_driver+0xb8/0x140
[    4.463821]  bus_for_each_drv+0x88/0xe8
[    4.463827]  __device_attach+0xa0/0x198
[    4.463834]  device_initial_probe+0x14/0x20
[    4.463841]  bus_probe_device+0xb4/0xc0
[    4.463847]  deferred_probe_work_func+0x90/0xcc
[    4.463854]  process_one_work+0x214/0x64c
[    4.463860]  worker_thread+0x1bc/0x360
[    4.463866]  kthread+0x14c/0x220
[    4.463871]  ret_from_fork+0x10/0x20
[   77.265041] random: crng init done

Fortunately, at the time of creating of the auxiliary device, we already
know the correct entry so let's store it as the device's platform data.
We don't need to hold gpio_shared_lock in devm_gpiod_shared_get() as
we're not removing the entry or traversing the list anymore but we still
need to protect it from concurrent modification of its fields so add a
more fine-grained mutex.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Closes: https://lore.kernel.org/all/fimuvblfy2cmn7o4wzcxjzrux5mwhvlvyxfsgeqs6ore2xg75i@ax46d3sfmdux/
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Tested-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20251128-gpio-shared-deadlock-v2-1-9f3ae8ddcb09@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-28 09:48:28 +01:00
Bartosz Golaszewski
114e594e6c gpio: shared: ignore GPIO hogs when traversing the device tree
GPIO hogs have a "gpios" property but it's not a phandle to a remote
node - it references the parent GPIO controller. We must not try to
parse it as a phandle.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Cosmin Tanislav <demonsingur@gmail.com>
Closes: https://lore.kernel.org/all/2d96e464-e17c-4ff5-9a08-b215b77da04f@gmail.com/
Link: https://lore.kernel.org/r/20251126-gpio-shared-fixes-v1-2-18309c0e87b5@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-28 09:29:14 +01:00
Bartosz Golaszewski
cfab6dc070 gpio: shared: ignore special __symbols__ node when traversing device tree
The __symbols__ node is a special, internal node and its properties must
not be considered when scanning the device-tree for shared GPIOs.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Jon Hunter <jonathanh@nvidia.com>
Closes: https://lore.kernel.org/all/0829a21c-f97d-41b6-90bc-2acaec42caab@nvidia.com/
Tested-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Link: https://lore.kernel.org/r/20251126-gpio-shared-fixes-v1-1-18309c0e87b5@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-28 09:29:14 +01:00
Bartosz Golaszewski
7b78b26757 gpio: shared: handle the reset-gpios corner case
There's an unexpected interaction between the reset-gpio driver and the
shared GPIO support. The reset-gpio device is an auxiliary device that's
created dynamically and fulfills a similar role to the gpio-shared-proxy
driver but is limited in scope to just supporting the "reset-gpios"
property.

The shared GPIO core code does not take into account that the machine
lookup entry we create when scanning the device-tree must connect the
reset-gpio device - that is the actual consumer of the GPIO and not the
consumer defined on the device tree, which in turn consumes the shared
reset control exposed by the reset-gpio device - to the GPIO controller.

We also must not skip the gpio-shared-proxy driver as it's possible that
a shared GPIO may be used by one consumer as a reset-gpios going through
the reset-gpio device and another that uses GPIOLIB.

We need to make it a special case handled in gpiolib-shared.c. Add a new
function - gpio_shared_dev_is_reset_gpio() - whose role it is to verify
if a non-matching consumer of a shared pin is a reset-gpio device and
make sure it's the right one for this pin. To that end make sure that
its parent is the GPIO controller in question and that the fwnode we
identified as sharing the pin references that controller via the
"reset-gpios" property.

Only include that code if the reset-gpio driver is enabled.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Val Packett <val@packett.cool>
Closes: https://lore.kernel.org/all/3b5d9df5-934d-4591-8827-6c9573a6f7ba@packett.cool/
Tested-by: Val Packett <val@packett.cool>
Tested-by: Abel Vesa <abel.vesa@linaro.org>
Link: https://lore.kernel.org/r/20251125-gpiolib-shared-reset-gpio-fix-v2-1-4eb6fa41f1dd@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-27 09:21:12 +01:00
Bartosz Golaszewski
8ad236f8a4 gpio: shared: extend the ifdef guard to gpio_shared_find_entry()
While this function is supposed to be used by all scanning functions, so
far we only have a single one for OF trees. Once we add support for ACPI
and software nodes, we'll drop the CONFIG_OF guard around this routine
but in order to avoid build warnings, let's extend it to cover it in the
meantime.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511180232.EItKeYjY-lkp@intel.com/
Link: https://lore.kernel.org/r/20251118-gpiolib-shared-of-guard-v1-1-e4ef149a2e0b@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-19 08:33:55 +01:00
Bartosz Golaszewski
01be904798 gpio: shared: fix a NULL-pointer dereference
The fact that CONFIG_OF is enabled does not mean that the device tree is
populated and that of_root points to a valid device node. Check if it's
NULL before trying to traverse the tree.

Fixes: a060b8c511 ("gpiolib: implement low-level, shared GPIO support")
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/all/dbe20642-9662-40af-a593-c1263baea73b@sirena.org.uk/
Tested-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20251118200459.13969-1-brgl@bgdev.pl
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-19 08:32:38 +01:00
Bartosz Golaszewski
82e71fe436 Merge tag 'gpio/shared-gpios-for-v6.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git into gpio/for-next
Immutable branch between the GPIO, ASoC and regulator trees for v6.19-rc1

Add better support for GPIOs shared by multiple consumers.
2025-11-17 10:37:37 +01:00
Bartosz Golaszewski
eb374f764a gpio: provide gpiod_is_shared()
Provide an interface allowing consumers to check if a GPIO descriptor
represents a GPIO that can potentially be shared by multiple consumers
at the same time. This is exposed to allow subsystems that already
work around the limitations of the current non-exclusive GPIO handling
in some ways, to gradually convert to relying on the new shared GPIO
feature of GPIOLIB.

Extend the gpiolib-shared module to mark the GPIO shared proxy
descriptors with a flag checked by the new interface.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20251112-gpio-shared-v4-6-b51f97b1abd8@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-17 10:16:51 +01:00
Bartosz Golaszewski
a060b8c511 gpiolib: implement low-level, shared GPIO support
This module scans the device tree (for now only OF nodes are supported
but care is taken to make other fwnode implementations easy to
integrate) and determines which GPIO lines are shared by multiple users.
It stores that information in memory. When the GPIO chip exposing shared
lines is registered, the shared GPIO descriptors it exposes are marked
as shared and virtual "proxy" devices that mediate access to the shared
lines are created. When a consumer of a shared GPIO looks it up, its
fwnode lookup is redirected to a just-in-time machine lookup that points
to this proxy device.

This code can be compiled out on platforms which don't use shared GPIOs.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20251112-gpio-shared-v4-3-b51f97b1abd8@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
2025-11-17 10:16:51 +01:00