Re: [PATCH 1/2] gpiolib: Introduce gpiod_device_add_link()

[Saravana Kannan]

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/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