[U-Boot] [PATCH v2 00/14] dm: add support of new binding in gpio and pincontrol

[U-Boot] [PATCH v2 00/14] dm: add support of new binding in gpio and pincontrol

[Patrick Delaunay]

Hi,

I create this patchset to prepare alignment of stm32mp157c-ev1
device-tree with the linux kernel v5.4.

One node for touch screen support use the IRQ line configuration
using the new kernel binding introduced by the linux kernel
commit ede033e1e863c from v5.1 ('dt-bindings:
gpio: document the new pull-up/pull-down flags')
https://github.com/torvalds/linux/commit/ede033e1e863c

gt9147: goodix_ts at 5d {
	compatible = "goodix,gt9147";
	reg = <0x5d>;
	status = "okay";
	irq-gpios = <&stmfx_pinctrl 14
			(GPIO_ACTIVE_HIGH | GPIO_PULL_DOWN)>;
	irq-flags = <IRQ_TYPE_EDGE_RISING>;
};

In Linux Kernel, the GPIO configuration (pull down/up) are now
managed by GPIO lib but they are not yet supported by U-boot in
uclass GPIO or pincontrol (when used by gpio-hog).

This serie adds the support of theses new defines (with ops
get_config/set_config) added in kernel GPIO binding and adds
the test in sandbox.

NB: In a second phase, the removed set_open_drain/get_open_drain
    API could be replaced by the new ops get_config/set_config.

I also align the gpio binding file with the version from the
latest kernel v5.3 (I don't think I remove any U-Boot specific
part with this patch)
I move the added information on gpio hog code in a new file
doc/README.gpio.

To have functional test, I convert pinctrl-generic to
livetree otherwise the pins are not correctly configured in
sandbox, when CONFIG_OF_LIVE is activated.

For the test on gpio I add information for pin configuration in
output of the command "pinmux status" with a simple pincontrol
driver associated to GPIO driver.

NB: after rebase on master branch, the sandbox test "ut dm power_domain"
   is failing; it was not the case for v2019.10.

=>  ut dm power_domain
Test: dm_test_power_domain: power-domain.c
../test/dm/test-main.c:73, dm_test_destroy(): 0 == uclass_destroy(uc): Expected 0, got -22
../test/dm/test-main.c:108, dm_do_test(): 0 == dm_test_destroy(uts): Expected 0, got 1
../test/dm/test-main.c:170, dm_test_main(): 0 == dm_do_test(uts, test, 1): Expected 0, got 1

I think it is linked to commit 52edfed65de9 ("dm: core:
device: switch off power domain after device removal")

After some investigation :

1/ pincontrol is remove in uclass_destroy(0)

2/ power domain is removed in uclass_destroy(0)

3/ device_chld_unbind()
dev_power_domain_off
-> probe power domain (again)
--> pinctrl_select_state() in device_probe()
---> pincontrol is probed (again)

4/ at the end of  dev_power_domain_off() function the power domain
   is unbind in dev_power_domain_off
   device_remove(pd.dev, DM_REMOVE_NORMAL);

So power domain driver is correctly removed but it is not the
case for the pincontrol driver and that cause the issue.

The problem occurs after my serie only because I introduce
a second pincontrol driver for sandbox, so the dynamic changes
but my serie is not the root cause of the issue,
it should be investigated by power domain users.

Workaround to allow test execution:
  "dm: core: device: don't probe pinctrl for power domain"

My working branch:
https://github.com/patrickdelaunay/u-boot/tree/dm

CI-Travis build:
https://travis-ci.org/patrickdelaunay/u-boot/builds/616716294

Issue for tbs2910 board not reproduced locally with buildman
+u-boot.imx exceeds file size limit:
1405+  limit:  0x5fc00 bytes
1406+  actual: 0x60c00 bytes
1407+  excess: 0x1000 bytes

Regards

Patrick Delaunay


Changes in v2:
- use the dev_ API instead of ofnode_ function.
- Identify property with a new struct ofprop as proposed
  by Simon Glass
- Add dev_ iterate functions
- Change dev_ functions in pinctrl-generic
- remove the unused GPIOD_REQUESTED
- remove the open_drain API and ops
- add gpio descriptor initialization helper
- change the proposed ops for pin config to set_dir_flags/get_dir_flags
- reused the existing API dm_gpio_set_dir_flags/dm_gpio_set_dir
- add a new API dm_gpio_get_dir_flags
- simplify sandbox GPIO driver: save dir_flags (GPIOD_...)
  and remove internal flags (GPIOF_...); the previous path
  "gpio: sandbox: cleanup flag support" is no more needed
- add unitary test in dm_test_gpio for set_dir_flags
- add some function check in dm_test_gpio_phandles
- dm_test_gpio_pin_config change to dm_test_gpio_get_dir_flags
- Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in
  sandbox gpio driver
- rebase on v2020.01-rc3

Patrick Delaunay (14):
  dm: pinctrl: convert pinctrl-single to livetree
  dm: core: add ofnode and dev function to iterate on node property
  dm: pinctrl: migrate pinctrl-generic to livetree
  dt-bindings: gpio: document the new pull-up/pull-down flags
  gpio: remove GPIOD_REQUESTED
  gpio: remove the open_drain API and ops
  gpio: add gpio descriptor initialization helper
  gpio: add ops for configuration with dir flags
  dt-bindings: gpio: alignment with kernel v5.3
  pinctrl: sandbox: Add mux information in get_pin_muxing
  test: dm: update test for pins configuration in pinctrl node
  gpio: sandbox: cleanup binding support
  test: dm: update test for pins configuration in gpio
  test: pinmux: add pincontrol-gpio for pin configuration

 arch/sandbox/dts/test.dts               |  85 +++++--
 arch/sandbox/include/asm/gpio.h         |  38 +--
 doc/README.gpio                         |  42 ++++
 doc/device-tree-bindings/gpio/gpio.txt  | 304 ++++++++++++----------
 drivers/core/of_access.c                |  32 +++
 drivers/core/ofnode.c                   |  48 ++++
 drivers/core/read.c                     |  16 ++
 drivers/gpio/gpio-uclass.c              | 192 ++++++++++----
 drivers/gpio/mpc8xxx_gpio.c             |  22 --
 drivers/gpio/sandbox.c                  | 321 +++++++++++++++++++-----
 drivers/pinctrl/pinctrl-generic.c       |  30 +--
 drivers/pinctrl/pinctrl-sandbox.c       |  44 +++-
 drivers/pinctrl/pinctrl-single.c        |  27 +-
 include/asm-generic/gpio.h              | 104 ++++----
 include/dm/of_access.h                  |  40 +++
 include/dm/ofnode.h                     |  63 ++++-
 include/dm/read.h                       |  67 +++++
 include/dt-bindings/gpio/gpio.h         |   6 +
 include/dt-bindings/gpio/sandbox-gpio.h |  24 ++
 test/dm/gpio.c                          |  69 ++++-
 test/dm/test-fdt.c                      |   2 +-
 test/py/tests/test_pinmux.py            |  38 ++-
 22 files changed, 1216 insertions(+), 398 deletions(-)
 create mode 100644 doc/README.gpio
 create mode 100644 include/dt-bindings/gpio/sandbox-gpio.h

-- 
2.17.1

[U-Boot] [PATCH v2 01/14] dm: pinctrl: convert pinctrl-single to livetree

[Patrick Delaunay]

Convert 'pinctrl-single' using livetree functions
- dev_read_prop
- dev_read_u32_default
- dev_read_u32_array
- dev_read_bool
- dev_read_addr
and get rid of DECLARE_GLOBAL_DATA_PTR.

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

Changes in v2:
- use the dev_ API instead of ofnode_ function.

 drivers/pinctrl/pinctrl-single.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 1dfc97dcea..6d8f989ebd 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -9,8 +9,6 @@
 #include <linux/libfdt.h>
 #include <asm/io.h>
 
-DECLARE_GLOBAL_DATA_PTR;
-
 struct single_pdata {
 	fdt_addr_t base;	/* first configuration register */
 	int offset;		/* index of last configuration register */
@@ -117,13 +115,11 @@ static int single_configure_bits(struct udevice *dev,
 static int single_set_state(struct udevice *dev,
 			    struct udevice *config)
 {
-	const void *fdt = gd->fdt_blob;
 	const struct single_fdt_pin_cfg *prop;
 	const struct single_fdt_bits_cfg *prop_bits;
 	int len;
 
-	prop = fdt_getprop(fdt, dev_of_offset(config), "pinctrl-single,pins",
-			   &len);
+	prop = dev_read_prop(dev, "pinctrl-single,pins", &len);
 
 	if (prop) {
 		dev_dbg(dev, "configuring pins for %s\n", config->name);
@@ -136,9 +132,7 @@ static int single_set_state(struct udevice *dev,
 	}
 
 	/* pinctrl-single,pins not found so check for pinctrl-single,bits */
-	prop_bits = fdt_getprop(fdt, dev_of_offset(config),
-				"pinctrl-single,bits",
-				    &len);
+	prop_bits = dev_read_prop(dev, "pinctrl-single,bits", &len);
 	if (prop_bits) {
 		dev_dbg(dev, "configuring pins for %s\n", config->name);
 		if (len % sizeof(struct single_fdt_bits_cfg)) {
@@ -160,27 +154,24 @@ static int single_ofdata_to_platdata(struct udevice *dev)
 	int res;
 	struct single_pdata *pdata = dev->platdata;
 
-	pdata->width = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
-				      "pinctrl-single,register-width", 0);
+	pdata->width =
+		dev_read_u32_default(dev, "pinctrl-single,register-width", 0);
 
-	res = fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev),
-				   "reg", of_reg, 2);
+	res = dev_read_u32_array(dev, "reg", of_reg, 2);
 	if (res)
 		return res;
 	pdata->offset = of_reg[1] - pdata->width / 8;
 
-	addr = devfdt_get_addr(dev);
+	addr = dev_read_addr(dev);
 	if (addr == FDT_ADDR_T_NONE) {
 		dev_dbg(dev, "no valid base register address\n");
 		return -EINVAL;
 	}
 	pdata->base = addr;
 
-	pdata->mask = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
-				     "pinctrl-single,function-mask",
-				     0xffffffff);
-	pdata->bits_per_mux = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
-					      "pinctrl-single,bit-per-mux");
+	pdata->mask = dev_read_u32_default(dev, "pinctrl-single,function-mask",
+					   0xffffffff);
+	pdata->bits_per_mux = dev_read_bool(dev, "pinctrl-single,bit-per-mux");
 
 	return 0;
 }
-- 
2.17.1

[U-Boot] [PATCH v2 02/14] dm: core: add ofnode and dev function to iterate on node property

[Patrick Delaunay]

Add functions to iterate on all property with livetree
- dev_read_first_prop
- dev_read_next_prop
- dev_read_prop_by_prop
and
- ofnode_get_first_property
- ofnode_get_next_property
- ofnode_get_property_by_prop

For example:
struct ofprop property;

dev_for_each_property(property, config) {
	value = dev_read_prop_by_prop(&property, &propname, &len);

or:
for (prop = ofnode_get_first_property(dev_ofnode(dev));
     prop;
     prop = ofnode_get_next_property(dev_ofnode(dev),prop))
{
     value = ofnode_get_property_by_prop(dev_ofnode(dev), prop,
					 &propname, &len);
....
}

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

Changes in v2:
- Identify property with a new struct ofprop as proposed
  by Simon Glass
- Add dev_ iterate functions

 drivers/core/of_access.c | 32 +++++++++++++++++++
 drivers/core/ofnode.c    | 48 ++++++++++++++++++++++++++++
 drivers/core/read.c      | 16 ++++++++++
 include/dm/of_access.h   | 40 ++++++++++++++++++++++++
 include/dm/ofnode.h      | 63 ++++++++++++++++++++++++++++++++++++-
 include/dm/read.h        | 67 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 265 insertions(+), 1 deletion(-)

diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
index 945b81448c..86fe42ad14 100644
--- a/drivers/core/of_access.c
+++ b/drivers/core/of_access.c
@@ -170,6 +170,38 @@ const void *of_get_property(const struct device_node *np, const char *name,
 	return pp ? pp->value : NULL;
 }
 
+const struct property *of_get_first_property(const struct device_node *np)
+{
+	if (!np)
+		return NULL;
+
+	return  np->properties;
+}
+
+const struct property *of_get_next_property(const struct device_node *np,
+					    const struct property *property)
+{
+	if (!np)
+		return NULL;
+
+	return property->next;
+}
+
+const void *of_get_property_by_prop(const struct device_node *np,
+				    const struct property *property,
+				    const char **name,
+				    int *lenp)
+{
+	if (!np || !property)
+		return NULL;
+	if (name)
+		*name = property->name;
+	if (lenp)
+		*lenp = property->length;
+
+	return property->value;
+}
+
 static const char *of_prop_next_string(struct property *prop, const char *cur)
 {
 	const void *curv = cur;
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 8f0eab2ca6..cb27562fec 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -536,6 +536,54 @@ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp)
 				   propname, lenp);
 }
 
+int ofnode_get_first_property(ofnode node, struct ofprop *prop)
+{
+	prop->node = node;
+
+	if (ofnode_is_np(node)) {
+		prop->prop = of_get_first_property(ofnode_to_np(prop->node));
+		if (!prop->prop)
+			return -FDT_ERR_NOTFOUND;
+	} else {
+		prop->offset =
+			fdt_first_property_offset(gd->fdt_blob,
+						  ofnode_to_offset(prop->node));
+		if (prop->offset < 0)
+			return prop->offset;
+	}
+
+	return 0;
+}
+
+int ofnode_get_next_property(struct ofprop *prop)
+{
+	if (ofnode_is_np(prop->node)) {
+		prop->prop = of_get_next_property(ofnode_to_np(prop->node),
+						  prop->prop);
+		if (!prop->prop)
+			return -FDT_ERR_NOTFOUND;
+	} else {
+		prop->offset = fdt_next_property_offset(gd->fdt_blob,
+							prop->offset);
+		if (prop->offset  < 0)
+			return prop->offset;
+	}
+
+	return 0;
+}
+
+const void *ofnode_get_property_by_prop(const struct ofprop *prop,
+					const char **propname, int *lenp)
+{
+	if (ofnode_is_np(prop->node))
+		return of_get_property_by_prop(ofnode_to_np(prop->node),
+					       prop->prop, propname, lenp);
+	else
+		return fdt_getprop_by_offset(gd->fdt_blob,
+					     prop->offset,
+					     propname, lenp);
+}
+
 bool ofnode_is_available(ofnode node)
 {
 	if (ofnode_is_np(node))
diff --git a/drivers/core/read.c b/drivers/core/read.c
index 9602e52d1b..a9f5d147b2 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -238,6 +238,22 @@ const void *dev_read_prop(struct udevice *dev, const char *propname, int *lenp)
 	return ofnode_get_property(dev_ofnode(dev), propname, lenp);
 }
 
+int dev_read_first_prop(struct udevice *dev, struct ofprop *prop)
+{
+	return ofnode_get_first_property(dev_ofnode(dev), prop);
+}
+
+int dev_read_next_prop(struct ofprop *prop)
+{
+	return ofnode_get_next_property(prop);
+}
+
+const void *dev_read_prop_by_prop(struct ofprop *prop,
+				  const char **propname, int *lenp)
+{
+	return ofnode_get_property_by_prop(prop, propname, lenp);
+}
+
 int dev_read_alias_seq(struct udevice *dev, int *devnump)
 {
 	ofnode node = dev_ofnode(dev);
diff --git a/include/dm/of_access.h b/include/dm/of_access.h
index 13fedb7cf5..1eb1ce13d0 100644
--- a/include/dm/of_access.h
+++ b/include/dm/of_access.h
@@ -103,6 +103,46 @@ struct property *of_find_property(const struct device_node *np,
 const void *of_get_property(const struct device_node *np, const char *name,
 			    int *lenp);
 
+/**
+ * of_get_first_property()- get to the pointer of the first property
+ *
+ * Get pointer to the first property of the node, it is used to iterate
+ * and read all the property with of_get_next_property_by_prop().
+ *
+ * @np: Pointer to device node
+ * @return pointer to property or NULL if not found
+ */
+const struct property *of_get_first_property(const struct device_node *np);
+
+/**
+ * of_get_next_property() - get to the pointer of the next property
+ *
+ * Get pointer to the next property of the node, it is used to iterate
+ * and read all the property with of_get_property_by_prop().
+ *
+ * @np: Pointer to device node
+ * @property: pointer of the current property
+ * @return pointer to next property or NULL if not found
+ */
+const struct property *of_get_next_property(const struct device_node *np,
+					    const struct property *property);
+
+/**
+ * of_get_property_by_prop() - get a property value of a node property
+ *
+ * Get value for the property identified by node and property pointer.
+ *
+ * @node: node to read
+ * @property: pointer of the property to read
+ * @propname: place to property name on success
+ * @lenp: place to put length on success
+ * @return pointer to property value or NULL if error
+ */
+const void *of_get_property_by_prop(const struct device_node *np,
+				    const struct property *property,
+				    const char **name,
+				    int *lenp);
+
 /**
  * of_device_is_compatible() - Check if the node matches given constraints
  * @device: pointer to node
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 5c4cbf0998..02687eeeef 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -58,6 +58,31 @@ struct ofnode_phandle_args {
 	uint32_t args[OF_MAX_PHANDLE_ARGS];
 };
 
+/**
+ * ofprop - reference to a property of a device tree node
+ *
+ * This struct hold the reference on one property of one node,
+ * using struct ofnode and an offset within the flat device tree or either
+ * a pointer to a struct property in the live device tree.
+ *
+ * Thus we can reference arguments in both the live tree and the flat tree.
+ *
+ * The property reference can also hold a null reference. This corresponds to
+ * a struct property NULL pointer or an offset of -1.
+ *
+ * @node: Pointer to device node
+ * @offset: Pointer into flat device tree, used for flat tree.
+ * @prop: Pointer to property, used for live treee.
+ */
+
+struct ofprop {
+	ofnode node;
+	union {
+		int offset;
+		const struct property *prop;
+	};
+};
+
 /**
  * _ofnode_to_np() - convert an ofnode to a live DT node pointer
  *
@@ -543,7 +568,7 @@ int ofnode_decode_display_timing(ofnode node, int index,
 				 struct display_timing *config);
 
 /**
- * ofnode_get_property()- - get a pointer to the value of a node property
+ * ofnode_get_property() - get a pointer to the value of a node property
  *
  * @node: node to read
  * @propname: property to read
@@ -552,6 +577,42 @@ int ofnode_decode_display_timing(ofnode node, int index,
  */
 const void *ofnode_get_property(ofnode node, const char *propname, int *lenp);
 
+/**
+ * ofnode_get_first_property()- get the reference of the first property
+ *
+ * Get reference to the first property of the node, it is used to iterate
+ * and read all the property with ofnode_get_property_by_prop().
+ *
+ * @node: node to read
+ * @prop: place to put argument reference
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+int ofnode_get_first_property(ofnode node, struct ofprop *prop);
+
+/**
+ * ofnode_get_next_property() - get the reference of the next property
+ *
+ * Get reference to the next property of the node, it is used to iterate
+ * and read all the property with ofnode_get_property_by_prop().
+ *
+ * @prop: reference of current argument and place to put reference of next one
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+int ofnode_get_next_property(struct ofprop *prop);
+
+/**
+ * ofnode_get_property_by_ofprop() - get a pointer to the value of a property
+ *
+ * Get value for the property identified by the provided reference.
+ *
+ * @prop: reference on property
+ * @propname: If non-NULL, place to property name on success,
+ * @lenp: If non-NULL, place to put length on success
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+const void *ofnode_get_property_by_prop(const struct ofprop *prop,
+					const char **propname, int *lenp);
+
 /**
  * ofnode_is_available() - check if a node is marked available
  *
diff --git a/include/dm/read.h b/include/dm/read.h
index d37fcb504d..3c6bed7bff 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -465,6 +465,42 @@ int dev_read_phandle(struct udevice *dev);
  */
 const void *dev_read_prop(struct udevice *dev, const char *propname, int *lenp);
 
+/**
+ * dev_read_first_prop()- get the reference of the first property
+ *
+ * Get reference to the first property of the node, it is used to iterate
+ * and read all the property with dev_read_prop_by_prop().
+ *
+ * @dev: device to check
+ * @prop: place to put argument reference
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+int dev_read_first_prop(struct udevice *dev, struct ofprop *prop);
+
+/**
+ * ofnode_get_next_property() - get the reference of the next property
+ *
+ * Get reference to the next property of the node, it is used to iterate
+ * and read all the property with dev_read_prop_by_prop().
+ *
+ * @prop: reference of current argument and place to put reference of next one
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+int dev_read_next_prop(struct ofprop *prop);
+
+/**
+ * dev_read_prop_by_prop() - get a pointer to the value of a property
+ *
+ * Get value for the property identified by the provided reference.
+ *
+ * @prop: reference on property
+ * @propname: If non-NULL, place to property name on success,
+ * @lenp: If non-NULL, place to put length on success
+ * @return 0 if OK, -ve on error. -FDT_ERR_NOTFOUND if not found
+ */
+const void *dev_read_prop_by_prop(struct ofprop *prop,
+				  const char **propname, int *lenp);
+
 /**
  * dev_read_alias_seq() - Get the alias sequence number of a node
  *
@@ -812,6 +848,23 @@ static inline const void *dev_read_prop(struct udevice *dev,
 	return ofnode_get_property(dev_ofnode(dev), propname, lenp);
 }
 
+static inline int dev_read_first_prop(struct udevice *dev, struct ofprop *prop)
+{
+	return ofnode_get_first_property(dev_ofnode(dev), prop);
+}
+
+static inline int dev_read_next_prop(struct ofprop *prop)
+{
+	return ofnode_get_next_property(prop);
+}
+
+static inline const void *dev_read_prop_by_prop(struct ofprop *prop,
+						const char **propname,
+						int *lenp)
+{
+	return ofnode_get_property_by_prop(prop, propname, lenp);
+}
+
 static inline int dev_read_alias_seq(struct udevice *dev, int *devnump)
 {
 	return fdtdec_get_alias_seq(gd->fdt_blob, dev->uclass->uc_drv->name,
@@ -889,4 +942,18 @@ static inline int dev_read_alias_highest_id(const char *stem)
 	     ofnode_valid(subnode); \
 	     subnode = ofnode_next_subnode(subnode))
 
+/**
+ * dev_for_each_property() - Helper function to iterate through property
+ *
+ * This creates a for() loop which works through the property in a device's
+ * device-tree node.
+ *
+ * @prop: struct ofprop holding the current property
+ * @dev: device to use for interation (struct udevice *)
+ */
+#define dev_for_each_property(prop, dev) \
+	for (int ret_prop = dev_read_first_prop(dev, &prop); \
+	     !ret_prop; \
+	     ret_prop = dev_read_next_prop(&prop))
+
 #endif
-- 
2.17.1

[U-Boot] [PATCH v2 03/14] dm: pinctrl: migrate pinctrl-generic to livetree

[Patrick Delaunay]

Migrate pinctrl-generic to livetree:
- dev_for_each_property
- dev_read_prop_by_prop
- dev_read_string_count
- dev_read_string_index
and get rid of DECLARE_GLOBAL_DATA_PTR.

This patch solves the parsing issue during sandbox tests for pin
configuration (OF_LIVE is activated in sandbox_defconfig
and sub node are not correctly parsed in
pinctrl_generic_set_state_subnode with fdt lib API).

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

Changes in v2:
- Change dev_ functions in pinctrl-generic

 drivers/pinctrl/pinctrl-generic.c | 30 +++++++++++-------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index eecf0f5dc1..9c4f73b1be 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -8,8 +8,6 @@
 #include <linux/compat.h>
 #include <dm/pinctrl.h>
 
-DECLARE_GLOBAL_DATA_PTR;
-
 /**
  * pinctrl_pin_name_to_selector() - return the pin selector for a pin
  *
@@ -243,18 +241,14 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 					 struct udevice *config,
 					 bool is_group, unsigned selector)
 {
-	const void *fdt = gd->fdt_blob;
-	int node_offset = dev_of_offset(config);
 	const char *propname;
 	const void *value;
-	int prop_offset, len, func_selector, param, ret;
+	struct ofprop property;
+	int len, func_selector, param, ret;
 	u32 arg, default_val;
 
-	for (prop_offset = fdt_first_property_offset(fdt, node_offset);
-	     prop_offset > 0;
-	     prop_offset = fdt_next_property_offset(fdt, prop_offset)) {
-		value = fdt_getprop_by_offset(fdt, prop_offset,
-					      &propname, &len);
+	dev_for_each_property(property, config) {
+		value = dev_read_prop_by_prop(&property, &propname, &len);
 		if (!value)
 			return -EINVAL;
 
@@ -298,19 +292,17 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 static int pinctrl_generic_set_state_subnode(struct udevice *dev,
 					     struct udevice *config)
 {
-	const void *fdt = gd->fdt_blob;
-	int node = dev_of_offset(config);
 	const char *subnode_target_type = "pins";
 	bool is_group = false;
 	const char *name;
 	int strings_count, selector, i, ret;
 
-	strings_count = fdt_stringlist_count(fdt, node, subnode_target_type);
+	strings_count = dev_read_string_count(config, subnode_target_type);
 	if (strings_count < 0) {
 		subnode_target_type = "groups";
 		is_group = true;
-		strings_count = fdt_stringlist_count(fdt, node,
-						     subnode_target_type);
+		strings_count = dev_read_string_count(config,
+						      subnode_target_type);
 		if (strings_count < 0) {
 			/* skip this node; may contain config child nodes */
 			return 0;
@@ -318,10 +310,10 @@ static int pinctrl_generic_set_state_subnode(struct udevice *dev,
 	}
 
 	for (i = 0; i < strings_count; i++) {
-		name = fdt_stringlist_get(fdt, node, subnode_target_type, i,
-					  NULL);
-		if (!name)
-			return -EINVAL;
+		ret = dev_read_string_index(config, subnode_target_type, i,
+					    &name);
+		if (ret)
+			return ret;
 
 		if (is_group)
 			selector = pinctrl_group_name_to_selector(dev, name);
-- 
2.17.1

[U-Boot] [PATCH v2 04/14] dt-bindings: gpio: document the new pull-up/pull-down flags

[Patrick Delaunay]

This commit extends the flags that can be used in GPIO specifiers to
indicate if a pull-up resistor or pull-down resistor should be
enabled.

It is the backport of linux commit ede033e1e863c ('dt-bindings:
gpio: document the new pull-up/pull-down flags')
from Thomas Petazzoni <thomas.petazzoni@bootlin.com>
and integrated in v5.1-rc1
https://github.com/torvalds/linux/commit/ede033e1e863c

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

Changes in v2: None

 doc/device-tree-bindings/gpio/gpio.txt | 24 ++++++++++++++++++++++++
 include/dt-bindings/gpio/gpio.h        |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt
index e146917ff3..e9ef0212af 100644
--- a/doc/device-tree-bindings/gpio/gpio.txt
+++ b/doc/device-tree-bindings/gpio/gpio.txt
@@ -65,6 +65,30 @@ Example of a node using GPIOs:
 GPIO_ACTIVE_HIGH is 0, so in this example gpio-specifier is "18 0" and encodes
 GPIO pin number, and GPIO flags as accepted by the "qe_pio_e" gpio-controller.
 
+Optional standard bitfield specifiers for the last cell:
+
+- Bit 0: 0 means active high, 1 means active low
+- Bit 1: 0 mean push-pull wiring, see:
+           https://en.wikipedia.org/wiki/Push-pull_output
+         1 means single-ended wiring, see:
+           https://en.wikipedia.org/wiki/Single-ended_triode
+- Bit 2: 0 means open-source, 1 means open drain, see:
+           https://en.wikipedia.org/wiki/Open_collector
+- Bit 3: 0 means the output should be maintained during sleep/low-power mode
+         1 means the output state can be lost during sleep/low-power mode
+- Bit 4: 0 means no pull-up resistor should be enabled
+         1 means a pull-up resistor should be enabled
+         This setting only applies to hardware with a simple on/off
+         control for pull-up configuration. If the hardware has more
+         elaborate pull-up configuration, it should be represented
+         using a pin control binding.
+- Bit 5: 0 means no pull-down resistor should be enabled
+         1 means a pull-down resistor should be enabled
+         This setting only applies to hardware with a simple on/off
+         control for pull-down configuration. If the hardware has more
+         elaborate pull-down configuration, it should be represented
+         using a pin control binding.
+
 1.1) GPIO specifier best practices
 ----------------------------------
 
diff --git a/include/dt-bindings/gpio/gpio.h b/include/dt-bindings/gpio/gpio.h
index 2cc10ae4bb..c029467e82 100644
--- a/include/dt-bindings/gpio/gpio.h
+++ b/include/dt-bindings/gpio/gpio.h
@@ -33,4 +33,10 @@
 #define GPIO_PERSISTENT 0
 #define GPIO_TRANSITORY 8
 
+/* Bit 4 express pull up */
+#define GPIO_PULL_UP 16
+
+/* Bit 5 express pull down */
+#define GPIO_PULL_DOWN 32
+
 #endif
-- 
2.17.1

[U-Boot] [PATCH v2 05/14] gpio: remove GPIOD_REQUESTED

[Patrick Delaunay]

Remove the define GPIOD_REQUESTED as it is never used
and use BIT() macro for other defines.

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

Changes in v2:
- remove the unused GPIOD_REQUESTED

 include/asm-generic/gpio.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6cf18744f..7da2015805 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -117,11 +117,10 @@ struct udevice;
 struct gpio_desc {
 	struct udevice *dev;	/* Device, NULL for invalid GPIO */
 	unsigned long flags;
-#define GPIOD_REQUESTED		(1 << 0)	/* Requested/claimed */
-#define GPIOD_IS_OUT		(1 << 1)	/* GPIO is an output */
-#define GPIOD_IS_IN		(1 << 2)	/* GPIO is an input */
-#define GPIOD_ACTIVE_LOW	(1 << 3)	/* value has active low */
-#define GPIOD_IS_OUT_ACTIVE	(1 << 4)	/* set output active */
+#define GPIOD_IS_OUT		BIT(1)	/* GPIO is an output */
+#define GPIOD_IS_IN		BIT(2)	/* GPIO is an input */
+#define GPIOD_ACTIVE_LOW	BIT(3)	/* value has active low */
+#define GPIOD_IS_OUT_ACTIVE	BIT(4)	/* set output active */
 
 	uint offset;		/* GPIO offset within the device */
 	/*
-- 
2.17.1

[U-Boot] [PATCH v2 06/14] gpio: remove the open_drain API and ops

[Patrick Delaunay]

This patch removes the ops get_open_drain/set_open_drain
and the API dm_gpio_get_open_drain/dm_gpio_set_open_drain.

The ops only provided in one driver (mpc8xxx gpio) and the
associated API is never called in boards.

This patch prepare a more generic set/get_dir_flags ops,
including the open drain property.

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

Changes in v2:
- remove the open_drain API and ops

 arch/sandbox/include/asm/gpio.h | 20 ------------------
 drivers/gpio/gpio-uclass.c      | 36 ---------------------------------
 drivers/gpio/mpc8xxx_gpio.c     | 22 --------------------
 drivers/gpio/sandbox.c          | 35 --------------------------------
 include/asm-generic/gpio.h      | 34 -------------------------------
 test/dm/gpio.c                  |  7 -------
 6 files changed, 154 deletions(-)

diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h
index de8ac37f42..cfb803bb3b 100644
--- a/arch/sandbox/include/asm/gpio.h
+++ b/arch/sandbox/include/asm/gpio.h
@@ -42,26 +42,6 @@ int sandbox_gpio_get_value(struct udevice *dev, unsigned int offset);
  */
 int sandbox_gpio_set_value(struct udevice *dev, unsigned int offset, int value);
 
-/**
- * Set or reset the simulated open drain mode of a GPIO (used only in sandbox
- * test code)
- *
- * @param gp	GPIO number
- * @param value	value to set (0 for enabled open drain mode, non-zero for
- * 		disabled)
- * @return -1 on error, 0 if ok
- */
-int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value);
-
-/**
- * Return the state of the simulated open drain mode of a GPIO (used only in
- * sandbox test code)
- *
- * @param gp	GPIO number
- * @return -1 on error, 0 if GPIO is input, >0 if output
- */
-int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset);
-
 /**
  * Return the simulated direction of a GPIO (used only in sandbox test code)
  *
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 90fbed455b..eb599cbcfd 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -491,38 +491,6 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value)
 	return 0;
 }
 
-int dm_gpio_get_open_drain(struct gpio_desc *desc)
-{
-	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
-	int ret;
-
-	ret = check_reserved(desc, "get_open_drain");
-	if (ret)
-		return ret;
-
-	if (ops->set_open_drain)
-		return ops->get_open_drain(desc->dev, desc->offset);
-	else
-		return -ENOSYS;
-}
-
-int dm_gpio_set_open_drain(struct gpio_desc *desc, int value)
-{
-	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
-	int ret;
-
-	ret = check_reserved(desc, "set_open_drain");
-	if (ret)
-		return ret;
-
-	if (ops->set_open_drain)
-		ret = ops->set_open_drain(desc->dev, desc->offset, value);
-	else
-		return 0; /* feature not supported -> ignore setting */
-
-	return ret;
-}
-
 int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
 {
 	struct udevice *dev = desc->dev;
@@ -1053,10 +1021,6 @@ static int gpio_post_bind(struct udevice *dev)
 			ops->get_value += gd->reloc_off;
 		if (ops->set_value)
 			ops->set_value += gd->reloc_off;
-		if (ops->get_open_drain)
-			ops->get_open_drain += gd->reloc_off;
-		if (ops->set_open_drain)
-			ops->set_open_drain += gd->reloc_off;
 		if (ops->get_function)
 			ops->get_function += gd->reloc_off;
 		if (ops->xlate)
diff --git a/drivers/gpio/mpc8xxx_gpio.c b/drivers/gpio/mpc8xxx_gpio.c
index c273c2c8a4..c23f5b5227 100644
--- a/drivers/gpio/mpc8xxx_gpio.c
+++ b/drivers/gpio/mpc8xxx_gpio.c
@@ -144,26 +144,6 @@ static int mpc8xxx_gpio_get_value(struct udevice *dev, uint gpio)
 	return !!mpc8xxx_gpio_get_val(data->base, gpio_mask(gpio));
 }
 
-static int mpc8xxx_gpio_get_open_drain(struct udevice *dev, uint gpio)
-{
-	struct mpc8xxx_gpio_data *data = dev_get_priv(dev);
-
-	return !!mpc8xxx_gpio_open_drain_val(data->base, gpio_mask(gpio));
-}
-
-static int mpc8xxx_gpio_set_open_drain(struct udevice *dev, uint gpio,
-				       int value)
-{
-	struct mpc8xxx_gpio_data *data = dev_get_priv(dev);
-
-	if (value)
-		mpc8xxx_gpio_open_drain_on(data->base, gpio_mask(gpio));
-	else
-		mpc8xxx_gpio_open_drain_off(data->base, gpio_mask(gpio));
-
-	return 0;
-}
-
 static int mpc8xxx_gpio_get_function(struct udevice *dev, uint gpio)
 {
 	struct mpc8xxx_gpio_data *data = dev_get_priv(dev);
@@ -240,8 +220,6 @@ static const struct dm_gpio_ops gpio_mpc8xxx_ops = {
 	.direction_output	= mpc8xxx_gpio_direction_output,
 	.get_value		= mpc8xxx_gpio_get_value,
 	.set_value		= mpc8xxx_gpio_set_value,
-	.get_open_drain		= mpc8xxx_gpio_get_open_drain,
-	.set_open_drain		= mpc8xxx_gpio_set_open_drain,
 	.get_function		= mpc8xxx_gpio_get_function,
 };
 
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 2ef5c67ad5..91e8e0677e 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -14,7 +14,6 @@
 /* Flags for each GPIO */
 #define GPIOF_OUTPUT	(1 << 0)	/* Currently set as an output */
 #define GPIOF_HIGH	(1 << 1)	/* Currently set high */
-#define GPIOF_ODR	(1 << 2)	/* Currently set to open drain mode */
 
 struct gpio_state {
 	const char *label;	/* label given by requester */
@@ -70,16 +69,6 @@ int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value)
 	return set_gpio_flag(dev, offset, GPIOF_HIGH, value);
 }
 
-int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset)
-{
-	return get_gpio_flag(dev, offset, GPIOF_ODR);
-}
-
-int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value)
-{
-	return set_gpio_flag(dev, offset, GPIOF_ODR, value);
-}
-
 int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset)
 {
 	return get_gpio_flag(dev, offset, GPIOF_OUTPUT);
@@ -134,28 +123,6 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value)
 	return sandbox_gpio_set_value(dev, offset, value);
 }
 
