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

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

[Heiko Schocher]

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>

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

Changes in v4:
- rework as suggested by Patrick
  based on patch:
  http://patchwork.ozlabs.org/patch/1098913/
  use new UCLASS_NOP
  hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
  gpio configuration for hog (request and set_dir) => probe function

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             | 181 ++++++++++++++++++++++---
 include/asm-generic/gpio.h             |  32 +++++
 5 files changed, 268 insertions(+), 16 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..308d0863ad 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -5,6 +5,9 @@
 
 #include <common.h>
 #include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/uclass-internal.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -141,6 +144,118 @@ 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)
+
+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);
+	const char *nodename;
+	int ret;
+
+	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;
+		plat->gpiod_flags = GPIOD_IS_OUT;
+	} else if (dev_read_bool(dev, "output-low")) {
+		plat->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 %d\n",
+		       __func__, ret);
+		return ret;
+	}
+	nodename = dev_read_string(dev, "line-name");
+	if (!nodename)
+		nodename = dev_read_name(dev);
+	device_set_name(dev, nodename);
+
+	return 0;
+}
+
+static int gpio_hog_probe(struct udevice *dev)
+{
+	struct gpio_hog_data *plat = dev_get_platdata(dev);
+	struct gpio_hog_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = gpio_dev_request_index(dev->parent, 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__,
+		      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);
+
+	return 0;
+}
+
+int gpio_hog_probe_all(void)
+{
+	struct udevice *dev;
+	int ret;
+
+	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;
+		}
+	}
+
+	return 0;
+}
+
+struct gpio_desc *gpio_hog_lookup_name(const char *name)
+{
+	struct udevice *dev;
+
+	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;
+	}
+
+	return NULL;
+}
+
+U_BOOT_DRIVER(gpio_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),
+};
+#else
+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;
@@ -640,22 +755,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 +781,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 +795,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 +809,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 +824,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,
@@ -854,8 +972,28 @@ 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_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);
 	static int reloc_done;
@@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
 		reloc_done++;
 	}
 #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);
+		}
+	}
+#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

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

[Michal Simek]

On 12. 06. 19 6:11, Heiko Schocher wrote:
> 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>
> 
> ---
> clean travis build, see result:
> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> 
> Changes in v4:
> - rework as suggested by Patrick
>   based on patch:
>   http://patchwork.ozlabs.org/patch/1098913/
>   use new UCLASS_NOP
>   hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>   gpio configuration for hog (request and set_dir) => probe function
> 
> 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             | 181 ++++++++++++++++++++++---
>  include/asm-generic/gpio.h             |  32 +++++
>  5 files changed, 268 insertions(+), 16 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..308d0863ad 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -5,6 +5,9 @@
>  
>  #include <common.h>
>  #include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/uclass-internal.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <errno.h>
>  #include <fdtdec.h>
> @@ -141,6 +144,118 @@ 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)
> +
> +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);
> +	const char *nodename;
> +	int ret;
> +
> +	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;
> +		plat->gpiod_flags = GPIOD_IS_OUT;
> +	} else if (dev_read_bool(dev, "output-low")) {
> +		plat->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 %d\n",
> +		       __func__, ret);
> +		return ret;
> +	}
> +	nodename = dev_read_string(dev, "line-name");
> +	if (!nodename)
> +		nodename = dev_read_name(dev);
> +	device_set_name(dev, nodename);
> +
> +	return 0;
> +}
> +
> +static int gpio_hog_probe(struct udevice *dev)
> +{
> +	struct gpio_hog_data *plat = dev_get_platdata(dev);
> +	struct gpio_hog_priv *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = gpio_dev_request_index(dev->parent, 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__,
> +		      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);
> +
> +	return 0;
> +}
> +
> +int gpio_hog_probe_all(void)
> +{
> +	struct udevice *dev;
> +	int ret;
> +
> +	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;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +struct gpio_desc *gpio_hog_lookup_name(const char *name)
> +{
> +	struct udevice *dev;
> +
> +	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;
> +	}
> +
> +	return NULL;
> +}
> +
> +U_BOOT_DRIVER(gpio_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),
> +};
> +#else
> +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;
> @@ -640,22 +755,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 +781,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 +795,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 +809,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 +824,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,
> @@ -854,8 +972,28 @@ 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_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);
>  	static int reloc_done;
> @@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
>  		reloc_done++;
>  	}
>  #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);
> +		}
> +	}
> +#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
>   *
> 

Tested-by: Michal Simek <michal.simek@xilinx.com> (zcu102)

Thanks,
Michal

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

[Patrick DELAUNAY]

Hi Mickael

> Subject: Re: [PATCH v4] gpio: add gpio-hog support
> Importance: High
> 
> On 12. 06. 19 6:11, Heiko Schocher wrote:
> > 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>
> >
> > ---
> > clean travis build, see result:
> > https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> >
> > Changes in v4:
> > - rework as suggested by Patrick
> >   based on patch:
> >   http://patchwork.ozlabs.org/patch/1098913/
> >   use new UCLASS_NOP
> >   hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
> >   gpio configuration for hog (request and set_dir) => probe function
> >
> > 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             | 181 ++++++++++++++++++++++---
> >  include/asm-generic/gpio.h             |  32 +++++
> >  5 files changed, 268 insertions(+), 16 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

Any reason to have hog at this place ? 
Before board_late_init....

I expect more before board_init() for my future use-case, 
to replace the current (dirty)  implementation with pinctrl-0

#if defined(CONFIG_WDT)
	initr_watchdog,
#endif
+#if defined(CONFIG_DM_GPIO_HOG)
+	gpio_hog_probe_all,
+#endif
#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
	defined(CONFIG_SANDBOX)
	board_init,	/* Setup chipselects */
#endif


