[PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Sep 8 17:26:45 CEST 2020
Am 8. September 2020 17:15:25 MESZ schrieb Pratyush Yadav <p.yadav at ti.com>:
>On 08/09/20 04:12PM, Heinrich Schuchardt wrote:
>> On 08.09.20 07:40, Pratyush Yadav wrote:
>> > From: Jean-Jacques Hiblot <jjhiblot at 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 <jjhiblot at ti.com>
>> > Reviewed-by: Simon Glass <sjg at chromium.org>
>> > Signed-off-by: Pratyush Yadav <p.yadav at 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?
Please, add a new file doc/api/gpio.rst and include your include there. Add gpio.rst to doc/api/index.rst.
Check that make htmldocs does not show warnings.
The documentation will be generated in doc/output.
Best regards
Heinrich
>
>> 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
>> >
>>
More information about the U-Boot
mailing list