-/* read GPIO ODR value of port 'offset' */
-static int sb_gpio_get_open_drain(struct udevice *dev, unsigned offset)
-{
-	debug("%s: offset:%u\n", __func__, offset);
-
-	return sandbox_gpio_get_open_drain(dev, offset);
-}
-
-/* write GPIO ODR value to port 'offset' */
-static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value)
-{
-	debug("%s: offset:%u, value = %d\n", __func__, offset, value);
-
-	if (!sandbox_gpio_get_direction(dev, offset)) {
-		printf("sandbox_gpio: error: set_open_drain on input gpio %u\n",
-		       offset);
-		return -1;
-	}
-
-	return sandbox_gpio_set_open_drain(dev, offset, value);
-}
-
 static int sb_gpio_get_function(struct udevice *dev, unsigned offset)
 {
 	if (get_gpio_flag(dev, offset, GPIOF_OUTPUT))
@@ -186,8 +153,6 @@ static const struct dm_gpio_ops gpio_sandbox_ops = {
 	.direction_output	= sb_gpio_direction_output,
 	.get_value		= sb_gpio_get_value,
 	.set_value		= sb_gpio_set_value,
-	.get_open_drain		= sb_gpio_get_open_drain,
-	.set_open_drain		= sb_gpio_set_open_drain,
 	.get_function		= sb_gpio_get_function,
 	.xlate			= sb_gpio_xlate,
 };
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 7da2015805..454578c8d2 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -253,8 +253,6 @@ struct dm_gpio_ops {
 				int value);
 	int (*get_value)(struct udevice *dev, unsigned offset);
 	int (*set_value)(struct udevice *dev, unsigned offset, int value);
-	int (*get_open_drain)(struct udevice *dev, unsigned offset);
-	int (*set_open_drain)(struct udevice *dev, unsigned offset, int value);
 	/**
 	 * get_function() Get the GPIO function
 	 *
@@ -585,38 +583,6 @@ int dm_gpio_get_value(const struct gpio_desc *desc);
 
 int dm_gpio_set_value(const struct gpio_desc *desc, int value);
 
-/**
- * dm_gpio_get_open_drain() - Check if open-drain-mode of a GPIO is active
- *
- * This checks if open-drain-mode for a GPIO is enabled or not. This method is
- * optional.
- *
- * @desc:	GPIO description containing device, offset and flags,
- *		previously returned by gpio_request_by_name()
- * @return Value of open drain mode for GPIO (0 for inactive, 1 for active) or
- *	   -ve on error
- */
-int dm_gpio_get_open_drain(struct gpio_desc *desc);
-
-/**
- * dm_gpio_set_open_drain() - Switch open-drain-mode of a GPIO on or off
- *
- * This enables or disables open-drain mode for a GPIO. This method is
- * optional; if the driver does not support it, nothing happens when the method
- * is called.
- *
- * In open-drain mode, instead of actively driving the output (Push-pull
- * output), the GPIO's pin is connected to the collector (for a NPN transistor)
- * or the drain (for a MOSFET) of a transistor, respectively. The pin then
- * either forms an open circuit or a connection to ground, depending on the
- * state of the transistor.
- *
- * @desc:	GPIO description containing device, offset and flags,
- *		previously returned by gpio_request_by_name()
- * @return 0 if OK, -ve on error
- */
-int dm_gpio_set_open_drain(struct gpio_desc *desc, int value);
-
 /**
  * dm_gpio_set_dir() - Set the direction for a GPIO
  *
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index bb4b20cea9..8abf0cd8c8 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -72,13 +72,6 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	ut_assertok(ops->set_value(dev, offset, 1));
 	ut_asserteq(1, ops->get_value(dev, offset));
 
-	/* Make it an open drain output, and reset it */
-	ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));
-	ut_assertok(ops->set_open_drain(dev, offset, 1));
-	ut_asserteq(1, sandbox_gpio_get_open_drain(dev, offset));
-	ut_assertok(ops->set_open_drain(dev, offset, 0));
-	ut_asserteq(0, sandbox_gpio_get_open_drain(dev, offset));
-
 	/* Make it an input */
 	ut_assertok(ops->direction_input(dev, offset));
 	ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