In my device tree I have :

	stmfx: stmfx at 42 {
		compatible = "st,stmfx-0300";
		reg = <0x42>;
		interrupts = <8 IRQ_TYPE_EDGE_RISING>;
		interrupt-parent = <&gpioi>;
		vdd-supply = <&v3v3>;

		stmfx_pinctrl: stmfx-pin-controller {
			compatible = "st,stmfx-0300-pinctrl";
			gpio-controller;
			#gpio-cells = <2>;
			interrupt-controller;
			#interrupt-cells = <2>;
			gpio-ranges = <&stmfx_pinctrl 0 0 24>;
			pinctrl-names = "default";
			pinctrl-0 = <&hog_pins>;

			hog_pins: hog {
				pins = "gpio14";
				drive-push-pull;
				bias-pull-down;
			};
	};


And in board, I force probe to configure HOG....before to initialize the DISPLAY, so in 

./board/st/stm32mp1/stm32mp1.c : board _init()

/* board dependent setup after realloc */
int board_init(void)
{
.....
	/* probe all PINCTRL for hog */
	for (uclass_first_device(UCLASS_PINCTRL, &dev);
	     dev;
	     uclass_next_device(&dev)) {
		pr_debug("probe pincontrol = %s\n", dev->name);
	}


Or you can let each board choose when call the function
as it is already done for other function as regulators_enable_boot_on()
So  don't add it in init_sequence_r[]  / board_r.c ?...

Anyway it is not blocking for me, it just a question.

> >  #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..308d0863ad 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -5,6 +5,9 @@
> >
> >  #include <common.h>
> >  #include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include <dm/lists.h>
> > +#include <dm/uclass-internal.h>
> >  #include <dt-bindings/gpio/gpio.h>
> >  #include <errno.h>
> >  #include <fdtdec.h>
> > @@ -141,6 +144,118 @@ 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)
> > +
> > +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);
> > +	const char *nodename;
> > +	int ret;
> > +
> > +	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;
> > +		plat->gpiod_flags = GPIOD_IS_OUT;
> > +	} else if (dev_read_bool(dev, "output-low")) {
> > +		plat->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 %d\n",
> > +		       __func__, ret);
> > +		return ret;
> > +	}
> > +	nodename = dev_read_string(dev, "line-name");
> > +	if (!nodename)
> > +		nodename = dev_read_name(dev);
> > +	device_set_name(dev, nodename);
> > +
> > +	return 0;
> > +}
> > +
> > +static int gpio_hog_probe(struct udevice *dev) {
> > +	struct gpio_hog_data *plat = dev_get_platdata(dev);
> > +	struct gpio_hog_priv *priv = dev_get_priv(dev);
> > +	int ret;
> > +
> > +	ret = gpio_dev_request_index(dev->parent, 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__,
> > +		      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);
> > +
> > +	return 0;
> > +}
> > +
> > +int gpio_hog_probe_all(void)
> > +{
> > +	struct udevice *dev;
> > +	int ret;
> > +
> > +	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;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +struct gpio_desc *gpio_hog_lookup_name(const char *name) {
> > +	struct udevice *dev;
> > +
> > +	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;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +U_BOOT_DRIVER(gpio_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), }; #else
> > +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;
> > @@ -640,22 +755,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 +781,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 +795,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 +809,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 +824,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, @@ -854,8 +972,28 @@ 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_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);
> >  	static int reloc_done;
> > @@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
> >  		reloc_done++;
> >  	}
> >  #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);
> > +		}
> > +	}
> > +#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
> >   *
> >
> 
> Tested-by: Michal Simek <michal.simek@xilinx.com> (zcu102)
> 
> Thanks,
> Michal


For the patch Review and tested on stm32mp1 board.

Tested-by: Patrick Delaunay <patrick.delaunay@st.com>

Regards
PAtrick

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

[Heiko Schocher]

Hello Patrick,

Am 21.06.2019 um 18:09 schrieb Patrick DELAUNAY:
> Hi Mickael
> 
>> Subject: Re: [PATCH v4] gpio: add gpio-hog support
>> Importance: High
>>
>> On 12. 06. 19 6:11, Heiko Schocher wrote:
>>> 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>
>>>
>>> ---
>>> clean travis build, see result:
>>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>>>
>>> Changes in v4:
>>> - rework as suggested by Patrick
>>>    based on patch:
>>>    http://patchwork.ozlabs.org/patch/1098913/
>>>    use new UCLASS_NOP
>>>    hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>>>    gpio configuration for hog (request and set_dir) => probe function
>>>
>>> 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             | 181 ++++++++++++++++++++++---
>>>   include/asm-generic/gpio.h             |  32 +++++
>>>   5 files changed, 268 insertions(+), 16 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
> 
> Any reason to have hog at this place ?

Not really, but we need to check at some point if all gpio hogs
are processed, and I thought at a late time it does not break
anything...

