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

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