-- 
2.17.1

[U-Boot] [PATCH v2 07/14] gpio: add gpio descriptor initialization helper

[Patrick Delaunay]

Add a helper function gpio_desc_init() to initialize the gpio descriptor;
with this function the flags will be always set to 0.

It wasn't the case before this patch in dm_gpio_lookup_name.

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

Changes in v2:
- add gpio descriptor initialization helper

 drivers/gpio/gpio-uclass.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index eb599cbcfd..0870458e96 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -18,6 +18,22 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/**
+ * gpio_desc_init() - Initialize the GPIO descriptor
+ *
+ * @desc:	GPIO descriptor to initialize
+ * @dev:	GPIO device
+ * @offset:	Offset of device GPIO
+ */
+static void gpio_desc_init(struct gpio_desc *desc,
+			   struct udevice *dev,
+			   uint offset)
+{
+	desc->dev = dev;
+	desc->offset = offset;
+	desc->flags = 0;
+}
+
 /**
  * gpio_to_device() - Convert global GPIO number to device, number
  *
@@ -41,9 +57,7 @@ static int gpio_to_device(unsigned int gpio, struct gpio_desc *desc)
 		uc_priv = dev_get_uclass_priv(dev);
 		if (gpio >= uc_priv->gpio_base &&
 		    gpio < uc_priv->gpio_base + uc_priv->gpio_count) {
-			desc->dev = dev;
-			desc->offset = gpio - uc_priv->gpio_base;
-			desc->flags = 0;
+			gpio_desc_init(desc, dev, gpio - uc_priv->gpio_base);
 			return 0;
 		}
 	}
@@ -85,8 +99,7 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 	if (!dev)
 		return ret ? ret : -EINVAL;
 
-	desc->dev = dev;
-	desc->offset = offset;
+	gpio_desc_init(desc, dev, offset);
 
 	return 0;
 }
@@ -772,9 +785,7 @@ static int gpio_request_tail(int ret, const char *nodename,
 			     struct gpio_desc *desc, int flags,
 			     bool add_index, struct udevice *gpio_dev)
 {
-	desc->dev = gpio_dev;
-	desc->offset = 0;
-	desc->flags = 0;
+	gpio_desc_init(desc, gpio_dev, 0);
 	if (ret)
 		goto err;
 
-- 
2.17.1

[U-Boot] [PATCH v2 08/14] gpio: add ops for configuration with dir flags

[Patrick Delaunay]

This commit manages the dir flags that can be used in gpio specifiers
to indicate if a pull-up resistor or pull-down resistor should be
enabled for output gpio (GPIO_PULL_UP, GPIO_PULL_DOWN) and the
Open Drain/Open Source configuration for input gpio
(GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE).

These flags are already supported in Linux kernel in gpiolib;
this patch provides the same support in U-Boot.

The dir flags are managed in gpio drivers with two optional ops in gpio
uclass: set_dir_flags and get_dir_flags.

- ops set_dir_flags() set the direction and configuration of each GPIO
  with a only call to dm_gpio_set_dir_flags() / dm_gpio_set_dir()
  and respecting the configuration provided by device tree
  (saved in desc->flags).

- ops get_dir_flags() return dynamically the current gpio configuration,
  it is used by the new API dm_gpio_get_dir_flags().

When these optional ops are absent, the gpio uclass use the mandatory ops
(direction_output, direction_input, get_value) and desc->flags to manage
only the main dir flags:
- GPIOD_IS_IN
- GPIOD_IS_OUT
- GPIOD_IS_OUT_ACTIVE
- GPIOD_ACTIVE_LOW

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

Changes in v2:
- change the proposed ops for pin config to set_dir_flags/get_dir_flags
- reused the existing API dm_gpio_set_dir_flags/dm_gpio_set_dir
- add a new API dm_gpio_get_dir_flags

 drivers/gpio/gpio-uclass.c | 157 +++++++++++++++++++++++++++++++------
 include/asm-generic/gpio.h |  65 +++++++++++++--
 2 files changed, 192 insertions(+), 30 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 0870458e96..241293f4b4 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -140,8 +140,27 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
 	if (args->args_count < 2)
 		return 0;
 
+	desc->flags = 0;
 	if (args->args[1] & GPIO_ACTIVE_LOW)
-		desc->flags = GPIOD_ACTIVE_LOW;
+		desc->flags |= GPIOD_ACTIVE_LOW;
+
+	/*
+	 * need to test 2 bits for gpio output binding:
+	 * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN (0x4)
+	 * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) | LINE_OPEN_SOURCE (0x0)
+	 */
+	if (args->args[1] & GPIO_SINGLE_ENDED) {
+		if (args->args[1] & GPIO_LINE_OPEN_DRAIN)
+			desc->flags |= GPIOD_OPEN_DRAIN;
+		else
+			desc->flags |= GPIOD_OPEN_SOURCE;
+	}
+
+	if (args->args[1] & GPIO_PULL_UP)
+		desc->flags |= GPIOD_PULL_UP;
+
+	if (args->args[1] & GPIO_PULL_DOWN)
+		desc->flags |= GPIOD_PULL_DOWN;
 
 	return 0;
 }
@@ -476,18 +495,24 @@ int gpio_direction_output(unsigned gpio, int value)
 							desc.offset, value);
 }
 
-int dm_gpio_get_value(const struct gpio_desc *desc)
+static int _gpio_get_value(const struct gpio_desc *desc)
 {
 	int value;
+
+	value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset);
+
+	return desc->flags & GPIOD_ACTIVE_LOW ? !value : value;
+}
+
+int dm_gpio_get_value(const struct gpio_desc *desc)
+{
 	int ret;
 
 	ret = check_reserved(desc, "get_value");
 	if (ret)
 		return ret;
 
-	value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset);
-
-	return desc->flags & GPIOD_ACTIVE_LOW ? !value : value;
+	return _gpio_get_value(desc);
 }
 
 int dm_gpio_set_value(const struct gpio_desc *desc, int value)
@@ -504,39 +529,119 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value)
 	return 0;
 }
 
-int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
+/* check dir flags invalid configuration */
+static int check_dir_flags(ulong flags)
+{
+	if ((flags & GPIOD_IS_OUT) && (flags & GPIOD_IS_IN))
+		return -EINVAL;
+
+	if ((flags & GPIOD_PULL_UP) && (flags & GPIOD_PULL_DOWN))
+		return -EINVAL;
+
+	if ((flags & GPIOD_OPEN_DRAIN) && (flags & GPIOD_OPEN_SOURCE))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
 {
 	struct udevice *dev = desc->dev;
 	struct dm_gpio_ops *ops = gpio_get_ops(dev);
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
 	int ret;
 
-	ret = check_reserved(desc, "set_dir");
-	if (ret)
-		return ret;
+	ret = check_dir_flags(flags);
+	if (ret) {
+		dev_err(dev,
+			"%s error: set_dir_flags for gpio %s%d has invalid dir flags 0x%lx\n",
+			desc->dev->name,
+			uc_priv->bank_name ? uc_priv->bank_name : "",
+			desc->offset, flags);
 
-	if (flags & GPIOD_IS_OUT) {
-		int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
+		return ret;
+	}
 
-		if (flags & GPIOD_ACTIVE_LOW)
-			value = !value;
-		ret = ops->direction_output(dev, desc->offset, value);
-	} else  if (flags & GPIOD_IS_IN) {
-		ret = ops->direction_input(dev, desc->offset);
+	/* GPIOD_ are directly managed by driver in set_dir_flags*/
+	if (ops->set_dir_flags) {
+		ret = ops->set_dir_flags(dev, desc->offset, flags);
+	} else {
+		if (flags & GPIOD_IS_OUT) {
+			ret = ops->direction_output(dev, desc->offset,
+						    GPIOD_FLAGS_OUTPUT(flags));
+		} else if (flags & GPIOD_IS_IN) {
+			ret = ops->direction_input(dev, desc->offset);
+		}
 	}
+
+	return ret;
+}
+
+int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
+{
+	int ret;
+
+	ret = check_reserved(desc, "set_dir_flags");
 	if (ret)
 		return ret;
-	/*
-	 * Update desc->flags here, so that GPIO_ACTIVE_LOW is honoured in
-	 * futures
-	 */
-	desc->flags = flags;
 
-	return 0;
+	/* combine the requested flags (for IN/OUT) and the descriptor flags */
+	flags |= desc->flags;
+	ret = _dm_gpio_set_dir_flags(desc, flags);
+
+	/* update the descriptor flags */
+	if (ret)
+		desc->flags = flags;
+
+	return ret;
 }
 
 int dm_gpio_set_dir(struct gpio_desc *desc)
 {
-	return dm_gpio_set_dir_flags(desc, desc->flags);
+	int ret;
+
+	ret = check_reserved(desc, "set_dir");
+	if (ret)
+		return ret;
+
+	return _dm_gpio_set_dir_flags(desc, desc->flags);
+}
+
+int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags)
+{
+	struct udevice *dev = desc->dev;
+	struct dm_gpio_ops *ops = gpio_get_ops(dev);
+	int ret, value;
+	ulong dir_flags;
+
+	ret = check_reserved(desc, "get_dir_flags");
+	if (ret)
+		return ret;
+
+	/* GPIOD_ are directly provided by driver except GPIOD_ACTIVE_LOW*/
+	if (ops->get_dir_flags) {
+		ret = ops->get_dir_flags(dev, desc->offset, &dir_flags);
+		if (ret)
+			return ret;
+
+		/* GPIOD_ACTIVE_LOW is saved in desc->flags */
+		value = dir_flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
+		if (desc->flags & GPIOD_ACTIVE_LOW)
+			value = !value;
+		dir_flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE);
+		dir_flags |= (desc->flags & GPIOD_ACTIVE_LOW);
+		if (value)
+			dir_flags |= GPIOD_IS_OUT_ACTIVE;
+	} else {
+		dir_flags = desc->flags;
+		/* only GPIOD_IS_OUT_ACTIVE is provided by uclass */
+		dir_flags &= ~GPIOD_IS_OUT_ACTIVE;
+		if ((desc->flags & GPIOD_IS_OUT) && _gpio_get_value(desc))
+			dir_flags |= GPIOD_IS_OUT_ACTIVE;
+	}
+	*flags = dir_flags;
+
+	return 0;
 }
 
 /**
@@ -809,7 +914,7 @@ static int gpio_request_tail(int ret, const char *nodename,
 		debug("%s: dm_gpio_requestf failed\n", __func__);
 		goto err;
 	}
-	ret = dm_gpio_set_dir_flags(desc, flags | desc->flags);
+	ret = dm_gpio_set_dir_flags(desc, flags);
 	if (ret) {
 		debug("%s: dm_gpio_set_dir failed\n", __func__);
 		goto err;
@@ -1036,6 +1141,10 @@ static int gpio_post_bind(struct udevice *dev)
 			ops->get_function += gd->reloc_off;
 		if (ops->xlate)
 			ops->xlate += gd->reloc_off;
+		if (ops->set_dir_flags)
+			ops->set_dir_flags += gd->reloc_off;
+		if (ops->get_dir_flags)
+			ops->get_dir_flags += gd->reloc_off;
 
 		reloc_done++;
 	}
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 454578c8d2..c6991be1c9 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -117,10 +117,15 @@ struct udevice;
 struct gpio_desc {
 	struct udevice *dev;	/* Device, NULL for invalid GPIO */
 	unsigned long flags;
+
 #define GPIOD_IS_OUT		BIT(1)	/* GPIO is an output */
 #define GPIOD_IS_IN		BIT(2)	/* GPIO is an input */
 #define GPIOD_ACTIVE_LOW	BIT(3)	/* value has active low */
 #define GPIOD_IS_OUT_ACTIVE	BIT(4)	/* set output active */
+#define GPIOD_OPEN_DRAIN	BIT(5)	/* GPIO is open drain type */
+#define GPIOD_OPEN_SOURCE	BIT(6)	/* GPIO is open source type */
+#define GPIOD_PULL_UP		BIT(7)	/* GPIO has pull up enabled */
+#define GPIOD_PULL_DOWN		BIT(8)	/* GPIO has pull down enabled */
 
 	uint offset;		/* GPIO offset within the device */
 	/*
@@ -129,6 +134,12 @@ struct gpio_desc {
 	 */
 };
 
