recently added gpio hog patch was "in discussion" state with Simon Glass. This patch now adds most of comments from Simon Glass.
Signed-off-by: Heiko Schocher hs@denx.de
--- clean travis build, see result: https://travis-ci.org/hsdenx/u-boot-test/builds/558774593
Based on mainline: 6070ef409c - Merge branch '2019-07-12-master-imports'
Tom applied version 4 of the patchset, but patch is in discussion with Simon and I prepared already a v5
This patch includes now comments from Simon: - add example for line-name in Documentation - drop .gpio-hog suffix in Documentation as not longer needed. - rework gpio_hog_lookup_name() which now returns error code and pointer to gpio desc is passed. - rename DM_GPIO_HOG to GPIO_HOG - set device name only if line-name property is set - remove dm_gpio_set_dir() call as this is already done through gpio_dev_request_index() - check some error codes - tried to add a comment to gpio_dev_request_index()) and renamed function paramter "dev" to "gpio_dev" - use IS_ENABLED() - fix comments for gpio_dev_request_index() - add warning if probing one gpio_hog fails and continue with probing the next gpio hog.
not yet implemented: - question if gpio_desc can be stored in private device data, but this can be fixed later.
common/board_r.c | 4 +- doc/device-tree-bindings/gpio/gpio.txt | 17 ++-- drivers/gpio/Kconfig | 2 +- drivers/gpio/gpio-uclass.c | 103 ++++++++++++++++++------- include/asm-generic/gpio.h | 12 +-- 5 files changed, 96 insertions(+), 42 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index abc31b17b8..e7ce11c30a 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -49,7 +49,7 @@ #include <linux/err.h> #include <efi_loader.h> #include <wdt.h> -#if defined(CONFIG_DM_GPIO_HOG) +#if defined(CONFIG_GPIO_HOG) #include <asm/gpio.h> #endif
@@ -799,7 +799,7 @@ static init_fnc_t init_sequence_r[] = { #ifdef CONFIG_CMD_NET initr_ethaddr, #endif -#if defined(CONFIG_DM_GPIO_HOG) +#if defined(CONFIG_GPIO_HOG) gpio_hog_probe_all, #endif #ifdef CONFIG_BOARD_LATE_INIT diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt index e774439369..e146917ff3 100644 --- a/doc/device-tree-bindings/gpio/gpio.txt +++ b/doc/device-tree-bindings/gpio/gpio.txt @@ -252,6 +252,7 @@ Example: boot_rescue { gpio-hog; input; + line-name = "foo-bar-gpio"; gpios = <7 GPIO_ACTIVE_LOW>; }; }; @@ -259,9 +260,13 @@ Example: For the above Example you can than access the gpio in your boardcode with:
- desc = gpio_hog_lookup_name("boot_rescue.gpio-hog"); - if (desc) { - if (dm_gpio_get_value(desc)) - printf("\nBooting into Rescue System\n"); - else - printf("\nBoot normal\n"); + struct gpio_desc *desc; + int ret; + + ret = gpio_hog_lookup_name("boot_rescue", &desc); + if (ret) + return; + if (dm_gpio_get_value(desc) == 1) + printf("\nBooting into Rescue System\n"); + else if (dm_gpio_get_value(desc) == 0) + printf("\nBoot normal\n"); diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index fa1c99700f..3f1a24013c 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -14,7 +14,7 @@ config DM_GPIO particular GPIOs that they provide. The uclass interface is defined in include/asm-generic/gpio.h.
-config DM_GPIO_HOG +config GPIO_HOG bool "Enable GPIO hog support" depends on DM_GPIO default n diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 308d0863ad..01cfa2f788 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -144,7 +144,7 @@ static int gpio_find_and_xlate(struct gpio_desc *desc, return gpio_xlate_offs_flags(desc->dev, desc, args); }
-#if defined(CONFIG_DM_GPIO_HOG) +#if defined(CONFIG_GPIO_HOG)
struct gpio_hog_priv { struct gpio_desc gpiod; @@ -181,9 +181,8 @@ static int gpio_hog_ofdata_to_platdata(struct udevice *dev) return ret; } nodename = dev_read_string(dev, "line-name"); - if (!nodename) - nodename = dev_read_name(dev); - device_set_name(dev, nodename); + if (nodename) + device_set_name(dev, nodename);
return 0; } @@ -202,9 +201,15 @@ static int gpio_hog_probe(struct udevice *dev) dev->name); return ret; } - dm_gpio_set_dir(&priv->gpiod); - if (plat->gpiod_flags == GPIOD_IS_OUT) - dm_gpio_set_value(&priv->gpiod, plat->value); + + if (plat->gpiod_flags == GPIOD_IS_OUT) { + ret = dm_gpio_set_value(&priv->gpiod, plat->value); + if (ret < 0) { + debug("%s: node %s could not set gpio.\n", __func__, + dev->name); + return ret; + } + }
return 0; } @@ -213,32 +218,38 @@ int gpio_hog_probe_all(void) { struct udevice *dev; int ret; + int retval = 0;
for (uclass_first_device(UCLASS_NOP, &dev); dev; uclass_find_next_device(&dev)) { if (dev->driver == DM_GET_DRIVER(gpio_hog)) { ret = device_probe(dev); - if (ret) - return ret; + if (ret) { + printf("Failed to probe device %s err: %d\n", + dev->name, ret); + retval = ret; + } } }
- return 0; + return retval; }
-struct gpio_desc *gpio_hog_lookup_name(const char *name) +int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc) { struct udevice *dev;
+ *desc = NULL; gpio_hog_probe_all(); if (!uclass_get_device_by_name(UCLASS_NOP, name, &dev)) { struct gpio_hog_priv *priv = dev_get_priv(dev);
- return &priv->gpiod; + *desc = &priv->gpiod; + return 0; }
- return NULL; + return -ENODEV; }
U_BOOT_DRIVER(gpio_hog) = { @@ -250,9 +261,9 @@ U_BOOT_DRIVER(gpio_hog) = { .platdata_auto_alloc_size = sizeof(struct gpio_hog_data), }; #else -struct gpio_desc *gpio_hog_lookup_name(const char *name) +int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc) { - return NULL; + return 0; } #endif
@@ -755,13 +766,45 @@ int dm_gpio_get_values_as_int(const struct gpio_desc *desc_list, int count) return vector; }
+/** + * gpio_request_tail: common work for requesting a gpio. + * + * ret: return value from previous work in function which calls + * this function. + * This seems bogus (why calling this function instead not + * calling it and end caller function instead?). + * Because on error in caller function we want to set some + * default values in gpio desc and have a common error + * debug message, which provides this function. + * nodename: Name of node for which gpio gets requested + * used for gpio label name. + * args: pointer to output arguments structure + * list_name: Name of GPIO list + * used for gpio label name. + * index: gpio index in gpio list + * used for gpio label name. + * desc: pointer to gpio descriptor, filled from this + * function. + * flags: gpio flags to use. + * add_index: should index added to gpio label name + * gpio_dev: pointer to gpio device from which the gpio + * will be requested. If NULL try to get the + * gpio device with uclass_get_device_by_ofnode() + * + * return: In error case this function sets default values in + * gpio descriptor, also emmits a debug message. + * On success it returns 0 else the error code from + * function calls, or the error code passed through + * ret to this function. + * + */ static int gpio_request_tail(int ret, const char *nodename, struct ofnode_phandle_args *args, const char *list_name, int index, struct gpio_desc *desc, int flags, - bool add_index, struct udevice *dev) + bool add_index, struct udevice *gpio_dev) { - desc->dev = dev; + desc->dev = gpio_dev; desc->offset = 0; desc->flags = 0; if (ret) @@ -771,7 +814,8 @@ static int gpio_request_tail(int ret, const char *nodename, ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node, &desc->dev); if (ret) { - debug("%s: uclass_get_device_by_ofnode failed\n", __func__); + debug("%s: uclass_get_device_by_ofnode failed\n", + __func__); goto err; } } @@ -989,10 +1033,8 @@ int gpio_dev_request_index(struct udevice *dev, const char *nodename,
static int gpio_post_bind(struct udevice *dev) { -#if defined(CONFIG_DM_GPIO_HOG) struct udevice *child; ofnode node; -#endif
#if defined(CONFIG_NEEDS_MANUAL_RELOC) struct dm_gpio_ops *ops = (struct dm_gpio_ops *)device_get_ops(dev); @@ -1024,16 +1066,21 @@ static int gpio_post_bind(struct udevice *dev) } #endif
-#if defined(CONFIG_DM_GPIO_HOG) - dev_for_each_subnode(node, dev) { - if (ofnode_read_bool(node, "gpio-hog")) { - const char *name = ofnode_get_name(node); - - device_bind_driver_to_node(dev, "gpio_hog", name, - node, &child); + if (IS_ENABLED(CONFIG_GPIO_HOG)) { + dev_for_each_subnode(node, dev) { + if (ofnode_read_bool(node, "gpio-hog")) { + const char *name = ofnode_get_name(node); + int ret; + + ret = device_bind_driver_to_node(dev, + "gpio_hog", + name, node, + &child); + if (ret) + return ret; + } } } -#endif return 0; }
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index 37f71e5708..d6cf18744f 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -352,9 +352,10 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc); * gpio_hog_lookup_name() - Look up a named GPIO and return the gpio descr. * * @name: Name to look up - * @return: Returns gpio_desc for gpio + * @desc: Returns GPIO description, on success, else NULL + * @return: Returns 0 if OK, else -ENODEV */ -struct gpio_desc *gpio_hog_lookup_name(const char *name); +int gpio_hog_lookup_name(const char *name, struct gpio_desc **desc);
/** * gpio_hog_probe_all() - probe all gpio devices with @@ -523,12 +524,13 @@ int gpio_request_list_by_name_nodev(ofnode node, const char *list_name, * gpio_dev_request_index() - request single GPIO from gpio device * * @dev: GPIO device - * @nodename: Name of node + * @nodename: Name of node for which gpio gets requested, used + * for the gpio label name * @list_name: Name of GPIO list (e.g. "board-id-gpios") * @index: Index number of the GPIO in that list use request (0=first) * @flags: GPIOD_* flags - * @dtflags: GPIO flags read from DT - * @desc: GPIO descriotor filled from this function + * @dtflags: GPIO flags read from DT defined see GPIOD_* + * @desc: returns GPIO descriptor filled from this function * @return: return value from gpio_request_tail() */ int gpio_dev_request_index(struct udevice *dev, const char *nodename,