Re: [U-Boot] [PATCH v3] gpio: add gpio-hog support

Hi Heiko,

add gpio-hog support. GPIO hogging is a mechanism providing automatic GPIO request and configuration as part of the gpio-controller's driver probe function.

for more infos see: doc/device-tree-bindings/gpio/gpio.txt

Signed-off-by: Heiko Schocher hs@denx.de

It seens the hog function can be called several times, for gpio bank probed in pre-reloc phasis: - before recocation : gpio_post_probe() => gpio_hog() .... gpio_hogs_probed = 0 - after relocation : gpio_post_probe() => gpio_hog()... gpio_hogs_probed = 0 gpio_hog_probe_all() => gpio_post_probe() => gpio_hog()... gpio_hogs_probed = 1 I don't known if it is expected behavior: hog configuration 2 times in Uboot = before and after relocation

---

clean travis build, see result: https://travis-ci.org/hsdenx/u-boot-test/builds/538734780

Changes in v3:

• probe now all gpio devices with gpio-hogs from board_r before board_late_init() call or if someone wants to access gpio-hog based gpios with gpio_hog_lookup_name() This is needed, because we cannot be sure, if a gpio device which contains gpio-hogs is probed.
• add line-name property as Michal recommended
• renamed gpio_priv_one into gpio_priv_one_hog and protected through CONFIG_DM_GPIO_HOG

Changes in v2:

• move gpio_hog() call from post_probe() to post_bind() call. Check if current gpio device has gpio-hog definitions, if so, probe it.

common/board_r.c | 6 + doc/device-tree-bindings/gpio/gpio.txt | 55 +++++++ drivers/gpio/Kconfig | 10 ++ drivers/gpio/gpio-uclass.c | 217 ++++++++++++++++++++++--- include/asm-generic/gpio.h | 32 ++++ 5 files changed, 301 insertions(+), 19 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c index 150e8cd424..507da4c74a 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -49,6 +49,9 @@ #include <linux/err.h> #include <efi_loader.h> #include <wdt.h> +#if defined(CONFIG_DM_GPIO_HOG) +#include <asm/gpio.h> +#endif

DECLARE_GLOBAL_DATA_PTR;

@@ -796,6 +799,9 @@ static init_fnc_t init_sequence_r[] = { #ifdef CONFIG_CMD_NET initr_ethaddr, #endif +#if defined(CONFIG_DM_GPIO_HOG)

• gpio_hog_probe_all,

+#endif #ifdef CONFIG_BOARD_LATE_INIT board_late_init, #endif diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree- bindings/gpio/gpio.txt index f7a158d858..e774439369 100644 --- a/doc/device-tree-bindings/gpio/gpio.txt +++ b/doc/device-tree-bindings/gpio/gpio.txt @@ -210,3 +210,58 @@ Example 2: Here, three GPIO ranges are defined wrt. two pin controllers. pinctrl1 GPIO ranges are defined using pin numbers whereas the GPIO ranges wrt. pinctrl2 are named "foo" and "bar".

+3) GPIO hog definitions +-----------------------

+The GPIO chip may contain GPIO hog definitions. GPIO hogging is a +mechanism providing automatic GPIO request and configuration as part of +the gpio-controller's driver probe function.

+Each GPIO hog definition is represented as a child node of the GPIO controller. +Required properties: +- gpio-hog: A property specifying that this child node represents a GPIO hog. +- gpios: Store the GPIO information (id, flags) for the GPIO to

•      affect.
  

•          ! Not yet support more than one gpio !
  

+Only one of the following properties scanned in the order shown below. +- input: A property specifying to set the GPIO direction as input. +- output-low A property specifying to set the GPIO direction as output with

•      the value low.
  

+- output-high A property specifying to set the GPIO direction as output with

•      the value high.
  

+Optional properties: +- line-name: The GPIO label name. If not present the node name is used.

+Example:

•    tca6416@20 {
  

•            compatible = "ti,tca6416";
  

•            reg = <0x20>;
  

•            #gpio-cells = <2>;
  

•            gpio-controller;
  

•            env_reset {
  

•                    gpio-hog;
  

•                    input;
  

•                    gpios = <6 GPIO_ACTIVE_LOW>;
  

•            };
  

•            boot_rescue {
  

•                    gpio-hog;
  

•                    input;
  

•                    gpios = <7 GPIO_ACTIVE_LOW>;
  

•            };
  

•    };
  

+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");
  

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e36a8abc42..fa1c99700f 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -14,6 +14,16 @@ config DM_GPIO particular GPIOs that they provide. The uclass interface is defined in include/asm-generic/gpio.h.

+config DM_GPIO_HOG

• bool "Enable GPIO hog support"
• depends on DM_GPIO
• default n
• help

•  Enable gpio hog support
  

•  The GPIO chip may contain GPIO hog definitions. GPIO hogging
  

•  is a mechanism providing automatic GPIO request and config-
  

•  uration as part of the gpio-controller's driver probe function.
  

config ALTERA_PIO bool "Altera PIO driver" depends on DM_GPIO diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index da5e9ba6e5..4ecff115f1 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -5,6 +5,7 @@

#include <common.h> #include <dm.h> +#include <dm/device-internal.h> #include <dt-bindings/gpio/gpio.h> #include <errno.h> #include <fdtdec.h> @@ -15,6 +16,18 @@

DECLARE_GLOBAL_DATA_PTR;

+#if defined(CONFIG_DM_GPIO_HOG) +struct gpio_priv_one_hog {

• struct list_head list;
• char *name;
• struct gpio_desc gpiod;

+};

+static struct list_head hogs; +static int list_init; +static int gpio_hogs_probed; +#endif

/**

• gpio_to_device() - Convert global GPIO number to device, number

@@ -141,6 +154,147 @@ 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) +int gpio_hog_probe_all(void) +{

• struct udevice *dev;
• ofnode node;
• int ret;
• /*

• * We need to probe all gpio-hog devices
  

• * at some point, as we cannot be sure,
  

• * that all gpio devices, which contain a
  

• * gpio-hog definition are probed.
  

• *
  

• */
  

• if (gpio_hogs_probed)

• return 0;
  

• for (uclass_first_device(UCLASS_GPIO, &dev);

•     dev;
  

•     uclass_next_device(&dev)) {
  

uclass_next_device automatically call probe for the class (in uclass_get_device_tail) in this context, I think that uclass_find_next_device is better

• dev_for_each_subnode(node, dev) {
  

• 	if (ofnode_read_bool(node, "gpio-hog")) {
  

• 		ret = device_probe(dev);
  

• 		if (ret)
  

• 			return ret;
  

• 		break;
  

• 	}
  

• }
  

• }
• gpio_hogs_probed = 1;
• return 0;

+}

+struct gpio_desc *gpio_hog_lookup_name(const char *name) {

• struct list_head *entry;
• struct gpio_priv_one_hog *cur;
• /* be sure, all gpio devices with gpio-hogs are probed */
• gpio_hog_probe_all();
• list_for_each(entry, &hogs) {

• cur = list_entry(entry, struct gpio_priv_one_hog, list);
  

• if (strcmp(cur->name, name) == 0)
  

• 	return &cur->gpiod;
  

• }
• return NULL;

+}