+/* helper to compute the value of the gpio output */
+#define GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE)
+#define GPIOD_FLAGS_OUTPUT(flags) \
+	(((((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_IS_OUT_ACTIVE) || \
+	  (((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_ACTIVE_LOW)) ? 1 : 0)
+
 /**
  * dm_gpio_is_valid() - Check if a GPIO is valid
  *
@@ -253,6 +264,7 @@ struct dm_gpio_ops {
 				int value);
 	int (*get_value)(struct udevice *dev, unsigned offset);
 	int (*set_value)(struct udevice *dev, unsigned offset, int value);
+
 	/**
 	 * get_function() Get the GPIO function
 	 *
@@ -287,6 +299,37 @@ struct dm_gpio_ops {
 	 */
 	int (*xlate)(struct udevice *dev, struct gpio_desc *desc,
 		     struct ofnode_phandle_args *args);
+
+	/**
+	 * set_dir_flags() - Set GPIO dir flags
+	 *
+	 * This function should set up the GPIO configuration according to the
+	 * information provide by the direction flags bitfield.
+	 *
+	 * This method is optional.
+	 *
+	 * @dev:	GPIO device
+	 * @offset:	GPIO offset within that device
+	 * @flags:	GPIO configuration to use
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*set_dir_flags)(struct udevice *dev, unsigned int offset,
+			     ulong flags);
+
+	/**
+	 * get_dir_flags() - Get GPIO dir flags
+	 *
+	 * This function return the GPIO direction flags used.
+	 *
+	 * This method is optional.
+	 *
+	 * @dev:	GPIO device
+	 * @offset:	GPIO offset within that device
+	 * @flags:	place to put the used direction flags by GPIO
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*get_dir_flags)(struct udevice *dev, unsigned int offset,
+			     ulong *flags);
 };
 
 /**
@@ -586,8 +629,7 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value);
 /**
  * dm_gpio_set_dir() - Set the direction for a GPIO
  *
- * This sets up the direction according tot the provided flags. It will do
- * nothing unless the direction is actually specified.
+ * This sets up the direction according to the GPIO flags: desc->flags.
  *
  * @desc:	GPIO description containing device, offset and flags,
  *		previously returned by gpio_request_by_name()
@@ -596,11 +638,10 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value);
 int dm_gpio_set_dir(struct gpio_desc *desc);
 
 /**
- * dm_gpio_set_dir_flags() - Set direction using specific flags
+ * dm_gpio_set_dir_flags() - Set direction using add flags
  *
- * This is like dm_gpio_set_dir() except that the flags value is provided
- * instead of being used from desc->flags. This is needed because in many
- * cases the GPIO description does not include direction information.
+ * This sets up the direction according tot the provided flags and the GPIO
+ * description (desc->flags) which include direction information.
  * Note that desc->flags is updated by this function.
  *
  * @desc:	GPIO description containing device, offset and flags,
@@ -610,6 +651,18 @@ int dm_gpio_set_dir(struct gpio_desc *desc);
  */
 int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags);
 
+/**
+ * dm_gpio_get_dir_flags() - Get direction flags
+ *
+ * read the current direction flags
+ *
+ * @desc:	GPIO description containing device, offset and flags,
+ *		previously returned by gpio_request_by_name()
+ * @flags:	place to put the used flags
+ * @return 0 if OK, -ve on error, in which case desc->flags is not updated
+ */
+int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags);
+
 /**
  * gpio_get_number() - Get the global GPIO number of a GPIO
  *
-- 
2.17.1

[U-Boot] [PATCH v2 09/14] dt-bindings: gpio: alignment with kernel v5.3

[Patrick Delaunay]

Update the binding file for gpio, it is just an alignment
with kernel v5.3.
The U-Boot code example for gpio-hog (not directly linked
to binding) is moved in a new file doc/README.gpio.
[commit 21676b706e99 ("gpio: fixes for gpio-hog support")
& 'commit 4762a9988ede ("gpio: add gpio-hog support")']

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

Changes in v2: None

 doc/README.gpio                        |  42 ++++
 doc/device-tree-bindings/gpio/gpio.txt | 280 ++++++++++++++-----------
 2 files changed, 196 insertions(+), 126 deletions(-)
 create mode 100644 doc/README.gpio

diff --git a/doc/README.gpio b/doc/README.gpio
new file mode 100644
index 0000000000..548ff37b8c
--- /dev/null
+++ b/doc/README.gpio
@@ -0,0 +1,42 @@
+
+GPIO hog (CONFIG_GPIO_HOG)
+--------
+
+All the GPIO hog are initialized in gpio_hog_probe_all() function called in
+board_r.c just before board_late_init() but you can also acces directly to
+the gpio with gpio_hog_lookup_name().
+
+
+Example, for the device tree:
+
+        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;
+                        line-name = "foo-bar-gpio";
+                        gpios = <7 GPIO_ACTIVE_LOW>;
+                };
+        };
+
+You can than access the gpio in your board code with:
+
+	struct gpio_desc *desc;
+	int ret;
+
+	ret = gpio_hog_lookup_name("boot_rescue", &desc);
+	if (ret)
+		return;
+	if (dm_gpio_get_value(desc) == 1)
+		printf("\nBooting into Rescue System\n");
+	else if (dm_gpio_get_value(desc) == 0)
+		printf("\nBoot normal\n");
diff --git a/doc/device-tree-bindings/gpio/gpio.txt b/doc/device-tree-bindings/gpio/gpio.txt
index e9ef0212af..1481ed607d 100644
--- a/doc/device-tree-bindings/gpio/gpio.txt
+++ b/doc/device-tree-bindings/gpio/gpio.txt
@@ -4,19 +4,12 @@ Specifying GPIO information for devices
 1) gpios property
 -----------------
 
-Nodes that makes use of GPIOs should specify them using one or more
-properties, each containing a 'gpio-list':
-
-	gpio-list ::= <single-gpio> [gpio-list]
-	single-gpio ::= <gpio-phandle> <gpio-specifier>
-	gpio-phandle : phandle to gpio controller node
-	gpio-specifier : Array of #gpio-cells specifying specific gpio
-			 (controller specific)
-
 GPIO properties should be named "[<name>-]gpios", with <name> being the purpose
 of this GPIO for the device. While a non-existent <name> is considered valid
 for compatibility reasons (resolving to the "gpios" property), it is not allowed
-for new bindings.
+for new bindings. Also, GPIO properties named "[<name>-]gpio" are valid and old
+bindings use it, but are only supported for compatibility reasons and should not
+be used for newer bindings since it has been deprecated.
 
 GPIO properties can contain one or more GPIO phandles, but only in exceptional
 cases should they contain more than one. If your device uses several GPIOs with
@@ -31,30 +24,28 @@ The following example could be used to describe GPIO pins used as device enable
 and bit-banged data signals:
 
 	gpio1: gpio1 {
-		gpio-controller
-		 #gpio-cells = <2>;
-	};
-	gpio2: gpio2 {
-		gpio-controller
-		 #gpio-cells = <1>;
+		gpio-controller;
+		#gpio-cells = <2>;
 	};
 	[...]
 
-	enable-gpios = <&gpio2 2>;
 	data-gpios = <&gpio1 12 0>,
 		     <&gpio1 13 0>,
 		     <&gpio1 14 0>,
 		     <&gpio1 15 0>;
 
-Note that gpio-specifier length is controller dependent.  In the
-above example, &gpio1 uses 2 cells to specify a gpio, while &gpio2
-only uses one.
+In the above example, &gpio1 uses 2 cells to specify a gpio. The first cell is
+a local offset to the GPIO line and the second cell represent consumer flags,
+such as if the consumer desire the line to be active low (inverted) or open
+drain. This is the recommended practice.
 
-gpio-specifier may encode: bank, pin position inside the bank,
-whether pin is open-drain and whether pin is logically inverted.
-Exact meaning of each specifier cell is controller specific, and must
-be documented in the device tree binding for the device. Use the macros
-defined in include/dt-bindings/gpio/gpio.h whenever possible:
+The exact meaning of each specifier cell is controller specific, and must be
+documented in the device tree binding for the device, but it is strongly
+recommended to use the two-cell approach.
+
+Most controllers are specifying a generic flag bitfield in the last cell, so
+for these, use the macros defined in
+include/dt-bindings/gpio/gpio.h whenever possible:
 
 Example of a node using GPIOs:
 
@@ -140,6 +131,80 @@ Every GPIO controller node must contain both an empty "gpio-controller"
 property, and a #gpio-cells integer property, which indicates the number of
 cells in a gpio-specifier.
 
+Some system-on-chips (SoCs) use the concept of GPIO banks. A GPIO bank is an
+instance of a hardware IP core on a silicon die, usually exposed to the
+programmer as a coherent range of I/O addresses. Usually each such bank is
+exposed in the device tree as an individual gpio-controller node, reflecting
+the fact that the hardware was synthesized by reusing the same IP block a
+few times over.
+
+Optionally, a GPIO controller may have a "ngpios" property. This property
+indicates the number of in-use slots of available slots for GPIOs. The
+typical example is something like this: the hardware register is 32 bits
+wide, but only 18 of the bits have a physical counterpart. The driver is
+generally written so that all 32 bits can be used, but the IP block is reused
+in a lot of designs, some using all 32 bits, some using 18 and some using
+12. In this case, setting "ngpios = <18>;" informs the driver that only the
+first 18 GPIOs, at local offset 0 .. 17, are in use.
+
+If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, an
+additional set of tuples is needed to specify which GPIOs are unusable, with
+the gpio-reserved-ranges binding. This property indicates the start and size
+of the GPIOs that can't be used.
+
+Optionally, a GPIO controller may have a "gpio-line-names" property. This is
+an array of strings defining the names of the GPIO lines going out of the
+GPIO controller. This name should be the most meaningful producer name
+for the system, such as a rail name indicating the usage. Package names
+such as pin name are discouraged: such lines have opaque names (since they
+are by definition generic purpose) and such names are usually not very
+helpful. For example "MMC-CD", "Red LED Vdd" and "ethernet reset" are
+reasonable line names as they describe what the line is used for. "GPIO0"
+is not a good name to give to a GPIO line. Placeholders are discouraged:
+rather use the "" (blank string) if the use of the GPIO line is undefined
+in your design. The names are assigned starting from line offset 0 from
+left to right from the passed array. An incomplete array (where the number
+of passed named are less than ngpios) will still be used up until the last
+provided valid line index.
+
+Example:
+
+gpio-controller at 00000000 {
+	compatible = "foo";
+	reg = <0x00000000 0x1000>;
+	gpio-controller;
+	#gpio-cells = <2>;
+	ngpios = <18>;
+	gpio-reserved-ranges = <0 4>, <12 2>;
+	gpio-line-names = "MMC-CD", "MMC-WP", "VDD eth", "RST eth", "LED R",
+		"LED G", "LED B", "Col A", "Col B", "Col C", "Col D",
+		"Row A", "Row B", "Row C", "Row D", "NMI button",
+		"poweroff", "reset";
+}
+
+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 each GPIO to
+	      affect. Shall contain an integer multiple of the number of cells
+	      specified in its parent node (GPIO controller node).
+Only one of the following properties scanned in the order shown below.
+This means that when multiple properties are present they will be searched
+in the order presented below and the first match is taken as the intended
+configuration.
+- 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 of two SOC GPIO banks defined as gpio-controller nodes:
 
 	qe_pio_a: gpio-controller at 1400 {
@@ -161,46 +226,40 @@ Example of two SOC GPIO banks defined as gpio-controller nodes:
 
 Some or all of the GPIOs provided by a GPIO controller may be routed to pins
 on the package via a pin controller. This allows muxing those pins between
-GPIO and other functions.
+GPIO and other functions. It is a fairly common practice among silicon
+engineers.
+
+2.2) Ordinary (numerical) GPIO ranges
+-------------------------------------
 
 It is useful to represent which GPIOs correspond to which pins on which pin
-controllers. The gpio-ranges property described below represents this, and
-contains information structures as follows:
-
-	gpio-range-list ::= <single-gpio-range> [gpio-range-list]
-	single-gpio-range ::= <numeric-gpio-range> | <named-gpio-range>
-	numeric-gpio-range ::=
-			<pinctrl-phandle> <gpio-base> <pinctrl-base> <count>
-	named-gpio-range ::= <pinctrl-phandle> <gpio-base> '<0 0>'
-	pinctrl-phandle : phandle to pin controller node
-	gpio-base : Base GPIO ID in the GPIO controller
-	pinctrl-base : Base pinctrl pin ID in the pin controller
-	count : The number of GPIOs/pins in this range
-
-The "pin controller node" mentioned above must conform to the bindings
-described in ../pinctrl/pinctrl-bindings.txt.
-
-In case named gpio ranges are used (ranges with both <pinctrl-base> and
-<count> set to 0), the property gpio-ranges-group-names contains one string
-for every single-gpio-range in gpio-ranges:
-	gpiorange-names-list ::= <gpiorange-name> [gpiorange-names-list]
-	gpiorange-name : Name of the pingroup associated to the GPIO range in
-			the respective pin controller.
-
-Elements of gpiorange-names-list corresponding to numeric ranges contain
-the empty string. Elements of gpiorange-names-list corresponding to named
-ranges contain the name of a pin group defined in the respective pin
-controller. The number of pins/GPIOs in the range is the number of pins in
-that pin group.
+controllers. The gpio-ranges property described below represents this with
+a discrete set of ranges mapping pins from the pin controller local number space
+to pins in the GPIO controller local number space.
 
-Previous versions of this binding required all pin controller nodes that
-were referenced by any gpio-ranges property to contain a property named
-#gpio-range-cells with value <3>. This requirement is now deprecated.
-However, that property may still exist in older device trees for
-compatibility reasons, and would still be required even in new device
-trees that need to be compatible with older software.
+The format is: <[pin controller phandle], [GPIO controller offset],
+                [pin controller offset], [number of pins]>;
+
+The GPIO controller offset pertains to the GPIO controller node containing the
+range definition.
+
+The pin controller node referenced by the phandle must conform to the bindings
+described in pinctrl/pinctrl-bindings.txt.
+
+Each offset runs from 0 to N. It is perfectly fine to pile any number of
+ranges with just one pin-to-GPIO line mapping if the ranges are concocted, but
+in practice these ranges are often lumped in discrete sets.
+
+Example:
 
-Example 1:
+    gpio-ranges = <&foo 0 20 10>, <&bar 10 50 20>;
+
+This means:
+- pins 20..29 on pin controller "foo" is mapped to GPIO line 0..9 and
+- pins 50..69 on pin controller "bar" is mapped to GPIO line 10..29
+
+
+Verbose example:
 
 	qe_pio_e: gpio-controller at 1460 {
 		#gpio-cells = <2>;
@@ -211,12 +270,33 @@ Example 1:
 	};
 
 Here, a single GPIO controller has GPIOs 0..9 routed to pin controller
-pinctrl1's pins 20..29, and GPIOs 10..19 routed to pin controller pinctrl2's
-pins 50..59.
+pinctrl1's pins 20..29, and GPIOs 10..29 routed to pin controller pinctrl2's
+pins 50..69.
+
+
+2.3) GPIO ranges from named pin groups
+--------------------------------------
 
-Example 2:
+It is also possible to use pin groups for gpio ranges when pin groups are the
+easiest and most convenient mapping.
 
-	gpio_pio_i: gpio-controller at 14B0 {
+Both both <pinctrl-base> and <count> must set to 0 when using named pin groups
+names.
+
+The property gpio-ranges-group-names must contain exactly one string for each
+range.
+
+Elements of gpio-ranges-group-names must contain the name of a pin group
+defined in the respective pin controller. The number of pins/GPIO lines in the
+range is the number of pins in that pin group. The number of pins of that
+group is defined int the implementation and not in the device tree.
+
+If numerical and named pin groups are mixed, the string corresponding to a
+numerical pin range in gpio-ranges-group-names must be empty.
+
+Example:
+
+	gpio_pio_i: gpio-controller at 14b0 {
 		#gpio-cells = <2>;
 		compatible = "fsl,qe-pario-bank-e", "fsl,qe-pario-bank";
 		reg = <0x1480 0x18>;
@@ -231,66 +311,14 @@ Example 2:
 						"bar";
 	};
 
-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 !
+Here, three GPIO ranges are defined referring to two pin controllers.
 
-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.
+pinctrl1 GPIO ranges are defined using pin numbers whereas the GPIO ranges
+in pinctrl2 are defined using the pin groups named "foo" and "bar".
 
-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;
-                        line-name = "foo-bar-gpio";
-                        gpios = <7 GPIO_ACTIVE_LOW>;
-                };
-        };
-
-For the above Example you can than access the gpio in your boardcode
-with:
-
-	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");
+Previous versions of this binding required all pin controller nodes that
+were referenced by any gpio-ranges property to contain a property named
+#gpio-range-cells with value <3>. This requirement is now deprecated.
+However, that property may still exist in older device trees for
+compatibility reasons, and would still be required even in new device
+trees that need to be compatible with older software.
-- 
2.17.1

[U-Boot] [PATCH v2 10/14] pinctrl: sandbox: Add mux information in get_pin_muxing

[Patrick Delaunay]

Add param information in pin information output.
This update prepare unitary test for pin configuration
in pinctrl node.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 drivers/pinctrl/pinctrl-sandbox.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c
index 0786afe747..d1a21f0f19 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -54,6 +54,10 @@ static const struct pinconf_param sandbox_conf_params[] = {
 	{ "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 },
 };
 
+/* bitfield used to save param and value of each pin/selector */
+static unsigned int sandbox_pins_param[ARRAY_SIZE(sandbox_pins)];
+static unsigned int sandbox_pins_value[ARRAY_SIZE(sandbox_pins)];
+
 static int sandbox_get_pins_count(struct udevice *dev)
 {
 	return ARRAY_SIZE(sandbox_pins);
@@ -68,8 +72,25 @@ static int sandbox_get_pin_muxing(struct udevice *dev,
 				  unsigned int selector,
 				  char *buf, int size)
 {
+	const struct pinconf_param *p;
+	int i;
+
 	snprintf(buf, size, "%s", sandbox_pins_muxing[selector]);
 
+	if (sandbox_pins_param[selector]) {
+		for (i = 0, p = sandbox_conf_params;
+		     i < ARRAY_SIZE(sandbox_conf_params);
+		     i++, p++) {
+			if ((sandbox_pins_param[selector] & BIT(p->param)) &&
+			    (!!(sandbox_pins_value[selector] & BIT(p->param)) ==
+			     p->default_value)) {
+				strncat(buf, " ", size);
+				strncat(buf, p->property, size);
+			}
+		}
+	}
+	strncat(buf, ".", size);
+
 	return 0;
 }
 
@@ -102,6 +123,9 @@ static int sandbox_pinmux_set(struct udevice *dev, unsigned pin_selector,
 	      pin_selector, sandbox_get_pin_name(dev, pin_selector),
 	      func_selector, sandbox_get_function_name(dev, func_selector));
 
+	sandbox_pins_param[pin_selector] = 0;
+	sandbox_pins_value[pin_selector] = 0;
+
 	return 0;
 }
 
@@ -123,6 +147,12 @@ static int sandbox_pinconf_set(struct udevice *dev, unsigned pin_selector,
 	      pin_selector, sandbox_get_pin_name(dev, pin_selector),
 	      param, argument);
 
+	sandbox_pins_param[pin_selector] |= BIT(param);
+	if (argument)
+		sandbox_pins_value[pin_selector] |= BIT(param);
+	else
+		sandbox_pins_value[pin_selector] &= ~BIT(param);
+
 	return 0;
 }
 
-- 
2.17.1

[U-Boot] [PATCH v2 11/14] test: dm: update test for pins configuration in pinctrl node

[Patrick Delaunay]

Add test for "pins" configuration in gpio uclass with set_state() ops
and test for generic parsing of pinconf_param array).

