From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pratyush Yadav Date: Tue, 8 Sep 2020 20:45:25 +0530 Subject: [PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree In-Reply-To: <6d10c9b7-8e91-3ece-80e1-0f1bf174faec@gmx.de> References: <20200908054011.18125-1-p.yadav@ti.com> <20200908054011.18125-2-p.yadav@ti.com> <6d10c9b7-8e91-3ece-80e1-0f1bf174faec@gmx.de> Message-ID: <20200908151523.bq5tbaxyuvs7shee@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/09/20 04:12PM, Heinrich Schuchardt wrote: > On 08.09.20 07:40, Pratyush Yadav wrote: > > From: Jean-Jacques Hiblot > > > > 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 > > Reviewed-by: Simon Glass > > Signed-off-by: Pratyush Yadav > > --- > > 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 > > #include > > #include > > +#include > > +#include > > #include > > #include > > #include > > @@ -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