On 08/09/20 04:12PM, Heinrich Schuchardt wrote:
> On 08.09.20 07:40, Pratyush Yadav wrote:
> > From: Jean-Jacques Hiblot <jjhib...@ti.com>
> >
> > Add managed functions to get a gpio from the devce-tree, based on a
> > property name (minus the '-gpios' suffix) and optionally an index.
> >
> > When the device is unbound, the GPIO is automatically released and the
> > data structure is freed.
> >
> > Signed-off-by: Jean-Jacques Hiblot <jjhib...@ti.com>
> > Reviewed-by: Simon Glass <s...@chromium.org>
> > Signed-off-by: Pratyush Yadav <p.ya...@ti.com>
> > ---
> > drivers/gpio/gpio-uclass.c | 71 ++++++++++++++++++++++++++++++++++++++
> > include/asm-generic/gpio.h | 47 +++++++++++++++++++++++++
> > 2 files changed, 118 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> > index 9c53299b6a..0c01413b58 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -6,6 +6,8 @@
> > #include <common.h>
> > #include <dm.h>
> > #include <log.h>
> > +#include <dm/devres.h>
> > +#include <dm/device_compat.h>
> > #include <dm/device-internal.h>
> > #include <dm/lists.h>
> > #include <dm/uclass-internal.h>
> > @@ -1209,6 +1211,75 @@ int gpio_dev_request_index(struct udevice *dev,
> > const char *nodename,
> > flags, 0, dev);
> > }
> >
> > +static void devm_gpiod_release(struct udevice *dev, void *res)
> > +{
> > + dm_gpio_free(dev, res);
> > +}
> > +
> > +static int devm_gpiod_match(struct udevice *dev, void *res, void *data)
> > +{
> > + return res == data;
> > +}
> > +
> > +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
> > + unsigned int index, int flags)
> > +{
> > + int rc;
> > + struct gpio_desc *desc;
> > + char *propname;
> > + static const char suffix[] = "-gpios";
> > +
> > + propname = malloc(strlen(id) + sizeof(suffix));
> > + if (!propname) {
> > + rc = -ENOMEM;
> > + goto end;
> > + }
> > +
> > + strcpy(propname, id);
> > + strcat(propname, suffix);
> > +
> > + desc = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc),
> > + __GFP_ZERO);
> > + if (unlikely(!desc)) {
> > + rc = -ENOMEM;
> > + goto end;
> > + }
> > +
> > + rc = gpio_request_by_name(dev, propname, index, desc, flags);
> > +
> > +end:
> > + if (propname)
> > + free(propname);
> > +
> > + if (rc)
> > + return ERR_PTR(rc);
> > +
> > + devres_add(dev, desc);
> > +
> > + return desc;
> > +}
> > +
> > +struct gpio_desc *devm_gpiod_get_index_optional(struct udevice *dev,
> > + const char *id,
> > + unsigned int index,
> > + int flags)
> > +{
> > + struct gpio_desc *desc = devm_gpiod_get_index(dev, id, index, flags);
> > +
> > + if (IS_ERR(desc))
> > + return NULL;
> > +
> > + return desc;
> > +}
> > +
> > +void devm_gpiod_put(struct udevice *dev, struct gpio_desc *desc)
> > +{
> > + int rc;
> > +
> > + rc = devres_release(dev, devm_gpiod_release, devm_gpiod_match, desc);
> > + WARN_ON(rc);
> > +}
> > +
> > static int gpio_post_bind(struct udevice *dev)
> > {
> > struct udevice *child;
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index a57dd2665c..ad6979a8ce 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -701,4 +701,51 @@ int gpio_get_number(const struct gpio_desc *desc);
> > */
> > int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio);
> >
> > +/**
> > + * devm_gpiod_get_index - Resource-managed gpiod_get()
> > + * @dev: GPIO consumer
> > + * @con_id: function within the GPIO consumer
> > + * @index: index of the GPIO to obtain in the consumer
> > + * @flags: optional GPIO initialization flags
> > + *
> > + * Managed gpiod_get(). GPIO descriptors returned from this function are
> > + * automatically disposed on driver detach.
>
> You pass in a device but write "driver detach". Shouldn't the object be
> "device" in the description as in your commit message?
Yes, it should be device.
> Devices have methods remove() and unbind() but no detach. In the commit
> message you speak of unbind(). Please, don't use alternative language.
Ok.
> Please, include the API in the HTML documentation created by 'make
> htmldocs'.
I tried searching for the GPIO API documentation under doc/ but I can't
find anything. README.gpio doesn't mention it anywhere and doc/api/ has
no file for gpio. Where do I document the newly added APIs then?
> Why did you choose the unbind() and not the remove() event for releasing
> the GPIOs?
I did not. Whoever added the devres API did (git blames Masahiro
Yamada). device_unbind() calls devres_release_all() so as a consequence
GPIO is released when the device is unbound.
> Best regards
>
> Heinrich
>
> > + * Return the GPIO descriptor corresponding to the function con_id of
> > device
> > + * dev, -ENOENT if no GPIO has been assigned to the requested function, or
> > + * another IS_ERR() code if an error occurred while trying to acquire the
> > GPIO.
> > + */
> > +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
> > + unsigned int index, int flags);
> > +
> > +#define devm_gpiod_get(dev, id, flags) devm_gpiod_get_index(dev, id, 0,
> > flags)
> > +/**
> > + * gpiod_get_optional - obtain an optional GPIO for a given GPIO function
> > + * @dev: GPIO consumer, can be NULL for system-global GPIOs
> > + * @con_id: function within the GPIO consumer
> > + * @index: index of the GPIO to obtain in the consumer
> > + * @flags: optional GPIO initialization flags
> > + *
> > + * This is equivalent to devm_gpiod_get(), except that when no GPIO was
> > + * assigned to the requested function it will return NULL. This is
> > convenient
> > + * for drivers that need to handle optional GPIOs.
> > + */
> > +struct gpio_desc *devm_gpiod_get_index_optional(struct udevice *dev,
> > + const char *id,
> > + unsigned int index,
> > + int flags);
> > +
> > +#define devm_gpiod_get_optional(dev, id, flags) \
> > + devm_gpiod_get_index_optional(dev, id, 0, flags)
> > +
> > +/**
> > + * devm_gpiod_put - Resource-managed gpiod_put()
> > + * @dev: GPIO consumer
> > + * @desc: GPIO descriptor to dispose of
> > + *
> > + * Dispose of a GPIO descriptor obtained with devm_gpiod_get() or
> > + * devm_gpiod_get_index(). Normally this function will not be called as
> > the GPIO
> > + * will be disposed of by the resource management code.
> > + */
> > +void devm_gpiod_put(struct udevice *dev, struct gpio_desc *desc);
> > +
> > #endif /* _ASM_GENERIC_GPIO_H_ */
> > --
> > 2.28.0
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments India