set_state() is called by:
- pinctrl_generic_set_state
 |- pinctrl_generic_set_state_subnode

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/dts/test.dts         | 25 +++++++++++++++++++++++++
 drivers/pinctrl/pinctrl-sandbox.c | 14 +++++++++++++-
 test/py/tests/test_pinmux.py      | 28 ++++++++++++++++++----------
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fdb08f2111..b57f152fa3 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -851,6 +851,31 @@
 
 	pinctrl {
 		compatible = "sandbox,pinctrl";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&gpios>;
+
+		gpios: gpios {
+			gpio0 {
+				pins = "GPIO0";
+				bias-pull-up;
+				input-disable;
+			};
+			gpio1 {
+				pins = "GPIO1";
+				output-high;
+				drive-open-drain;
+			};
+			gpio2 {
+				pins = "GPIO2";
+				bias-pull-down;
+				input-enable;
+			};
+			gpio3 {
+				pins = "GPIO3";
+				bias-disable;
+			};
+		};
 	};
 
 	hwspinlock at 0 {
diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c
index d1a21f0f19..3ee75fbbee 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -14,7 +14,11 @@ static const char * const sandbox_pins[] = {
 	"SDA",
 	"TX",
 	"RX",
-	"W1"
+	"W1",
+	"GPIO0",
+	"GPIO1",
+	"GPIO2",
+	"GPIO3",
 };
 
 static const char * const sandbox_pins_muxing[] = {
@@ -23,6 +27,10 @@ static const char * const sandbox_pins_muxing[] = {
 	"Uart TX",
 	"Uart RX",
 	"1-wire gpio",
+	"gpio",
+	"gpio",
+	"gpio",
+	"gpio",
 };
 
 static const char * const sandbox_groups[] = {
@@ -38,6 +46,10 @@ static const char * const sandbox_functions[] = {
 	"serial",
 	"spi",
 	"w1",
+	"gpio",
+	"gpio",
+	"gpio",
+	"gpio",
 };
 
 static const struct pinconf_param sandbox_conf_params[] = {
diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py
index 25394f1faf..5ca0b4b630 100644
--- a/test/py/tests/test_pinmux.py
+++ b/test/py/tests/test_pinmux.py
@@ -22,11 +22,15 @@ def test_pinmux_usage_2(u_boot_console):
 def test_pinmux_status_all(u_boot_console):
     """Test that 'pinmux status -a' displays pin's muxing."""
     output = u_boot_console.run_command('pinmux status -a')
-    assert ('SCL       : I2C SCL' in output)
-    assert ('SDA       : I2C SDA' in output)
-    assert ('TX        : Uart TX' in output)
-    assert ('RX        : Uart RX' in output)
-    assert ('W1        : 1-wire gpio' in output)
+    assert ('SCL       : I2C SCL.' in output)
+    assert ('SDA       : I2C SDA.' in output)
+    assert ('TX        : Uart TX.' in output)
+    assert ('RX        : Uart RX.' in output)
+    assert ('W1        : 1-wire gpio.' in output)
+    assert ('GPIO0     : gpio bias-pull-up input-disable.' in output)
+    assert ('GPIO1     : gpio drive-open-drain.' in output)
+    assert ('GPIO2     : gpio bias-pull-down input-enable.' in output)
+    assert ('GPIO3     : gpio bias-disable.' in output)
 
 @pytest.mark.buildconfigspec('cmd_pinmux')
 @pytest.mark.boardspec('sandbox')
@@ -59,8 +63,12 @@ def test_pinmux_status(u_boot_console):
     """Test that 'pinmux status' displays selected pincontroller's pin
     muxing descriptions."""
     output = u_boot_console.run_command('pinmux status')
-    assert ('SCL       : I2C SCL' in output)
-    assert ('SDA       : I2C SDA' in output)
-    assert ('TX        : Uart TX' in output)
-    assert ('RX        : Uart RX' in output)
-    assert ('W1        : 1-wire gpio' in output)
+    assert ('SCL       : I2C SCL.' in output)
+    assert ('SDA       : I2C SDA.' in output)
+    assert ('TX        : Uart TX.' in output)
+    assert ('RX        : Uart RX.' in output)
+    assert ('W1        : 1-wire gpio.' in output)
+    assert ('GPIO0     : gpio bias-pull-up input-disable.' in output)
+    assert ('GPIO1     : gpio drive-open-drain.' in output)
+    assert ('GPIO2     : gpio bias-pull-down input-enable.' in output)
+    assert ('GPIO3     : gpio bias-disable.' in output)
-- 
2.17.1

[U-Boot] [PATCH v2 12/14] gpio: sandbox: cleanup binding support

[Patrick Delaunay]

Cleanup binding support, use the generic binding by default
(test u-class gpio_xlate_offs_flags function) and add
specific binding for added value.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/dts/test.dts               | 14 ++++++++++----
 drivers/gpio/sandbox.c                  | 13 ++++++++-----
 include/dt-bindings/gpio/sandbox-gpio.h | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 9 deletions(-)
 create mode 100644 include/dt-bindings/gpio/sandbox-gpio.h

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index b57f152fa3..df58a0b488 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1,5 +1,8 @@
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/gpio/sandbox-gpio.h>
+
 / {
 	model = "sandbox";
 	compatible = "sandbox";
@@ -86,11 +89,14 @@
 		ping-expect = <0>;
 		ping-add = <0>;
 		u-boot,dm-pre-reloc;
-		test-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 5 0 3 2 1>,
+		test-gpios = <&gpio_a 1>, <&gpio_a 4>,
+			<&gpio_b 5 GPIO_ACTIVE_HIGH 3 2 1>,
 			<0>, <&gpio_a 12>;
-		test2-gpios = <&gpio_a 1>, <&gpio_a 4>, <&gpio_b 6 1 3 2 1>,
-			<&gpio_b 7 2 3 2 1>, <&gpio_b 8 4 3 2 1>,
-			<&gpio_b 9 0xc 3 2 1>;
+		test2-gpios = <&gpio_a 1>, <&gpio_a 4>,
+			<&gpio_b 6 GPIO_ACTIVE_LOW 3 2 1>,
+			<&gpio_b 7 GPIO_IN 3 2 1>,
+			<&gpio_b 8 GPIO_OUT 3 2 1>,
+			<&gpio_b 9 (GPIO_OUT|GPIO_OUT_ACTIVE) 3 2 1>;
 		int-value = <1234>;
 		uint-value = <(-1234)>;
 	};
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 91e8e0677e..c2a8adc647 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -10,6 +10,7 @@
 #include <asm/gpio.h>
 #include <dm/of.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/gpio/sandbox-gpio.h>
 
 /* Flags for each GPIO */
 #define GPIOF_OUTPUT	(1 << 0)	/* Currently set as an output */
@@ -136,13 +137,15 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
 	desc->offset = args->args[0];
 	if (args->args_count < 2)
 		return 0;
-	if (args->args[1] & GPIO_ACTIVE_LOW)
-		desc->flags |= GPIOD_ACTIVE_LOW;
-	if (args->args[1] & 2)
+	/* treat generic binding with gpio uclass */
+	gpio_xlate_offs_flags(dev, desc, args);
+
+	/* sandbox test specific, not defined in gpio.h */
+	if (args->args[1] & GPIO_IN)
 		desc->flags |= GPIOD_IS_IN;
-	if (args->args[1] & 4)
+	if (args->args[1] & GPIO_OUT)
 		desc->flags |= GPIOD_IS_OUT;
-	if (args->args[1] & 8)
+	if (args->args[1] & GPIO_OUT_ACTIVE)
 		desc->flags |= GPIOD_IS_OUT_ACTIVE;
 
 	return 0;
diff --git a/include/dt-bindings/gpio/sandbox-gpio.h b/include/dt-bindings/gpio/sandbox-gpio.h
new file mode 100644
index 0000000000..e4bfdb3ce1
--- /dev/null
+++ b/include/dt-bindings/gpio/sandbox-gpio.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides constants for binding sandbox,gpio
+ *
+ */
+#ifndef _DT_BINDINGS_GPIO_SANDBOX_GPIO_H
+#define _DT_BINDINGS_GPIO_SANDBOX_GPIO_H
+
+/*
+ * Add a specific binding for sandbox gpio.
+ * The value need to be after the generic defines of
+ * dt-bindings/gpio/gpio.h
+ */
+
+/* Bit 16 express GPIO input mode */
+#define GPIO_IN			0x10000
+
+/* Bit 17 express GPIO output mode */
+#define GPIO_OUT		0x20000
+
+/* Bit 18 express GPIO output is active */
+#define GPIO_OUT_ACTIVE		0x40000
+
+#endif
-- 
2.17.1

[U-Boot] [PATCH v2 13/14] test: dm: update test for pins configuration in gpio

[Patrick Delaunay]

Add tests for new API set_dir_flags and set_dir_flags and associated
code in gpio uclass.

Test support for new flags GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE
GPIO_PULL_UP and GPIO_PULL_DOWN.

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

Changes in v2:
- simplify sandbox GPIO driver: save dir_flags (GPIOD_...)
  and remove internal flags (GPIOF_...); the previous path
  "gpio: sandbox: cleanup flag support" is no more needed
- add unitary test in dm_test_gpio for set_dir_flags
- add some function check in dm_test_gpio_phandles
- dm_test_gpio_pin_config change to dm_test_gpio_get_dir_flags

 arch/sandbox/dts/test.dts       | 16 ++++++
 arch/sandbox/include/asm/gpio.h | 20 ++++++++
 drivers/gpio/sandbox.c          | 86 +++++++++++++++++++++++++--------
 test/dm/gpio.c                  | 66 ++++++++++++++++++++++---
 test/dm/test-fdt.c              |  2 +-
 5 files changed, 163 insertions(+), 27 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index df58a0b488..8ab22c0c81 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -16,6 +16,7 @@
 		eth5 = &eth_5;
 		gpio1 = &gpio_a;
 		gpio2 = &gpio_b;
+		gpio3 = &gpio_c;
 		i2c0 = "/i2c at 0";
 		mmc0 = "/mmc0";
 		mmc1 = "/mmc1";
@@ -97,6 +98,13 @@
 			<&gpio_b 7 GPIO_IN 3 2 1>,
 			<&gpio_b 8 GPIO_OUT 3 2 1>,
 			<&gpio_b 9 (GPIO_OUT|GPIO_OUT_ACTIVE) 3 2 1>;
+		test3-gpios =
+			<&gpio_c 0 (GPIO_OUT|GPIO_OPEN_DRAIN)>,
+			<&gpio_c 1 (GPIO_OUT|GPIO_OPEN_SOURCE)>,
+			<&gpio_c 2 GPIO_OUT>,
+			<&gpio_c 3 (GPIO_IN|GPIO_PULL_UP)>,
+			<&gpio_c 4 (GPIO_IN|GPIO_PULL_DOWN)>,
+			<&gpio_c 5 GPIO_IN>;
 		int-value = <1234>;
 		uint-value = <(-1234)>;
 	};
@@ -296,6 +304,14 @@
 		sandbox,gpio-count = <10>;
 	};
 
+	gpio_c: extra2-gpios {
+		compatible = "sandbox,gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-bank-name = "c";
+		sandbox,gpio-count = <10>;
+	};
+
 	i2c at 0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h
index cfb803bb3b..df4ba4fb5f 100644
--- a/arch/sandbox/include/asm/gpio.h
+++ b/arch/sandbox/include/asm/gpio.h
@@ -62,4 +62,24 @@ int sandbox_gpio_get_direction(struct udevice *dev, unsigned int offset);
 int sandbox_gpio_set_direction(struct udevice *dev, unsigned int offset,
 			       int output);
 
+/**
+ * Return the simulated flags of a GPIO (used only in sandbox test code)
+ *
+ * @param dev		device to use
+ * @param offset	GPIO offset within bank
+ * @return dir_flags: bitfield accesses by GPIOD_ defines
+ */
+ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset);
+
+/**
+ * Set the simulated flags of a GPIO (used only in sandbox test code)
+ *
+ * @param dev		device to use
+ * @param offset	GPIO offset within bank
+ * @param flags		dir_flags: bitfield accesses by GPIOD_ defines
+ * @return -1 on error, 0 if ok
+ */
+int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset,
+			       ulong flags);
+
 #endif
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index c2a8adc647..a9c470ee5e 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -8,43 +8,43 @@
 #include <fdtdec.h>
 #include <malloc.h>
 #include <asm/gpio.h>
+#include <dm/lists.h>
 #include <dm/of.h>
+#include <dm/pinctrl.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/gpio/sandbox-gpio.h>
 
-/* Flags for each GPIO */
-#define GPIOF_OUTPUT	(1 << 0)	/* Currently set as an output */
-#define GPIOF_HIGH	(1 << 1)	/* Currently set high */
 
 struct gpio_state {
 	const char *label;	/* label given by requester */
-	u8 flags;		/* flags (GPIOF_...) */
+	ulong dir_flags;	/* dir_flags (GPIOD_...) */
 };
 
-/* Access routines for GPIO state */
-static u8 *get_gpio_flags(struct udevice *dev, unsigned offset)
+/* Access routines for GPIO dir flags */
+static ulong *get_gpio_dir_flags(struct udevice *dev, unsigned int offset)
 {
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct gpio_state *state = dev_get_priv(dev);
 
 	if (offset >= uc_priv->gpio_count) {
-		static u8 invalid_flags;
+		static ulong invalid_dir_flags;
 		printf("sandbox_gpio: error: invalid gpio %u\n", offset);
-		return &invalid_flags;
+		return &invalid_dir_flags;
 	}
 
-	return &state[offset].flags;
+	return &state[offset].dir_flags;
+
 }
 
-static int get_gpio_flag(struct udevice *dev, unsigned offset, int flag)
+static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag)
 {
-	return (*get_gpio_flags(dev, offset) & flag) != 0;
+	return (*get_gpio_dir_flags(dev, offset) & flag) != 0;
 }
 
-static int set_gpio_flag(struct udevice *dev, unsigned offset, int flag,
+static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag,
 			 int value)
 {
-	u8 *gpio = get_gpio_flags(dev, offset);
+	ulong *gpio = get_gpio_dir_flags(dev, offset);
 
 	if (value)
 		*gpio |= flag;
@@ -60,24 +60,40 @@ static int set_gpio_flag(struct udevice *dev, unsigned offset, int flag,
 
 int sandbox_gpio_get_value(struct udevice *dev, unsigned offset)
 {
-	if (get_gpio_flag(dev, offset, GPIOF_OUTPUT))
+	if (get_gpio_flag(dev, offset, GPIOD_IS_OUT))
 		debug("sandbox_gpio: get_value on output gpio %u\n", offset);
-	return get_gpio_flag(dev, offset, GPIOF_HIGH);
+	return get_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE);
 }
 
 int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value)
 {
-	return set_gpio_flag(dev, offset, GPIOF_HIGH, value);
+	return set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE, value);
 }
 
 int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset)
 {
-	return get_gpio_flag(dev, offset, GPIOF_OUTPUT);
+	return get_gpio_flag(dev, offset, GPIOD_IS_OUT);
 }
 
 int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output)
 {
-	return set_gpio_flag(dev, offset, GPIOF_OUTPUT, output);
+	set_gpio_flag(dev, offset, GPIOD_IS_OUT, output);
+	set_gpio_flag(dev, offset, GPIOD_IS_IN, !(output));
+
+	return 0;
+}
+
+ulong sandbox_gpio_get_dir_flags(struct udevice *dev, unsigned int offset)
+{
+	return *get_gpio_dir_flags(dev, offset);
+}
+
+int sandbox_gpio_set_dir_flags(struct udevice *dev, unsigned int offset,
+			       ulong flags)
+{
+	*get_gpio_dir_flags(dev, offset) = flags;
+
+	return 0;
 }
 
 /*
@@ -126,9 +142,12 @@ static int sb_gpio_set_value(struct udevice *dev, unsigned offset, int value)
 
 static int sb_gpio_get_function(struct udevice *dev, unsigned offset)
 {
-	if (get_gpio_flag(dev, offset, GPIOF_OUTPUT))
+	if (get_gpio_flag(dev, offset, GPIOD_IS_OUT))
 		return GPIOF_OUTPUT;
-	return GPIOF_INPUT;
+	if (get_gpio_flag(dev, offset, GPIOD_IS_IN))
+		return GPIOF_INPUT;
+
+	return GPIOF_INPUT; /*GPIO is not configurated */
 }
 
 static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
@@ -143,14 +162,39 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
 	/* sandbox test specific, not defined in gpio.h */
 	if (args->args[1] & GPIO_IN)
 		desc->flags |= GPIOD_IS_IN;
+
 	if (args->args[1] & GPIO_OUT)
 		desc->flags |= GPIOD_IS_OUT;
+
 	if (args->args[1] & GPIO_OUT_ACTIVE)
 		desc->flags |= GPIOD_IS_OUT_ACTIVE;
 
 	return 0;
 }
 
+static int sb_gpio_set_dir_flags(struct udevice *dev, unsigned int offset,
+				 ulong flags)
+{
+	ulong *dir_flags;
+
+	debug("%s: offset:%u, dir_flags = %lx\n", __func__, offset, flags);
+
+	dir_flags = get_gpio_dir_flags(dev, offset);
+
+	*dir_flags = flags;
+
+	return 0;
+}
+
+static int sb_gpio_get_dir_flags(struct udevice *dev, unsigned int offset,
+				 ulong *flags)
+{
+	debug("%s: offset:%u\n", __func__, offset);
+	*flags = *get_gpio_dir_flags(dev, offset);
+
+	return 0;
+}
+
 static const struct dm_gpio_ops gpio_sandbox_ops = {
 	.direction_input	= sb_gpio_direction_input,
 	.direction_output	= sb_gpio_direction_output,
@@ -158,6 +202,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = {
 	.set_value		= sb_gpio_set_value,
 	.get_function		= sb_gpio_get_function,
 	.xlate			= sb_gpio_xlate,
+	.set_dir_flags		= sb_gpio_set_dir_flags,
+	.get_dir_flags		= sb_gpio_get_dir_flags,
 };
 
 static int sandbox_gpio_ofdata_to_platdata(struct udevice *dev)
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index 8abf0cd8c8..5992d938d7 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -23,9 +23,9 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	char buf[80];
 
 	/*
-	 * We expect to get 3 banks. One is anonymous (just numbered) and
-	 * comes from platdata. The other two are named a (20 gpios)
-	 * and b (10 gpios) and come from the device tree. See
+	 * We expect to get 4 banks. One is anonymous (just numbered) and
+	 * comes from platdata. The other are named a (20 gpios),
+	 * b (10 gpios) and c (10 gpios) and come from the device tree. See
 	 * test/dm/test.dts.
 	 */
 	ut_assertok(gpio_lookup_name("b4", &dev, &offset, &gpio));
@@ -72,6 +72,18 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	ut_assertok(ops->set_value(dev, offset, 1));
 	ut_asserteq(1, ops->get_value(dev, offset));
 
+	/* Make it an open drain output, and reset it */
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE,
+		    sandbox_gpio_get_dir_flags(dev, offset));
+	ut_assertok(ops->set_dir_flags(dev, offset,
+				       GPIOD_IS_OUT | GPIOD_OPEN_DRAIN));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN,
+		    sandbox_gpio_get_dir_flags(dev, offset));
+	ut_assertok(ops->set_dir_flags(dev, offset,
+				       GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE,
+		    sandbox_gpio_get_dir_flags(dev, offset));
+
 	/* Make it an input */
 	ut_assertok(ops->direction_input(dev, offset));
 	ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
@@ -207,11 +219,14 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts)
 						      desc_list2,
 						      ARRAY_SIZE(desc_list2),
 						      0));
+	ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_a, 4, NULL));
 	ut_assertok(gpio_free_list(dev, desc_list, 3));
+	ut_asserteq(GPIOF_UNUSED, gpio_get_function(gpio_a, 4, NULL));
 	ut_asserteq(3, gpio_request_list_by_name(dev,  "test-gpios", desc_list,
 						 ARRAY_SIZE(desc_list),
 						 GPIOD_IS_OUT |
 						 GPIOD_IS_OUT_ACTIVE));
+	ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_a, 4, NULL));
 	ut_asserteq_ptr(gpio_a, desc_list[0].dev);
 	ut_asserteq(1, desc_list[0].offset);
 	ut_asserteq_ptr(gpio_a, desc_list[1].dev);
@@ -221,10 +236,14 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts)
 	ut_asserteq(1, dm_gpio_get_value(desc_list));
 	ut_assertok(gpio_free_list(dev, desc_list, 3));
 
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE,
+		    sandbox_gpio_get_dir_flags(gpio_a, 1));
 	ut_asserteq(6, gpio_request_list_by_name(dev, "test2-gpios", desc_list,
 						 ARRAY_SIZE(desc_list), 0));
-	/* This was set to output previously, so still will be */
-	ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_a, 1, NULL));
+
+	/* This was set to output previously but flags resetted to 0 = INPUT */
+	ut_asserteq(0, sandbox_gpio_get_dir_flags(gpio_a, 1));
+	ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_a, 1, NULL));
 
 	/* Active low should invert the input value */
 	ut_asserteq(GPIOF_INPUT, gpio_get_function(gpio_b, 6, NULL));
@@ -236,7 +255,42 @@ static int dm_test_gpio_phandles(struct unit_test_state *uts)
 	ut_asserteq(GPIOF_OUTPUT, gpio_get_function(gpio_b, 9, NULL));
 	ut_asserteq(1, dm_gpio_get_value(&desc_list[5]));
 
