Received-SPF: pass (google.com: domain of linux-kernel+bounces-75624-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240220111111.133826-1-herve.codina@bootlin.com>
 <20240220111111.133826-2-herve.codina@bootlin.com> <CACRpkdYWiwR_QBsiCESPYfQSsoiThn6hZyLAJA3u0bzTUNvBYQ@mail.gmail.com>
In-Reply-To: <CACRpkdYWiwR_QBsiCESPYfQSsoiThn6hZyLAJA3u0bzTUNvBYQ@mail.gmail.com>
From: Saravana Kannan <saravanak@google.com>
Date: Wed, 21 Feb 2024 14:58:44 -0800
Message-ID: <CAGETcx9zWsZcWteGDcKA-H67JCwJCsLPHWb0PH28T62M7ZLo2Q@mail.gmail.com>
Subject: Re: [PATCH 1/2] gpiolib: Introduce gpiod_device_add_link()
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Herve Codina <herve.codina@bootlin.com>, Bartosz Golaszewski <brgl@bgdev.pl>, Pavel Machek <pavel@ucw.cz>, 
	Lee Jones <lee@kernel.org>, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-leds@vger.kernel.org, Luca Ceresoli <luca.ceresoli@bootlin.com>, 
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 21, 2024 at 2:44=E2=80=AFPM Linus Walleij <linus.walleij@linaro=
org> 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 thre=
ad.

https://lore.kernel.org/all/20240220133950.138452-1-herve.codina@bootlin.co=
m/

-Saravana
>
> On Tue, Feb 20, 2024 at 12:11=E2=80=AFPM Herve Codina <herve.codina@bootl=
in.com> 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() ca=
n
> > be related to an of-node parent of the of-node used for the already don=
e
> > 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 addition=
al
> > devlinks, introduce gpiod_device_add_link().
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  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 GP=
IO.
> > + * @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 additiona=
l
> > + * 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 =3D device_link_add(consumer, desc->gdev->dev.parent, flag=
s);
> > +       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/consume=
r.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 gpi=
o_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 sort=
ing out
> the dependencies in the device tree e.g.
>
> Yours,
> Linus Walleij