> Before board_late_init....
> 
> I expect more before board_init() for my future use-case,
> to replace the current (dirty)  implementation with pinctrl-0
> 
> #if defined(CONFIG_WDT)
> 	initr_watchdog,
> #endif
> +#if defined(CONFIG_DM_GPIO_HOG)
> +	gpio_hog_probe_all,
> +#endif
> #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
> 	defined(CONFIG_SANDBOX)
> 	board_init,	/* Setup chipselects */
> #endif
> 
> 
> In my device tree I have :
> 
> 	stmfx: stmfx at 42 {
> 		compatible = "st,stmfx-0300";
> 		reg = <0x42>;
> 		interrupts = <8 IRQ_TYPE_EDGE_RISING>;
> 		interrupt-parent = <&gpioi>;
> 		vdd-supply = <&v3v3>;
> 
> 		stmfx_pinctrl: stmfx-pin-controller {
> 			compatible = "st,stmfx-0300-pinctrl";
> 			gpio-controller;
> 			#gpio-cells = <2>;
> 			interrupt-controller;
> 			#interrupt-cells = <2>;
> 			gpio-ranges = <&stmfx_pinctrl 0 0 24>;
> 			pinctrl-names = "default";
> 			pinctrl-0 = <&hog_pins>;
> 
> 			hog_pins: hog {
> 				pins = "gpio14";
> 				drive-push-pull;
> 				bias-pull-down;
> 			};
> 	};
> 
> 
> And in board, I force probe to configure HOG....before to initialize the DISPLAY, so in
> 
> ./board/st/stm32mp1/stm32mp1.c : board _init()
> 
> /* board dependent setup after realloc */
> int board_init(void)
> {
> .....
> 	/* probe all PINCTRL for hog */
> 	for (uclass_first_device(UCLASS_PINCTRL, &dev);
> 	     dev;
> 	     uclass_next_device(&dev)) {
> 		pr_debug("probe pincontrol = %s\n", dev->name);
> 	}
> 
> 
> Or you can let each board choose when call the function
> as it is already done for other function as regulators_enable_boot_on()
> So  don't add it in init_sequence_r[]  / board_r.c ?...

Hmm... I do not like this, because if you enable gpio-hog support,
you don;t want to think to call gpio_hog_probe_all().

But it should be possible to call gpio_hog_probe_all() earlier from board
code, as gpio_hog_probe_all() calls device_probe():

  212 int gpio_hog_probe_all(void)
  213 {
  214         struct udevice *dev;
  215         int ret;
  216
  217         for (uclass_first_device(UCLASS_NOP, &dev);
  218              dev;
  219              uclass_find_next_device(&dev)) {
  220                 if (dev->driver == DM_GET_DRIVER(gpio_hog)) {
  221                         ret = device_probe(dev);


device_probe() checks if it is already probed, and simply returns if
so, see:

http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/core/device.c;h=0d15e5062b66123cd364bd9803e076db7e7dd97c;hb=77f6e2dd0551d8a825bab391a1bd6b838874bcd4#l319

May you can test if this works for you?

> 
> Anyway it is not blocking for me, it just a question.

Fine.

>>>   #ifdef CONFIG_BOARD_LATE_INIT
>>>   	board_late_init,
[...]
> 
> For the patch Review and tested on stm32mp1 board.
> 
> Tested-by: Patrick Delaunay <patrick.delaunay@st.com>

Thanks for testing!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Patrick DELAUNAY]

Hi Heiko,

> From: Heiko Schocher <hs@denx.de>
> Sent: samedi 22 juin 2019 07:32
> 
> Hello Patrick,
> 
> Am 21.06.2019 um 18:09 schrieb Patrick DELAUNAY:
> > Hi Mickael
> >
> >> Subject: Re: [PATCH v4] gpio: add gpio-hog support
> >> Importance: High
> >>
> >> On 12. 06. 19 6:11, Heiko Schocher wrote:
> >>> 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>
> >>>
> >>> ---
> >>> clean travis build, see result:
> >>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> >>>
> >>> Changes in v4:
> >>> - rework as suggested by Patrick
> >>>    based on patch:
> >>>    http://patchwork.ozlabs.org/patch/1098913/
> >>>    use new UCLASS_NOP
> >>>    hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
> >>>    gpio configuration for hog (request and set_dir) => probe
> >>> function
> >>>
> >>> 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             | 181 ++++++++++++++++++++++---
> >>>   include/asm-generic/gpio.h             |  32 +++++
> >>>   5 files changed, 268 insertions(+), 16 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
> >
> > Any reason to have hog at this place ?
> 
> Not really, but we need to check at some point if all gpio hogs are processed, and
> I thought at a late time it does not break anything...

Ok. It is safe.... 

and anyway any board can choose the call the function earlier if the hog was required 
(for nay initcall before board_late_init : initr_nand, initr_mmc ....)


> > Before board_late_init....
> >
> > I expect more before board_init() for my future use-case, to replace
> > the current (dirty)  implementation with pinctrl-0
> >
> > #if defined(CONFIG_WDT)
> > 	initr_watchdog,
> > #endif
> > +#if defined(CONFIG_DM_GPIO_HOG)
> > +	gpio_hog_probe_all,
> > +#endif
> > #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) ||
> defined(CONFIG_RISCV) || \
> > 	defined(CONFIG_SANDBOX)
> > 	board_init,	/* Setup chipselects */
> > #endif
> >
> >
> > In my device tree I have :
> >
> > 	stmfx: stmfx at 42 {
> > 		compatible = "st,stmfx-0300";
> > 		reg = <0x42>;
> > 		interrupts = <8 IRQ_TYPE_EDGE_RISING>;
> > 		interrupt-parent = <&gpioi>;
> > 		vdd-supply = <&v3v3>;
> >
> > 		stmfx_pinctrl: stmfx-pin-controller {
> > 			compatible = "st,stmfx-0300-pinctrl";
> > 			gpio-controller;
> > 			#gpio-cells = <2>;
> > 			interrupt-controller;
> > 			#interrupt-cells = <2>;
> > 			gpio-ranges = <&stmfx_pinctrl 0 0 24>;
> > 			pinctrl-names = "default";
> > 			pinctrl-0 = <&hog_pins>;
> >
> > 			hog_pins: hog {
> > 				pins = "gpio14";
> > 				drive-push-pull;
> > 				bias-pull-down;
> > 			};
> > 	};
> >
> >
> > And in board, I force probe to configure HOG....before to initialize
> > the DISPLAY, so in
> >
> > ./board/st/stm32mp1/stm32mp1.c : board _init()
> >
> > /* board dependent setup after realloc */ int board_init(void) { .....
> > 	/* probe all PINCTRL for hog */
> > 	for (uclass_first_device(UCLASS_PINCTRL, &dev);
> > 	     dev;
> > 	     uclass_next_device(&dev)) {
> > 		pr_debug("probe pincontrol = %s\n", dev->name);
> > 	}
> >
> >
> > Or you can let each board choose when call the function as it is
> > already done for other function as regulators_enable_boot_on() So
> > don't add it in init_sequence_r[]  / board_r.c ?...
> 
> Hmm... I do not like this, because if you enable gpio-hog support, you don;t want
> to think to call gpio_hog_probe_all().
> 
> But it should be possible to call gpio_hog_probe_all() earlier from board code, as
> gpio_hog_probe_all() calls device_probe():

Fine.

>   212 int gpio_hog_probe_all(void)
>   213 {
>   214         struct udevice *dev;
>   215         int ret;
>   216
>   217         for (uclass_first_device(UCLASS_NOP, &dev);
>   218              dev;
>   219              uclass_find_next_device(&dev)) {
>   220                 if (dev->driver == DM_GET_DRIVER(gpio_hog)) {
>   221                         ret = device_probe(dev);
> 
> 
> device_probe() checks if it is already probed, and simply returns if so, see:
> 
> http://git.denx.de/?p=u-
> boot.git;a=blob;f=drivers/core/device.c;h=0d15e5062b66123cd364bd9803e076db7
> e7dd97c;hb=77f6e2dd0551d8a825bab391a1bd6b838874bcd4#l319
> 
> May you can test if this works for you?

I have the same idea, it is why I say is was no't blocking for me.

Already tested and it is working (gpio hog is called only one time even if gpio_hog_probe_all several time).

> >
> > Anyway it is not blocking for me, it just a question.
> 
> Fine.
> 
> >>>   #ifdef CONFIG_BOARD_LATE_INIT
> >>>   	board_late_init,
> [...]
> >
> > For the patch Review and tested on stm32mp1 board.
> >
> > Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> 
> Thanks for testing!
> 
> bye,
> Heiko 

Regards
Patrick

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

[Simon Glass]

Hi Heiko,

On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
>
> 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>
>
> ---
> clean travis build, see result:
> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>
> Changes in v4:
> - rework as suggested by Patrick
>   based on patch:
>   http://patchwork.ozlabs.org/patch/1098913/
>   use new UCLASS_NOP
>   hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>   gpio configuration for hog (request and set_dir) => probe function


Sorry for not getting to this earlier.

>
>
> 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             | 181 ++++++++++++++++++++++---
>  include/asm-generic/gpio.h             |  32 +++++
>  5 files changed, 268 insertions(+), 16 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.

This seems a strange name. I suppose this is what linux called it.

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

Should have an example for line-name

> +                };
> +        };
> +
> +For the above Example you can than access the gpio in your boardcode
> +with:
> +
> +        desc = gpio_hog_lookup_name("boot_rescue.gpio-hog");

Could we drop the .gpio-hog suffix? It doesn't seem necessary, but
perhaps you are worried about people getting confused otherwise?

Also this function should be passed a desc struct pointer, not return
static data.

> +        if (desc) {
> +                if (dm_gpio_get_value(desc))

You should check the error here.

dm_gpio_get_value() returns -ve on error, including if desc returns an
invalid GPIO (dev=NULL), so you should be able to do:

gpio_hog_lookup_name("boot_rescue.gpio-hog", &desc);
if (dm_gpio_get_value(desc) == 1)

So in fact
> +                        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

Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
don't want to rename these sorts of options.

> +       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..308d0863ad 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -5,6 +5,9 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/uclass-internal.h>
>  #include <dt-bindings/gpio/gpio.h>
>  #include <errno.h>
>  #include <fdtdec.h>
> @@ -141,6 +144,118 @@ 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)
> +
> +struct gpio_hog_priv {
> +       struct gpio_desc gpiod;

As above I don't think you need this.

> +};
> +
> +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);
> +       const char *nodename;
> +       int ret;
> +
> +       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;
> +               plat->gpiod_flags = GPIOD_IS_OUT;
> +       } else if (dev_read_bool(dev, "output-low")) {
> +               plat->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 %d\n",
> +                      __func__, ret);
> +               return ret;
> +       }
> +       nodename = dev_read_string(dev, "line-name");
> +       if (!nodename)
> +               nodename = dev_read_name(dev);
> +       device_set_name(dev, nodename);

I don't think you should set the device name unless needed. But why
not store this in the device data, rather than changing its name? If
we want to change the name, why not just use that for the node name in
the .dts?

> +
> +       return 0;
> +}
> +
> +static int gpio_hog_probe(struct udevice *dev)
> +{
> +       struct gpio_hog_data *plat = dev_get_platdata(dev);
> +       struct gpio_hog_priv *priv = dev_get_priv(dev);
> +       int ret;
> +
> +       ret = gpio_dev_request_index(dev->parent, 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__,
> +                     dev->name);
> +               return ret;
> +       }
> +       dm_gpio_set_dir(&priv->gpiod);

Check error

> +       if (plat->gpiod_flags == GPIOD_IS_OUT)
> +               dm_gpio_set_value(&priv->gpiod, plat->value);

check error

> +
> +       return 0;
> +}
> +
> +int gpio_hog_probe_all(void)
> +{
> +       struct udevice *dev;
> +       int ret;
> +
> +       for (uclass_first_device(UCLASS_NOP, &dev);

Perhaps you should create a new uclass for this?

> +            dev;
> +            uclass_find_next_device(&dev)) {
> +               if (dev->driver == DM_GET_DRIVER(gpio_hog)) {
> +                       ret = device_probe(dev);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +struct gpio_desc *gpio_hog_lookup_name(const char *name)
> +{
> +       struct udevice *dev;
> +
> +       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;
> +       }
> +
> +       return NULL;
> +}
> +
> +U_BOOT_DRIVER(gpio_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),
> +};
> +#else
> +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;
> @@ -640,22 +755,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)

This function badly needs a comment now. Also 'dev' is not a great name.

>  {
> -       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 +781,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 +795,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 +809,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 +824,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,
> @@ -854,8 +972,28 @@ 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_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);
>         static int reloc_done;
> @@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
>                 reloc_done++;
>         }
>  #endif
> +
> +#if defined(CONFIG_DM_GPIO_HOG)

Can you use if (IS_ENABLED(CONFIG....

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

Check error? This might run out of memory, perhaps.

> +               }
> +       }
> +#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()

What if one fails and the others succeed?

> + */
> +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

What is a node? Can you expand this a bit?

> + * @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

Reference GPIOD_... mask here?

> + * @desc:      GPIO descriotor filled from this function

returns GPIO descriptor...

> + * @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,
Simon

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

[Heiko Schocher]

Hello Simon,

Am 22.06.2019 um 21:09 schrieb Simon Glass:
> Hi Heiko,
> 
> On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
>>
>> 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>
>>
>> ---
>> clean travis build, see result:
>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>>
>> Changes in v4:
>> - rework as suggested by Patrick
>>    based on patch:
>>    http://patchwork.ozlabs.org/patch/1098913/
>>    use new UCLASS_NOP
>>    hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>>    gpio configuration for hog (request and set_dir) => probe function
> 
> 
> Sorry for not getting to this earlier.

no problem, thanks for your time!

>> 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             | 181 ++++++++++++++++++++++---
>>   include/asm-generic/gpio.h             |  32 +++++
>>   5 files changed, 268 insertions(+), 16 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.
> 
> This seems a strange name. I suppose this is what linux called it.

Yes.

>> +
>> +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>;
> 
> Should have an example for line-name

Oh, yes of course, I added the "line-name" part later, and forgot the
example.
> 
>> +                };
>> +        };
>> +
>> +For the above Example you can than access the gpio in your boardcode
>> +with:
>> +
>> +        desc = gpio_hog_lookup_name("boot_rescue.gpio-hog");
> 
> Could we drop the .gpio-hog suffix? It doesn't seem necessary, but
> perhaps you are worried about people getting confused otherwise?