-
 	return 0;
 }
 DM_TEST(dm_test_gpio_phandles, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Check the gpio pin configuration get from device tree information */
+static int dm_test_gpio_get_dir_flags(struct unit_test_state *uts)
+{
+	struct gpio_desc desc_list[6];
+	struct udevice *dev;
+	ulong flags;
+
+	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+
+	ut_asserteq(6, gpio_request_list_by_name(dev, "test3-gpios", desc_list,
+						 ARRAY_SIZE(desc_list), 0));
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[0], &flags));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN, flags);
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[1], &flags));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE, flags);
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[2], &flags));
+	ut_asserteq(GPIOD_IS_OUT, flags);
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[3], &flags));
+	ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, flags);
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[4], &flags));
+	ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_DOWN, flags);
+
+	ut_assertok(dm_gpio_get_dir_flags(&desc_list[5], &flags));
+	ut_asserteq(GPIOD_IS_IN, flags);
+
+	ut_assertok(gpio_free_list(dev, desc_list, 6));
+
+	return 0;
+}
+DM_TEST(dm_test_gpio_get_dir_flags, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 1fb8b5c248..3451146d04 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -227,7 +227,7 @@ static int dm_test_alias_highest_id(struct unit_test_state *uts)
 	ut_asserteq(5, ret);
 
 	ret = dev_read_alias_highest_id("gpio");
-	ut_asserteq(2, ret);
+	ut_asserteq(3, ret);
 
 	ret = dev_read_alias_highest_id("pci");
 	ut_asserteq(2, ret);
-- 
2.17.1

[U-Boot] [PATCH v2 14/14] test: pinmux: add pincontrol-gpio for pin configuration

[Patrick Delaunay]

Add a simple pincontrol associated to the sandbox gpio driver,
that allows to check pin configuration with the command pinmux.

The pmux test is also updated to test behavior with 2 pincontrols.

Example to check LED pin configuration:

=> pinmux list
| Device                        | Driver                        | Parent
| pinctrl-gpio                  | sandbox_pinctrl_gpio          | root_driver
| pinctrl                       | sandbox_pinctrl               | root_driver

=> pinmux dev pinctrl-gpio

=> pinmux status

a0        : gpio input .
a1        : gpio input .
a2        : gpio input .
a3        : gpio input .
a4        : gpio input .
a5        : gpio output .
a6        : gpio output .
...

Serie-cc: Heiko Schocher <hs@denx.de>
Serie-cc: Simon Glass <sjg@chromium.org>

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

---

Changes in v2:
- Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in
  sandbox gpio driver
- rebase on v2020.01-rc3

 arch/sandbox/dts/test.dts    |  48 +++++----
 drivers/gpio/sandbox.c       | 195 +++++++++++++++++++++++++++++++++++
 test/py/tests/test_pinmux.py |  10 ++
 3 files changed, 231 insertions(+), 22 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 8ab22c0c81..b69f0da871 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -288,28 +288,32 @@
 		};
 	};
 
-	gpio_a: base-gpios {
-		compatible = "sandbox,gpio";
-		gpio-controller;
-		#gpio-cells = <1>;
-		gpio-bank-name = "a";
-		sandbox,gpio-count = <20>;
-	};
-
-	gpio_b: extra-gpios {
-		compatible = "sandbox,gpio";
-		gpio-controller;
-		#gpio-cells = <5>;
-		gpio-bank-name = "b";
-		sandbox,gpio-count = <10>;
-	};
-
-	gpio_c: extra2-gpios {
-		compatible = "sandbox,gpio";
-		gpio-controller;
-		#gpio-cells = <2>;
-		gpio-bank-name = "c";
-		sandbox,gpio-count = <10>;
+	pinctrl-gpio {
+		compatible = "sandbox,pinctrl-gpio";
+
+		gpio_a: base-gpios {
+			compatible = "sandbox,gpio";
+			gpio-controller;
+			#gpio-cells = <1>;
+			gpio-bank-name = "a";
+			sandbox,gpio-count = <20>;
+		};
+
+		gpio_b: extra-gpios {
+			compatible = "sandbox,gpio";
+			gpio-controller;
+			#gpio-cells = <5>;
+			gpio-bank-name = "b";
+			sandbox,gpio-count = <10>;
+		};
+
+		gpio_c: pinmux-gpios {
+			compatible = "sandbox,gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-bank-name = "c";
+			sandbox,gpio-count = <10>;
+		};
 	};
 
 	i2c at 0 {
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index a9c470ee5e..bf6287f029 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -251,3 +251,198 @@ U_BOOT_DRIVER(gpio_sandbox) = {
 	.remove	= gpio_sandbox_remove,
 	.ops	= &gpio_sandbox_ops,
 };
+
+/* pincontrol: used only to check GPIO pin configuration (pinmux command) */
+
+struct sb_pinctrl_priv {
+	int pinctrl_ngpios;
+	struct list_head gpio_dev;
+};
+
+struct sb_gpio_bank {
+	struct udevice *gpio_dev;
+	struct list_head list;
+};
+
+static int sb_populate_gpio_dev_list(struct udevice *dev)
+{
+	struct sb_pinctrl_priv *priv = dev_get_priv(dev);
+	struct udevice *gpio_dev;
+	struct udevice *child;
+	struct sb_gpio_bank *gpio_bank;
+	int ret;
+
+	/*
+	 * parse pin-controller sub-nodes (ie gpio bank nodes) and fill
+	 * a list with all gpio device reference which belongs to the
+	 * current pin-controller. This list is used to find pin_name and
+	 * pin muxing
+	 */
+	list_for_each_entry(child, &dev->child_head, sibling_node) {
+		ret = uclass_get_device_by_name(UCLASS_GPIO, child->name,
+						&gpio_dev);
+		if (ret < 0)
+			continue;
+
+		gpio_bank = malloc(sizeof(*gpio_bank));
+		if (!gpio_bank) {
+			dev_err(dev, "Not enough memory\n");
+			return -ENOMEM;
+		}
+
+		gpio_bank->gpio_dev = gpio_dev;
+		list_add_tail(&gpio_bank->list, &priv->gpio_dev);
+	}
+
+	return 0;
+}
+
+static int sb_pinctrl_get_pins_count(struct udevice *dev)
+{
+	struct sb_pinctrl_priv *priv = dev_get_priv(dev);
+	struct gpio_dev_priv *uc_priv;
+	struct sb_gpio_bank *gpio_bank;
+
+	/*
+	 * if get_pins_count has already been executed once on this
+	 * pin-controller, no need to run it again
+	 */
+	if (priv->pinctrl_ngpios)
+		return priv->pinctrl_ngpios;
+
+	if (list_empty(&priv->gpio_dev))
+		sb_populate_gpio_dev_list(dev);
+	/*
+	 * walk through all banks to retrieve the pin-controller
+	 * pins number
+	 */
+	list_for_each_entry(gpio_bank, &priv->gpio_dev, list) {
+		uc_priv = dev_get_uclass_priv(gpio_bank->gpio_dev);
+
+		priv->pinctrl_ngpios += uc_priv->gpio_count;
+	}
+
+	return priv->pinctrl_ngpios;
+}
+
+static struct udevice *sb_pinctrl_get_gpio_dev(struct udevice *dev,
+					       unsigned int selector,
+					       unsigned int *idx)
+{
+	struct sb_pinctrl_priv *priv = dev_get_priv(dev);
+	struct sb_gpio_bank *gpio_bank;
+	struct gpio_dev_priv *uc_priv;
+	int pin_count = 0;
+
+	if (list_empty(&priv->gpio_dev))
+		sb_populate_gpio_dev_list(dev);
+
+	/* look up for the bank which owns the requested pin */
+	list_for_each_entry(gpio_bank, &priv->gpio_dev, list) {
+		uc_priv = dev_get_uclass_priv(gpio_bank->gpio_dev);
+
+		if (selector < (pin_count + uc_priv->gpio_count)) {
+			/*
+			 * we found the bank, convert pin selector to
+			 * gpio bank index
+			 */
+			*idx = selector - pin_count;
+
+			return gpio_bank->gpio_dev;
+		}
+		pin_count += uc_priv->gpio_count;
+	}
+
+	return NULL;
+}
+
+static const char *sb_pinctrl_get_pin_name(struct udevice *dev,
+					   unsigned int selector)
+{
+	struct gpio_dev_priv *uc_priv;
+	struct udevice *gpio_dev;
+	unsigned int gpio_idx;
+	static char pin_name[PINNAME_SIZE];
+
+	/* look up for the bank which owns the requested pin */
+	gpio_dev = sb_pinctrl_get_gpio_dev(dev, selector, &gpio_idx);
+	if (!gpio_dev) {
+		snprintf(pin_name, PINNAME_SIZE, "Error");
+	} else {
+		uc_priv = dev_get_uclass_priv(gpio_dev);
+
+		snprintf(pin_name, PINNAME_SIZE, "%s%d",
+			 uc_priv->bank_name,
+			 gpio_idx);
+	}
+
+	return pin_name;
+}
+
+static char *get_dir_flags_string(ulong flags)
+{
+	if (flags & GPIOD_OPEN_DRAIN)
+		return "drive-open-drain";
+	if (flags & GPIOD_OPEN_SOURCE)
+		return "drive-open-source";
+	if (flags & GPIOD_PULL_UP)
+		return "bias-pull-up";
+	if (flags & GPIOD_PULL_DOWN)
+		return "bias-pull-down";
+	return ".";
+}
+
+static int sb_pinctrl_get_pin_muxing(struct udevice *dev,
+				     unsigned int selector,
+				     char *buf, int size)
+{
+	struct udevice *gpio_dev;
+	unsigned int gpio_idx;
+	ulong dir_flags;
+	int function;
+
+	/* look up for the bank which owns the requested pin */
+	gpio_dev = sb_pinctrl_get_gpio_dev(dev, selector, &gpio_idx);
+	if (!gpio_dev) {
+		snprintf(buf, size, "Error");
+	} else {
+		function = sb_gpio_get_function(gpio_dev, gpio_idx);
+		dir_flags = *get_gpio_dir_flags(gpio_dev, gpio_idx);
+
+		snprintf(buf, size, "gpio %s %s",
+			 function == GPIOF_OUTPUT ? "output" : "input",
+			 get_dir_flags_string(dir_flags));
+	}
+
+	return 0;
+}
+
+static int sandbox_pinctrl_probe(struct udevice *dev)
+{
+	struct sb_pinctrl_priv *priv = dev_get_priv(dev);
+
+	INIT_LIST_HEAD(&priv->gpio_dev);
+
+	return 0;
+}
+
+static struct pinctrl_ops sandbox_pinctrl_gpio_ops = {
+	.get_pin_name		= sb_pinctrl_get_pin_name,
+	.get_pins_count		= sb_pinctrl_get_pins_count,
+	.get_pin_muxing		= sb_pinctrl_get_pin_muxing,
+};
+
+static const struct udevice_id sandbox_pinctrl_gpio_match[] = {
+	{ .compatible = "sandbox,pinctrl-gpio" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(sandbox_pinctrl_gpio) = {
+	.name = "sandbox_pinctrl_gpio",
+	.id = UCLASS_PINCTRL,
+	.of_match = sandbox_pinctrl_gpio_match,
+	.ops = &sandbox_pinctrl_gpio_ops,
+	.bind = dm_scan_fdt_dev,
+	.probe = sandbox_pinctrl_probe,
+	.priv_auto_alloc_size	= sizeof(struct sb_pinctrl_priv),
+};
diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py
index 5ca0b4b630..4e6df992a4 100644
--- a/test/py/tests/test_pinmux.py
+++ b/test/py/tests/test_pinmux.py
@@ -22,6 +22,12 @@ def test_pinmux_usage_2(u_boot_console):
 def test_pinmux_status_all(u_boot_console):
     """Test that 'pinmux status -a' displays pin's muxing."""
     output = u_boot_console.run_command('pinmux status -a')
+
+    assert ('pinctrl-gpio:' in output)
+    assert ('a5        : gpio output .' in output)
+    assert ('a6        : gpio output .' in output)
+
+    assert ('pinctrl:' in output)
     assert ('SCL       : I2C SCL.' in output)
     assert ('SDA       : I2C SDA.' in output)
     assert ('TX        : Uart TX.' in output)
@@ -63,6 +69,10 @@ def test_pinmux_status(u_boot_console):
     """Test that 'pinmux status' displays selected pincontroller's pin
     muxing descriptions."""
     output = u_boot_console.run_command('pinmux status')
+
+    assert (not 'pinctrl-gpio:' in output)
+    assert (not 'pinctrl:' in output)
+
     assert ('SCL       : I2C SCL.' in output)
     assert ('SDA       : I2C SDA.' in output)
     assert ('TX        : Uart TX.' in output)
-- 
2.17.1

[PATCH v2 01/14] dm: pinctrl: convert pinctrl-single to livetree

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Convert 'pinctrl-single' using livetree functions
> - dev_read_prop
> - dev_read_u32_default
> - dev_read_u32_array
> - dev_read_bool
> - dev_read_addr
> and get rid of DECLARE_GLOBAL_DATA_PTR.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - use the dev_ API instead of ofnode_ function.
>
>  drivers/pinctrl/pinctrl-single.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 02/14] dm: core: add ofnode and dev function to iterate on node property

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add functions to iterate on all property with livetree
> - dev_read_first_prop
> - dev_read_next_prop
> - dev_read_prop_by_prop
> and
> - ofnode_get_first_property
> - ofnode_get_next_property
> - ofnode_get_property_by_prop
>
> For example:
> struct ofprop property;
>
> dev_for_each_property(property, config) {
>         value = dev_read_prop_by_prop(&property, &propname, &len);
>
> or:
> for (prop = ofnode_get_first_property(dev_ofnode(dev));
>      prop;
>      prop = ofnode_get_next_property(dev_ofnode(dev),prop))
> {
>      value = ofnode_get_property_by_prop(dev_ofnode(dev), prop,
>                                          &propname, &len);
> ....
> }
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - Identify property with a new struct ofprop as proposed
>   by Simon Glass
> - Add dev_ iterate functions
>
>  drivers/core/of_access.c | 32 +++++++++++++++++++
>  drivers/core/ofnode.c    | 48 ++++++++++++++++++++++++++++
>  drivers/core/read.c      | 16 ++++++++++
>  include/dm/of_access.h   | 40 ++++++++++++++++++++++++
>  include/dm/ofnode.h      | 63 ++++++++++++++++++++++++++++++++++++-
>  include/dm/read.h        | 67 ++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 265 insertions(+), 1 deletion(-)

These look good, but please add a few simple tests like in your commit message.

See test/dm/ofnode.c, or you could add test/dm/read.c and just use the
read interface which would be good enough It think, since tests run
with and without livetree.

Regards,
Simon

[PATCH v2 03/14] dm: pinctrl: migrate pinctrl-generic to livetree

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Migrate pinctrl-generic to livetree:
> - dev_for_each_property
> - dev_read_prop_by_prop
> - dev_read_string_count
> - dev_read_string_index
> and get rid of DECLARE_GLOBAL_DATA_PTR.
>
> This patch solves the parsing issue during sandbox tests for pin
> configuration (OF_LIVE is activated in sandbox_defconfig
> and sub node are not correctly parsed in
> pinctrl_generic_set_state_subnode with fdt lib API).
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - Change dev_ functions in pinctrl-generic
>
>  drivers/pinctrl/pinctrl-generic.c | 30 +++++++++++-------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 06/14] gpio: remove the open_drain API and ops

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> This patch removes the ops get_open_drain/set_open_drain
> and the API dm_gpio_get_open_drain/dm_gpio_set_open_drain.
>
> The ops only provided in one driver (mpc8xxx gpio) and the
> associated API is never called in boards.
>
> This patch prepare a more generic set/get_dir_flags ops,
> including the open drain property.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - remove the open_drain API and ops
>
>  arch/sandbox/include/asm/gpio.h | 20 ------------------
>  drivers/gpio/gpio-uclass.c      | 36 ---------------------------------
>  drivers/gpio/mpc8xxx_gpio.c     | 22 --------------------
>  drivers/gpio/sandbox.c          | 35 --------------------------------
>  include/asm-generic/gpio.h      | 34 -------------------------------
>  test/dm/gpio.c                  |  7 -------
>  6 files changed, 154 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 07/14] gpio: add gpio descriptor initialization helper

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add a helper function gpio_desc_init() to initialize the gpio descriptor;
> with this function the flags will be always set to 0.
>
> It wasn't the case before this patch in dm_gpio_lookup_name.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - add gpio descriptor initialization helper
>
>  drivers/gpio/gpio-uclass.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 08/14] gpio: add ops for configuration with dir flags

[Simon Glass]

Hi Patrick,

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> This commit manages the dir flags that can be used in gpio specifiers
> to indicate if a pull-up resistor or pull-down resistor should be
> enabled for output gpio (GPIO_PULL_UP, GPIO_PULL_DOWN) and the
> Open Drain/Open Source configuration for input gpio
> (GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE).
>
> These flags are already supported in Linux kernel in gpiolib;
> this patch provides the same support in U-Boot.
>
> The dir flags are managed in gpio drivers with two optional ops in gpio
> uclass: set_dir_flags and get_dir_flags.
>
> - ops set_dir_flags() set the direction and configuration of each GPIO
>   with a only call to dm_gpio_set_dir_flags() / dm_gpio_set_dir()
>   and respecting the configuration provided by device tree
>   (saved in desc->flags).
>
> - ops get_dir_flags() return dynamically the current gpio configuration,
>   it is used by the new API dm_gpio_get_dir_flags().
>
> When these optional ops are absent, the gpio uclass use the mandatory ops
> (direction_output, direction_input, get_value) and desc->flags to manage
> only the main dir flags:
> - GPIOD_IS_IN
> - GPIOD_IS_OUT
> - GPIOD_IS_OUT_ACTIVE
> - GPIOD_ACTIVE_LOW
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - change the proposed ops for pin config to set_dir_flags/get_dir_flags
> - reused the existing API dm_gpio_set_dir_flags/dm_gpio_set_dir
> - add a new API dm_gpio_get_dir_flags
>
>  drivers/gpio/gpio-uclass.c | 157 +++++++++++++++++++++++++++++++------
>  include/asm-generic/gpio.h |  65 +++++++++++++--
>  2 files changed, 192 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index 0870458e96..241293f4b4 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -140,8 +140,27 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
>         if (args->args_count < 2)
>                 return 0;
>
> +       desc->flags = 0;
>         if (args->args[1] & GPIO_ACTIVE_LOW)
> -               desc->flags = GPIOD_ACTIVE_LOW;
> +               desc->flags |= GPIOD_ACTIVE_LOW;
> +
> +       /*
> +        * need to test 2 bits for gpio output binding:
> +        * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN (0x4)
> +        * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) | LINE_OPEN_SOURCE (0x0)
> +        */
> +       if (args->args[1] & GPIO_SINGLE_ENDED) {
> +               if (args->args[1] & GPIO_LINE_OPEN_DRAIN)
> +                       desc->flags |= GPIOD_OPEN_DRAIN;
> +               else
> +                       desc->flags |= GPIOD_OPEN_SOURCE;
> +       }
> +
> +       if (args->args[1] & GPIO_PULL_UP)
> +               desc->flags |= GPIOD_PULL_UP;
> +
> +       if (args->args[1] & GPIO_PULL_DOWN)
> +               desc->flags |= GPIOD_PULL_DOWN;
>
>         return 0;
>  }
> @@ -476,18 +495,24 @@ int gpio_direction_output(unsigned gpio, int value)
>                                                         desc.offset, value);
>  }
>
> -int dm_gpio_get_value(const struct gpio_desc *desc)
> +static int _gpio_get_value(const struct gpio_desc *desc)
>  {
>         int value;
> +
> +       value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset);
> +
> +       return desc->flags & GPIOD_ACTIVE_LOW ? !value : value;
> +}
> +
> +int dm_gpio_get_value(const struct gpio_desc *desc)
> +{
>         int ret;
>
>         ret = check_reserved(desc, "get_value");
>         if (ret)
>                 return ret;
>
> -       value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset);
> -
> -       return desc->flags & GPIOD_ACTIVE_LOW ? !value : value;
> +       return _gpio_get_value(desc);
>  }
>
>  int dm_gpio_set_value(const struct gpio_desc *desc, int value)
> @@ -504,39 +529,119 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value)
>         return 0;
>  }
>
> -int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
> +/* check dir flags invalid configuration */
> +static int check_dir_flags(ulong flags)
> +{
> +       if ((flags & GPIOD_IS_OUT) && (flags & GPIOD_IS_IN))
> +               return -EINVAL;
> +
> +       if ((flags & GPIOD_PULL_UP) && (flags & GPIOD_PULL_DOWN))
> +               return -EINVAL;
> +
> +       if ((flags & GPIOD_OPEN_DRAIN) && (flags & GPIOD_OPEN_SOURCE))
> +               return -EINVAL;

Can we use different error numbers here, so that people can figure out
what is going on?

> +
> +       return 0;
> +}
> +
> +static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
>  {
>         struct udevice *dev = desc->dev;
>         struct dm_gpio_ops *ops = gpio_get_ops(dev);
> +       struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>         int ret;
>
> -       ret = check_reserved(desc, "set_dir");
> -       if (ret)
> -               return ret;
> +       ret = check_dir_flags(flags);
> +       if (ret) {
> +               dev_err(dev,
> +                       "%s error: set_dir_flags for gpio %s%d has invalid dir flags 0x%lx\n",
> +                       desc->dev->name,
> +                       uc_priv->bank_name ? uc_priv->bank_name : "",
> +                       desc->offset, flags);

log_debug()
Also should return error

>
> -       if (flags & GPIOD_IS_OUT) {
> -               int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
> +               return ret;
> +       }
>
> -               if (flags & GPIOD_ACTIVE_LOW)
> -                       value = !value;
> -               ret = ops->direction_output(dev, desc->offset, value);
> -       } else  if (flags & GPIOD_IS_IN) {
> -               ret = ops->direction_input(dev, desc->offset);
> +       /* GPIOD_ are directly managed by driver in set_dir_flags*/
> +       if (ops->set_dir_flags) {
> +               ret = ops->set_dir_flags(dev, desc->offset, flags);
> +       } else {
> +               if (flags & GPIOD_IS_OUT) {
> +                       ret = ops->direction_output(dev, desc->offset,
> +                                                   GPIOD_FLAGS_OUTPUT(flags));
> +               } else if (flags & GPIOD_IS_IN) {
> +                       ret = ops->direction_input(dev, desc->offset);
> +               }
>         }
> +
> +       return ret;
> +}
> +
> +int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
> +{
> +       int ret;
> +
> +       ret = check_reserved(desc, "set_dir_flags");
>         if (ret)
>                 return ret;
> -       /*
> -        * Update desc->flags here, so that GPIO_ACTIVE_LOW is honoured in
> -        * futures
> -        */
> -       desc->flags = flags;
>
> -       return 0;
> +       /* combine the requested flags (for IN/OUT) and the descriptor flags */
> +       flags |= desc->flags;
> +       ret = _dm_gpio_set_dir_flags(desc, flags);
> +
> +       /* update the descriptor flags */
> +       if (ret)
> +               desc->flags = flags;
> +
> +       return ret;
>  }
>
>  int dm_gpio_set_dir(struct gpio_desc *desc)
>  {
> -       return dm_gpio_set_dir_flags(desc, desc->flags);
> +       int ret;
> +
> +       ret = check_reserved(desc, "set_dir");
> +       if (ret)
> +               return ret;
> +
> +       return _dm_gpio_set_dir_flags(desc, desc->flags);
> +}
> +
> +int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags)
> +{
> +       struct udevice *dev = desc->dev;
> +       struct dm_gpio_ops *ops = gpio_get_ops(dev);
> +       int ret, value;
> +       ulong dir_flags;
> +
> +       ret = check_reserved(desc, "get_dir_flags");
> +       if (ret)
> +               return ret;
> +
> +       /* GPIOD_ are directly provided by driver except GPIOD_ACTIVE_LOW*/

Space before *

> +       if (ops->get_dir_flags) {
> +               ret = ops->get_dir_flags(dev, desc->offset, &dir_flags);
> +               if (ret)
> +                       return ret;
> +
> +               /* GPIOD_ACTIVE_LOW is saved in desc->flags */
> +               value = dir_flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
> +               if (desc->flags & GPIOD_ACTIVE_LOW)
> +                       value = !value;
> +               dir_flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE);
> +               dir_flags |= (desc->flags & GPIOD_ACTIVE_LOW);
> +               if (value)
> +                       dir_flags |= GPIOD_IS_OUT_ACTIVE;
> +       } else {
> +               dir_flags = desc->flags;
> +               /* only GPIOD_IS_OUT_ACTIVE is provided by uclass */
> +               dir_flags &= ~GPIOD_IS_OUT_ACTIVE;
> +               if ((desc->flags & GPIOD_IS_OUT) && _gpio_get_value(desc))
> +                       dir_flags |= GPIOD_IS_OUT_ACTIVE;
> +       }
> +       *flags = dir_flags;
> +
> +       return 0;
>  }
>
>  /**
> @@ -809,7 +914,7 @@ static int gpio_request_tail(int ret, const char *nodename,
>                 debug("%s: dm_gpio_requestf failed\n", __func__);
>                 goto err;
>         }
> -       ret = dm_gpio_set_dir_flags(desc, flags | desc->flags);
> +       ret = dm_gpio_set_dir_flags(desc, flags);
>         if (ret) {
>                 debug("%s: dm_gpio_set_dir failed\n", __func__);
>                 goto err;
> @@ -1036,6 +1141,10 @@ static int gpio_post_bind(struct udevice *dev)
>                         ops->get_function += gd->reloc_off;
>                 if (ops->xlate)
>                         ops->xlate += gd->reloc_off;
> +               if (ops->set_dir_flags)
> +                       ops->set_dir_flags += gd->reloc_off;
> +               if (ops->get_dir_flags)
> +                       ops->get_dir_flags += gd->reloc_off;
>
>                 reloc_done++;
>         }
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 454578c8d2..c6991be1c9 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -117,10 +117,15 @@ struct udevice;
>  struct gpio_desc {
>         struct udevice *dev;    /* Device, NULL for invalid GPIO */
>         unsigned long flags;
> +
>  #define GPIOD_IS_OUT           BIT(1)  /* GPIO is an output */
>  #define GPIOD_IS_IN            BIT(2)  /* GPIO is an input */
>  #define GPIOD_ACTIVE_LOW       BIT(3)  /* value has active low */

