| Message ID | 20231011130204.52265-3-brgl@bgdev.pl |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | i2c: mux: don't access GPIOLIB internal structures | expand |
Hi! 2023-10-11 at 15:02, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@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> Cheers, Peter
On Wed, Oct 11, 2023 at 4:02 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@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. My concern with this API is the following scenario: 1. One driver requests the GPIO descriptor. 2. Another driver does take an arbitrary number, converts to a descriptor and calls for this API. Is there any (potential) problem?
On Wed, Oct 11, 2023 at 4:02 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: ... > +struct gpio_device *gpiod_to_gpio_device(struct gpio_desc *desc) > +{ > + if (!desc) > + return NULL; > + > + return desc->gdev; Wondering if we may use (part of) the validate_desc() (in a form of VALIDATE_DESC_VOID() macro call). > +}
On Wed, 11 Oct 2023 at 17:23, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Wed, Oct 11, 2023 at 4:02 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > From: Bartosz Golaszewski <bartosz.golaszewski@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. > > My concern with this API is the following scenario: > 1. One driver requests the GPIO descriptor. > 2. Another driver does take an arbitrary number, converts to a > descriptor and calls for this API. > > Is there any (potential) problem? YES! And I have it already on my TODO list! But it's great to know I'm not the only one seeing it. Basically we need to The end-goal should be to make gpio_to_desc() an internal GPIOLIB symbol. There are still around 10 users outside drivers/gpio/ that will need to be addressed in one way or another. Preferably by being converted to using descriptors. Bart > > -- > With Best Regards, > Andy Shevchenko
On Wed, Oct 11, 2023 at 5:39 PM Bartosz Golaszewski <bartosz.golaszewski@linaro.org> wrote: > The end-goal should be to make gpio_to_desc() an internal GPIOLIB > symbol. There are still around 10 users outside drivers/gpio/ that > will need to be addressed in one way or another. Preferably by being > converted to using descriptors. Conversely desc_to_gpio() as well, here is one fix for the current merge window (yeah I keep churning away at this...) https://lore.kernel.org/alsa-devel/20230926-descriptors-asoc-ti-v1-2-60cf4f8adbc5@linaro.org/ Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index ca2b5b424284..1e0ed6f5bcd5 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -220,6 +220,27 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc) } EXPORT_SYMBOL_GPL(gpiod_to_chip); +/** + * gpiod_to_gpio_device() - Return the GPIO device to which this descriptor + * belongs. + * @desc: Descriptor for which to return the GPIO device. + * + * This *DOES NOT* increase the reference count of the GPIO device as it's + * expected that the descriptor is requested and the users already holds a + * reference to the device. + * + * Returns: + * Address of the GPIO device owning this descriptor. + */ +struct gpio_device *gpiod_to_gpio_device(struct gpio_desc *desc) +{ + if (!desc) + return NULL; + + return desc->gdev; +} +EXPORT_SYMBOL_GPL(gpiod_to_gpio_device); + /** * gpio_device_get_chip() - Get the gpio_chip implementation of this GPIO device * @gdev: GPIO device diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 0484bf90b25d..7a8725be1225 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -784,6 +784,7 @@ int gpiochip_lock_as_irq(struct gpio_chip *gc, unsigned int offset); void gpiochip_unlock_as_irq(struct gpio_chip *gc, unsigned int offset); struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc); +struct gpio_device *gpiod_to_gpio_device(struct gpio_desc *desc); #else /* CONFIG_GPIOLIB */