- 26 Feb, 2024 2 commits
-
-
Bartosz Golaszewski authored
We have three functions in gpio-sim that are called with the device lock already held. We use the "_unlocked" suffix in their names to indicate that. This has proven to be confusing though as the naming convention in the kernel varies between using "_locked" or "_unlocked" for this purpose. Naming convention also doesn't enforce anything. Let's remove the suffix and add lockdep annotation at the top of these functions. This makes it clear the function requires a lock to be held (and which one specifically!) as well as results in a warning if it's not the case. The only place where the information is lost is the place where the function is called but the caller doesn't care about that information anyway. Signed-off-by:Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
Stephen Boyd authored
The ChromeOS embedded controller (EC) supports setting the state of GPIOs when the system is unlocked, and getting the state of GPIOs in all cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO expander. Add a driver to get and set the GPIOs on the EC through the host command interface. Signed-off-by:
Stephen Boyd <swboyd@chromium.org> Reviewed-by:
Linus Walleij <linus.walleij@linaro.org> Signed-off-by:
-
- 20 Feb, 2024 1 commit
-
-
Krzysztof Kozlowski authored
Pointer to the struct of_phandle_args can be made const after gpio_device_find() arguments got constified. This should be part of commit 4a92857d ("gpio: constify opaque pointer "data" in gpio_device_find()"). Signed-off-by:
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by:
-
- 19 Feb, 2024 1 commit
-
-
Xiaolei Wang authored
Since commit 1f2bcb8c ("gpio: protect the descriptor label with SRCU"), desc_set_label() already allocates memory for the label, so there is no need to allocate it again. If we do, we leak it. unreferenced object 0xffff0000c3e4d0c0 (size 32): comm "kworker/u16:4", pid 60, jiffies 4294894555 hex dump (first 32 bytes): 72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73 regulator-can2-s 74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff tby............. backtrace (crc 2c3a0350): [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40 [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524 [<000000000dd1c057>] kstrdup+0x4c/0x98 [<00000000b513a96a>] kstrdup_const+0x34/0x40 [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358 [<00000000fc71ad64>] gpiod_request+0xd8/0x204 [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780 [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0 [<000000004aef97f9>] gpiod_get_optional+0x18/0x30 [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8 [<00000000e6f47635>] platform_probe+0xc4/0x198 [<00000000cf78fbdb>] really_probe+0x204/0x5a8 [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4 [<00000000e4fe452b>] driver_probe_device+0x60/0x18c [<00000000479fcf5d>] __device_attach_driver+0x168/0x208 [<000000007d389f38>] bus_for_each_drv+0x104/0x190 Fixes: 1f2bcb8c ("gpio: protect the descriptor label with SRCU") Signed-off-by:
Xiaolei Wang <xiaolei.wang@windriver.com> [Bartosz: tweaked the commit message] Signed-off-by:
-
- 17 Feb, 2024 1 commit
-
-
Krzysztof Kozlowski authored
The opaque pointer "data" in each match function used by gpio_device_find() is a pointer to const, thus the same argument passed to gpio_device_find() can adjusted similarly. Signed-off-by:
-
- 16 Feb, 2024 1 commit
-
-
Bartosz Golaszewski authored
We are actually passing the gc pointer to chip_dbg() so we have to srcu_dereference() it. Fixes: 8574b5b4 ("gpio: cdev: use correct pointer accessors with SRCU") Reported-by:
Marek Szyprowski <m.szyprowski@samsung.com> Closes: https://lore.kernel.org/lkml/179caa10-5f86-4707-8bb0-fe1b316326d6@samsung.com/Signed-off-by:
-
- 15 Feb, 2024 6 commits
-
-
Kent Gibson authored
The documentation for default_values mentions high/low which can be confusing, particularly when the ACTIVE_LOW flag is set. Replace high/low with active/inactive to clarify that the values are logical not physical. Similarly, clarify the interpretation of values in struct gpiohandle_data. Signed-off-by:
Kent Gibson <warthog618@gmail.com> Reviewed-by:
Andy Shevchenko <andy.shevchenko@gmail.com> Acked-by:
-
Bartosz Golaszewski authored
The logic is inverted, we want to return if the chip *IS* NULL. Fixes: d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU") Reported-by:
Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/linux-gpio/15671341-0b29-40e0-b487-0a4cdc414d8e@moroto.mountain/Signed-off-by:
-
Bartosz Golaszewski authored
There are two legacy, deprecated functions - gpiod_to_chip() and gpio_device_get_chip() - that still have users in tree. They return the address of the SRCU-protected chip outside of the read-only critical sections. They are inherently dangerous and the users should convert to safer alternatives. Let's explicitly silence lockdep warnings by using rcu_dereference_check(ptr, 1). While at it: reuse gpio_device_get_chip() in gpiod_to_chip(). Fixes: d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU") Reported-by:
kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.comSigned-off-by:
-
Bartosz Golaszewski authored
Lockdep with CONFIG_PROVE_RCU enabled reports false positives about suspicious rcu_dereference() usage. Let's silence it by using srcu_dereference() which is the correct helper with SRCU. Fixes: d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU") Reported-by:
Paul E. McKenney <paulmck@kernel.org>
-
Bartosz Golaszewski authored
We never dereference the chip pointer in character device code so we can use the lighter rcu_access_pointer() helper. This also makes lockep happier as it no longer complains about suspicious rcu_dereference() usage. Fixes: d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU") Reported-by:
-
Bartosz Golaszewski authored
gpiod_hog() may be called without the gpio_device SRCU read lock taken so we need to do it here as well. It's alright if someone else is already holding the lock as SRCU read critical sections can be nested. Fixes: d83cee3d ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU") Reported-by:
-
- 14 Feb, 2024 3 commits
-
-
Kent Gibson authored
Consistently use active/inactive to describe logical line values, rather than high/low, which is used for physical values, or asserted/de-asserted which is awkward. Signed-off-by:
-
Kent Gibson authored
Clarify that line values are logical, not physical, by replacing high/low terminology with active/inactive. Signed-off-by:
Andy Shevchenko <amdy.shevchenko@gmail.com> Signed-off-by:
-
Bartosz Golaszewski authored
In certain situations we may end up taking the GPIO descriptor SRCU read lock in of_gpiochip_add() before the SRCU struct is initialized. Move the initialization before the call to of_gpiochip_add(). Fixes: be711caa ("gpio: add SRCU infrastructure to struct gpio_desc") Fixes: 1f2bcb8c ("gpio: protect the descriptor label with SRCU") Reported-by:
kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-lkp/202402122228.e607a080-lkp@intel.comSigned-off-by:
-
- 12 Feb, 2024 25 commits
-
-
Bartosz Golaszewski authored
We still have some functions that return the address of the GPIO chip associated with the GPIO device. This is dangerous and the users should find a better solution. Let's add appropriate comments to the kernel docs. Signed-off-by:
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
Bartosz Golaszewski authored
With all accesses to gdev->chip being protected with SRCU, we can now remove the RW-semaphore specific to the character device which fulfilled the same role up to this point. Signed-off-by:
-
Bartosz Golaszewski authored
Ensure we cannot crash if the GPIO device gets unregistered (and the chip pointer set to NULL) during any of the API calls. To that end: wait for all users of gdev->chip to exit their read-only SRCU critical sections in gpiochip_remove(). For brevity: add a guard class which can be instantiated at the top of every function requiring read-only access to the chip pointer and use it in all API calls taking a GPIO descriptor as argument. In places where we only deal with the GPIO device - use regular guard() helpers and rcu_dereference() for chip access. Do the same in API calls taking a const pointer to gpio_desc. Signed-off-by:
-
Bartosz Golaszewski authored
Add the SRCU struct to GPIO device. It will be used to serialize access to the GPIO chip pointer. Initialize and clean it up where applicable. Signed-off-by:
-
Bartosz Golaszewski authored
Duplicating the can_sleep value in GPIO device will allow us to not needlessly dereference the chip pointer in several places and reduce the number of SRCU read-only critical sections. Signed-off-by:
-
Bartosz Golaszewski authored
We don't need to check the gdev pointer in struct gpio_desc - it's always assigned and never cleared. It's also pointless to check gdev->chip before we actually serialize access to it. Signed-off-by:
-
Bartosz Golaszewski authored
Checking desc->gdev->chip for NULL without holding it in place with some serializing mechanism is pointless. Remove this check. Also don't check desc->gdev for NULL as it can never happen. We'll be protecting gdev->chip with SRCU soon but we will provide a dedicated, automatic class for that. Signed-off-by:
-
Bartosz Golaszewski authored
We don't need to dereference gdev->chip in gpiochip_setup_dev() as at the time it's called, the label in the associated struct gpio_device is already set. Signed-off-by:
-
Bartosz Golaszewski authored
Don't dereference gdev->chip if the same information can be obtained from struct gpio_device. Suggested-by:
-
Bartosz Golaszewski authored
The variable holding the number of GPIO lines is duplicated in GPIO device so read it instead of unnecessarily dereferencing the chip pointer. Signed-off-by:
-
Bartosz Golaszewski authored
gpio_device_get_desc() is the safer alternative to gpiochip_get_desc(). As we don't really need to dereference the chip pointer to retrieve the descriptors in character device code, let's use it. Signed-off-by:
-
Bartosz Golaszewski authored
We're working towards protecting the chip pointer in struct gpio_device with SRCU. In order to use it in sysfs callbacks we must pass the pointer to the GPIO device that wraps the chip instead of the address of the chip itself as the user data. Signed-off-by:
-
Bartosz Golaszewski authored
Checking the gdev->mockdev pointer for NULL must be part of the critical section. Signed-off-by:
-
Bartosz Golaszewski authored
The GPIO chip pointer is unused. Let's remove it. Signed-off-by:
-
Bartosz Golaszewski authored
We now removed the gpio_lock spinlock and modified the places previously protected by it to handle desc->flags access in a consistent way. Let's improve other places that were previously unprotected by reading the flags field of gpio_desc once and using the stored value for logic consistency. If we need to modify the field, let's also write it back once with a consistent value resulting from the function's logic. Signed-off-by:
-
Bartosz Golaszewski authored
The "multi-function" gpio_lock is pretty much useless with how it's used in GPIOLIB currently. Because many GPIO API calls can be called from all contexts but may also call into sleeping driver callbacks, there are many places with utterly broken workarounds like yielding the lock to call a possibly sleeping function and then re-acquiring it again without taking into account that the protected state may have changed. It was also used to protect several unrelated things: like individual descriptors AND the GPIO device list. We now serialize access to these two with SRCU and so can finally remove the spinlock. There is of course the question of consistency of lockless access to GPIO descriptors. Because we only support exclusive access to GPIOs (officially anyway, I'm looking at you broken GPIOD_FLAGS_BIT_NONEXCLUSIVE bit...) and the API contract with providers does not guarantee serialization, it's enough to ensure we cannot accidentally dereference an invalid pointer and that the state we present to both users and providers remains consistent. To achieve that: read the flags field atomically except for a few special cases. Read their current value before executing callback code and use this value for any subsequent logic. Modifying the flags depends on the particular use-case and can differ. For instance: when requesting a GPIO, we need to set the REQUESTED bit immediately so that the next user trying to request the same line sees -EBUSY. While at it: the allocations that used GFP_ATOMIC until this point can now switch to GFP_KERNEL. Signed-off-by:
-
Bartosz Golaszewski authored
With the list of GPIO devices now protected with SRCU we can use gpio_device_find() to traverse it from sysfs. Signed-off-by:
-
Bartosz Golaszewski authored
In order to ensure that the label is not freed while it's being accessed, let's protect it with SRCU and synchronize it everytime it's changed. Let's modify desc_set_label() to manage the memory used for the label as it can only be freed once synchronize_srcu() returns. Signed-off-by:
-
Bartosz Golaszewski authored
Extend the GPIO descriptor with an SRCU structure in order to serialize the access to the label. Initialize and clean it up where applicable. Signed-off-by:
-
Bartosz Golaszewski authored
We will soon serialize access to the descriptor label using SRCU. The write-side of the protection will require calling synchronize_srcu() which must not be called from atomic context. We have two irq helpers: gpiochip_lock_as_irq() and gpiochip_unlock_as_irq() that set the label if the GPIO is not requested but is being used as interrupt. They are called with a spinlock held from the interrupt subsystem. They must not do it if we are to use SRCU so instead let's move the special corner case to a dedicated getter. Don't actually set the label to "interrupt" in the above case but rather use the newly added gpiod_get_label() helper to hide the logic that atomically checks the descriptor flags and returns the address of a static "interrupt" string. Signed-off-by:
-
Bartosz Golaszewski authored
We will soon serialize access to the descriptor label using SRCU. The write-side of the protection will require calling synchronize_srcu() which must not be called from atomic context. We have two irq helpers: gpiochip_lock_as_irq() and gpiochip_unlock_as_irq() that set the label if the GPIO is not requested but is being used as interrupt. They are called with a spinlock held from the interrupt subsystem. They must not do it if we are to use SRCU so instead let's move the special corner case to a dedicated getter. First: let's implement and use the getter where it's applicable. Signed-off-by:
-
Bartosz Golaszewski authored
The general rule of the kernel is to not provide symbols that have no users upstream. Let's remove logging helpers that are not used anywhere. This will save us work later when we'll be modifying them to use the upcoming SRCU infrastructure. Signed-off-by:
-
Bartosz Golaszewski authored
The device nodes representing GPIO hogs cannot be deleted without unregistering the GPIO chip so there's no need to serialize their access. However we must ensure that users can get the right address so write and read it atomically. Signed-off-by:
-
Bartosz Golaszewski authored
We're working towards removing the "multi-function" GPIO spinlock that's implemented terribly wrong. We tried using an RW-semaphore to protect the list of GPIO devices but it turned out that we still have old code using legacy GPIO calls that need to translate the global GPIO number to the address of the associated descriptor and - to that end - traverse the list while holding the lock. If we change the spinlock to a sleeping lock then we'll end up with "scheduling while atomic" bugs. Let's allow lockless traversal of the list using SRCU and only use the mutex when modyfing the list. While at it: let's protect the period between when we start the lookup and when we finally request the descriptor (increasing the reference count of the GPIO device) with the SRCU read lock. Signed-off-by:
-
Krzysztof Kozlowski authored
The match function used in gpio_device_find() should not modify the contents of passed opaque pointer, because such modification would not be necessary for actual matching and it could lead to quite unreadable, spaghetti code. Signed-off-by:
Mika Westerberg <mika.westerberg@linux.intel.com> Reviewed-by:
-