s/has/is/ ?

>  #define GPIOD_IS_OUT_ACTIVE    BIT(4)  /* set output active */
> +#define GPIOD_OPEN_DRAIN       BIT(5)  /* GPIO is open drain type */
> +#define GPIOD_OPEN_SOURCE      BIT(6)  /* GPIO is open source type */
> +#define GPIOD_PULL_UP          BIT(7)  /* GPIO has pull up enabled */
> +#define GPIOD_PULL_DOWN                BIT(8)  /* GPIO has pull down enabled */

pull-down or pulldown

>
>         uint offset;            /* GPIO offset within the device */
>         /*
> @@ -129,6 +134,12 @@ struct gpio_desc {
>          */
>  };
>
> +/* helper to compute the value of the gpio output */
> +#define GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE)
> +#define GPIOD_FLAGS_OUTPUT(flags) \
> +       (((((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_IS_OUT_ACTIVE) || \
> +         (((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_ACTIVE_LOW)) ? 1 : 0)
> +

You can drop the ? 1 : 0 since boolean expressions do that anyway.

>  /**
>   * dm_gpio_is_valid() - Check if a GPIO is valid
>   *
> @@ -253,6 +264,7 @@ struct dm_gpio_ops {
>                                 int value);
>         int (*get_value)(struct udevice *dev, unsigned offset);
>         int (*set_value)(struct udevice *dev, unsigned offset, int value);
> +
>         /**
>          * get_function() Get the GPIO function
>          *
> @@ -287,6 +299,37 @@ struct dm_gpio_ops {
>          */
>         int (*xlate)(struct udevice *dev, struct gpio_desc *desc,
>                      struct ofnode_phandle_args *args);
> +
> +       /**
> +        * set_dir_flags() - Set GPIO dir flags
> +        *
> +        * This function should set up the GPIO configuration according to the
> +        * information provide by the direction flags bitfield.
> +        *
> +        * This method is optional.
> +        *
> +        * @dev:        GPIO device
> +        * @offset:     GPIO offset within that device
> +        * @flags:      GPIO configuration to use
> +        * @return 0 if OK, -ve on error
> +        */
> +       int (*set_dir_flags)(struct udevice *dev, unsigned int offset,
> +                            ulong flags);
> +
> +       /**
> +        * get_dir_flags() - Get GPIO dir flags
> +        *
> +        * This function return the GPIO direction flags used.
> +        *
> +        * This method is optional.
> +        *
> +        * @dev:        GPIO device
> +        * @offset:     GPIO offset within that device
> +        * @flags:      place to put the used direction flags by GPIO
> +        * @return 0 if OK, -ve on error
> +        */
> +       int (*get_dir_flags)(struct udevice *dev, unsigned int offset,
> +                            ulong *flags);
>  };
>
>  /**
> @@ -586,8 +629,7 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value);
>  /**
>   * dm_gpio_set_dir() - Set the direction for a GPIO
>   *
> - * This sets up the direction according tot the provided flags. It will do
> - * nothing unless the direction is actually specified.
> + * This sets up the direction according to the GPIO flags: desc->flags.
>   *
>   * @desc:      GPIO description containing device, offset and flags,
>   *             previously returned by gpio_request_by_name()
> @@ -596,11 +638,10 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value);
>  int dm_gpio_set_dir(struct gpio_desc *desc);
>
>  /**
> - * dm_gpio_set_dir_flags() - Set direction using specific flags
> + * dm_gpio_set_dir_flags() - Set direction using add flags
>   *
> - * This is like dm_gpio_set_dir() except that the flags value is provided
> - * instead of being used from desc->flags. This is needed because in many
> - * cases the GPIO description does not include direction information.
> + * This sets up the direction according tot the provided flags and the GPIO
> + * description (desc->flags) which include direction information.
>   * Note that desc->flags is updated by this function.
>   *
>   * @desc:      GPIO description containing device, offset and flags,
> @@ -610,6 +651,18 @@ int dm_gpio_set_dir(struct gpio_desc *desc);
>   */
>  int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags);
>
> +/**
> + * dm_gpio_get_dir_flags() - Get direction flags
> + *
> + * read the current direction flags
> + *
> + * @desc:      GPIO description containing device, offset and flags,
> + *             previously returned by gpio_request_by_name()
> + * @flags:     place to put the used flags
> + * @return 0 if OK, -ve on error, in which case desc->flags is not updated
> + */
> +int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags);
> +
>  /**
>   * gpio_get_number() - Get the global GPIO number of a GPIO
>   *
> --
> 2.17.1
>

There's a lot going on in this patch.

Regards,
Simon

[PATCH v2 13/14] test: dm: update test for pins configuration in gpio

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add tests for new API set_dir_flags and set_dir_flags and associated
> code in gpio uclass.
>
> Test support for new flags GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE
> GPIO_PULL_UP and GPIO_PULL_DOWN.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v2:
> - simplify sandbox GPIO driver: save dir_flags (GPIOD_...)
>   and remove internal flags (GPIOF_...); the previous path
>   "gpio: sandbox: cleanup flag support" is no more needed
> - add unitary test in dm_test_gpio for set_dir_flags
> - add some function check in dm_test_gpio_phandles
> - dm_test_gpio_pin_config change to dm_test_gpio_get_dir_flags
>
>  arch/sandbox/dts/test.dts       | 16 ++++++
>  arch/sandbox/include/asm/gpio.h | 20 ++++++++
>  drivers/gpio/sandbox.c          | 86 +++++++++++++++++++++++++--------
>  test/dm/gpio.c                  | 66 ++++++++++++++++++++++---
>  test/dm/test-fdt.c              |  2 +-
>  5 files changed, 163 insertions(+), 27 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 14/14] test: pinmux: add pincontrol-gpio for pin configuration

[Simon Glass]

On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add a simple pincontrol associated to the sandbox gpio driver,
> that allows to check pin configuration with the command pinmux.
>
> The pmux test is also updated to test behavior with 2 pincontrols.
>
> Example to check LED pin configuration:
>
> => pinmux list
> | Device                        | Driver                        | Parent
> | pinctrl-gpio                  | sandbox_pinctrl_gpio          | root_driver
> | pinctrl                       | sandbox_pinctrl               | root_driver
>
> => pinmux dev pinctrl-gpio
>
> => pinmux status
>
> a0        : gpio input .
> a1        : gpio input .
> a2        : gpio input .
> a3        : gpio input .
> a4        : gpio input .
> a5        : gpio output .
> a6        : gpio output .
> ...
>
> Serie-cc: Heiko Schocher <hs@denx.de>
> Serie-cc: Simon Glass <sjg@chromium.org>

Series-cc :-)

>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
>
> ---
>
> Changes in v2:
> - Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in
>   sandbox gpio driver
> - rebase on v2020.01-rc3
>
>  arch/sandbox/dts/test.dts    |  48 +++++----
>  drivers/gpio/sandbox.c       | 195 +++++++++++++++++++++++++++++++++++
>  test/py/tests/test_pinmux.py |  10 ++
>  3 files changed, 231 insertions(+), 22 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 02/14] dm: core: add ofnode and dev function to iterate on node property

[Patrick DELAUNAY]

Hi,

> From: Simon Glass <sjg@chromium.org>
> Sent: lundi 30 décembre 2019 02:21
> 
> On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > Add functions to iterate on all property with livetree
> > - dev_read_first_prop
> > - dev_read_next_prop
> > - dev_read_prop_by_prop
> > and
> > - ofnode_get_first_property
> > - ofnode_get_next_property
> > - ofnode_get_property_by_prop
> >
> > For example:
> > struct ofprop property;
> >
> > dev_for_each_property(property, config) {
> >         value = dev_read_prop_by_prop(&property, &propname, &len);
> >
> > or:
> > for (prop = ofnode_get_first_property(dev_ofnode(dev));
> >      prop;
> >      prop = ofnode_get_next_property(dev_ofnode(dev),prop))
> > {
> >      value = ofnode_get_property_by_prop(dev_ofnode(dev), prop,
> >                                          &propname, &len); ....
> > }
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > Changes in v2:
> > - Identify property with a new struct ofprop as proposed
> >   by Simon Glass
> > - Add dev_ iterate functions
> >
> >  drivers/core/of_access.c | 32 +++++++++++++++++++
> >  drivers/core/ofnode.c    | 48 ++++++++++++++++++++++++++++
> >  drivers/core/read.c      | 16 ++++++++++
> >  include/dm/of_access.h   | 40 ++++++++++++++++++++++++
> >  include/dm/ofnode.h      | 63 ++++++++++++++++++++++++++++++++++++-
> >  include/dm/read.h        | 67
> ++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 265 insertions(+), 1 deletion(-)
> 
> These look good, but please add a few simple tests like in your commit message.
> 
> See test/dm/ofnode.c, or you could add test/dm/read.c and just use the read
> interface which would be good enough It think, since tests run with and without
> livetree.

Ok I will add  it.

Regards
Patrick

[PATCH v2 08/14] gpio: add ops for configuration with dir flags

[Patrick DELAUNAY]

Hi Simon

> From: Simon Glass <sjg@chromium.org>
> Sent: lundi 30 décembre 2019 02:21
> 
> Hi Patrick,
> 
> On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > This commit manages the dir flags that can be used in gpio specifiers
> > to indicate if a pull-up resistor or pull-down resistor should be
> > enabled for output gpio (GPIO_PULL_UP, GPIO_PULL_DOWN) and the Open
> > Drain/Open Source configuration for input gpio (GPIO_OPEN_DRAIN,
> > GPIO_OPEN_SOURCE).
> >
> > These flags are already supported in Linux kernel in gpiolib; this
> > patch provides the same support in U-Boot.
> >
> > The dir flags are managed in gpio drivers with two optional ops in
> > gpio
> > uclass: set_dir_flags and get_dir_flags.
> >
> > - ops set_dir_flags() set the direction and configuration of each GPIO
> >   with a only call to dm_gpio_set_dir_flags() / dm_gpio_set_dir()
> >   and respecting the configuration provided by device tree
> >   (saved in desc->flags).
> >
> > - ops get_dir_flags() return dynamically the current gpio configuration,
> >   it is used by the new API dm_gpio_get_dir_flags().
> >
> > When these optional ops are absent, the gpio uclass use the mandatory
> > ops (direction_output, direction_input, get_value) and desc->flags to
> > manage only the main dir flags:
> > - GPIOD_IS_IN
> > - GPIOD_IS_OUT
> > - GPIOD_IS_OUT_ACTIVE
> > - GPIOD_ACTIVE_LOW
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > Changes in v2:
> > - change the proposed ops for pin config to
> > set_dir_flags/get_dir_flags
> > - reused the existing API dm_gpio_set_dir_flags/dm_gpio_set_dir
> > - add a new API dm_gpio_get_dir_flags
> >
> >  drivers/gpio/gpio-uclass.c | 157
> > +++++++++++++++++++++++++++++++------
> >  include/asm-generic/gpio.h |  65 +++++++++++++--
> >  2 files changed, 192 insertions(+), 30 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> >
> > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> > index 0870458e96..241293f4b4 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -140,8 +140,27 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct
> gpio_desc *desc,
> >         if (args->args_count < 2)
> >                 return 0;
> >
> > +       desc->flags = 0;
> >         if (args->args[1] & GPIO_ACTIVE_LOW)
> > -               desc->flags = GPIOD_ACTIVE_LOW;
> > +               desc->flags |= GPIOD_ACTIVE_LOW;
> > +
> > +       /*
> > +        * need to test 2 bits for gpio output binding:
> > +        * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN
> (0x4)
> > +        * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) |
> LINE_OPEN_SOURCE (0x0)
> > +        */
> > +       if (args->args[1] & GPIO_SINGLE_ENDED) {
> > +               if (args->args[1] & GPIO_LINE_OPEN_DRAIN)
> > +                       desc->flags |= GPIOD_OPEN_DRAIN;
> > +               else
> > +                       desc->flags |= GPIOD_OPEN_SOURCE;
> > +       }
> > +
> > +       if (args->args[1] & GPIO_PULL_UP)
> > +               desc->flags |= GPIOD_PULL_UP;
> > +
> > +       if (args->args[1] & GPIO_PULL_DOWN)
> > +               desc->flags |= GPIOD_PULL_DOWN;
> >
> >         return 0;
> >  }
> > @@ -476,18 +495,24 @@ int gpio_direction_output(unsigned gpio, int value)
> >                                                         desc.offset,
> > value);  }
> >
> > -int dm_gpio_get_value(const struct gpio_desc *desc)
> > +static int _gpio_get_value(const struct gpio_desc *desc)
> >  {
> >         int value;
> > +
> > +       value = gpio_get_ops(desc->dev)->get_value(desc->dev,
> > + desc->offset);
> > +
> > +       return desc->flags & GPIOD_ACTIVE_LOW ? !value : value; }
> > +
> > +int dm_gpio_get_value(const struct gpio_desc *desc) {
> >         int ret;
> >
> >         ret = check_reserved(desc, "get_value");
> >         if (ret)
> >                 return ret;
> >
> > -       value = gpio_get_ops(desc->dev)->get_value(desc->dev, desc->offset);
> > -
> > -       return desc->flags & GPIOD_ACTIVE_LOW ? !value : value;
> > +       return _gpio_get_value(desc);
> >  }
> >
> >  int dm_gpio_set_value(const struct gpio_desc *desc, int value) @@
> > -504,39 +529,119 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int
> value)
> >         return 0;
> >  }
> >
> > -int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
> > +/* check dir flags invalid configuration */ static int
> > +check_dir_flags(ulong flags) {
> > +       if ((flags & GPIOD_IS_OUT) && (flags & GPIOD_IS_IN))
> > +               return -EINVAL;
> > +
> > +       if ((flags & GPIOD_PULL_UP) && (flags & GPIOD_PULL_DOWN))
> > +               return -EINVAL;
> > +
> > +       if ((flags & GPIOD_OPEN_DRAIN) && (flags &
> GPIOD_OPEN_SOURCE))
> > +               return -EINVAL;
> 
> Can we use different error numbers here, so that people can figure out what is
> going on?

It is really a basic error that should be never occur,
I think that EINVAL is the correct return value here (it is difficult to have 3 different errno)

But I will add a debug trace before each return...
 
> > +
> > +       return 0;
> > +}
> > +
> > +static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong
> > +flags)
> >  {
> >         struct udevice *dev = desc->dev;
> >         struct dm_gpio_ops *ops = gpio_get_ops(dev);
> > +       struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> >         int ret;
> >
> > -       ret = check_reserved(desc, "set_dir");
> > -       if (ret)
> > -               return ret;
> > +       ret = check_dir_flags(flags);
> > +       if (ret) {
> > +               dev_err(dev,
> > +                       "%s error: set_dir_flags for gpio %s%d has invalid dir flags
> 0x%lx\n",
> > +                       desc->dev->name,
> > +                       uc_priv->bank_name ? uc_priv->bank_name : "",
> > +                       desc->offset, flags);
> 
> log_debug()

Ok change to dev_dbg
log_debug is preferered to dev_dbg ?

for me dev_.... functions are preferred when 'dev' is available.

> Also should return error
> 
> >
> > -       if (flags & GPIOD_IS_OUT) {
> > -               int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
> > +               return ret;
> > +       }

In fact return is inserted here (test moved in check_dir_flags)

The final code become :

	ret = check_dir_flags(flags);
	if (ret) {
		dev_dbg(dev,
			"%s error: set_dir_flags for gpio %s%d has invalid dir flags 0x%lx\n",
			desc->dev->name,
			uc_priv->bank_name ? uc_priv->bank_name : "",
			desc->offset, flags);

		return ret;
	}

	/* GPIOD_ are directly managed by driver in set_dir_flags*/

> >
> > -               if (flags & GPIOD_ACTIVE_LOW)
> > -                       value = !value;
> > -               ret = ops->direction_output(dev, desc->offset, value);
> > -       } else  if (flags & GPIOD_IS_IN) {
> > -               ret = ops->direction_input(dev, desc->offset);
> > +       /* GPIOD_ are directly managed by driver in set_dir_flags*/
> > +       if (ops->set_dir_flags) {
> > +               ret = ops->set_dir_flags(dev, desc->offset, flags);
> > +       } else {
> > +               if (flags & GPIOD_IS_OUT) {
> > +                       ret = ops->direction_output(dev, desc->offset,
> > +                                                   GPIOD_FLAGS_OUTPUT(flags));
> > +               } else if (flags & GPIOD_IS_IN) {
> > +                       ret = ops->direction_input(dev, desc->offset);
> > +               }
> >         }
> > +
> > +       return ret;
> > +}
> > +
> > +int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags) {
> > +       int ret;
> > +
> > +       ret = check_reserved(desc, "set_dir_flags");
> >         if (ret)
> >                 return ret;
> > -       /*
> > -        * Update desc->flags here, so that GPIO_ACTIVE_LOW is honoured in
> > -        * futures
> > -        */
> > -       desc->flags = flags;
> >
> > -       return 0;
> > +       /* combine the requested flags (for IN/OUT) and the descriptor flags */
> > +       flags |= desc->flags;
> > +       ret = _dm_gpio_set_dir_flags(desc, flags);
> > +
> > +       /* update the descriptor flags */
> > +       if (ret)
> > +               desc->flags = flags;
> > +
> > +       return ret;
> >  }
> >
> >  int dm_gpio_set_dir(struct gpio_desc *desc)  {
> > -       return dm_gpio_set_dir_flags(desc, desc->flags);
> > +       int ret;
> > +
> > +       ret = check_reserved(desc, "set_dir");
> > +       if (ret)
> > +               return ret;
> > +
> > +       return _dm_gpio_set_dir_flags(desc, desc->flags); }
> > +
> > +int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong *flags) {
> > +       struct udevice *dev = desc->dev;
> > +       struct dm_gpio_ops *ops = gpio_get_ops(dev);
> > +       int ret, value;
> > +       ulong dir_flags;
> > +
> > +       ret = check_reserved(desc, "get_dir_flags");
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* GPIOD_ are directly provided by driver except
> > + GPIOD_ACTIVE_LOW*/
> 
> Space before *

ok

> > +       if (ops->get_dir_flags) {
> > +               ret = ops->get_dir_flags(dev, desc->offset, &dir_flags);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               /* GPIOD_ACTIVE_LOW is saved in desc->flags */
> > +               value = dir_flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
> > +               if (desc->flags & GPIOD_ACTIVE_LOW)
> > +                       value = !value;
> > +               dir_flags &= ~(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE);
> > +               dir_flags |= (desc->flags & GPIOD_ACTIVE_LOW);
> > +               if (value)
> > +                       dir_flags |= GPIOD_IS_OUT_ACTIVE;
> > +       } else {
> > +               dir_flags = desc->flags;
> > +               /* only GPIOD_IS_OUT_ACTIVE is provided by uclass */
> > +               dir_flags &= ~GPIOD_IS_OUT_ACTIVE;
> > +               if ((desc->flags & GPIOD_IS_OUT) && _gpio_get_value(desc))
> > +                       dir_flags |= GPIOD_IS_OUT_ACTIVE;
> > +       }
> > +       *flags = dir_flags;
> > +
> > +       return 0;
> >  }
> >
> >  /**
> > @@ -809,7 +914,7 @@ static int gpio_request_tail(int ret, const char
> *nodename,
> >                 debug("%s: dm_gpio_requestf failed\n", __func__);
> >                 goto err;
> >         }
> > -       ret = dm_gpio_set_dir_flags(desc, flags | desc->flags);
> > +       ret = dm_gpio_set_dir_flags(desc, flags);
> >         if (ret) {
> >                 debug("%s: dm_gpio_set_dir failed\n", __func__);
> >                 goto err;
> > @@ -1036,6 +1141,10 @@ static int gpio_post_bind(struct udevice *dev)
> >                         ops->get_function += gd->reloc_off;
> >                 if (ops->xlate)
> >                         ops->xlate += gd->reloc_off;
> > +               if (ops->set_dir_flags)
> > +                       ops->set_dir_flags += gd->reloc_off;
> > +               if (ops->get_dir_flags)
> > +                       ops->get_dir_flags += gd->reloc_off;
> >
> >                 reloc_done++;
> >         }
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index 454578c8d2..c6991be1c9 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -117,10 +117,15 @@ struct udevice;
> >  struct gpio_desc {
> >         struct udevice *dev;    /* Device, NULL for invalid GPIO */
> >         unsigned long flags;
> > +
> >  #define GPIOD_IS_OUT           BIT(1)  /* GPIO is an output */
> >  #define GPIOD_IS_IN            BIT(2)  /* GPIO is an input */
> >  #define GPIOD_ACTIVE_LOW       BIT(3)  /* value has active low */
> 
> s/has/is/ ?

