- 01 Mar, 2024 2 commits
-
-
Andy Shevchenko authored
Make acpi_gpio_count() take firmware node as a parameter in order to be aligned with other functions and decouple from unused device pointer. The latter helps to create a common fwnode_gpio_count() in the future. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
Andy Shevchenko authored
Make of_gpio_get_count() take firmware node as a parameter in order to be aligned with other functions and decouple from unused device pointer. The latter helps to create a common fwnode_gpio_count() in the future. While at it, rename to be of_gpio_count() to be aligned with the others. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 27 Feb, 2024 1 commit
-
-
Stephen Boyd authored
This devm API takes a consumer device as an argument to setup the devm action, but throws it away when calling further into gpiolib. This leads to odd debug messages like this: (NULL device *): using DT '/gpio-keys/switch-pen-insert' for '(null)' GPIO lookup Let's pass the consumer device down, by directly calling what fwnode_gpiod_get_index() calls but pass the device used for devm. This changes the message to look like this instead: gpio-keys gpio-keys: using DT '/gpio-keys/switch-pen-insert' for '(null)' GPIO lookup Note that callers of fwnode_gpiod_get_index() will still see the NULL device pointer debug message, but there's not much we can do about that because the API doesn't take a struct device. Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Fixes: 8eb1f71e ("gpiolib: consolidate GPIO lookups") Signed-off-by: Stephen Boyd <swboyd@chromium.org> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 26 Feb, 2024 6 commits
-
-
Bartosz Golaszewski authored
Display debugfs information about all simulated GPIOs, not only the requested ones. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
-
Bartosz Golaszewski authored
We only provide iterators for requested GPIOs to provider drivers. In order to allow them to display debug information about all GPIOs, let's provide a variant for iterating over all GPIOs. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
-
Bartosz Golaszewski authored
With SRCU we can now correctly handle the situation when a GPIO provider is removed while having users still holding references to GPIO descriptors. Remove all warnings emitted in this situation. Suggested-by: Kent Gibson <warthog618@gmail.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Herve Codina <herve.codina@bootlin.com>
-
Bartosz Golaszewski authored
Typically, whenever a human-readable name is created for objects using a software node, its name is delimited with ":" as dashes are often used in other parts of the name. Make gpio-sim use the same pattern. This results in better looking default names: gpio-sim.0:node0 gpio-sim.0:node1 gpio-sim.1:node0 Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Acked-by: Linus Walleij <linus.walleij@linaro.org>
-
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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 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: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
-
- 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: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
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 <bartosz.golaszewski@linaro.org>
-
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 <bartosz.golaszewski@linaro.org>
-
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: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.comSigned-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Acked-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: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.comSigned-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Paul E. McKenney <paulmck@kernel.org>
-
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: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.comSigned-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Paul E. McKenney <paulmck@kernel.org>
-
- 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 <warthog618@gmail.com> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> [Bartosz: tweaked the commit subject to use imperative mood] Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
Kent Gibson authored
Clarify that line values are logical, not physical, by replacing high/low terminology with active/inactive. Signed-off-by: Kent Gibson <warthog618@gmail.com> Reviewed-by: Andy Shevchenko <amdy.shevchenko@gmail.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
-
- 12 Feb, 2024 18 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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
Bartosz Golaszewski authored
Don't dereference gdev->chip if the same information can be obtained from struct gpio_device. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
Bartosz Golaszewski authored
Checking the gdev->mockdev pointer for NULL must be part of the critical section. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
Bartosz Golaszewski authored
The GPIO chip pointer is unused. Let's remove it. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-
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 <bartosz.golaszewski@linaro.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-