On Wed, Feb 21, 2024 at 2:44 PM Linus Walleij <linus.walleij@xxxxxxxxxx> 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/20240220133950.138452-1-herve.codina@xxxxxxxxxxx/ -Saravana > > On Tue, Feb 20, 2024 at 12:11 PM Herve Codina <herve.codina@xxxxxxxxxxx> 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 <herve.codina@xxxxxxxxxxx> > > --- > > 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