Hi,
When a gpio used by the leds-gpio device is removed, the leds-gpio
device continues to use this gpio. Also, when the gpio is back, the
leds-gpio still uses the old removed gpio.
A consumer/supplier relationship is missing between the leds-gpio device
(consumer) and the gpio used (supplier).
This series adds an addionnal devlink between this two device.
With this link when the gpio is removed, the leds-gpio device is also
removed.
Best regards,
Hervé Codina
Herve Codina (2):
gpiolib: Introduce gpiod_device_add_link()
leds: gpio: Add devlinks between the gpio consumed and the gpio leds
device
drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
drivers/leds/leds-gpio.c | 15 +++++++++++++++
include/linux/gpio/consumer.h | 5 +++++
3 files changed, 52 insertions(+)
--
2.43.0
With device-tree, some devlink related to gpios are automatically added
when a consumer device is added and the attached node has phandles
related to GPIOs.
In some cases, the real device used to get the gpio during a probe() can
be related to an of-node parent of the of-node used for the already done
automatically devlink creation.
For instance, a driver can be bound to a device and, during the
probe(), the driver can walk its of-node children to get the GPIO
described in these children nodes.
In that case, an additional devlink between the device attached to the
driver and the gpio consumer need to be created.
Indeed, if the GPIO is removed, the consumer/supplier dependency should
lead to remove first the consuming driver before removing the supplier.
In order to give the possibility to this kind of driver to add additional
devlinks, introduce gpiod_device_add_link().
Signed-off-by: Herve Codina <[email protected]>
---
drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
include/linux/gpio/consumer.h | 5 +++++
2 files changed, 37 insertions(+)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8b3a0f45b574..416ab334b02d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4195,6 +4195,38 @@ static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
return desc;
}
+/**
+ * gpiod_device_add_link - Add a link between a GPIO consumer and a GPIO.
+ * @consumer: GPIO consumer.
+ * @desc: GPIO consumed.
+ * @flags: Link flags, see device_link_add().
+ *
+ * This function can be used for drivers that need to add an additional
+ * consumer/supplier device link to a GPIO.
+ *
+ * Returns:
+ * On successful, the link created.
+ * NULL if the link was not created due to a missing GPIO parent.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct device_link *gpiod_device_add_link(struct device *consumer,
+ struct gpio_desc *desc,
+ u32 flags)
+{
+ struct device_link *link;
+
+ if (!desc->gdev->dev.parent)
+ return NULL;
+
+ link = device_link_add(consumer, desc->gdev->dev.parent, flags);
+ if (!link)
+ return ERR_PTR(-EINVAL);
+
+ return link;
+}
+EXPORT_SYMBOL_GPL(gpiod_device_add_link);
+
/**
* fwnode_gpiod_get_index - obtain a GPIO from firmware node
* @fwnode: handle of the firmware node
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index db2dfbae8edb..4feed4e166b0 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -7,6 +7,7 @@
struct acpi_device;
struct device;
+struct device_link;
struct fwnode_handle;
struct gpio_array;
@@ -106,6 +107,10 @@ void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
+struct device_link *gpiod_device_add_link(struct device *consumer,
+ struct gpio_desc *desc,
+ u32 flags);
+
int gpiod_get_direction(struct gpio_desc *desc);
int gpiod_direction_input(struct gpio_desc *desc);
int gpiod_direction_output(struct gpio_desc *desc, int value);
--
2.43.0
On Tue, Feb 20, 2024 at 12:11 PM Herve Codina <herve.codina@bootlincom> wrote:
>
> Hi,
>
> When a gpio used by the leds-gpio device is removed, the leds-gpio
> device continues to use this gpio. Also, when the gpio is back, the
> leds-gpio still uses the old removed gpio.
>
> A consumer/supplier relationship is missing between the leds-gpio device
> (consumer) and the gpio used (supplier).
>
> This series adds an addionnal devlink between this two device.
> With this link when the gpio is removed, the leds-gpio device is also
> removed.
>
> Best regards,
> Hervé Codina
>
> Herve Codina (2):
> gpiolib: Introduce gpiod_device_add_link()
> leds: gpio: Add devlinks between the gpio consumed and the gpio leds
> device
>
> drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
> drivers/leds/leds-gpio.c | 15 +++++++++++++++
> include/linux/gpio/consumer.h | 5 +++++
> 3 files changed, 52 insertions(+)
>
> --
> 2.43.0
>
You should probably Cc Saravana on anything devlink related.
Bart
Hi Bartosz,
On Tue, 20 Feb 2024 12:40:17 +0100
Bartosz Golaszewski <[email protected]> wrote:
> On Tue, Feb 20, 2024 at 12:11 PM Herve Codina <[email protected]> wrote:
> >
> > Hi,
> >
> > When a gpio used by the leds-gpio device is removed, the leds-gpio
> > device continues to use this gpio. Also, when the gpio is back, the
> > leds-gpio still uses the old removed gpio.
> >
> > A consumer/supplier relationship is missing between the leds-gpio device
> > (consumer) and the gpio used (supplier).
> >
> > This series adds an addionnal devlink between this two device.
> > With this link when the gpio is removed, the leds-gpio device is also
> > removed.
> >
> > Best regards,
> > Hervé Codina
> >
> > Herve Codina (2):
> > gpiolib: Introduce gpiod_device_add_link()
> > leds: gpio: Add devlinks between the gpio consumed and the gpio leds
> > device
> >
> > drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
> > drivers/leds/leds-gpio.c | 15 +++++++++++++++
> > include/linux/gpio/consumer.h | 5 +++++
> > 3 files changed, 52 insertions(+)
> >
> > --
> > 2.43.0
> >
>
> You should probably Cc Saravana on anything devlink related.
>
Sure, I am going to resend the whole series adding Saravana.
Regards,
Hervé
Looping in Saravana, he should always look at patches like this.
On Tue, Feb 20, 2024 at 12:11 PM Herve Codina <herve.codina@bootlincom> wrote:
>
> With device-tree, some devlink related to gpios are automatically added
> when a consumer device is added and the attached node has phandles
> related to GPIOs.
> In some cases, the real device used to get the gpio during a probe() can
> be related to an of-node parent of the of-node used for the already done
> automatically devlink creation.
> For instance, a driver can be bound to a device and, during the
> probe(), the driver can walk its of-node children to get the GPIO
> described in these children nodes.
> In that case, an additional devlink between the device attached to the
> driver and the gpio consumer need to be created.
> Indeed, if the GPIO is removed, the consumer/supplier dependency should
> lead to remove first the consuming driver before removing the supplier.
>
> In order to give the possibility to this kind of driver to add additional
> devlinks, introduce gpiod_device_add_link().
>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
> include/linux/gpio/consumer.h | 5 +++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 8b3a0f45b574..416ab334b02d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4195,6 +4195,38 @@ static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> return desc;
> }
>
> +/**
> + * gpiod_device_add_link - Add a link between a GPIO consumer and a GPIO.
> + * @consumer: GPIO consumer.
> + * @desc: GPIO consumed.
> + * @flags: Link flags, see device_link_add().
> + *
> + * This function can be used for drivers that need to add an additional
> + * consumer/supplier device link to a GPIO.
> + *
> + * Returns:
> + * On successful, the link created.
> + * NULL if the link was not created due to a missing GPIO parent.
> + *
> + * In case of error an ERR_PTR() is returned.
> + */
> +struct device_link *gpiod_device_add_link(struct device *consumer,
> + struct gpio_desc *desc,
> + u32 flags)
> +{
> + struct device_link *link;
> +
> + if (!desc->gdev->dev.parent)
> + return NULL;
> +
> + link = device_link_add(consumer, desc->gdev->dev.parent, flags);
> + if (!link)
> + return ERR_PTR(-EINVAL);
> +
> + return link;
> +}
> +EXPORT_SYMBOL_GPL(gpiod_device_add_link);
> +
> /**
> * fwnode_gpiod_get_index - obtain a GPIO from firmware node
> * @fwnode: handle of the firmware node
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index db2dfbae8edb..4feed4e166b0 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -7,6 +7,7 @@
>
> struct acpi_device;
> struct device;
> +struct device_link;
> struct fwnode_handle;
>
> struct gpio_array;
> @@ -106,6 +107,10 @@ void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
> void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
> void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
>
> +struct device_link *gpiod_device_add_link(struct device *consumer,
> + struct gpio_desc *desc,
> + u32 flags);
> +
> int gpiod_get_direction(struct gpio_desc *desc);
> int gpiod_direction_input(struct gpio_desc *desc);
> int gpiod_direction_output(struct gpio_desc *desc, int value);
The function as such is pretty straight forward, but the cross call
here happens on instatiated
devices etc, and we need to know why this can't be done earlier when sorting out
the dependencies in the device tree e.g.
Yours,
Linus Walleij
On Wed, Feb 21, 2024 at 2:44 PM Linus Walleij <linus.walleij@linaroorg> wrote:
>
> Looping in Saravana, he should always look at patches like this.
Thanks Linus. Bartosz suggested a RESEND with me and I replied in that thread.
https://lore.kernel.org/all/[email protected]/
-Saravana
>
> On Tue, Feb 20, 2024 at 12:11 PM Herve Codina <[email protected]> wrote:
> >
> > With device-tree, some devlink related to gpios are automatically added
> > when a consumer device is added and the attached node has phandles
> > related to GPIOs.
> > In some cases, the real device used to get the gpio during a probe() can
> > be related to an of-node parent of the of-node used for the already done
> > automatically devlink creation.
> > For instance, a driver can be bound to a device and, during the
> > probe(), the driver can walk its of-node children to get the GPIO
> > described in these children nodes.
> > In that case, an additional devlink between the device attached to the
> > driver and the gpio consumer need to be created.
> > Indeed, if the GPIO is removed, the consumer/supplier dependency should
> > lead to remove first the consuming driver before removing the supplier.
> >
> > In order to give the possibility to this kind of driver to add additional
> > devlinks, introduce gpiod_device_add_link().
> >
> > Signed-off-by: Herve Codina <[email protected]>
> > ---
> > drivers/gpio/gpiolib.c | 32 ++++++++++++++++++++++++++++++++
> > include/linux/gpio/consumer.h | 5 +++++
> > 2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 8b3a0f45b574..416ab334b02d 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -4195,6 +4195,38 @@ static struct gpio_desc *gpiod_find_and_request(struct device *consumer,
> > return desc;
> > }
> >
> > +/**
> > + * gpiod_device_add_link - Add a link between a GPIO consumer and a GPIO.
> > + * @consumer: GPIO consumer.
> > + * @desc: GPIO consumed.
> > + * @flags: Link flags, see device_link_add().
> > + *
> > + * This function can be used for drivers that need to add an additional
> > + * consumer/supplier device link to a GPIO.
> > + *
> > + * Returns:
> > + * On successful, the link created.
> > + * NULL if the link was not created due to a missing GPIO parent.
> > + *
> > + * In case of error an ERR_PTR() is returned.
> > + */
> > +struct device_link *gpiod_device_add_link(struct device *consumer,
> > + struct gpio_desc *desc,
> > + u32 flags)
> > +{
> > + struct device_link *link;
> > +
> > + if (!desc->gdev->dev.parent)
> > + return NULL;
> > +
> > + link = device_link_add(consumer, desc->gdev->dev.parent, flags);
> > + if (!link)
> > + return ERR_PTR(-EINVAL);
> > +
> > + return link;
> > +}
> > +EXPORT_SYMBOL_GPL(gpiod_device_add_link);
> > +
> > /**
> > * fwnode_gpiod_get_index - obtain a GPIO from firmware node
> > * @fwnode: handle of the firmware node
> > diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> > index db2dfbae8edb..4feed4e166b0 100644
> > --- a/include/linux/gpio/consumer.h
> > +++ b/include/linux/gpio/consumer.h
> > @@ -7,6 +7,7 @@
> >
> > struct acpi_device;
> > struct device;
> > +struct device_link;
> > struct fwnode_handle;
> >
> > struct gpio_array;
> > @@ -106,6 +107,10 @@ void devm_gpiod_put(struct device *dev, struct gpio_desc *desc);
> > void devm_gpiod_unhinge(struct device *dev, struct gpio_desc *desc);
> > void devm_gpiod_put_array(struct device *dev, struct gpio_descs *descs);
> >
> > +struct device_link *gpiod_device_add_link(struct device *consumer,
> > + struct gpio_desc *desc,
> > + u32 flags);
> > +
> > int gpiod_get_direction(struct gpio_desc *desc);
> > int gpiod_direction_input(struct gpio_desc *desc);
> > int gpiod_direction_output(struct gpio_desc *desc, int value);
>
> The function as such is pretty straight forward, but the cross call
> here happens on instatiated
> devices etc, and we need to know why this can't be done earlier when sorting out
> the dependencies in the device tree e.g.
>
> Yours,
> Linus Walleij