+static int gpio_hog(struct udevice *dev) {

• ofnode node;
• int found = 0;
• int ret;
• struct gpio_dev_priv *uc_priv = NULL;
• if (!list_init) {

• INIT_LIST_HEAD(&hogs);
  

• list_init = 1;
  

• }
• dev_for_each_subnode(node, dev) {

• if (ofnode_read_bool(node, "gpio-hog")) {
  

• 	found = 1;
  

• 	break;
  

• }
  

• }

This check is really needed, as it is already done in the main loop ? I think you can remove this first 'found' loop.

• if (!found)

• return 0;
  

• uc_priv = dev_get_uclass_priv(dev);
• if (!uc_priv) {

• printf("%s: missing private data.\n", __func__);
  

• return 0;
  

• }

It is prossible to have uc_priv = 0 ? by construction, is is not allowed in driver mode, so this test can be remove I think.

• /* scan for gpio-hog subnodes */
• dev_for_each_subnode(node, dev) {

• u32 val[2];
  

• int value = 0;
  

• int gpiod_flags;
  

• struct gpio_priv_one_hog *new;
  

• const char *nodename;
  

• if (!ofnode_read_bool(node, "gpio-hog"))
  

• 	continue;
  

• if (ofnode_read_bool(node, "input")) {
  

• 	gpiod_flags = GPIOD_IS_IN;
  

• } else if (ofnode_read_bool(node, "output-high")) {
  

• 	value = 1;
  

• 	gpiod_flags = GPIOD_IS_OUT;
  

• } else if (ofnode_read_bool(node, "output-low")) {
  

• 	gpiod_flags = GPIOD_IS_OUT;
  

• } else {
  

• 	printf("%s: missing gpio-hog state.\n", __func__);
  

• 	return -EINVAL;
  

• }
  

• ret = ofnode_read_u32_array(node, "gpios", val, 2);
  

• if (ret) {
  

• 	printf("%s: wrong gpios property, 2 values needed, ret:
  

%d\n", __func__, ret);

• 	return ret;
  

• }
  

• new = calloc(1, sizeof(struct gpio_priv_one_hog));
  

• nodename = ofnode_read_string(node, "line-name");
  

• if (!nodename)
  

• 	nodename = ofnode_get_name(node);
  

• ret = gpio_dev_request_index(dev, nodename, "gpio-hog",
  

• 			     val[0], gpiod_flags, val[1],
  

• 			     &new->gpiod);
  

• if (ret < 0) {
  

• 	debug("%s: node %s could not get gpio.\n", __func__,
  

• 	      ofnode_get_name(node));
  

• 	free(new);
  

• 	return ret;
  

• }
  

• new->name = uc_priv->name[new->gpiod.offset];
  

• list_add_tail(&new->list, &hogs);
  

• dm_gpio_set_dir(&new->gpiod);
  

• if (gpiod_flags == GPIOD_IS_OUT)
  

• 	dm_gpio_set_value(&new->gpiod, value);
  

• }
• return 0;

+} +#else +static int gpio_hog(struct udevice *dev) {

• return 0;

+}

+struct gpio_desc *gpio_hog_lookup_name(const char *name) {

• return NULL;

+} +#endif

int dm_gpio_request(struct gpio_desc *desc, const char *label) { struct udevice *dev = desc->dev; @@ -149,8 +303,9 @@ int dm_gpio_request(struct gpio_desc *desc, const char *label) int ret;

uc_priv = dev_get_uclass_priv(dev);

• if (uc_priv->name[desc->offset])

• return -EBUSY;
  

• if (uc_priv)

• if (uc_priv->name[desc->offset])
  

• 	return -EBUSY;
  

  str = strdup(label); if (!str) return -ENOMEM;

@@ -640,22 +795,25 @@ int dm_gpio_get_values_as_int(const struct gpio_desc *desc_list, int count) return vector; }

-static int gpio_request_tail(int ret, ofnode node, +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 gpio_desc *desc, int flags,
  

• 	     bool add_index, struct udevice *dev)
  

{

• desc->dev = NULL;

• desc->dev = dev; desc->offset = 0; desc->flags = 0; if (ret) goto err;

• ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node,

• 			  &desc->dev);
  

