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

Patrick DELAUNAY patrick.delaunay at st.com
Tue Jun 4 11:49:06 UTC 2019


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 at 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 at 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


More information about the U-Boot mailing list