All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol
@ 2019-10-23 13:44 Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
                   ` (12 more replies)
  0 siblings, 13 replies; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot


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 phasis set_open_drain/get_open_drain could be
    replaced by the new ops get_config/set_config to avoid to
    manage 2 API in parallel.

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.

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

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


Regards

Patrick Delaunay



Patrick Delaunay (13):
  pinctrol: dm: remove the function pinctrl_decode_pin_config
  dm: pinctrl: convert pinctrl-single to livetree
  dm: core: add ofnode 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: add support for new flags on gpio configuration
  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 flag support
  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 +++++--
 doc/README.gpio                         |  42 ++++
 doc/device-tree-bindings/gpio/gpio.txt  | 304 ++++++++++++++----------
 drivers/core/of_access.c                |  32 +++
 drivers/core/ofnode.c                   |  45 ++++
 drivers/gpio/gpio-uclass.c              |  62 ++++-
 drivers/gpio/sandbox.c                  | 281 ++++++++++++++++++++--
 drivers/pinctrl/pinctrl-generic.c       |  36 ++-
 drivers/pinctrl/pinctrl-sandbox.c       |  44 +++-
 drivers/pinctrl/pinctrl-single.c        |  33 ++-
 drivers/pinctrl/pinctrl-uclass.c        |  12 -
 include/asm-generic/gpio.h              |  56 +++++
 include/dm/of_access.h                  |  40 ++++
 include/dm/ofnode.h                     |  39 ++-
 include/dm/pinctrl.h                    |  13 -
 include/dt-bindings/gpio/gpio.h         |   6 +
 include/dt-bindings/gpio/sandbox-gpio.h |  24 ++
 test/dm/gpio.c                          |  31 ++-
 test/dm/test-fdt.c                      |   2 +-
 test/py/tests/test_pinmux.py            |  38 ++-
 20 files changed, 984 insertions(+), 241 deletions(-)
 create mode 100644 doc/README.gpio
 create mode 100644 include/dt-bindings/gpio/sandbox-gpio.h

-- 
2.17.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-11-14 13:43   ` sjg at google.com
  2019-10-23 13:44 ` [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree Patrick Delaunay
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Remove the pinctrl_decode_pin_config() API, because this
function is unused and not compatible with livetree
(it uses fdtdec_get_bool instead of ofnode API).

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

 drivers/pinctrl/pinctrl-uclass.c | 12 ------------
 include/dm/pinctrl.h             | 13 -------------
 2 files changed, 25 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-uclass.c b/drivers/pinctrl/pinctrl-uclass.c
index 761ee29f41..3425ed11b1 100644
--- a/drivers/pinctrl/pinctrl-uclass.c
+++ b/drivers/pinctrl/pinctrl-uclass.c
@@ -15,18 +15,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int pinctrl_decode_pin_config(const void *blob, int node)
-{
-	int flags = 0;
-
-	if (fdtdec_get_bool(blob, node, "bias-pull-up"))
-		flags |= 1 << PIN_CONFIG_BIAS_PULL_UP;
-	else if (fdtdec_get_bool(blob, node, "bias-pull-down"))
-		flags |= 1 << PIN_CONFIG_BIAS_PULL_DOWN;
-
-	return flags;
-}
-
 #if CONFIG_IS_ENABLED(PINCTRL_FULL)
 /**
  * pinctrl_config_one() - apply pinctrl settings for a single node
diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
index 3eca34fbf7..692e5fc8cb 100644
--- a/include/dm/pinctrl.h
+++ b/include/dm/pinctrl.h
@@ -369,19 +369,6 @@ int pinctrl_request_noflags(struct udevice *dev, int func);
  */
 int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
 
-/**
- * pinctrl_decode_pin_config() - decode pin configuration flags
- *
- * This decodes some of the PIN_CONFIG values into flags, with each value
- * being (1 << pin_cfg). This does not support things with values like the
- * slew rate.
- *
- * @blob:	Device tree blob
- * @node:	Node containing the PIN_CONFIG values
- * @return decoded flag value, or -ve on error
- */
-int pinctrl_decode_pin_config(const void *blob, int node);
-
 /**
  * pinctrl_get_gpio_mux() - get the mux value for a particular GPIO
  *
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property Patrick Delaunay
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Convert 'pinctrl-single' using livetree functions
- ofnode_get_property
- ofnode_read_u32_default
- ofnode_read_u32_array
- ofnode_read_bool
- dev_read_addr
and get rid of DECLARE_GLOBAL_DATA_PTR.

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

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

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 1dfc97dcea..67429d6995 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,12 @@ 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 = ofnode_get_property(dev_ofnode(dev), "pinctrl-single,pins",
+				   &len);
 
 	if (prop) {
 		dev_dbg(dev, "configuring pins for %s\n", config->name);
@@ -136,9 +133,9 @@ 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 = ofnode_get_property(dev_ofnode(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 +157,27 @@ 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 = ofnode_read_u32_default(dev_ofnode(dev),
+					       "pinctrl-single,register-width",
+					       0);
 
-	res = fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev),
-				   "reg", of_reg, 2);
+	res = ofnode_read_u32_array(dev_ofnode(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 = ofnode_read_u32_default(dev_ofnode(dev),
+					      "pinctrl-single,function-mask",
+					      0xffffffff);
+	pdata->bits_per_mux = ofnode_read_bool(dev_ofnode(dev),
+					       "pinctrl-single,bit-per-mux");
 
 	return 0;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree Patrick Delaunay
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Add functions to iterate on all property with livetree
- ofnode_get_first_property
- ofnode_get_next_property
- ofnode_get_property_by_prop

For example:
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>
---

 drivers/core/of_access.c | 32 ++++++++++++++++++++++++++++
 drivers/core/ofnode.c    | 45 ++++++++++++++++++++++++++++++++++++++++
 include/dm/of_access.h   | 40 +++++++++++++++++++++++++++++++++++
 include/dm/ofnode.h      | 39 +++++++++++++++++++++++++++++++++-
 4 files changed, 155 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 297f0a0c7c..4169befd92 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -536,6 +536,51 @@ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp)
 				   propname, lenp);
 }
 
+const void *ofnode_get_first_property(ofnode node)
+{
+	int prop_offset;
+
+	if (ofnode_is_np(node)) {
+		return of_get_first_property(ofnode_to_np(node));
+	} else {
+		prop_offset = fdt_first_property_offset(gd->fdt_blob,
+							ofnode_to_offset(node));
+		if (prop_offset < 0)
+			return NULL;
+
+		return (void *)(uintptr_t)prop_offset;
+	}
+}
+
+const void *ofnode_get_next_property(ofnode node, const void *property)
+{
+	int prop_offset;
+
+	if (ofnode_is_np(node)) {
+		return of_get_next_property(ofnode_to_np(node), property);
+	} else {
+		prop_offset = (uintptr_t)property;
+		prop_offset = fdt_next_property_offset(gd->fdt_blob,
+						       prop_offset);
+		if (prop_offset < 0)
+			return NULL;
+
+		return (void *)(uintptr_t)prop_offset;
+	}
+}
+
+const void *ofnode_get_property_by_prop(ofnode node, const void *property,
+					const char **propname, int *lenp)
+{
+	if (ofnode_is_np(node))
+		return of_get_property_by_prop(ofnode_to_np(node),
+					       property, propname, lenp);
+	else
+		return fdt_getprop_by_offset(gd->fdt_blob,
+					     (uintptr_t)property,
+					     propname, lenp);
+}
+
 bool ofnode_is_available(ofnode node)
 {
 	if (ofnode_is_np(node))
diff --git a/include/dm/of_access.h b/include/dm/of_access.h
index 13fedb7cf5..0418782aa2 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().
+ *
+ * @p: 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().
+ *
+ * @p: 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..08d684cea0 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -543,7 +543,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 +552,43 @@ 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 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 ofnode_get_property_by_prop().
+ *
+ * @node: node to read
+ * @return pointer or offset to property, used to iterate, or NULL
+ */
+const void *ofnode_get_first_property(ofnode node);
+
+/**
+ * ofnode_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 ofnode_get_property_by_prop().
+ *
+ * @node: node to read
+ * @property: pointer or offset of the current property
+ * @return pointer or offset to next property or NULL
+ */
+const void *ofnode_get_next_property(ofnode node, const void *property);
+
+/**
+ * ofnode_get_property_by_prop() - get a pointer to the value of a node property
+ *
+ * Get value for the property identified by node and property.
+ *
+ * @node: node to read
+ * @property: pointer or offset of the property to read
+ * @propname: place to property name on success
+ * @lenp: place to put length on success
+ * @return pointer to property or NULL if error
+ */
+const void *ofnode_get_property_by_prop(ofnode node, const void *property,
+					const char **propname, int *lenp);
+
 /**
  * ofnode_is_available() - check if a node is marked available
  *
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (2 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 05/13] dt-bindings: gpio: document the new pull-up/pull-down flags Patrick Delaunay
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Migrate pinctrl-generic to livetree:
- ofnode_get_first_property
- ofnode_get_next_property
- ofnode_get_property_by_prop
- ofnode_read_string_count
- ofnode_read_string_index
and get rid of DECLARE_GLOBAL_DATA_PTR.

This solve parsing issue during test in sandbox 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>
---

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

diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index eecf0f5dc1..474a2e65ca 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,18 @@ 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;
+	const void *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);
+	for (property = ofnode_get_first_property(dev_ofnode(config));
+	     property;
+	     property = ofnode_get_next_property(dev_ofnode(config),
+						 property)) {
+		value = ofnode_get_property_by_prop(dev_ofnode(config),
+						    property, &propname, &len);
 		if (!value)
 			return -EINVAL;
 
@@ -298,19 +296,18 @@ 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 = ofnode_read_string_count(dev_ofnode(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 = ofnode_read_string_count(dev_ofnode(config),
+							 subnode_target_type);
 		if (strings_count < 0) {
 			/* skip this node; may contain config child nodes */
 			return 0;
@@ -318,10 +315,11 @@ 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 = ofnode_read_string_index(dev_ofnode(config),
+					       subnode_target_type, i,
+					       &name);
+		if (ret)
+			return ret;
 
 		if (is_group)
 			selector = pinctrl_group_name_to_selector(dev, name);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 05/13] dt-bindings: gpio: document the new pull-up/pull-down flags
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (3 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration Patrick Delaunay
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (4 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 05/13] dt-bindings: gpio: document the new pull-up/pull-down flags Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 07/13] dt-bindings: gpio: alignment with kernel v5.3 Patrick Delaunay
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

This commit manages the 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 and the Open Drain/Open Source configuration
for input GPIO.

It is managed in driver with a new ops in gpio uclass set_config.

These flags are already support in Linux kernel in gpiolib.

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

 drivers/gpio/gpio-uclass.c | 62 +++++++++++++++++++++++++++++++++++++-
 include/asm-generic/gpio.h | 56 ++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 90fbed455b..c6c6e8b343 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -128,7 +128,19 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
 		return 0;
 
 	if (args->args[1] & GPIO_ACTIVE_LOW)
-		desc->flags = GPIOD_ACTIVE_LOW;
+		desc->flags |= GPIOD_ACTIVE_LOW;
+
+	if ((args->args[1] & GPIO_OPEN_DRAIN) == GPIO_OPEN_DRAIN)
+		desc->flags |= GPIOD_OPEN_DRAIN;
+
+	if ((args->args[1] & GPIO_OPEN_SOURCE) == GPIO_OPEN_SOURCE)
+		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;
 }
@@ -517,12 +529,31 @@ int dm_gpio_set_open_drain(struct gpio_desc *desc, int value)
 
 	if (ops->set_open_drain)
 		ret = ops->set_open_drain(desc->dev, desc->offset, value);
+	else if (ops->set_config)
+		ret = ops->set_config(desc->dev, desc->offset,
+				      value ? GPIO_CONF_DRIVE_OPEN_DRAIN :
+					      GPIO_CONF_DRIVE_PULL_PUSH);
 	else
 		return 0; /* feature not supported -> ignore setting */
 
 	return ret;
 }
 
+int gpio_get_config(const struct gpio_desc *desc)
+{
+	struct dm_gpio_ops *ops = gpio_get_ops(desc->dev);
+	int ret;
+
+	ret = check_reserved(desc, "get_config");
+	if (ret)
+		return ret;
+
+	if (ops->set_config)
+		return ops->get_config(desc->dev, desc->offset);
+	else
+		return -ENOSYS;
+}
+
 int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
 {
 	struct udevice *dev = desc->dev;
@@ -533,14 +564,39 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
 	if (ret)
 		return ret;
 
+	/* both pull-up and pull-down enabled, invalid configuration */
+	if ((flags & GPIOD_PULL_UP) && (flags & GPIOD_PULL_DOWN))
+		return -EINVAL;
+
 	if (flags & GPIOD_IS_OUT) {
 		int value = flags & GPIOD_IS_OUT_ACTIVE ? 1 : 0;
 
+		if (ops->set_config) {
+			if (flags & GPIOD_OPEN_DRAIN)
+				ops->set_config(dev, desc->offset,
+						GPIO_CONF_DRIVE_OPEN_DRAIN);
+			else if (flags & GPIOD_OPEN_SOURCE)
+				ops->set_config(dev, desc->offset,
+						GPIO_CONF_DRIVE_OPEN_SOURCE);
+			else
+				ops->set_config(dev, desc->offset,
+						GPIO_CONF_DRIVE_PULL_PUSH);
+		}
+
 		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);
+
+		if (ops->set_config) {
+			if (flags & GPIOD_PULL_UP)
+				ops->set_config(dev, desc->offset,
+						GPIO_CONF_BIAS_PULL_UP);
+			else if (flags & GPIOD_PULL_DOWN)
+				ops->set_config(dev, desc->offset,
+						GPIO_CONF_BIAS_PULL_DOWN);
+		}
 	}
 	if (ret)
 		return ret;
@@ -1061,6 +1117,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_config)
+			ops->set_config += gd->reloc_off;
+		if (ops->get_config)
+			ops->get_config += gd->reloc_off;
 
 		reloc_done++;
 	}
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index d6cf18744f..9441e7dccc 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -112,6 +112,17 @@ enum gpio_func_t {
 	GPIOF_COUNT,
 };
 
+enum gpio_config {
+	GPIO_CONF_NONE,
+	GPIO_CONF_DRIVE_OPEN_DRAIN,
+	GPIO_CONF_DRIVE_OPEN_SOURCE,
+	GPIO_CONF_DRIVE_PULL_PUSH,
+	GPIO_CONF_BIAS_PULL_DOWN,
+	GPIO_CONF_BIAS_PULL_UP,
+
+	GPIO_CONF_COUNT,
+};
+
 struct udevice;
 
 struct gpio_desc {
@@ -122,6 +133,10 @@ struct gpio_desc {
 #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_OPEN_DRAIN	(1 << 5)	/* GPIO is open drain type */
+#define GPIOD_OPEN_SOURCE	(1 << 6)	/* GPIO is open source type */
+#define GPIOD_PULL_UP		(1 << 7)	/* GPIO has pull up enabled */
+#define GPIOD_PULL_DOWN		(1 << 8)	/* GPIO has pull down enabled */
 
 	uint offset;		/* GPIO offset within the device */
 	/*
@@ -290,6 +305,36 @@ struct dm_gpio_ops {
 	 */
 	int (*xlate)(struct udevice *dev, struct gpio_desc *desc,
 		     struct ofnode_phandle_args *args);
+
+	/**
+	 * set_config() - Set GPIO configuration
+	 *
+	 * This function should set up the GPIO configuration according to the
+	 * information in the arguments.
+	 *
+	 * This method is optional.
+	 *
+	 * @dev:	GPIO device
+	 * @offset:	GPIO offset within that device
+	 * @config:	GPIO configuration
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*set_config)(struct udevice *dev, unsigned offset,
+			  enum gpio_config config);
+
+	/**
+	 * set_config() - Set GPIO configuration
+	 *
+	 * This function return the GPIO configuration according to the
+	 * information in the arguments.
+	 *
+	 * This method is optional.
+	 *
+	 * @dev:	GPIO device
+	 * @offset:	GPIO offset within that device
+	 * @return GPIO configuration (GPIO_CONF_) if OK, -ve on error
+	 */
+	int (*get_config)(struct udevice *dev, unsigned offset);
 };
 
 /**
@@ -657,4 +702,15 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags);
  */
 int gpio_get_number(const struct gpio_desc *desc);
 
+/**
+ * gpio_get_config() - get the current GPIO pin configuration
+ *
+ * Obtain the current GPIO pin configuration
+ *
+ * @desc:	GPIO description containing device, offset and flags,
+ *		previously returned by gpio_request_by_name()
+ * @return GPIO configuration (GPIO_CONF_) if OK, -ve on error
+ */
+int gpio_get_config(const struct gpio_desc *desc);
+
 #endif	/* _ASM_GENERIC_GPIO_H_ */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 07/13] dt-bindings: gpio: alignment with kernel v5.3
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (5 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing Patrick Delaunay
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (6 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 07/13] dt-bindings: gpio: alignment with kernel v5.3 Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node Patrick Delaunay
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (7 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support Patrick Delaunay
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

 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 42b41fbf62..35e80219a8 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -847,6 +847,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (8 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support Patrick Delaunay
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Replace the GPIOF_ defines of gpio UCLASS (they are not bitfields but
enum gpio_func_t = State of a GPIO, as reported by get_function())
by GPIO_FLAG to access to the bitfield 'flags' of struct gpio_state.

This patch avoid confusion between sandbox and gpio UCLASS defines.

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

 drivers/gpio/sandbox.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 2ef5c67ad5..b6c78cc46a 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -11,14 +11,14 @@
 #include <dm/of.h>
 #include <dt-bindings/gpio/gpio.h>
 
-/* 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 */
+/* Internal flags for each GPIO : bitfield*/
+#define GPIO_FLAG_OUTPUT	BIT(0)	/* Currently set as an output */
+#define GPIO_FLAG_HIGH		BIT(1)	/* Currently set high */
+#define GPIO_FLAG_ODR		BIT(2)	/* Currently set to open drain mode */
 
 struct gpio_state {
 	const char *label;	/* label given by requester */
-	u8 flags;		/* flags (GPIOF_...) */
+	u8 flags;		/* bitfield flags (GPIO_FLAG_...) */
 };
 
 /* Access routines for GPIO state */
@@ -60,34 +60,34 @@ 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, GPIO_FLAG_OUTPUT))
 		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, GPIO_FLAG_HIGH);
 }
 
 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, GPIO_FLAG_HIGH, value);
 }
 
 int sandbox_gpio_get_open_drain(struct udevice *dev, unsigned offset)
 {
-	return get_gpio_flag(dev, offset, GPIOF_ODR);
+	return get_gpio_flag(dev, offset, GPIO_FLAG_ODR);
 }
 
 int sandbox_gpio_set_open_drain(struct udevice *dev, unsigned offset, int value)
 {
-	return set_gpio_flag(dev, offset, GPIOF_ODR, value);
+	return set_gpio_flag(dev, offset, GPIO_FLAG_ODR, 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, GPIO_FLAG_OUTPUT);
 }
 
 int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output)
 {
-	return set_gpio_flag(dev, offset, GPIOF_OUTPUT, output);
+	return set_gpio_flag(dev, offset, GPIO_FLAG_OUTPUT, output);
 }
 
 /*
@@ -158,7 +158,7 @@ static int sb_gpio_set_open_drain(struct udevice *dev, unsigned offset, int valu
 
 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, GPIO_FLAG_OUTPUT))
 		return GPIOF_OUTPUT;
 	return GPIOF_INPUT;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (9 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  2019-10-23 13:44 ` [U-Boot] [PATCH 12/13] test: dm: update test for pins configuration in gpio Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration Patrick Delaunay
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

 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 35e80219a8..160eaa0ff7 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 b6c78cc46a..89e4505859 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>
 
 /* Internal flags for each GPIO : bitfield*/
 #define GPIO_FLAG_OUTPUT	BIT(0)	/* Currently set as an output */