Good catch! Since v4 it is already without this suffix, so I correct
the doc here.

> Also this function should be passed a desc struct pointer, not return
> static data.
> 
>> +        if (desc) {
>> +                if (dm_gpio_get_value(desc))
> 
> You should check the error here.
> 
> dm_gpio_get_value() returns -ve on error, including if desc returns an
> invalid GPIO (dev=NULL), so you should be able to do:
> 
> gpio_hog_lookup_name("boot_rescue.gpio-hog", &desc);

Also we should check here the return value.

> if (dm_gpio_get_value(desc) == 1)

Good point. I rework this, thanks!

> So in fact
>> +                        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
> 
> Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
> don't want to rename these sorts of options.

Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
I can change this.

>> +       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..308d0863ad 100644
>> --- a/drivers/gpio/gpio-uclass.c
>> +++ b/drivers/gpio/gpio-uclass.c
>> @@ -5,6 +5,9 @@
>>
>>   #include <common.h>
>>   #include <dm.h>
>> +#include <dm/device-internal.h>
>> +#include <dm/lists.h>
>> +#include <dm/uclass-internal.h>
>>   #include <dt-bindings/gpio/gpio.h>
>>   #include <errno.h>
>>   #include <fdtdec.h>
>> @@ -141,6 +144,118 @@ 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)
>> +
>> +struct gpio_hog_priv {
>> +       struct gpio_desc gpiod;
> 
> As above I don't think you need this.

Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
for which I need to pass a pointer to struct gpio_desc ...

And later I want to have the possibility to access the gpio for example
with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
store it somewhere. Or do I overlook something obvious?

>> +};
>> +
>> +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);
>> +       const char *nodename;
>> +       int ret;
>> +
>> +       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;
>> +               plat->gpiod_flags = GPIOD_IS_OUT;
>> +       } else if (dev_read_bool(dev, "output-low")) {
>> +               plat->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 %d\n",
>> +                      __func__, ret);
>> +               return ret;
>> +       }
>> +       nodename = dev_read_string(dev, "line-name");
>> +       if (!nodename)
>> +               nodename = dev_read_name(dev);
>> +       device_set_name(dev, nodename);
> 
> I don't think you should set the device name unless needed. But why
> not store this in the device data, rather than changing its name? If
> we want to change the name, why not just use that for the node name in
> the .dts?

Because I use the device name for finding the gpio through gpio_hog_lookup_name().

Ok, if there is no line-name property, we do not need to set
the device name, as it has already the correct name, so this can be
changed to:

         nodename = dev_read_string(dev, "line-name");
         if (nodename)
                 device_set_name(dev, nodename);

>> +
>> +       return 0;
>> +}
>> +
>> +static int gpio_hog_probe(struct udevice *dev)
>> +{
>> +       struct gpio_hog_data *plat = dev_get_platdata(dev);
>> +       struct gpio_hog_priv *priv = dev_get_priv(dev);
>> +       int ret;
>> +
>> +       ret = gpio_dev_request_index(dev->parent, 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__,
>> +                     dev->name);
>> +               return ret;
>> +       }
>> +       dm_gpio_set_dir(&priv->gpiod);
> 
> Check error

Yes ... Hmm, while checking, this is not needed anymore, as
gpio_dev_request_index() calls gpio_request_tail() which calls
dm_gpio_set_dir(), so I remove this.

> 
>> +       if (plat->gpiod_flags == GPIOD_IS_OUT)
>> +               dm_gpio_set_value(&priv->gpiod, plat->value);
> 
> check error

done.