GPIO is active when value is low

> 
> >  #define GPIOD_IS_OUT_ACTIVE    BIT(4)  /* set output active */
> > +#define GPIOD_OPEN_DRAIN       BIT(5)  /* GPIO is open drain type */
> > +#define GPIOD_OPEN_SOURCE      BIT(6)  /* GPIO is open source type */
> > +#define GPIOD_PULL_UP          BIT(7)  /* GPIO has pull up enabled */
> > +#define GPIOD_PULL_DOWN                BIT(8)  /* GPIO has pull down enabled
> */
> 
> pull-down or pulldown

Ok => pull-down

> >
> >         uint offset;            /* GPIO offset within the device */
> >         /*
> > @@ -129,6 +134,12 @@ struct gpio_desc {
> >          */
> >  };
> >
> > +/* helper to compute the value of the gpio output */ #define
> > +GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW |
> GPIOD_IS_OUT_ACTIVE)
> > +#define GPIOD_FLAGS_OUTPUT(flags) \
> > +       (((((flags) & GPIOD_FLAGS_OUTPUT_MASK) ==
> GPIOD_IS_OUT_ACTIVE) || \
> > +         (((flags) & GPIOD_FLAGS_OUTPUT_MASK) ==
> GPIOD_ACTIVE_LOW)) ?
> > +1 : 0)
> > +
> 
> You can drop the ? 1 : 0 since boolean expressions do that anyway.

Yes I drop it .... 

> >  /**
> >   * dm_gpio_is_valid() - Check if a GPIO is valid
> >   *
> > @@ -253,6 +264,7 @@ struct dm_gpio_ops {
> >                                 int value);
> >         int (*get_value)(struct udevice *dev, unsigned offset);
> >         int (*set_value)(struct udevice *dev, unsigned offset, int
> > value);
> > +
> >         /**
> >          * get_function() Get the GPIO function
> >          *
> > @@ -287,6 +299,37 @@ struct dm_gpio_ops {
> >          */
> >         int (*xlate)(struct udevice *dev, struct gpio_desc *desc,
> >                      struct ofnode_phandle_args *args);
> > +
> > +       /**
> > +        * set_dir_flags() - Set GPIO dir flags
> > +        *
> > +        * This function should set up the GPIO configuration according to the
> > +        * information provide by the direction flags bitfield.
> > +        *
> > +        * This method is optional.
> > +        *
> > +        * @dev:        GPIO device
> > +        * @offset:     GPIO offset within that device
> > +        * @flags:      GPIO configuration to use
> > +        * @return 0 if OK, -ve on error
> > +        */
> > +       int (*set_dir_flags)(struct udevice *dev, unsigned int offset,
> > +                            ulong flags);
> > +
> > +       /**
> > +        * get_dir_flags() - Get GPIO dir flags
> > +        *
> > +        * This function return the GPIO direction flags used.
> > +        *
> > +        * This method is optional.
> > +        *
> > +        * @dev:        GPIO device
> > +        * @offset:     GPIO offset within that device
> > +        * @flags:      place to put the used direction flags by GPIO
> > +        * @return 0 if OK, -ve on error
> > +        */
> > +       int (*get_dir_flags)(struct udevice *dev, unsigned int offset,
> > +                            ulong *flags);
> >  };
> >
> >  /**
> > @@ -586,8 +629,7 @@ int dm_gpio_set_value(const struct gpio_desc
> > *desc, int value);
> >  /**
> >   * dm_gpio_set_dir() - Set the direction for a GPIO
> >   *
> > - * This sets up the direction according tot the provided flags. It
> > will do
> > - * nothing unless the direction is actually specified.
> > + * This sets up the direction according to the GPIO flags: desc->flags.
> >   *
> >   * @desc:      GPIO description containing device, offset and flags,
> >   *             previously returned by gpio_request_by_name()
> > @@ -596,11 +638,10 @@ int dm_gpio_set_value(const struct gpio_desc
> > *desc, int value);  int dm_gpio_set_dir(struct gpio_desc *desc);
> >
> >  /**
> > - * dm_gpio_set_dir_flags() - Set direction using specific flags
> > + * dm_gpio_set_dir_flags() - Set direction using add flags
> >   *
> > - * This is like dm_gpio_set_dir() except that the flags value is
> > provided
> > - * instead of being used from desc->flags. This is needed because in
> > many
> > - * cases the GPIO description does not include direction information.
> > + * This sets up the direction according tot the provided flags and
> > + the GPIO
> > + * description (desc->flags) which include direction information.
> >   * Note that desc->flags is updated by this function.
> >   *
> >   * @desc:      GPIO description containing device, offset and flags,
> > @@ -610,6 +651,18 @@ int dm_gpio_set_dir(struct gpio_desc *desc);
> >   */
> >  int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags);
> >
> > +/**
> > + * dm_gpio_get_dir_flags() - Get direction flags
> > + *
> > + * read the current direction flags
> > + *
> > + * @desc:      GPIO description containing device, offset and flags,
> > + *             previously returned by gpio_request_by_name()
> > + * @flags:     place to put the used flags
> > + * @return 0 if OK, -ve on error, in which case desc->flags is not
> > +updated  */ int dm_gpio_get_dir_flags(struct gpio_desc *desc, ulong
> > +*flags);
> > +
> >  /**
> >   * gpio_get_number() - Get the global GPIO number of a GPIO
> >   *
> > --
> > 2.17.1
> >
> 
> There's a lot going on in this patch.

Yes, I can split the work to help the review

1/ prepare
2/ add get_dir_flags
3/ add set_dir_flags

> Regards,
> Simon

Regards
Patrick

[PATCH v2 14/14] test: pinmux: add pincontrol-gpio for pin configuration

[Patrick DELAUNAY]

Hi Simon,

> From: Simon Glass <sjg@chromium.org>
> Sent: lundi 30 décembre 2019 02:21
> 
> On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > Add a simple pincontrol associated to the sandbox gpio driver, that
> > allows to check pin configuration with the command pinmux.
> >
> > The pmux test is also updated to test behavior with 2 pincontrols.
> >
> > Example to check LED pin configuration:
> >
> > => pinmux list
> > | Device                        | Driver                        | Parent
> > | pinctrl-gpio                  | sandbox_pinctrl_gpio          | root_driver
> > | pinctrl                       | sandbox_pinctrl               | root_driver
> >
> > => pinmux dev pinctrl-gpio
> >
> > => pinmux status
> >
> > a0        : gpio input .
> > a1        : gpio input .
> > a2        : gpio input .
> > a3        : gpio input .
> > a4        : gpio input .
> > a5        : gpio output .
> > a6        : gpio output .
> > ...
> >
> > Serie-cc: Heiko Schocher <hs@denx.de>
> > Serie-cc: Simon Glass <sjg@chromium.org>
> 
> Series-cc :-)

Oups :-)

 
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> >
> > ---
> >
> > Changes in v2:
> > - Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in
> >   sandbox gpio driver
> > - rebase on v2020.01-rc3
> >
> >  arch/sandbox/dts/test.dts    |  48 +++++----
> >  drivers/gpio/sandbox.c       | 195 +++++++++++++++++++++++++++++++++++
> >  test/py/tests/test_pinmux.py |  10 ++
> >  3 files changed, 231 insertions(+), 22 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks for the review

I am preparing a v3.

Do you think I need to split the patch 
	[PATCH v2 08/14] gpio: add ops for configuration with dir flags

Regards.

Patrick

[PATCH v2 14/14] test: pinmux: add pincontrol-gpio for pin configuration

[Simon Glass]

Hi Patrick,

On Thu, 9 Jan 2020 at 23:30, Patrick DELAUNAY <patrick.delaunay@st.com> wrote:
>
> Hi Simon,
>
> > From: Simon Glass <sjg@chromium.org>
> > Sent: lundi 30 décembre 2019 02:21
> >
> > On Tue, 26 Nov 2019 at 01:49, Patrick Delaunay <patrick.delaunay@st.com>
> > wrote:
> > >
> > > Add a simple pincontrol associated to the sandbox gpio driver, that
> > > allows to check pin configuration with the command pinmux.
> > >
> > > The pmux test is also updated to test behavior with 2 pincontrols.
> > >
> > > Example to check LED pin configuration:
> > >
> > > => pinmux list
> > > | Device                        | Driver                        | Parent
> > > | pinctrl-gpio                  | sandbox_pinctrl_gpio          | root_driver
> > > | pinctrl                       | sandbox_pinctrl               | root_driver
> > >
> > > => pinmux dev pinctrl-gpio
> > >
> > > => pinmux status
> > >
> > > a0        : gpio input .
> > > a1        : gpio input .
> > > a2        : gpio input .
> > > a3        : gpio input .
> > > a4        : gpio input .
> > > a5        : gpio output .
> > > a6        : gpio output .
> > > ...
> > >
> > > Serie-cc: Heiko Schocher <hs@denx.de>
> > > Serie-cc: Simon Glass <sjg@chromium.org>
> >
> > Series-cc :-)
>
> Oups :-)
>
>
> > >
> > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Adapt sandbox_pinctrl_gpio driver with the saved dir_flags in
> > >   sandbox gpio driver
> > > - rebase on v2020.01-rc3
> > >
> > >  arch/sandbox/dts/test.dts    |  48 +++++----
> > >  drivers/gpio/sandbox.c       | 195 +++++++++++++++++++++++++++++++++++
> > >  test/py/tests/test_pinmux.py |  10 ++
> > >  3 files changed, 231 insertions(+), 22 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Thanks for the review
>
> I am preparing a v3.
>
> Do you think I need to split the patch
>         [PATCH v2 08/14] gpio: add ops for configuration with dir flags
>

I am in favour of that.

Regards,
Simon