[PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree

[Heinrich Schuchardt]

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?

Devices have methods remove() and unbind() but no detach. In the commit
message you speak of unbind(). Please, don't use alternative language.

Please, include the API in the HTML documentation created by 'make
htmldocs'.

Why did you choose the unbind() and not the remove() event for releasing
the GPIOs?

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
>