@@ -169,13 +170,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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 12/13] test: dm: update test for pins configuration in gpio
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (10 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-23 13:44 ` [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration Patrick Delaunay
  12 siblings, 0 replies; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

Add tests for new API gpio_get_config and gpio_set_config 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>
---

 arch/sandbox/dts/test.dts | 16 +++++++++++++
 drivers/gpio/sandbox.c    | 49 +++++++++++++++++++++++++++++++++++++++
 test/dm/gpio.c            | 31 +++++++++++++++++++++----
 test/dm/test-fdt.c        |  2 +-
 4 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 160eaa0ff7..05f6f84fe3 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)>;
 	};
@@ -292,6 +300,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/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 89e4505859..7a3e9ed520 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -20,6 +20,7 @@
 struct gpio_state {
 	const char *label;	/* label given by requester */
 	u8 flags;		/* bitfield flags (GPIO_FLAG_...) */
+	u8 config;		/* config (GPIO_CONF_...) */
 };
 
 /* Access routines for GPIO state */
@@ -55,6 +56,37 @@ static int set_gpio_flag(struct udevice *dev, unsigned offset, int flag,
 	return 0;
 }
 
+/* Access routines for GPIO configs */
+static u8 *get_sb_gpio_configs(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);
+	static u8 invalid_configs = GPIO_CONF_NONE;
+
+	if (offset >= uc_priv->gpio_count) {
+		printf("sandbox_gpio: error: invalid gpio %u\n", offset);
+
+		return &invalid_configs;
+	}
+
+	return &state[offset].config;
+}
+
+static int get_sb_gpio_config(struct udevice *dev, unsigned int offset)
+{
+	return *get_sb_gpio_configs(dev, offset);
+}
+
+static int set_sb_gpio_config(struct udevice *dev, unsigned int offset,
+			      u8 config)
+{
+	u8 *gpio = get_sb_gpio_configs(dev, offset);
+
+	*gpio = config;
+
+	return 0;
+}
+
 /*
  * Back-channel sandbox-internal-only access to GPIO state
  */
@@ -184,6 +216,21 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
 	return 0;
 }
 
+static int sb_gpio_set_config(struct udevice *dev, unsigned int offset,
+			      enum gpio_config config)
+{
+	debug("%s: offset:%u, config = %d\n", __func__, offset, config);
+
+	return set_sb_gpio_config(dev, offset, config);
+}
+
+static int sb_gpio_get_config(struct udevice *dev, unsigned int offset)
+{
+	debug("%s: offset:%u\n", __func__, offset);
+
+	return get_sb_gpio_config(dev, offset);
+}
+
 static const struct dm_gpio_ops gpio_sandbox_ops = {
 	.direction_input	= sb_gpio_direction_input,
 	.direction_output	= sb_gpio_direction_output,
@@ -193,6 +240,8 @@ static const struct dm_gpio_ops gpio_sandbox_ops = {
 	.set_open_drain		= sb_gpio_set_open_drain,
 	.get_function		= sb_gpio_get_function,
 	.xlate			= sb_gpio_xlate,
+	.set_config		= sb_gpio_set_config,
+	.get_config		= sb_gpio_get_config,
 };
 
 static int sandbox_gpio_ofdata_to_platdata(struct udevice *dev)
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index bb4b20cea9..b1bb5c2453 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));
@@ -243,7 +243,30 @@ 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_pin_config(struct unit_test_state *uts)
+{
+	struct gpio_desc desc_list[6];
+	struct udevice *dev;
+
+	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_asserteq(GPIO_CONF_DRIVE_OPEN_DRAIN, gpio_get_config(&desc_list[0]));
+	ut_asserteq(GPIO_CONF_DRIVE_OPEN_SOURCE,
+		    gpio_get_config(&desc_list[1]));
+	ut_asserteq(GPIO_CONF_DRIVE_PULL_PUSH, gpio_get_config(&desc_list[2]));
+	ut_asserteq(GPIO_CONF_BIAS_PULL_UP, gpio_get_config(&desc_list[3]));
+	ut_asserteq(GPIO_CONF_BIAS_PULL_DOWN, gpio_get_config(&desc_list[4]));
+	ut_asserteq(GPIO_CONF_NONE, gpio_get_config(&desc_list[5]));
+
+	ut_assertok(gpio_free_list(dev, desc_list, 6));
+
+	return 0;
+}
+DM_TEST(dm_test_gpio_pin_config, 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration
  2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
                   ` (11 preceding siblings ...)
  2019-10-23 13:44 ` [U-Boot] [PATCH 12/13] test: dm: update test for pins configuration in gpio Patrick Delaunay
@ 2019-10-23 13:44 ` Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
  12 siblings, 1 reply; 28+ messages in thread
From: Patrick Delaunay @ 2019-10-23 13:44 UTC (permalink / raw)
  To: u-boot

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

The pinux 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 drive-pull-push
a6        : gpio output drive-pull-push
...

Serie-cc: Heiko Schocher <hs@denx.de>

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

---

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

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 05f6f84fe3..a975b2edca 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -284,28 +284,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 7a3e9ed520..3ca8933e84 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -8,7 +8,9 @@
 #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>
 
@@ -23,6 +25,15 @@ struct gpio_state {
 	u8 config;		/* config (GPIO_CONF_...) */
 };
 
+static const char * const gpio_config[GPIO_CONF_COUNT] = {
+	[GPIO_CONF_NONE] = ".",
+	[GPIO_CONF_DRIVE_OPEN_DRAIN] = "drive-open-drain",
+	[GPIO_CONF_DRIVE_OPEN_SOURCE] = "drive-push-pull",
+	[GPIO_CONF_DRIVE_PULL_PUSH] = "drive-pull-push",
+	[GPIO_CONF_BIAS_PULL_DOWN] = "bias-pull-down",
+	[GPIO_CONF_BIAS_PULL_UP] = "bias-pull-up",
+};
+
 /* Access routines for GPIO state */
 static u8 *get_gpio_flags(struct udevice *dev, unsigned offset)
 {
@@ -289,3 +300,185 @@ 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 int sb_pinctrl_get_pin_muxing(struct udevice *dev,
+				     unsigned int selector,
+				     char *buf, int size)
+{
+	struct udevice *gpio_dev;
+	unsigned int gpio_idx;
+	u8 config;
+	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);
+		config = *get_sb_gpio_configs(gpio_dev, gpio_idx);
+
+		snprintf(buf, size, "gpio %s %s",
+			 function == GPIOF_OUTPUT ? "output" : "input",
+			 gpio_config[config]);
+	}
+
+	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..f6e6093bbf 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 drive-pull-push' in output)
+    assert ('a6        : gpio output drive-pull-push' 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

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config
  2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  2019-11-14 13:43   ` sjg at google.com
  1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:44, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Remove the pinctrl_decode_pin_config() API, because this
> function is unused and not compatible with livetree
> (it uses fdtdec_get_bool instead of ofnode API).
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/pinctrl/pinctrl-uclass.c | 12 ------------
>  include/dm/pinctrl.h             | 13 -------------
>  2 files changed, 25 deletions(-)

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree
  2019-10-23 13:44 ` [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Convert 'pinctrl-single' using livetree functions
> - ofnode_get_property
> - ofnode_read_u32_default
> - ofnode_read_u32_array
> - ofnode_read_bool
> - dev_read_addr
> and get rid of DECLARE_GLOBAL_DATA_PTR.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/pinctrl/pinctrl-single.c | 33 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 1dfc97dcea..67429d6995 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,12 @@ 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 = ofnode_get_property(dev_ofnode(dev), "pinctrl-single,pins",
> +                                  &len);

dev_read_prop()

Similarly below

>
>         if (prop) {
>                 dev_dbg(dev, "configuring pins for %s\n", config->name);
> @@ -136,9 +133,9 @@ 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 = ofnode_get_property(dev_ofnode(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 +157,27 @@ 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 = ofnode_read_u32_default(dev_ofnode(dev),
> +                                              "pinctrl-single,register-width",
> +                                              0);
>
> -       res = fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev),
> -                                  "reg", of_reg, 2);
> +       res = ofnode_read_u32_array(dev_ofnode(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 = ofnode_read_u32_default(dev_ofnode(dev),
> +                                             "pinctrl-single,function-mask",
> +                                             0xffffffff);
> +       pdata->bits_per_mux = ofnode_read_bool(dev_ofnode(dev),
> +                                              "pinctrl-single,bit-per-mux");
>
>         return 0;
>  }
> --
> 2.17.1

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property
  2019-10-23 13:44 ` [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  2019-10-31 12:17     ` Patrick DELAUNAY
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add functions to iterate on all property with livetree
> - ofnode_get_first_property
> - ofnode_get_next_property
> - ofnode_get_property_by_prop
>
> For example:
> 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>
> ---
>
>  drivers/core/of_access.c | 32 ++++++++++++++++++++++++++++
>  drivers/core/ofnode.c    | 45 ++++++++++++++++++++++++++++++++++++++++
>  include/dm/of_access.h   | 40 +++++++++++++++++++++++++++++++++++
>  include/dm/ofnode.h      | 39 +++++++++++++++++++++++++++++++++-
>  4 files changed, 155 insertions(+), 1 deletion(-)

Please can you add the dev_read() interface too?  Also need to support
CONFIG_DM_DEV_READ_INLINE in read.h

[..]

> diff --git a/include/dm/of_access.h b/include/dm/of_access.h
> index 13fedb7cf5..0418782aa2 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().
> + *
> + * @p: Pointer to device node

np

> + * @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().
> + *
> + * @p: Pointer to device node

np

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

This can be NULL so please document that

> + * @lenp: place to put length on success

This can be NULL so please document that

> + * @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..08d684cea0 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -543,7 +543,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 +552,43 @@ 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 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 ofnode_get_property_by_prop().
> + *
> + * @node: node to read
> + * @return pointer or offset to property, used to iterate, or NULL
> + */
> +const void *ofnode_get_first_property(ofnode node);
> +
> +/**
> + * ofnode_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 ofnode_get_property_by_prop().
> + *
> + * @node: node to read
> + * @property: pointer or offset of the current property
> + * @return pointer or offset to next property or NULL
> + */
> +const void *ofnode_get_next_property(ofnode node, const void *property);
> +
> +/**
> + * ofnode_get_property_by_prop() - get a pointer to the value of a node property
> + *
> + * Get value for the property identified by node and property.
> + *
> + * @node: node to read
> + * @property: pointer or offset of the property to read

Perhaps you should define an ofprop type for this? It is pretty ugly
to use a pointer.

In fact I wonder if ofprop should be:

struct ofprop {
   ofnode node;
   union {
      int offset;
      struct property *prop;
   };
}


> + * @propname: place to property name on success
> + * @lenp: place to put length on success

These two above can be NULL so please document that

> + * @return pointer to property or NULL if error
> + */
> +const void *ofnode_get_property_by_prop(ofnode node, const void *property,
> +                                       const char **propname, int *lenp);
> +
>  /**
>   * ofnode_is_available() - check if a node is marked available
>   *
> --
> 2.17.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree
  2019-10-23 13:44 ` [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  2019-10-31 14:43     ` Patrick DELAUNAY
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Migrate pinctrl-generic to livetree:
> - ofnode_get_first_property
> - ofnode_get_next_property
> - ofnode_get_property_by_prop
> - ofnode_read_string_count
> - ofnode_read_string_index
> and get rid of DECLARE_GLOBAL_DATA_PTR.
>
> This solve parsing issue during test in sandbox 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>
> ---
>
>  drivers/pinctrl/pinctrl-generic.c | 36 +++++++++++++++----------------
>  1 file changed, 17 insertions(+), 19 deletions(-)
>

It looks like this should use dev_read_...() too.

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing
  2019-10-23 13:44 ` [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> 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>
> ---
>
>  drivers/pinctrl/pinctrl-sandbox.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node
  2019-10-23 13:44 ` [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> 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>
> ---
>
>  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(-)

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support
  2019-10-23 13:44 ` [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Replace the GPIOF_ defines of gpio UCLASS (they are not bitfields but
> enum gpio_func_t = State of a GPIO, as reported by get_function())
> by GPIO_FLAG to access to the bitfield 'flags' of struct gpio_state.
>
> This patch avoid confusion between sandbox and gpio UCLASS defines.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/gpio/sandbox.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support
  2019-10-23 13:44 ` [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> 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>
> ---
>
>  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

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration
  2019-10-23 13:44 ` [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Add a simple pincontrol associated to the sandbox gpio driver,
> that allow to check pin configuration check with the
> command pinmux.
>
> The pinux 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 drive-pull-push
> a6        : gpio output drive-pull-push
> ...
>
> Serie-cc: Heiko Schocher <hs@denx.de>
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
>
> ---
>
>  arch/sandbox/dts/test.dts    |  48 +++++----
>  drivers/gpio/sandbox.c       | 193 +++++++++++++++++++++++++++++++++++
>  test/py/tests/test_pinmux.py |  10 ++
>  3 files changed, 229 insertions(+), 22 deletions(-)

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration
  2019-10-23 13:44 ` [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration Patrick Delaunay
@ 2019-10-30  1:48   ` Simon Glass
  2019-10-31 14:59     ` Patrick DELAUNAY
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2019-10-30  1:48 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> This commit manages the 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 and the Open Drain/Open Source configuration
> for input GPIO.
>
> It is managed in driver with a new ops in gpio uclass set_config.
>
> These flags are already support in Linux kernel in gpiolib.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/gpio/gpio-uclass.c | 62 +++++++++++++++++++++++++++++++++++++-
>  include/asm-generic/gpio.h | 56 ++++++++++++++++++++++++++++++++++
>  2 files changed, 117 insertions(+), 1 deletion(-)
>

To me this seems like a pretty annoying interface. The uclass has to
call the driver multiple times with each enum and the driver may end
up reprogramming the pins multiple times to get it right.

Normally we want to program things correctly once, before enabling the function.

On the other handle I think what you have is better than adding new
methods like set_open_drain().

But overall I think it would be better to define a new struct like
gpio_config that holds some flags and perhaps a few other things. Then
the uclass can set up that struct and call the driver once with it, to
set up the pin. It could include input/output too, so that if
set_config() is defined, the uclass uses that instead of
direction_output(), etc.

What do you think?

Also we should update the sandbox driver to include tests for new
methods. It looks like you have done pinctrl but not this?

Regards,
Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property
  2019-10-30  1:48   ` Simon Glass
@ 2019-10-31 12:17     ` Patrick DELAUNAY
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick DELAUNAY @ 2019-10-31 12:17 UTC (permalink / raw)
  To: u-boot

Hi Simon,

> From: Simon Glass <sjg@chromium.org>
> Sent: mercredi 30 octobre 2019 02:48
> 
> On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > Add functions to iterate on all property with livetree
> > - ofnode_get_first_property
> > - ofnode_get_next_property
> > - ofnode_get_property_by_prop
> >
> > For example:
> > 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>
> > ---
> >
> >  drivers/core/of_access.c | 32 ++++++++++++++++++++++++++++
> >  drivers/core/ofnode.c    | 45 ++++++++++++++++++++++++++++++++++++++++
> >  include/dm/of_access.h   | 40 +++++++++++++++++++++++++++++++++++
> >  include/dm/ofnode.h      | 39 +++++++++++++++++++++++++++++++++-
> >  4 files changed, 155 insertions(+), 1 deletion(-)
> 
> Please can you add the dev_read() interface too?  Also need to support
> CONFIG_DM_DEV_READ_INLINE in read.h

Yes I will it,

 
> [..]
> 
> > diff --git a/include/dm/of_access.h b/include/dm/of_access.h index
> > 13fedb7cf5..0418782aa2 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().
> > + *
> > + * @p: Pointer to device node
> 
> np
> 
> > + * @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().
> > + *
> > + * @p: Pointer to device node
> 
> np
> 
> > + * @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
> 
> This can be NULL so please document that

ok

> > + * @lenp: place to put length on success
> 
> This can be NULL so please document that
> 
> > + * @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..08d684cea0 100644
> > --- a/include/dm/ofnode.h
> > +++ b/include/dm/ofnode.h
> > @@ -543,7 +543,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 +552,43 @@ 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 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 ofnode_get_property_by_prop().
> > + *
> > + * @node: node to read
> > + * @return pointer or offset to property, used to iterate, or NULL
> > +*/ const void *ofnode_get_first_property(ofnode node);
> > +
> > +/**
> > + * ofnode_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 ofnode_get_property_by_prop().
> > + *
> > + * @node: node to read
> > + * @property: pointer or offset of the current property
> > + * @return pointer or offset to next property or NULL  */ const void
> > +*ofnode_get_next_property(ofnode node, const void *property);
> > +
> > +/**
> > + * ofnode_get_property_by_prop() - get a pointer to the value of a
> > +node property
> > + *
> > + * Get value for the property identified by node and property.
> > + *
> > + * @node: node to read
> > + * @property: pointer or offset of the property to read
> 
> Perhaps you should define an ofprop type for this? It is pretty ugly to use a pointer.
> 
> In fact I wonder if ofprop should be:
> 
> struct ofprop {
>    ofnode node;
>    union {
>       int offset;
>       struct property *prop;
>    };
> }

Ok, I will do it in v2

The API become:

int ofnode_get_first_property(ofnode node, struct ofprop *prop);
int ofnode_get_next_property(struct ofprop *prop);

same prop is used as input parameter and ouput parameter.

const void *ofnode_get_property_by_prop(struct ofprop *prop,
					const char **propname, int *lenp);

same for dev_ function 


> > + * @propname: place to property name on success
> > + * @lenp: place to put length on success
> 
> These two above can be NULL so please document that

OK

> > + * @return pointer to property or NULL if error  */ const void
> > +*ofnode_get_property_by_prop(ofnode node, const void *property,
> > +                                       const char **propname, int
> > +*lenp);
> > +
> >  /**
> >   * ofnode_is_available() - check if a node is marked available
> >   *
> > --
> > 2.17.1
> >
> 
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree
  2019-10-30  1:48   ` Simon Glass
@ 2019-10-31 14:43     ` Patrick DELAUNAY
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick DELAUNAY @ 2019-10-31 14:43 UTC (permalink / raw)
  To: u-boot

Hi Simon

> From: Simon Glass <sjg@chromium.org>
> Sent: mercredi 30 octobre 2019 02:48
> 
> Hi Patrick,
> 
> On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > Migrate pinctrl-generic to livetree:
> > - ofnode_get_first_property
> > - ofnode_get_next_property
> > - ofnode_get_property_by_prop
> > - ofnode_read_string_count
> > - ofnode_read_string_index
> > and get rid of DECLARE_GLOBAL_DATA_PTR.
> >
> > This solve parsing issue during test in sandbox 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>
> > ---
> >
> >  drivers/pinctrl/pinctrl-generic.c | 36
> > +++++++++++++++----------------
> >  1 file changed, 17 insertions(+), 19 deletions(-)
> >
> 
> It looks like this should use dev_read_...() too.

Yes I miss it.

Thanks

 
> Regards,
> Simon

Regards
Patrick

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration
  2019-10-30  1:48   ` Simon Glass
@ 2019-10-31 14:59     ` Patrick DELAUNAY
  0 siblings, 0 replies; 28+ messages in thread
From: Patrick DELAUNAY @ 2019-10-31 14:59 UTC (permalink / raw)
  To: u-boot

Hi Simon,

> From: Simon Glass <sjg@chromium.org>
> Sent: mercredi 30 octobre 2019 02:49
> 
> Hi Patrick,
> 
> On Wed, 23 Oct 2019 at 07:45, Patrick Delaunay <patrick.delaunay@st.com>
> wrote:
> >
> > This commit manages the 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 and the Open Drain/Open Source configuration for input
> > GPIO.
> >
> > It is managed in driver with a new ops in gpio uclass set_config.
> >
> > These flags are already support in Linux kernel in gpiolib.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  drivers/gpio/gpio-uclass.c | 62
> > +++++++++++++++++++++++++++++++++++++-
> >  include/asm-generic/gpio.h | 56 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 117 insertions(+), 1 deletion(-)
> >
> 
> To me this seems like a pretty annoying interface. The uclass has to call the driver
> multiple times with each enum and the driver may end up reprogramming the pins
> multiple times to get it right.
> 
> Normally we want to program things correctly once, before enabling the function.
> 
> On the other handle I think what you have is better than adding new methods like
> set_open_drain().
> 
> But overall I think it would be better to define a new struct like gpio_config that
> holds some flags and perhaps a few other things. Then the uclass can set up that
> struct and call the driver once with it, to set up the pin. It could include input/output
> too, so that if
> set_config() is defined, the uclass uses that instead of direction_output(), etc.
> 
> What do you think?

I understand the issue.... 
You think something like the serial ops setconfig/getconfig.

So the API can evaluate without add new ops for each new parameter... 
It is clearly better.

I will think about and try to propose something without break nothing.

> Also we should update the sandbox driver to include tests for new methods. It
> looks like you have done pinctrl but not this?

I think test are done in 
[PATCH 12/13] test: dm: update test for pins configuration in gpio
- dm_test_gpio_pin_config

Do think about other tests ?

> Regards,
> Simon

Regards

Patrick

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config
  2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
  2019-10-30  1:48   ` Simon Glass
@ 2019-11-14 13:43   ` sjg at google.com
  1 sibling, 0 replies; 28+ messages in thread
From: sjg at google.com @ 2019-11-14 13:43 UTC (permalink / raw)
  To: u-boot

On Wed, 23 Oct 2019 at 07:44, Patrick Delaunay <patrick.delaunay@st.com> wrote:
>
> Remove the pinctrl_decode_pin_config() API, because this
> function is unused and not compatible with livetree
> (it uses fdtdec_get_bool instead of ofnode API).
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  drivers/pinctrl/pinctrl-uclass.c | 12 ------------
>  include/dm/pinctrl.h             | 13 -------------
>  2 files changed, 25 deletions(-)

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

Applied to u-boot-dm, thanks!

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-11-14 13:43 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 13:44 [U-Boot] [PATCH 00/13] dm: add support of new binding in gpio and pincontrol Patrick Delaunay
2019-10-23 13:44 ` [U-Boot] [PATCH 01/13] pinctrol: dm: remove the function pinctrl_decode_pin_config Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-11-14 13:43   ` sjg at google.com
2019-10-23 13:44 ` [U-Boot] [PATCH 02/13] dm: pinctrl: convert pinctrl-single to livetree Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-23 13:44 ` [U-Boot] [PATCH 03/13] dm: core: add ofnode function to iterate on node property Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-31 12:17     ` Patrick DELAUNAY
2019-10-23 13:44 ` [U-Boot] [PATCH 04/13] dm: pinctrl: migrate pinctrl-generic to livetree Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-31 14:43     ` Patrick DELAUNAY
2019-10-23 13:44 ` [U-Boot] [PATCH 05/13] dt-bindings: gpio: document the new pull-up/pull-down flags Patrick Delaunay
2019-10-23 13:44 ` [U-Boot] [PATCH 06/13] gpio: add support for new flags on gpio configuration Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-31 14:59     ` Patrick DELAUNAY
2019-10-23 13:44 ` [U-Boot] [PATCH 07/13] dt-bindings: gpio: alignment with kernel v5.3 Patrick Delaunay
2019-10-23 13:44 ` [U-Boot] [PATCH 08/13] pinctrl: sandbox: Add mux information in get_pin_muxing Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-23 13:44 ` [U-Boot] [PATCH 09/13] test: dm: update test for pins configuration in pinctrl node Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-23 13:44 ` [U-Boot] [PATCH 10/13] gpio: sandbox: cleanup flag support Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-23 13:44 ` [U-Boot] [PATCH 11/13] gpio: sandbox: cleanup binding support Patrick Delaunay
2019-10-30  1:48   ` Simon Glass
2019-10-23 13:44 ` [U-Boot] [PATCH 12/13] test: dm: update test for pins configuration in gpio Patrick Delaunay
2019-10-23 13:44 ` [U-Boot] [PATCH 13/13] test: pinmux: add pincontrol-gpio for pin configuration Patrick Delaunay
2019-10-30  1:48   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.