• if (ret) {

• debug("%s: uclass_get_device_by_ofnode failed\n", __func__);
  

• goto err;
  

• if (!desc->dev) {

• ret = uclass_get_device_by_ofnode(UCLASS_GPIO, args->node,
  

• 				  &desc->dev);
  

• if (ret) {
  

• 	debug("%s: uclass_get_device_by_ofnode failed\n",
  

__func__);

• 	goto err;
  

• }
  

  } ret = gpio_find_and_xlate(desc, args); if (ret) {

@@ -663,8 +821,7 @@ static int gpio_request_tail(int ret, ofnode node, goto err; } ret = dm_gpio_requestf(desc, add_index ? "%s.%s%d" : "%s.%s",

• 	       ofnode_get_name(node),
  

• 	       list_name, index);
  

• 	       nodename, list_name, index);
  

  if (ret) { debug("%s: dm_gpio_requestf failed\n", __func__); goto err;

@@ -678,7 +835,7 @@ static int gpio_request_tail(int ret, ofnode node, return 0; err: debug("%s: Node '%s', property '%s', failed to request GPIO index %d: %d\n",

•      __func__, ofnode_get_name(node), list_name, index, ret);
  

•      __func__, nodename, list_name, index, ret);
  

  return ret;

}

@@ -692,8 +849,8 @@ static int _gpio_request_by_name_nodev(ofnode node, const char *list_name, ret = ofnode_parse_phandle_with_args(node, list_name, "#gpio-cells", 0, index, &args);

• return gpio_request_tail(ret, node, &args, list_name, index, desc,

• 		 flags, add_index);
  

• return gpio_request_tail(ret, ofnode_get_name(node), &args, list_name,

• 		 index, desc, flags, add_index, NULL);
  

}

int gpio_request_by_name_nodev(ofnode node, const char *list_name, int index, @@ -707,13 +864,14 @@ int gpio_request_by_name(struct udevice *dev, const char *list_name, int index, struct gpio_desc *desc, int flags) { struct ofnode_phandle_args args;

• ofnode node; int ret;

  ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0, index, &args);

• return gpio_request_tail(ret, dev_ofnode(dev), &args, list_name,

• 		 index, desc, flags, index > 0);
  

• node = dev_ofnode(dev);
• return gpio_request_tail(ret, ofnode_get_name(node), &args, list_name,

• 		 index, desc, flags, index > 0, NULL);
  

}

int gpio_request_list_by_name_nodev(ofnode node, const char *list_name, @@ - 832,12 +990,17 @@ int gpio_get_number(const struct gpio_desc *desc) static int gpio_post_probe(struct udevice *dev) { struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);

• int ret;

  uc_priv->name = calloc(uc_priv->gpio_count, sizeof(char *)); if (!uc_priv->name) return -ENOMEM;

• return gpio_renumber(NULL);

• ret = gpio_renumber(NULL);
• if (ret)

• return ret;
  

It is strange for me to have many device tree parsing in probe function....

For me => hog detection + DT parsing in gpio_post_bind() .... info saved in platdata => gpio configuration for hog (request and set_dir) => probe function

• return gpio_hog(dev);

}

One other solution to do it is to manage a hog with uclass (using the new UCLASS_NOP or misc for example) http://patchwork.ozlabs.org/patch/1098913/ Ps: we can also create a new UCLASS_GPIOHOG

And we to bind this class during GPIO binding; the parsing of DT is done only one time and manage privdata and platdata as expected by driver model....

For example something like (not tested)

static int gpio_post_bind(struct udevice *dev) { struct udevice *child; ofnode node; .....

#if defined(CONFIG_DM_GPIO_HOG) dev_for_each_subnode(node, dev) { if (ofnode_read_bool(node, "gpio-hog")) { const char *name = fdt_get_name(fdt, node, NULL); device_bind_driver_to_node (dev, "gpio_hog", name, dev_ofnode(dev), &child); } } #endif return 0; }

#if defined(CONFIG_DM_GPIO_HOG)

struct gpio_hog_priv{ struct gpio_desc gpiod; }

struct gpio_hog_data { int gpiod_flags; int value; u32 val[2]; }

static int gpio_hog_ofdata_to_platdata(struct udevice *dev) { struct gpio_hog_data *plat = dev_get_platdata(dev); char *nodename;

plat->value = 0; if (dev_read_bool(dev, "input")) { plat->gpiod_flags = GPIOD_IS_IN; } else if (dev_read_bool(dev, "output-high")) { plat->value = 1; gpiod_flags = GPIOD_IS_OUT; } else if (dev_read_bool(dev, "output-low")) { gpiod_flags = GPIOD_IS_OUT; } else { printf("%s: missing gpio-hog state.\n", __func__); return -EINVAL; } ret = dev_read_u32_array(dev, "gpios", plat->val, 2); if (ret) { printf("%s: wrong gpios property, 2 values needed, ret: %d\n", __func__, ret); return ret; } nodename = dev_read_string(node, "line-name"); if (!nodename) nodename = dev_read_name(node); device_set_name(dev, nodename); }

static int gpio_hog_probe(struct udevice *dev) { struct gpio_hog_data *plat = dev_get_platdata(dev); struct gpio_hog_priv *priv = dev_get_privdata(dev);

ret = gpio_dev_request_index(dev, dev->name, "gpio-hog", plat->val[0], plat->gpiod_flags, plat->val[1], &priv->gpiod); if (ret < 0) { debug("%s: node %s could not get gpio.\n", __func__, ofnode_get_name(node)); return ret; } dm_gpio_set_dir_flags(&priv->gpiod); if (gpiod_flags == GPIOD_IS_OUT) dm_gpio_set_value(&priv->gpiod, plat->value); }

int gpio_hog_probe_all(void) { struct udevice *dev; ofnode node;

for (uclass_first_device(UCLASS_NOP, &dev); dev; uclass_find_next_device (&dev)) { if (dev->driver == M_GET_DRIVER(gpio_hog)) { ret = device_probe(dev); if (ret) ' return ret; break; } } } }

struct gpio_desc *gpio_hog_lookup_name(const char *name) { struct udevice *dev;

/* be sure, all gpio devices with gpio-hogs are probed */ gpio_hog_probe_all(); if (!uclass_get_device_by_name(UCLASS_NOP, name, &dev)) ' struct gpio_hog_priv *priv = dev_get_privdata(dev); return &priv->gpiod; }

return NULL; }

U_BOOT_DRIVER(gio_hog) = { .name = "gpio_hog", .id = UCLASS_NOP, .ofdata_to_platdata = gpio_hog_ofdata_to_platdata, .probe = gpio_hog_probe, .priv_auto_alloc_size = sizeof(struct gpio_hog_priv), .platdata_auto_alloc_size = sizeof(struct gpio_hog_data), } #endif

static int gpio_pre_remove(struct udevice *dev) @@ -854,6 +1017,21 @@ static int gpio_pre_remove(struct udevice *dev) return gpio_renumber(dev); }

+int gpio_dev_request_index(struct udevice *dev, const char *nodename,

• 	   char *list_name, int index, int flags,
  

• 	   int dtflags, struct gpio_desc *desc) {
  

• struct ofnode_phandle_args args;
• args.node = ofnode_null();
• args.args_count = 2;
• args.args[0] = index;
• args.args[1] = dtflags;
• return gpio_request_tail(0, nodename, &args, list_name, index, desc,

• 		 flags, 0, dev);
  

+}

static int gpio_post_bind(struct udevice *dev) { #if defined(CONFIG_NEEDS_MANUAL_RELOC) @@ -885,6 +1063,7 @@ static int gpio_post_bind(struct udevice *dev) reloc_done++; } #endif

• return 0;

}

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h index d03602696f..37f71e5708 100644 --- a/include/asm-generic/gpio.h +++ b/include/asm-generic/gpio.h @@ -348,6 +348,22 @@ const char *gpio_get_bank_info(struct udevice *dev, int *offset_count); */ 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
• */

+struct gpio_desc *gpio_hog_lookup_name(const char *name);

+/**

• • gpio_hog_probe_all() - probe all gpio devices with
• • gpio-hog subnodes.
• • @return: Returns return value from device_probe()
• */

+int gpio_hog_probe_all(void);

/**

• gpio_lookup_name - Look up a GPIO name and return its details

@@ -503,6 +519,22 @@ int gpio_request_list_by_name_nodev(ofnode node, const char *list_name, struct gpio_desc *desc_list, int max_count, int flags);

+/**

• • gpio_dev_request_index() - request single GPIO from gpio device
• • @dev: GPIO device
• • @nodename: Name of node
• • @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
• • @return: return value from gpio_request_tail()
• */

+int gpio_dev_request_index(struct udevice *dev, const char *nodename,

• 	   char *list_name, int index, int flags,
  

• 	   int dtflags, struct gpio_desc *desc);
  

/**

• dm_gpio_free() - Free a single GPIO

-- 2.21.0

Regards

Patrick