>> +
>> +       return 0;
>> +}
>> +
>> +int gpio_hog_probe_all(void)
>> +{
>> +       struct udevice *dev;
>> +       int ret;
>> +
>> +       for (uclass_first_device(UCLASS_NOP, &dev);
> 
> Perhaps you should create a new uclass for this?

Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
UCLASS_NOP is valid for this here to?

[1] http://patchwork.ozlabs.org/patch/1098913/

>> +            dev;
>> +            uclass_find_next_device(&dev)) {
>> +               if (dev->driver == DM_GET_DRIVER(gpio_hog)) {
>> +                       ret = device_probe(dev);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +struct gpio_desc *gpio_hog_lookup_name(const char *name)
>> +{
>> +       struct udevice *dev;
>> +
>> +       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;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>> +U_BOOT_DRIVER(gpio_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),
>> +};
>> +#else
>> +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;
>> @@ -640,22 +755,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)
> 
> This function badly needs a comment now. Also 'dev' is not a great name.

Before my patch I think, it needed a comment ;-)
I try to add one...

Hmm... the name "dev" is the same as in struct gpio_desc ... what is a good name?

parent? gpio_dev?

I tend to name it gpio_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 +781,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 +795,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 +809,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 +824,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,
>> @@ -854,8 +972,28 @@ 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_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);
>>          static int reloc_done;
>> @@ -885,6 +1023,17 @@ static int gpio_post_bind(struct udevice *dev)
>>                  reloc_done++;
>>          }
>>   #endif
>> +
>> +#if defined(CONFIG_DM_GPIO_HOG)
> 
> Can you use if (IS_ENABLED(CONFIG....

Yes, changed.

>> +       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);
> 
> Check error? This might run out of memory, perhaps.

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()
> 
> What if one fails and the others succeed?

Currently I return with error code from device_probe(). So the
rest of the gpio-hogs are not probed.

Hmm.. may it is better to emmit a debug message and continue?

>> + */
>> +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
> 
> What is a node? Can you expand this a bit?

changed to:

  * @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
> 
> Reference GPIOD_... mask here?

Yep, added.

>> + * @desc:      GPIO descriotor filled from this function
> 
> returns GPIO descriptor...

changed.

>> + * @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,
> Simon

Thanks for this review!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Simon Glass]

Hi Heiko,

On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <hs@denx.de> wrote:
>
> Hello Simon,
>
> Am 22.06.2019 um 21:09 schrieb Simon Glass:
> > Hi Heiko,
> >
> > On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
> >>
> >> 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>
> >>
> >> ---
> >> clean travis build, see result:
> >> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> >>
> >> Changes in v4:
> >> - rework as suggested by Patrick
> >>    based on patch:
> >>    http://patchwork.ozlabs.org/patch/1098913/
> >>    use new UCLASS_NOP
> >>    hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
> >>    gpio configuration for hog (request and set_dir) => probe function
> >
> >

[..]

> >> +config DM_GPIO_HOG
> >
> > Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
> > don't want to rename these sorts of options.
>
> Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
> DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
> I can change this.

Yes I understand that, but in general we hope one day to rename
DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
keep the DM_ suffix only for things that need it.

