| Age | Commit message (Collapse) | Author |
|---|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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 <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
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 <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
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 <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
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 <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
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: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
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: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
[Bartosz: fix coding style in header]
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
Linux 6.8-rc4
Pulling this for a bugfix upstream with which the gpio/for-next branch
conflicts.
|
|
It's useful to have the device type information for those sub-devices
that are actually GPIO chips registered with GPIOLIB. While at it: use
the device type struct to setup the release callback which is the
preferred way to use the device API.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
Since commit 48e1b4d369cf ("gpiolib: remove the GPIO device from the list
when it's unregistered") we remove the GPIO device entry from the global
list (used to order devices by their GPIO ranges) when unregistering the
chip, not when releasing the device. It will not happen when the last
reference is put anymore. This means, we need to remove it in error path
in gpiochip_add_data_with_key() unconditionally, without checking if the
device's .release() callback is set.
Fixes: 48e1b4d369cf ("gpiolib: remove the GPIO device from the list when it's unregistered")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
Now that the driver core can properly handle constant struct bus_type,
move the gpio_bus_type variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
There are no external users for the irq domain helpers so unexport them
and remove the prototypes from the driver header.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
This reverts commits 1979a2807547 ("gpiolib: replace the GPIO device
mutex with a read-write semaphore") and 65a828bab158 ("gpiolib: use
a mutex to protect the list of GPIO devices").
Unfortunately the legacy GPIO API that's still used in older code has to
translate numbers from the global GPIO numberspace to descriptors. This
results in a GPIO device lookup in every call to legacy functions. Some
of those functions - like gpio_set/get_value() - can be called from
atomic context so taking a sleeping lock that is an RW semaphore results
in an error.
We'll probably have to protect this list with SRCU.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-wireless/f7b5ff1e-8f34-4d98-a7be-b826cb897dc8@moroto.mountain/
Fixes: 1979a2807547 ("gpiolib: replace the GPIO device mutex with a read-write semaphore")
Fixes: 65a828bab158 ("gpiolib: use a mutex to protect the list of GPIO devices")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
There are only two spots where we modify (add to or remove objects from)
the GPIO device list. Readers should be able to access it concurrently.
Replace the mutex with a read-write semaphore and adjust the locking
operations accordingly.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
If we wait until the GPIO device's .release() callback gets invoked
before we remove it from the global device list, then we risk that
someone will look it up using gpio_device_find() between where we
dropped the last reference and before .release() is done taking a
reference again to an object that's being released.
The device must be removed when it's being unregistered - just like how
we remove it from the GPIO bus.
Fixes: ff2b13592299 ("gpio: make the gpiochip a real device")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Older code has an annoying habit of putting tabs between the type and the
name of the variable. This doesn't really add to readability and newer
code doesn't do it so make the entire file consistent.
While at it: convert 'unsigned' to 'unsigned int'.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
extra_checks is only used in a few places. It also depends on
a non-standard DEBUG define one needs to add to the source file. The
overhead of removing it should be minimal (we already use pure
might_sleep() in the code anyway) so drop it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
cdev is the only user of the debounce_period_us field in
struct gpio_desc, and it no longer uses it, so remove it.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
The global list of GPIO devices is never modified or accessed from
atomic context so it's fine to protect it using a mutex. Add a new
global lock dedicated to the gpio_devices list and use it whenever
accessing or modifying it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
Rename two functions that read or modify the global GPIO device list but
don't take the lock themselves (and need to be called with it already
acquired). Use the _unlocked() suffix which seems to be used quite
consistently across the kernel despite there also existing the _locked()
suffix for the same purpose.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
|
|
We will eventually switch to protecting the GPIO descriptors with a mutex
but until then, we need to allocate memory for the label copy atomically
while we're holding the global spinlock.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-gpio/62588146-eed6-42f7-ba26-160226b109fe@moroto.mountain/T/#u
Fixes: f8d05e276b45 ("gpiolib: remove gpiochip_is_requested()")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
gpio: remove gpiochip_is_requested()
- provide a safer alternative to gpiochip_is_requested()
- convert all existing users
- remove gpiochip_is_requested()
|
|
We have no external users of gpiochip_is_requested(). Let's remove it
and replace its internal calls with direct testing of the REQUESTED flag.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
gpiochip_is_requested() not only has a misleading name but it returns
a pointer to a string that is freed when the descriptor is released.
Provide a new helper meant to replace it, which returns a copy of the
label string instead.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Provide a getter for the GPIO device label string so that users don't
have to dereference struct gpio_chip directly.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
Now that pinctrl_gpio_set_config() is no longer used, let's drop the
'_new' suffix from its improved variant.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Now that pinctrl_gpio_free()() is no longer used, let's drop the '_new'
suffix from its improved variant.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Now that pinctrl_gpio_request() is no longer used, let's drop the '_new'
suffix from its improved variant.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Replace the pinctrl helpers taking the global GPIO number as argument
with the improved variants that instead take a pointer to the GPIO chip
and the controller-relative offset.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
With all users of gpiochip_find() converted to using gpio_device_find(),
we can now remove this function from the kernel.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Let's start adding getters for the opaque struct gpio_device. Start with
a function allowing to retrieve the base GPIO number.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Accessing struct gpio_chip backing a GPIO device is only allowed for the
actual providers of that chip.
Similarly to how we introduced gpio_device_find() in order to replace
the abused gpiochip_find(), let's introduce a counterpart to
gpiod_to_chip() that returns a reference to the GPIO device owning the
descriptor. This is done in order to later remove gpiod_to_chip()
entirely.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Peter Rosin <peda@axentia.se>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
There are users in the kernel who need to retrieve the address of the
struct device backing the GPIO device. Currently they needlessly poke in
the internals of GPIOLIB. Add a dedicated getter function.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Peter Rosin <peda@axentia.se>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
|
|
One of the ways of looking up GPIO devices is using their fwnode.
Provide a helper for that to avoid every user implementing their
own matching function.
Reviewed-by: Dipen Patel <dipenp@nvidia.com>
Tested-by: Dipen Patel <dipenp@nvidia.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20231010151709.4104747-2-andriy.shevchenko@linux.intel.com
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
|
|
Remove all remaining uses of find_chip_by_name() (and subsequently:
gpiochip_find()) from gpiolib.c and use the new
gpio_device_find_by_label() instead.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
The process of converting all unauthorized users of struct gpio_chip to
using dedicated struct gpio_device function will be long so in the
meantime we must provide a way of retrieving the pointer to struct
gpio_chip from a GPIO device.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
Getting the GPIO descriptor directly from the gpio_chip struct is
dangerous as we don't take the reference to the underlying GPIO device.
In order to start working towards removing gpiochip_get_desc(), let's
provide a safer variant that works with an existing reference to struct
gpio_device.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
|
By far the most common way of looking up GPIO devices is using their
label. Provide a helpers for that to avoid every user implementing their
own matching function.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
|
