Am 8. September 2020 17:15:25 MESZ schrieb Pratyush Yadav <[email protected]>: >On 08/09/20 04:12PM, Heinrich Schuchardt wrote: >> On 08.09.20 07:40, Pratyush Yadav wrote: >> > From: Jean-Jacques Hiblot <[email protected]> >> > >> > 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 <[email protected]> >> > Reviewed-by: Simon Glass <[email protected]> >> > Signed-off-by: Pratyush Yadav <[email protected]> >> > --- >> > 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 >> > >>