>
> >> +       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..308d0863ad 100644
> >> --- a/drivers/gpio/gpio-uclass.c
> >> +++ b/drivers/gpio/gpio-uclass.c
> >> @@ -5,6 +5,9 @@
> >>
> >>   #include <common.h>
> >>   #include <dm.h>
> >> +#include <dm/device-internal.h>
> >> +#include <dm/lists.h>
> >> +#include <dm/uclass-internal.h>
> >>   #include <dt-bindings/gpio/gpio.h>
> >>   #include <errno.h>
> >>   #include <fdtdec.h>
> >> @@ -141,6 +144,118 @@ 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)
> >> +
> >> +struct gpio_hog_priv {
> >> +       struct gpio_desc gpiod;
> >
> > As above I don't think you need this.
>
> Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
> for which I need to pass a pointer to struct gpio_desc ...

The caller can declare this in a local variable.

>
> And later I want to have the possibility to access the gpio for example
> with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
> store it somewhere. Or do I overlook something obvious?

In that case the caller should store it, perhaps in a local variable,
or perhaps in a private data structure if needed for a long time.

[..]

> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +int gpio_hog_probe_all(void)
> >> +{
> >> +       struct udevice *dev;
> >> +       int ret;
> >> +
> >> +       for (uclass_first_device(UCLASS_NOP, &dev);
> >
> > Perhaps you should create a new uclass for this?
>
> Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
> UCLASS_NOP is valid for this here to?
>
> [1] http://patchwork.ozlabs.org/patch/1098913/

I see your point and I can see arguments for each side. But the fact
that you have to check the driver suggests to me that a new uclass is
better.

[..]

> >> +/**
> >> + * gpio_hog_probe_all() - probe all gpio devices with
> >> + * gpio-hog subnodes.
> >> + *
> >> + * @return:    Returns return value from device_probe()
> >
> > What if one fails and the others succeed?
>
> Currently I return with error code from device_probe(). So the
> rest of the gpio-hogs are not probed.
>
> Hmm.. may it is better to emmit a debug message and continue?

Sounds reasonable. But it seems to me that if there is an error there
should be some visible report, so this could perhaps return the error
code? Then the caller could print a messag?

Regards,
SImon

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

[Heiko Schocher]

Hello Simon,

Am 26.06.2019 um 17:07 schrieb Simon Glass:
> Hi Heiko,
> 
> On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <hs@denx.de> wrote:
>>
>> Hello Simon,
>>
>> Am 22.06.2019 um 21:09 schrieb Simon Glass:
>>> Hi Heiko,
>>>
>>> On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
>>>>
>>>> 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>
>>>>
>>>> ---
>>>> clean travis build, see result:
>>>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>>>>
>>>> Changes in v4:
>>>> - rework as suggested by Patrick
>>>>     based on patch:
>>>>     http://patchwork.ozlabs.org/patch/1098913/
>>>>     use new UCLASS_NOP
>>>>     hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>>>>     gpio configuration for hog (request and set_dir) => probe function
>>>
>>>
> 
> [..]
> 
>>>> +config DM_GPIO_HOG
>>>
>>> Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
>>> don't want to rename these sorts of options.
>>
>> Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
>> DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
>> I can change this.
> 
> Yes I understand that, but in general we hope one day to rename
> DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
> keep the DM_ suffix only for things that need it.

Yep, already dropped, see temp version (travisbuild clean):
https://github.com/hsdenx/u-boot-test/commit/509bad758b8abc94dbbebc2a387b9e97c30b6543

>>
>>>> +       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..308d0863ad 100644
>>>> --- a/drivers/gpio/gpio-uclass.c
>>>> +++ b/drivers/gpio/gpio-uclass.c
>>>> @@ -5,6 +5,9 @@
>>>>
>>>>    #include <common.h>
>>>>    #include <dm.h>
>>>> +#include <dm/device-internal.h>
>>>> +#include <dm/lists.h>
>>>> +#include <dm/uclass-internal.h>
>>>>    #include <dt-bindings/gpio/gpio.h>
>>>>    #include <errno.h>
>>>>    #include <fdtdec.h>
>>>> @@ -141,6 +144,118 @@ 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)
>>>> +
>>>> +struct gpio_hog_priv {
>>>> +       struct gpio_desc gpiod;
>>>
>>> As above I don't think you need this.
>>
>> Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
>> for which I need to pass a pointer to struct gpio_desc ...
> 
> The caller can declare this in a local variable.

The caller is always the device probe function, as gpio_hog_probe_all()
loops over all gpio hog devices and probes them with device_probe()
which cannot return a gpio_desc pointer, nor can such a pointer passed to.

Why isn't it valid to store in each gpio hog device the gpio desc it
belongs too in device private data?

>> And later I want to have the possibility to access the gpio for example
>> with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
>> store it somewhere. Or do I overlook something obvious?
> 
> In that case the caller should store it, perhaps in a local variable,
> or perhaps in a private data structure if needed for a long time.
> 
> [..]

Of course I can introduce a list for each gpio hog definition filled
when calling gpio_hog_probe_all() ... but than I have a second list
which uses memory, may has to keep in sync the list if device are
removed ... I want to avoid such an approach ... as we have already a
list of gpio hog devices and the priv storage pointer for device private
data.

What do you think?

>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +int gpio_hog_probe_all(void)
>>>> +{
>>>> +       struct udevice *dev;
>>>> +       int ret;
>>>> +
>>>> +       for (uclass_first_device(UCLASS_NOP, &dev);
>>>
>>> Perhaps you should create a new uclass for this?
>>
>> Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
>> UCLASS_NOP is valid for this here to?
>>
>> [1] http://patchwork.ozlabs.org/patch/1098913/
> 
> I see your point and I can see arguments for each side. But the fact
> that you have to check the driver suggests to me that a new uclass is
> better.
> 
> [..]

Ok with this approach the gpio_hog additions to gpio-uclass can be
moved to a gpio-hog-uclass.c file. Also the problem where to store
the gpio descriptor moves from device private data to uclass private
data?

Is it worth?

>>>> +/**
>>>> + * gpio_hog_probe_all() - probe all gpio devices with
>>>> + * gpio-hog subnodes.
>>>> + *
>>>> + * @return:    Returns return value from device_probe()
>>>
>>> What if one fails and the others succeed?
>>
>> Currently I return with error code from device_probe(). So the
>> rest of the gpio-hogs are not probed.
>>
>> Hmm.. may it is better to emmit a debug message and continue?
> 
> Sounds reasonable. But it seems to me that if there is an error there
> should be some visible report, so this could perhaps return the error
> code? Then the caller could print a messag?

Ok, so I add a printf message. I like more to add it in
gpio_hog_probe_all(), because more than one gpio_hog can fail.

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Heiko Schocher]

Hello Simon,

gentle ping to my questions only ... hope I did not missed an
answer from you ...

Am 27.06.2019 um 06:12 schrieb Heiko Schocher:
> Hello Simon,
> 
> Am 26.06.2019 um 17:07 schrieb Simon Glass:
>> Hi Heiko,
>>
>> On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <hs@denx.de> wrote:
>>>
>>> Hello Simon,
>>>
>>> Am 22.06.2019 um 21:09 schrieb Simon Glass:
>>>> Hi Heiko,
>>>>
>>>> On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
>>>>>
>>>>> 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>
>>>>>
>>>>> ---
>>>>> clean travis build, see result:
>>>>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>>>>>
>>>>> Changes in v4:
>>>>> - rework as suggested by Patrick
>>>>>     based on patch:
>>>>>     http://patchwork.ozlabs.org/patch/1098913/
>>>>>     use new UCLASS_NOP
>>>>>     hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>>>>>     gpio configuration for hog (request and set_dir) => probe function
>>>>
>>>>
>>
>> [..]
>>
>>>>> +config DM_GPIO_HOG
>>>>
>>>> Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
>>>> don't want to rename these sorts of options.
>>>
>>> Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
>>> DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
>>> I can change this.
>>
>> Yes I understand that, but in general we hope one day to rename
>> DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
>> keep the DM_ suffix only for things that need it.
> 
> Yep, already dropped, see temp version (travisbuild clean):
> https://github.com/hsdenx/u-boot-test/commit/509bad758b8abc94dbbebc2a387b9e97c30b6543
> 
>>>
>>>>> +       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..308d0863ad 100644
>>>>> --- a/drivers/gpio/gpio-uclass.c
>>>>> +++ b/drivers/gpio/gpio-uclass.c
>>>>> @@ -5,6 +5,9 @@
>>>>>
>>>>>    #include <common.h>
>>>>>    #include <dm.h>
>>>>> +#include <dm/device-internal.h>
>>>>> +#include <dm/lists.h>
>>>>> +#include <dm/uclass-internal.h>
>>>>>    #include <dt-bindings/gpio/gpio.h>
>>>>>    #include <errno.h>
>>>>>    #include <fdtdec.h>
>>>>> @@ -141,6 +144,118 @@ 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)
>>>>> +
>>>>> +struct gpio_hog_priv {
>>>>> +       struct gpio_desc gpiod;
>>>>
>>>> As above I don't think you need this.
>>>
>>> Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
>>> for which I need to pass a pointer to struct gpio_desc ...
>>
>> The caller can declare this in a local variable.
> 
> The caller is always the device probe function, as gpio_hog_probe_all()
> loops over all gpio hog devices and probes them with device_probe()
> which cannot return a gpio_desc pointer, nor can such a pointer passed to.
> 
> Why isn't it valid to store in each gpio hog device the gpio desc it
> belongs too in device private data?

?

>>> And later I want to have the possibility to access the gpio for example
>>> with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
>>> store it somewhere. Or do I overlook something obvious?
>>
>> In that case the caller should store it, perhaps in a local variable,
>> or perhaps in a private data structure if needed for a long time.
>>
>> [..]
> 
> Of course I can introduce a list for each gpio hog definition filled
> when calling gpio_hog_probe_all() ... but than I have a second list
> which uses memory, may has to keep in sync the list if device are
> removed ... I want to avoid such an approach ... as we have already a
> list of gpio hog devices and the priv storage pointer for device private
> data.
> 
> What do you think?

?

>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +int gpio_hog_probe_all(void)
>>>>> +{
>>>>> +       struct udevice *dev;
>>>>> +       int ret;
>>>>> +
>>>>> +       for (uclass_first_device(UCLASS_NOP, &dev);
>>>>
>>>> Perhaps you should create a new uclass for this?
>>>
>>> Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
>>> UCLASS_NOP is valid for this here to?
>>>
>>> [1] http://patchwork.ozlabs.org/patch/1098913/
>>
>> I see your point and I can see arguments for each side. But the fact
>> that you have to check the driver suggests to me that a new uclass is
>> better.
>>
>> [..]
> 
> Ok with this approach the gpio_hog additions to gpio-uclass can be
> moved to a gpio-hog-uclass.c file. Also the problem where to store
> the gpio descriptor moves from device private data to uclass private
> data?
> 
> Is it worth?

?

>>>>> +/**
>>>>> + * gpio_hog_probe_all() - probe all gpio devices with
>>>>> + * gpio-hog subnodes.
>>>>> + *
>>>>> + * @return:    Returns return value from device_probe()
>>>>
>>>> What if one fails and the others succeed?
>>>
>>> Currently I return with error code from device_probe(). So the
>>> rest of the gpio-hogs are not probed.
>>>
>>> Hmm.. may it is better to emmit a debug message and continue?
>>
>> Sounds reasonable. But it seems to me that if there is an error there
>> should be some visible report, so this could perhaps return the error
>> code? Then the caller could print a messag?
> 
> Ok, so I add a printf message. I like more to add it in
> gpio_hog_probe_all(), because more than one gpio_hog can fail.
> 
> bye,
> Heiko

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Simon Glass]

Hi Heiko,

On Wed, 26 Jun 2019 at 22:12, Heiko Schocher <hs@denx.de> wrote:
>
> Hello Simon,
>
> Am 26.06.2019 um 17:07 schrieb Simon Glass:
> > Hi Heiko,
> >
> > On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <hs@denx.de> wrote:
> >>
> >> Hello Simon,
> >>
> >> Am 22.06.2019 um 21:09 schrieb Simon Glass:
> >>> Hi Heiko,
> >>>
> >>> On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
> >>>>
> >>>> 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>
> >>>>
> >>>> ---
> >>>> clean travis build, see result:
> >>>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
> >>>>
> >>>> Changes in v4:
> >>>> - rework as suggested by Patrick
> >>>>     based on patch:
> >>>>     http://patchwork.ozlabs.org/patch/1098913/
> >>>>     use new UCLASS_NOP
> >>>>     hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
> >>>>     gpio configuration for hog (request and set_dir) => probe function
> >>>
> >>>
> >
> > [..]
> >
> >>>> +config DM_GPIO_HOG
> >>>
> >>> Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
> >>> don't want to rename these sorts of options.
> >>
> >> Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
> >> DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
> >> I can change this.
> >
> > Yes I understand that, but in general we hope one day to rename
> > DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
> > keep the DM_ suffix only for things that need it.
>
> Yep, already dropped, see temp version (travisbuild clean):
> https://github.com/hsdenx/u-boot-test/commit/509bad758b8abc94dbbebc2a387b9e97c30b6543
>
> >>
> >>>> +       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..308d0863ad 100644
> >>>> --- a/drivers/gpio/gpio-uclass.c
> >>>> +++ b/drivers/gpio/gpio-uclass.c
> >>>> @@ -5,6 +5,9 @@
> >>>>
> >>>>    #include <common.h>
> >>>>    #include <dm.h>
> >>>> +#include <dm/device-internal.h>
> >>>> +#include <dm/lists.h>
> >>>> +#include <dm/uclass-internal.h>
> >>>>    #include <dt-bindings/gpio/gpio.h>
> >>>>    #include <errno.h>
> >>>>    #include <fdtdec.h>
> >>>> @@ -141,6 +144,118 @@ 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)
> >>>> +
> >>>> +struct gpio_hog_priv {
> >>>> +       struct gpio_desc gpiod;
> >>>
> >>> As above I don't think you need this.
> >>
> >> Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
> >> for which I need to pass a pointer to struct gpio_desc ...
> >
> > The caller can declare this in a local variable.
>
> The caller is always the device probe function, as gpio_hog_probe_all()
> loops over all gpio hog devices and probes them with device_probe()
> which cannot return a gpio_desc pointer, nor can such a pointer passed to.
>
> Why isn't it valid to store in each gpio hog device the gpio desc it
> belongs too in device private data?

I don't see any point. Your caller needs to store the pointer anyway,
or if not, could just as easily have a local variable with the info.

Also, passing a pointer to the struct is how we consistently do it so far.

What is the benefit of storing it in the callee rather than the caller?

>
> >> And later I want to have the possibility to access the gpio for example
> >> with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
> >> store it somewhere. Or do I overlook something obvious?
> >
> > In that case the caller should store it, perhaps in a local variable,
> > or perhaps in a private data structure if needed for a long time.
> >
> > [..]
>
> Of course I can introduce a list for each gpio hog definition filled
> when calling gpio_hog_probe_all() ... but than I have a second list
> which uses memory, may has to keep in sync the list if device are
> removed ... I want to avoid such an approach ... as we have already a
> list of gpio hog devices and the priv storage pointer for device private
> data.

I understand what you are saying about devices being removed, but
actually removable is fine, it's only unbinding that is dangerous. And
we already have that problem. We would need to add refcounts to fix
it, and given that bootloaders don't normally need to unbind, I have
not worried about it too much.

The approach I suggest would not use any more memory I think.

>
> What do you think?

Well I still think it is better to have the caller own the struct.

>
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>> +
> >>>> +int gpio_hog_probe_all(void)
> >>>> +{
> >>>> +       struct udevice *dev;
> >>>> +       int ret;
> >>>> +
> >>>> +       for (uclass_first_device(UCLASS_NOP, &dev);
> >>>
> >>> Perhaps you should create a new uclass for this?
> >>
> >> Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
> >> UCLASS_NOP is valid for this here to?
> >>
> >> [1] http://patchwork.ozlabs.org/patch/1098913/
> >
> > I see your point and I can see arguments for each side. But the fact
> > that you have to check the driver suggests to me that a new uclass is
> > better.
> >
> > [..]
>
> Ok with this approach the gpio_hog additions to gpio-uclass can be
> moved to a gpio-hog-uclass.c file. Also the problem where to store
> the gpio descriptor moves from device private data to uclass private
> data?

I don't think you have explained why you actually need to store the
GPIO descriptor. Where do you use it later? Is it just the
gpio_hog_probe_all() function?

>
> Is it worth?
>
> >>>> +/**
> >>>> + * gpio_hog_probe_all() - probe all gpio devices with
> >>>> + * gpio-hog subnodes.
> >>>> + *
> >>>> + * @return:    Returns return value from device_probe()
> >>>
> >>> What if one fails and the others succeed?
> >>
> >> Currently I return with error code from device_probe(). So the
> >> rest of the gpio-hogs are not probed.
> >>
> >> Hmm.. may it is better to emmit a debug message and continue?
> >
> > Sounds reasonable. But it seems to me that if there is an error there
> > should be some visible report, so this could perhaps return the error
> > code? Then the caller could print a messag?
>
> Ok, so I add a printf message. I like more to add it in
> gpio_hog_probe_all(), because more than one gpio_hog can fail.

Regards,
Simon

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

[Heiko Schocher]

Hello Simon,

Am 12.07.2019 um 23:11 schrieb Simon Glass:
> Hi Heiko,
> 
> On Wed, 26 Jun 2019 at 22:12, Heiko Schocher <hs@denx.de> wrote:
>>
>> Hello Simon,
>>
>> Am 26.06.2019 um 17:07 schrieb Simon Glass:
>>> Hi Heiko,
>>>
>>> On Mon, 24 Jun 2019 at 00:16, Heiko Schocher <hs@denx.de> wrote:
>>>>
>>>> Hello Simon,
>>>>
>>>> Am 22.06.2019 um 21:09 schrieb Simon Glass:
>>>>> Hi Heiko,
>>>>>
>>>>> On Wed, 12 Jun 2019 at 05:12, Heiko Schocher <hs@denx.de> wrote:
>>>>>>
>>>>>> 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>
>>>>>>
>>>>>> ---
>>>>>> clean travis build, see result:
>>>>>> https://travis-ci.org/hsdenx/u-boot-test/builds/544040419
>>>>>>
>>>>>> Changes in v4:
>>>>>> - rework as suggested by Patrick
>>>>>>      based on patch:
>>>>>>      http://patchwork.ozlabs.org/patch/1098913/
>>>>>>      use new UCLASS_NOP
>>>>>>      hog detection + DT parsing in gpio_post_bind() .... info saved in platdata
>>>>>>      gpio configuration for hog (request and set_dir) => probe function
>>>>>
>>>>>
>>>
>>> [..]
>>>
>>>>>> +config DM_GPIO_HOG
>>>>>
>>>>> Can we drop the DM_ prefix? At some point DM_GPIO will go away and we
>>>>> don't want to rename these sorts of options.
>>>>
>>>> Hmm... we also have the DM_GPIO symbol, as the gpio hog only works with
>>>> DM_GPIO enabled, I thought DM_GPIO_HOG is the better choice, but of course
>>>> I can change this.
>>>
>>> Yes I understand that, but in general we hope one day to rename
>>> DM_GPIO to GPIO, once everything uses DM. That's why I am trying to
>>> keep the DM_ suffix only for things that need it.
>>
>> Yep, already dropped, see temp version (travisbuild clean):
>> https://github.com/hsdenx/u-boot-test/commit/509bad758b8abc94dbbebc2a387b9e97c30b6543
>>
>>>>
>>>>>> +       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..308d0863ad 100644
>>>>>> --- a/drivers/gpio/gpio-uclass.c
>>>>>> +++ b/drivers/gpio/gpio-uclass.c
>>>>>> @@ -5,6 +5,9 @@
>>>>>>
>>>>>>     #include <common.h>
>>>>>>     #include <dm.h>
>>>>>> +#include <dm/device-internal.h>
>>>>>> +#include <dm/lists.h>
>>>>>> +#include <dm/uclass-internal.h>
>>>>>>     #include <dt-bindings/gpio/gpio.h>
>>>>>>     #include <errno.h>
>>>>>>     #include <fdtdec.h>
>>>>>> @@ -141,6 +144,118 @@ 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)
>>>>>> +
>>>>>> +struct gpio_hog_priv {
>>>>>> +       struct gpio_desc gpiod;
>>>>>
>>>>> As above I don't think you need this.
>>>>
>>>> Hmm.. when probing I need to request the gpio with gpio_dev_request_index()
>>>> for which I need to pass a pointer to struct gpio_desc ...
>>>
>>> The caller can declare this in a local variable.
>>
>> The caller is always the device probe function, as gpio_hog_probe_all()
>> loops over all gpio hog devices and probes them with device_probe()
>> which cannot return a gpio_desc pointer, nor can such a pointer passed to.
>>
>> Why isn't it valid to store in each gpio hog device the gpio desc it
>> belongs too in device private data?
> 
> I don't see any point. Your caller needs to store the pointer anyway,
> or if not, could just as easily have a local variable with the info.
> 
> Also, passing a pointer to the struct is how we consistently do it so far.
> 
> What is the benefit of storing it in the callee rather than the caller?
> 
>>
>>>> And later I want to have the possibility to access the gpio for example
>>>> with dm_gpio_set_value() and need therefore the gpio descriptor, so I must
>>>> store it somewhere. Or do I overlook something obvious?
>>>
>>> In that case the caller should store it, perhaps in a local variable,
>>> or perhaps in a private data structure if needed for a long time.
>>>
>>> [..]
>>
>> Of course I can introduce a list for each gpio hog definition filled
>> when calling gpio_hog_probe_all() ... but than I have a second list
>> which uses memory, may has to keep in sync the list if device are
>> removed ... I want to avoid such an approach ... as we have already a
>> list of gpio hog devices and the priv storage pointer for device private
>> data.
> 
> I understand what you are saying about devices being removed, but
> actually removable is fine, it's only unbinding that is dangerous. And
> we already have that problem. We would need to add refcounts to fix
> it, and given that bootloaders don't normally need to unbind, I have
> not worried about it too much.
> 
> The approach I suggest would not use any more memory I think.
> 
>>
>> What do you think?
> 
> Well I still think it is better to have the caller own the struct.
> 
>>
>>>>>> +
>>>>>> +       return 0;
>>>>>> +}
>>>>>> +
>>>>>> +int gpio_hog_probe_all(void)
>>>>>> +{
>>>>>> +       struct udevice *dev;
>>>>>> +       int ret;
>>>>>> +
>>>>>> +       for (uclass_first_device(UCLASS_NOP, &dev);
>>>>>
>>>>> Perhaps you should create a new uclass for this?
>>>>
>>>> Hmm.. yes, I had an approach with a new uclass, but I thought, the new [1]
>>>> UCLASS_NOP is valid for this here to?
>>>>
>>>> [1] http://patchwork.ozlabs.org/patch/1098913/
>>>
>>> I see your point and I can see arguments for each side. But the fact
>>> that you have to check the driver suggests to me that a new uclass is
>>> better.
>>>
>>> [..]
>>
>> Ok with this approach the gpio_hog additions to gpio-uclass can be
>> moved to a gpio-hog-uclass.c file. Also the problem where to store
>> the gpio descriptor moves from device private data to uclass private
>> data?
> 
> I don't think you have explained why you actually need to store the
> GPIO descriptor. Where do you use it later? Is it just the
> gpio_hog_probe_all() function?

You can use it later! See example this patch adds in
doc/device-tree-bindings/gpio/gpio.txt:

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

So in this example, gpio "board_rescue" is defined through DTS and code
which uses this gpio is generic ... makes it nice to handle board
differences (as I have on the aristainetos board, which I rework
for DM support).

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Tom Rini]

On Wed, Jun 12, 2019 at 06:11:46AM +0200, Heiko Schocher wrote:

> 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>
> Tested-by: Michal Simek <michal.simek@xilinx.com> (zcu102)
> Tested-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190714/d6b0ad81/attachment.sig>

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

[Heiko Schocher]

Hello Tom,

Am 14.07.2019 um 15:08 schrieb Tom Rini:
> On Wed, Jun 12, 2019 at 06:11:46AM +0200, Heiko Schocher wrote:
> 
>> 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>
>> Tested-by: Michal Simek <michal.simek@xilinx.com> (zcu102)
>> Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> 
> Applied to u-boot/master, thanks!

Ups, Simon (added to cc) has some comments to v4 I mainly worked in ...

Is it OK to send based on this commit a followup patch with the comments
from Simon worked in?

If you want to look into this patch, you will find current state here:
https://github.com/hsdenx/u-boot-test/commit/30134bab8d5e56708943473f294676b237e8614c

travis build in progress:
https://travis-ci.org/hsdenx/u-boot-test/builds/558774593

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

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

[Tom Rini]

On Mon, Jul 15, 2019 at 08:21:57AM +0200, Heiko Schocher wrote:
> Hello Tom,
> 
> Am 14.07.2019 um 15:08 schrieb Tom Rini:
> >On Wed, Jun 12, 2019 at 06:11:46AM +0200, Heiko Schocher wrote:
> >
> >>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>
> >>Tested-by: Michal Simek <michal.simek@xilinx.com> (zcu102)
> >>Tested-by: Patrick Delaunay <patrick.delaunay@st.com>
> >
> >Applied to u-boot/master, thanks!
> 
> Ups, Simon (added to cc) has some comments to v4 I mainly worked in ...
> 
> Is it OK to send based on this commit a followup patch with the comments
> from Simon worked in?
> 
> If you want to look into this patch, you will find current state here:
> https://github.com/hsdenx/u-boot-test/commit/30134bab8d5e56708943473f294676b237e8614c
> 
> travis build in progress:
> https://travis-ci.org/hsdenx/u-boot-test/builds/558774593

Yes please, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190715/d18fd994/attachment.sig>