All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 10:25 ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
	Pantelis Antoniou, Linus Walleij, Mark Brown

This is one small chunk of work related to DT overlays for expansion
boards. It would be good to have a way to expose #<list>-cells types of
providers through a connector in a standard way. So we introduce a way
to make "nexus" nodes for these types of properties to remap the consumer
number space to the other side of the connector's number space. It's
basically a copy of the interrupt nexus implementation, but without
the address space matching design and interrupt-parent walking.

The first patch implements a generic method to do this, and the second patch
adds a unit test for it. The third patch is more of an example than anything
else. It shows how we would modify frameworks to use the new API.

Stephen Boyd (3):
  of: Support parsing phandle argument lists through a nexus node
  of: unittest: Add phandle remapping test
  gpio: Support gpio nexus dt bindings

 drivers/gpio/gpiolib-of.c                   |   5 +-
 drivers/of/base.c                           | 146 ++++++++++++++++++++++++++++
 drivers/of/unittest-data/testcases.dts      |  11 +++
 drivers/of/unittest-data/tests-phandle.dtsi |  24 +++++
 drivers/of/unittest.c                       | 124 +++++++++++++++++++++++
 include/linux/of.h                          |  14 +++
 6 files changed, 322 insertions(+), 2 deletions(-)

-- 
2.10.0.297.gf6727b0


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

* [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 10:25 ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

This is one small chunk of work related to DT overlays for expansion
boards. It would be good to have a way to expose #<list>-cells types of
providers through a connector in a standard way. So we introduce a way
to make "nexus" nodes for these types of properties to remap the consumer
number space to the other side of the connector's number space. It's
basically a copy of the interrupt nexus implementation, but without
the address space matching design and interrupt-parent walking.

The first patch implements a generic method to do this, and the second patch
adds a unit test for it. The third patch is more of an example than anything
else. It shows how we would modify frameworks to use the new API.

Stephen Boyd (3):
  of: Support parsing phandle argument lists through a nexus node
  of: unittest: Add phandle remapping test
  gpio: Support gpio nexus dt bindings

 drivers/gpio/gpiolib-of.c                   |   5 +-
 drivers/of/base.c                           | 146 ++++++++++++++++++++++++++++
 drivers/of/unittest-data/testcases.dts      |  11 +++
 drivers/of/unittest-data/tests-phandle.dtsi |  24 +++++
 drivers/of/unittest.c                       | 124 +++++++++++++++++++++++
 include/linux/of.h                          |  14 +++
 6 files changed, 322 insertions(+), 2 deletions(-)

-- 
2.10.0.297.gf6727b0

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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
  2016-11-24 10:25 ` Stephen Boyd
  (?)
@ 2016-11-24 10:25   ` Stephen Boyd
  -1 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: devicetree, Linus Walleij, Pantelis Antoniou, linux-kernel,
	linux-gpio, Mark Brown, linux-arm-kernel

Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.

We need a way to describe the GPIOs routed through the connector
in an SoC agnostic way. Let's introduce nexus property parsing
into the OF core to do this. This is largely based on the
interrupt nexus support we already have. This allows us to remap
a phandle list in a consumer node (e.g. reset-gpios) through a
connector in a generic way (e.g. via gpio-map). Do this in a
generic routine so that we can remap any sort of variable length
phandle list.

Taking GPIOs as an example, the connector would be a GPIO nexus,
supporting the remapping of a GPIO specifier space to multiple
GPIO providers on the SoC. DT would look as shown below, where
'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
expansion port where boards can be plugged in, and
'expansion_device' is a device on the expansion board.

	soc {
		soc_gpio1: gpio-controller1 {
			#gpio-cells = <2>;
		};

		soc_gpio2: gpio-controller2 {
			#gpio-cells = <2>;
		};
	};

	connector: connector {
		#gpio-cells = <2>;
		gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
			   <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
			   <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
			   <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
		gpio-map-mask = <0xf 0x1>;
	};

	expansion_device {
		reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
	};

The GPIO core would use of_parse_phandle_with_args_map() instead
of of_parse_phandle_with_args() and arrive at the same type of
result, a phandle and argument list. The difference is that the
phandle and arguments will be remapped through the nexus node to
the underlying SoC GPIO controller node. In the example above,
we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |  14 +++++
 2 files changed, 160 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d687e6de24a0..693b73f33675 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
 /**
+ * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_args:	optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ *	#list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ *	#list-cells = <1>;
+ * }
+ *
+ * phandle3: node3 {
+ * 	#list-cells = <1>;
+ * 	list-map = <0 &phandle2 3>,
+ * 		   <1 &phandle2 2>,
+ * 		   <2 &phandle1 5 1>;
+ *	list-map-mask = <0x3>;
+ * };
+ *
+ * node4 {
+ *	list = <&phandle1 1 2 &phandle3 0>;
+ * }
+ *
+ * To get a device_node of the `node2' node you may call this:
+ * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
+ * 			      "list-map-mask", 1, &args);
+ */
+int of_parse_phandle_with_args_map(const struct device_node *np,
+				   const char *list_name,
+				   const char *cells_name,
+				   const char *map_name,
+				   const char *mask_name,
+				   int index, struct of_phandle_args *out_args)
+{
+	struct device_node *cur, *new = NULL;
+	const __be32 *map, *mask, *tmp;
+	const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
+	__be32 initial_match_array[MAX_PHANDLE_ARGS];
+	const __be32 *match_array = initial_match_array;
+	int i, ret, map_len, match;
+	u32 list_size, new_size;
+
+	if (index < 0)
+		return -EINVAL;
+
+	ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
+					   out_args);
+	if (ret)
+		return ret;
+
+	/* Get the #<list>-cells property */
+	cur = out_args->np;
+	ret = of_property_read_u32(cur, cells_name, &list_size);
+	if (ret < 0)
+		goto fail;
+
+	/* Precalculate the match array - this simplifies match loop */
+	for (i = 0; i < list_size; i++)
+		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
+
+	while (cur) {
+		/* Get the <list>-map property */
+		map = of_get_property(cur, map_name, &map_len);
+		if (!map)
+			return 0;
+		map_len /= sizeof(u32);
+
+		/* Get the <list>-map-mask property (optional) */
+		mask = of_get_property(cur, mask_name, NULL);
+		if (!mask)
+			mask = dummy_mask;
+
+		/* Iterate through <list>-map property */
+		match = 0;
+		while (map_len > (list_size + 1) && !match) {
+			/* Compare specifiers */
+			match = 1;
+			for (i = 0; i < list_size; i++, map_len--)
+				match &= !((match_array[i] ^ *map++) & mask[i]);
+
+			of_node_put(new);
+			new = of_find_node_by_phandle(be32_to_cpup(map));
+			map++;
+			map_len--;
+
+			/* Check if not found */
+			if (!new)
+				goto fail;
+
+			if (!of_device_is_available(new))
+				match = 0;
+
+			tmp = of_get_property(new, cells_name, NULL);
+			if (!tmp)
+				goto fail;
+
+			new_size = be32_to_cpu(*tmp);
+
+			/* Check for malformed properties */
+			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
+				goto fail;
+			if (map_len < new_size)
+				goto fail;
+
+			/* Move forward by new node's #<list>-cells amount */
+			map += new_size;
+			map_len -= new_size;
+		}
+		if (!match)
+			goto fail;
+
+		/*
+		 * Successfully parsed a <list>-map translation; copy new
+		 * specifier into the out_args structure.
+		 */
+		match_array = map - new_size;
+		for (i = 0; i < new_size; i++)
+			out_args->args[i] = be32_to_cpup(map - new_size + i);
+		out_args->args_count = list_size = new_size;
+		/* Iterate again with new provider */
+		out_args->np = new;
+		of_node_put(cur);
+		cur = new;
+	}
+fail:
+	of_node_put(cur);
+	of_node_put(new);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(of_parse_phandle_with_args_map);
+
+/**
  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
  * @np:		pointer to a device tree node containing a list
  * @list_name:	property name that contains a list
diff --git a/include/linux/of.h b/include/linux/of.h
index d3a9c2e69001..65ff306403a2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_args_map(const struct device_node *np,
+	const char *list_name, const char *cells_name, const char *map_name,
+	const char *mask_name, int index, struct of_phandle_args *out_args);
 extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args);
@@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_parse_phandle_with_args_map(const struct device_node *np,
+						 const char *list_name,
+						 const char *cells_name,
+						 const char *map_name,
+						 const char *mask_name,
+						 int index,
+						 struct of_phandle_args *out_args)
+{
+	return -ENOSYS;
+}
+
 static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args)
-- 
2.10.0.297.gf6727b0

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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-11-24 10:25   ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
	Pantelis Antoniou, Linus Walleij, Mark Brown

Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.

We need a way to describe the GPIOs routed through the connector
in an SoC agnostic way. Let's introduce nexus property parsing
into the OF core to do this. This is largely based on the
interrupt nexus support we already have. This allows us to remap
a phandle list in a consumer node (e.g. reset-gpios) through a
connector in a generic way (e.g. via gpio-map). Do this in a
generic routine so that we can remap any sort of variable length
phandle list.

Taking GPIOs as an example, the connector would be a GPIO nexus,
supporting the remapping of a GPIO specifier space to multiple
GPIO providers on the SoC. DT would look as shown below, where
'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
expansion port where boards can be plugged in, and
'expansion_device' is a device on the expansion board.

	soc {
		soc_gpio1: gpio-controller1 {
			#gpio-cells = <2>;
		};

		soc_gpio2: gpio-controller2 {
			#gpio-cells = <2>;
		};
	};

	connector: connector {
		#gpio-cells = <2>;
		gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
			   <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
			   <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
			   <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
		gpio-map-mask = <0xf 0x1>;
	};

	expansion_device {
		reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
	};

The GPIO core would use of_parse_phandle_with_args_map() instead
of of_parse_phandle_with_args() and arrive at the same type of
result, a phandle and argument list. The difference is that the
phandle and arguments will be remapped through the nexus node to
the underlying SoC GPIO controller node. In the example above,
we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |  14 +++++
 2 files changed, 160 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d687e6de24a0..693b73f33675 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
 /**
+ * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_args:	optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ *	#list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ *	#list-cells = <1>;
+ * }
+ *
+ * phandle3: node3 {
+ * 	#list-cells = <1>;
+ * 	list-map = <0 &phandle2 3>,
+ * 		   <1 &phandle2 2>,
+ * 		   <2 &phandle1 5 1>;
+ *	list-map-mask = <0x3>;
+ * };
+ *
+ * node4 {
+ *	list = <&phandle1 1 2 &phandle3 0>;
+ * }
+ *
+ * To get a device_node of the `node2' node you may call this:
+ * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
+ * 			      "list-map-mask", 1, &args);
+ */
+int of_parse_phandle_with_args_map(const struct device_node *np,
+				   const char *list_name,
+				   const char *cells_name,
+				   const char *map_name,
+				   const char *mask_name,
+				   int index, struct of_phandle_args *out_args)
+{
+	struct device_node *cur, *new = NULL;
+	const __be32 *map, *mask, *tmp;
+	const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
+	__be32 initial_match_array[MAX_PHANDLE_ARGS];
+	const __be32 *match_array = initial_match_array;
+	int i, ret, map_len, match;
+	u32 list_size, new_size;
+
+	if (index < 0)
+		return -EINVAL;
+
+	ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
+					   out_args);
+	if (ret)
+		return ret;
+
+	/* Get the #<list>-cells property */
+	cur = out_args->np;
+	ret = of_property_read_u32(cur, cells_name, &list_size);
+	if (ret < 0)
+		goto fail;
+
+	/* Precalculate the match array - this simplifies match loop */
+	for (i = 0; i < list_size; i++)
+		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
+
+	while (cur) {
+		/* Get the <list>-map property */
+		map = of_get_property(cur, map_name, &map_len);
+		if (!map)
+			return 0;
+		map_len /= sizeof(u32);
+
+		/* Get the <list>-map-mask property (optional) */
+		mask = of_get_property(cur, mask_name, NULL);
+		if (!mask)
+			mask = dummy_mask;
+
+		/* Iterate through <list>-map property */
+		match = 0;
+		while (map_len > (list_size + 1) && !match) {
+			/* Compare specifiers */
+			match = 1;
+			for (i = 0; i < list_size; i++, map_len--)
+				match &= !((match_array[i] ^ *map++) & mask[i]);
+
+			of_node_put(new);
+			new = of_find_node_by_phandle(be32_to_cpup(map));
+			map++;
+			map_len--;
+
+			/* Check if not found */
+			if (!new)
+				goto fail;
+
+			if (!of_device_is_available(new))
+				match = 0;
+
+			tmp = of_get_property(new, cells_name, NULL);
+			if (!tmp)
+				goto fail;
+
+			new_size = be32_to_cpu(*tmp);
+
+			/* Check for malformed properties */
+			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
+				goto fail;
+			if (map_len < new_size)
+				goto fail;
+
+			/* Move forward by new node's #<list>-cells amount */
+			map += new_size;
+			map_len -= new_size;
+		}
+		if (!match)
+			goto fail;
+
+		/*
+		 * Successfully parsed a <list>-map translation; copy new
+		 * specifier into the out_args structure.
+		 */
+		match_array = map - new_size;
+		for (i = 0; i < new_size; i++)
+			out_args->args[i] = be32_to_cpup(map - new_size + i);
+		out_args->args_count = list_size = new_size;
+		/* Iterate again with new provider */
+		out_args->np = new;
+		of_node_put(cur);
+		cur = new;
+	}
+fail:
+	of_node_put(cur);
+	of_node_put(new);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(of_parse_phandle_with_args_map);
+
+/**
  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
  * @np:		pointer to a device tree node containing a list
  * @list_name:	property name that contains a list
diff --git a/include/linux/of.h b/include/linux/of.h
index d3a9c2e69001..65ff306403a2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_args_map(const struct device_node *np,
+	const char *list_name, const char *cells_name, const char *map_name,
+	const char *mask_name, int index, struct of_phandle_args *out_args);
 extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args);
@@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_parse_phandle_with_args_map(const struct device_node *np,
+						 const char *list_name,
+						 const char *cells_name,
+						 const char *map_name,
+						 const char *mask_name,
+						 int index,
+						 struct of_phandle_args *out_args)
+{
+	return -ENOSYS;
+}
+
 static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args)
-- 
2.10.0.297.gf6727b0

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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-11-24 10:25   ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.

We need a way to describe the GPIOs routed through the connector
in an SoC agnostic way. Let's introduce nexus property parsing
into the OF core to do this. This is largely based on the
interrupt nexus support we already have. This allows us to remap
a phandle list in a consumer node (e.g. reset-gpios) through a
connector in a generic way (e.g. via gpio-map). Do this in a
generic routine so that we can remap any sort of variable length
phandle list.

Taking GPIOs as an example, the connector would be a GPIO nexus,
supporting the remapping of a GPIO specifier space to multiple
GPIO providers on the SoC. DT would look as shown below, where
'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
expansion port where boards can be plugged in, and
'expansion_device' is a device on the expansion board.

	soc {
		soc_gpio1: gpio-controller1 {
			#gpio-cells = <2>;
		};

		soc_gpio2: gpio-controller2 {
			#gpio-cells = <2>;
		};
	};

	connector: connector {
		#gpio-cells = <2>;
		gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
			   <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
			   <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
			   <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
		gpio-map-mask = <0xf 0x1>;
	};

	expansion_device {
		reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
	};

The GPIO core would use of_parse_phandle_with_args_map() instead
of of_parse_phandle_with_args() and arrive at the same type of
result, a phandle and argument list. The difference is that the
phandle and arguments will be remapped through the nexus node to
the underlying SoC GPIO controller node. In the example above,
we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h |  14 +++++
 2 files changed, 160 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d687e6de24a0..693b73f33675 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
 /**
+ * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
+ * @np:		pointer to a device tree node containing a list
+ * @list_name:	property name that contains a list
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_args:	optional pointer to output arguments structure (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_args, on error returns appropriate
+ * errno value.
+ *
+ * Caller is responsible to call of_node_put() on the returned out_args->np
+ * pointer.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ *	#list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ *	#list-cells = <1>;
+ * }
+ *
+ * phandle3: node3 {
+ * 	#list-cells = <1>;
+ * 	list-map = <0 &phandle2 3>,
+ * 		   <1 &phandle2 2>,
+ * 		   <2 &phandle1 5 1>;
+ *	list-map-mask = <0x3>;
+ * };
+ *
+ * node4 {
+ *	list = <&phandle1 1 2 &phandle3 0>;
+ * }
+ *
+ * To get a device_node of the `node2' node you may call this:
+ * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
+ * 			      "list-map-mask", 1, &args);
+ */
+int of_parse_phandle_with_args_map(const struct device_node *np,
+				   const char *list_name,
+				   const char *cells_name,
+				   const char *map_name,
+				   const char *mask_name,
+				   int index, struct of_phandle_args *out_args)
+{
+	struct device_node *cur, *new = NULL;
+	const __be32 *map, *mask, *tmp;
+	const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
+	__be32 initial_match_array[MAX_PHANDLE_ARGS];
+	const __be32 *match_array = initial_match_array;
+	int i, ret, map_len, match;
+	u32 list_size, new_size;
+
+	if (index < 0)
+		return -EINVAL;
+
+	ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
+					   out_args);
+	if (ret)
+		return ret;
+
+	/* Get the #<list>-cells property */
+	cur = out_args->np;
+	ret = of_property_read_u32(cur, cells_name, &list_size);
+	if (ret < 0)
+		goto fail;
+
+	/* Precalculate the match array - this simplifies match loop */
+	for (i = 0; i < list_size; i++)
+		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
+
+	while (cur) {
+		/* Get the <list>-map property */
+		map = of_get_property(cur, map_name, &map_len);
+		if (!map)
+			return 0;
+		map_len /= sizeof(u32);
+
+		/* Get the <list>-map-mask property (optional) */
+		mask = of_get_property(cur, mask_name, NULL);
+		if (!mask)
+			mask = dummy_mask;
+
+		/* Iterate through <list>-map property */
+		match = 0;
+		while (map_len > (list_size + 1) && !match) {
+			/* Compare specifiers */
+			match = 1;
+			for (i = 0; i < list_size; i++, map_len--)
+				match &= !((match_array[i] ^ *map++) & mask[i]);
+
+			of_node_put(new);
+			new = of_find_node_by_phandle(be32_to_cpup(map));
+			map++;
+			map_len--;
+
+			/* Check if not found */
+			if (!new)
+				goto fail;
+
+			if (!of_device_is_available(new))
+				match = 0;
+
+			tmp = of_get_property(new, cells_name, NULL);
+			if (!tmp)
+				goto fail;
+
+			new_size = be32_to_cpu(*tmp);
+
+			/* Check for malformed properties */
+			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
+				goto fail;
+			if (map_len < new_size)
+				goto fail;
+
+			/* Move forward by new node's #<list>-cells amount */
+			map += new_size;
+			map_len -= new_size;
+		}
+		if (!match)
+			goto fail;
+
+		/*
+		 * Successfully parsed a <list>-map translation; copy new
+		 * specifier into the out_args structure.
+		 */
+		match_array = map - new_size;
+		for (i = 0; i < new_size; i++)
+			out_args->args[i] = be32_to_cpup(map - new_size + i);
+		out_args->args_count = list_size = new_size;
+		/* Iterate again with new provider */
+		out_args->np = new;
+		of_node_put(cur);
+		cur = new;
+	}
+fail:
+	of_node_put(cur);
+	of_node_put(new);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL(of_parse_phandle_with_args_map);
+
+/**
  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
  * @np:		pointer to a device tree node containing a list
  * @list_name:	property name that contains a list
diff --git a/include/linux/of.h b/include/linux/of.h
index d3a9c2e69001..65ff306403a2 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
 extern int of_parse_phandle_with_args(const struct device_node *np,
 	const char *list_name, const char *cells_name, int index,
 	struct of_phandle_args *out_args);
+extern int of_parse_phandle_with_args_map(const struct device_node *np,
+	const char *list_name, const char *cells_name, const char *map_name,
+	const char *mask_name, int index, struct of_phandle_args *out_args);
 extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args);
@@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_parse_phandle_with_args_map(const struct device_node *np,
+						 const char *list_name,
+						 const char *cells_name,
+						 const char *map_name,
+						 const char *mask_name,
+						 int index,
+						 struct of_phandle_args *out_args)
+{
+	return -ENOSYS;
+}
+
 static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
 	const char *list_name, int cells_count, int index,
 	struct of_phandle_args *out_args)
-- 
2.10.0.297.gf6727b0

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

* [PATCH 2/3] of: unittest: Add phandle remapping test
  2016-11-24 10:25 ` Stephen Boyd
@ 2016-11-24 10:25   ` Stephen Boyd
  -1 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
	Pantelis Antoniou, Linus Walleij, Mark Brown

Test the functionality of of_parse_phandle_with_args_map().

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/unittest-data/testcases.dts      |  11 +++
 drivers/of/unittest-data/tests-phandle.dtsi |  24 ++++++
 drivers/of/unittest.c                       | 124 ++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)

diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index 12f7c3d649c8..f4c653418515 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -26,12 +26,23 @@
 / { __local_fixups__ {
 	testcase-data {
 		phandle-tests {
+			provider4 {
+				phandle-map = <0x00000008 0x00000018
+					       0x00000024 0x0000003c
+					       0x00000050 0x0000005c>;
+			};
 			consumer-a {
 				phandle-list = <0x00000000 0x00000008
 						0x00000018 0x00000028
 						0x00000034 0x00000038>;
 				phandle-list-bad-args = <0x00000000 0x0000000c>;
 			};
+			consumer-b {
+				phandle-list = <0x00000000 0x00000008
+						0x00000018 0x00000024
+						0x00000030 0x00000034>;
+				phandle-list-bad-args = <0x00000000 0x0000000c>;
+			};
 		};
 		interrupts {
 			intmap0 {
diff --git a/drivers/of/unittest-data/tests-phandle.dtsi b/drivers/of/unittest-data/tests-phandle.dtsi
index 5b1527e8a7fb..80428bfafa10 100644
--- a/drivers/of/unittest-data/tests-phandle.dtsi
+++ b/drivers/of/unittest-data/tests-phandle.dtsi
@@ -25,6 +25,17 @@
 				#phandle-cells = <3>;
 			};
 
+			provider4: provider4 {
+				#phandle-cells = <2>;
+				phandle-map = <0 1 &provider1 3>,
+					      <4 0 &provider0>,
+					      <16 5 &provider3 3 5 0>,
+					      <200 8 &provider2 23 54>,
+					      <19 0 &provider0>,
+					      <2 3 &provider3 2 5 3>;
+				phandle-map-mask = <0xff 0xf>;
+			};
+
 			consumer-a {
 				phandle-list =	<&provider1 1>,
 						<&provider2 2 0>,
@@ -43,6 +54,19 @@
 				unterminated-string = [40 41 42 43];
 				unterminated-string-list = "first", "second", [40 41 42 43];
 			};
+
+			consumer-b {
+				phandle-list =	<&provider1 1>,
+						<&provider4 2 3>,
+						<0>,
+						<&provider4 4 256>,
+						<&provider4 0 97>,
+						<&provider0>,
+						<&provider4 19 32>;
+				phandle-list-bad-phandle = <12345678 0 0>;
+				phandle-list-bad-args = <&provider2 1 0>,
+							<&provider4 0>;
+			};
 		};
 	};
 };
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 53c83d66eb7e..52a70da32f04 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -386,6 +386,129 @@ static void __init of_unittest_parse_phandle_with_args(void)
 	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 }
 
+static void __init of_unittest_parse_phandle_with_args_map(void)
+{
+	struct device_node *np, *p0, *p1, *p2, *p3;
+	struct of_phandle_args args;
+	int i, rc;
+
+	np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b");
+	if (!np) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
+	if (!p0) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
+	if (!p1) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
+	if (!p2) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
+	if (!p3) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells");
+	unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc);
+
+	for (i = 0; i < 8; i++) {
+		bool passed = true;
+
+		rc = of_parse_phandle_with_args_map(np, "phandle-list",
+						"#phandle-cells", "phandle-map",
+						"phandle-map-mask", i, &args);
+
+		/* Test the values from tests-phandle.dtsi */
+		switch (i) {
+		case 0:
+			passed &= !rc;
+			passed &= (args.np == p1);
+			passed &= (args.args_count == 1);
+			passed &= (args.args[0] == 1);
+			break;
+		case 1:
+			passed &= !rc;
+			passed &= (args.np == p3);
+			passed &= (args.args_count == 3);
+			passed &= (args.args[0] == 2);
+			passed &= (args.args[1] == 5);
+			passed &= (args.args[2] == 3);
+			break;
+		case 2:
+			passed &= (rc == -ENOENT);
+			break;
+		case 3:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 4:
+			passed &= !rc;
+			passed &= (args.np == p1);
+			passed &= (args.args_count == 1);
+			passed &= (args.args[0] == 3);
+			break;
+		case 5:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 6:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 7:
+			passed &= (rc == -ENOENT);
+			break;
+		default:
+			passed = false;
+		}
+
+		unittest(passed, "index %i - data error on node %s rc=%i\n",
+			 i, args.np->full_name, rc);
+	}
+
+	/* Check for missing list property */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-missing",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 0, &args);
+	unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
+
+	/* Check for missing cells property */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list",
+					    "#phandle-cells-missing",
+					    "phandle-map", "phandle-map-mask",
+					    0, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+	/* Check for bad phandle in list */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 0, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+	/* Check for incorrectly formed argument list */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 1, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+}
+
 static void __init of_unittest_property_string(void)
 {
 	const char *strings[4];
@@ -1951,6 +2074,7 @@ static int __init of_unittest(void)
 	of_unittest_find_node_by_name();
 	of_unittest_dynamic();
 	of_unittest_parse_phandle_with_args();
+	of_unittest_parse_phandle_with_args_map();
 	of_unittest_property_string();
 	of_unittest_property_copy();
 	of_unittest_changeset();
-- 
2.10.0.297.gf6727b0

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

* [PATCH 2/3] of: unittest: Add phandle remapping test
@ 2016-11-24 10:25   ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Test the functionality of of_parse_phandle_with_args_map().

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
 drivers/of/unittest-data/testcases.dts      |  11 +++
 drivers/of/unittest-data/tests-phandle.dtsi |  24 ++++++
 drivers/of/unittest.c                       | 124 ++++++++++++++++++++++++++++
 3 files changed, 159 insertions(+)

diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index 12f7c3d649c8..f4c653418515 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -26,12 +26,23 @@
 / { __local_fixups__ {
 	testcase-data {
 		phandle-tests {
+			provider4 {
+				phandle-map = <0x00000008 0x00000018
+					       0x00000024 0x0000003c
+					       0x00000050 0x0000005c>;
+			};
 			consumer-a {
 				phandle-list = <0x00000000 0x00000008
 						0x00000018 0x00000028
 						0x00000034 0x00000038>;
 				phandle-list-bad-args = <0x00000000 0x0000000c>;
 			};
+			consumer-b {
+				phandle-list = <0x00000000 0x00000008
+						0x00000018 0x00000024
+						0x00000030 0x00000034>;
+				phandle-list-bad-args = <0x00000000 0x0000000c>;
+			};
 		};
 		interrupts {
 			intmap0 {
diff --git a/drivers/of/unittest-data/tests-phandle.dtsi b/drivers/of/unittest-data/tests-phandle.dtsi
index 5b1527e8a7fb..80428bfafa10 100644
--- a/drivers/of/unittest-data/tests-phandle.dtsi
+++ b/drivers/of/unittest-data/tests-phandle.dtsi
@@ -25,6 +25,17 @@
 				#phandle-cells = <3>;
 			};
 
+			provider4: provider4 {
+				#phandle-cells = <2>;
+				phandle-map = <0 1 &provider1 3>,
+					      <4 0 &provider0>,
+					      <16 5 &provider3 3 5 0>,
+					      <200 8 &provider2 23 54>,
+					      <19 0 &provider0>,
+					      <2 3 &provider3 2 5 3>;
+				phandle-map-mask = <0xff 0xf>;
+			};
+
 			consumer-a {
 				phandle-list =	<&provider1 1>,
 						<&provider2 2 0>,
@@ -43,6 +54,19 @@
 				unterminated-string = [40 41 42 43];
 				unterminated-string-list = "first", "second", [40 41 42 43];
 			};
+
+			consumer-b {
+				phandle-list =	<&provider1 1>,
+						<&provider4 2 3>,
+						<0>,
+						<&provider4 4 256>,
+						<&provider4 0 97>,
+						<&provider0>,
+						<&provider4 19 32>;
+				phandle-list-bad-phandle = <12345678 0 0>;
+				phandle-list-bad-args = <&provider2 1 0>,
+							<&provider4 0>;
+			};
 		};
 	};
 };
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 53c83d66eb7e..52a70da32f04 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -386,6 +386,129 @@ static void __init of_unittest_parse_phandle_with_args(void)
 	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
 }
 
+static void __init of_unittest_parse_phandle_with_args_map(void)
+{
+	struct device_node *np, *p0, *p1, *p2, *p3;
+	struct of_phandle_args args;
+	int i, rc;
+
+	np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-b");
+	if (!np) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p0 = of_find_node_by_path("/testcase-data/phandle-tests/provider0");
+	if (!p0) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p1 = of_find_node_by_path("/testcase-data/phandle-tests/provider1");
+	if (!p1) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p2 = of_find_node_by_path("/testcase-data/phandle-tests/provider2");
+	if (!p2) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	p3 = of_find_node_by_path("/testcase-data/phandle-tests/provider3");
+	if (!p3) {
+		pr_err("missing testcase data\n");
+		return;
+	}
+
+	rc = of_count_phandle_with_args(np, "phandle-list", "#phandle-cells");
+	unittest(rc == 7, "of_count_phandle_with_args() returned %i, expected 7\n", rc);
+
+	for (i = 0; i < 8; i++) {
+		bool passed = true;
+
+		rc = of_parse_phandle_with_args_map(np, "phandle-list",
+						"#phandle-cells", "phandle-map",
+						"phandle-map-mask", i, &args);
+
+		/* Test the values from tests-phandle.dtsi */
+		switch (i) {
+		case 0:
+			passed &= !rc;
+			passed &= (args.np == p1);
+			passed &= (args.args_count == 1);
+			passed &= (args.args[0] == 1);
+			break;
+		case 1:
+			passed &= !rc;
+			passed &= (args.np == p3);
+			passed &= (args.args_count == 3);
+			passed &= (args.args[0] == 2);
+			passed &= (args.args[1] == 5);
+			passed &= (args.args[2] == 3);
+			break;
+		case 2:
+			passed &= (rc == -ENOENT);
+			break;
+		case 3:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 4:
+			passed &= !rc;
+			passed &= (args.np == p1);
+			passed &= (args.args_count == 1);
+			passed &= (args.args[0] == 3);
+			break;
+		case 5:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 6:
+			passed &= !rc;
+			passed &= (args.np == p0);
+			passed &= (args.args_count == 0);
+			break;
+		case 7:
+			passed &= (rc == -ENOENT);
+			break;
+		default:
+			passed = false;
+		}
+
+		unittest(passed, "index %i - data error on node %s rc=%i\n",
+			 i, args.np->full_name, rc);
+	}
+
+	/* Check for missing list property */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-missing",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 0, &args);
+	unittest(rc == -ENOENT, "expected:%i got:%i\n", -ENOENT, rc);
+
+	/* Check for missing cells property */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list",
+					    "#phandle-cells-missing",
+					    "phandle-map", "phandle-map-mask",
+					    0, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+	/* Check for bad phandle in list */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-phandle",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 0, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+
+	/* Check for incorrectly formed argument list */
+	rc = of_parse_phandle_with_args_map(np, "phandle-list-bad-args",
+					    "#phandle-cells", "phandle-map",
+					    "phandle-map-mask", 1, &args);
+	unittest(rc == -EINVAL, "expected:%i got:%i\n", -EINVAL, rc);
+}
+
 static void __init of_unittest_property_string(void)
 {
 	const char *strings[4];
@@ -1951,6 +2074,7 @@ static int __init of_unittest(void)
 	of_unittest_find_node_by_name();
 	of_unittest_dynamic();
 	of_unittest_parse_phandle_with_args();
+	of_unittest_parse_phandle_with_args_map();
 	of_unittest_property_string();
 	of_unittest_property_copy();
 	of_unittest_changeset();
-- 
2.10.0.297.gf6727b0

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

* [PATCH 3/3] gpio: Support gpio nexus dt bindings
  2016-11-24 10:25 ` Stephen Boyd
@ 2016-11-24 10:25   ` Stephen Boyd
  -1 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: Rob Herring, Frank Rowand
  Cc: linux-arm-kernel, linux-kernel, devicetree, linux-gpio,
	Pantelis Antoniou, Linus Walleij, Mark Brown

Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.

Now that we have nexus support in the OF core let's change the
function call here that parses the phandle lists of gpios to use
the nexus variant. This allows us to remap phandles and their
arguments through any number of nexus nodes and end up with the
actual gpio provider being used.

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---

TODO: Document gpio-map and gpio-map-mask in GPIO devicetree binding

 drivers/gpio/gpiolib-of.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ecad3f0e3b77..3117397c4c41 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -71,8 +71,9 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 	struct gpio_desc *desc;
 	int ret;
 
-	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
-					 &gpiospec);
+	ret = of_parse_phandle_with_args_map(np, propname, "#gpio-cells",
+					     "gpio-map", "gpio-map-mask",
+					     index, &gpiospec);
 	if (ret) {
 		pr_debug("%s: can't parse '%s' property of node '%s[%d]'\n",
 			__func__, propname, np->full_name, index);
-- 
2.10.0.297.gf6727b0


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

* [PATCH 3/3] gpio: Support gpio nexus dt bindings
@ 2016-11-24 10:25   ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-11-24 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Platforms like 96boards have a standardized connector/expansion
slot that exposes signals like GPIOs to expansion boards in an
SoC agnostic way. We'd like the DT overlays for the expansion
boards to be written once without knowledge of the SoC on the
other side of the connector. This avoids the unscalable
combinatorial explosion of a different DT overlay for each
expansion board and SoC pair.

Now that we have nexus support in the OF core let's change the
function call here that parses the phandle lists of gpios to use
the nexus variant. This allows us to remap phandles and their
arguments through any number of nexus nodes and end up with the
actual gpio provider being used.

Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---

TODO: Document gpio-map and gpio-map-mask in GPIO devicetree binding

 drivers/gpio/gpiolib-of.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index ecad3f0e3b77..3117397c4c41 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -71,8 +71,9 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
 	struct gpio_desc *desc;
 	int ret;
 
-	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
-					 &gpiospec);
+	ret = of_parse_phandle_with_args_map(np, propname, "#gpio-cells",
+					     "gpio-map", "gpio-map-mask",
+					     index, &gpiospec);
 	if (ret) {
 		pr_debug("%s: can't parse '%s' property of node '%s[%d]'\n",
 			__func__, propname, np->full_name, index);
-- 
2.10.0.297.gf6727b0

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

* Re: [PATCH 0/2] OF phandle nexus support + GPIO nexus
  2016-11-24 10:25 ` Stephen Boyd
  (?)
@ 2016-11-24 12:47   ` Pantelis Antoniou
  -1 siblings, 0 replies; 23+ messages in thread
From: Pantelis Antoniou @ 2016-11-24 12:47 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: devicetree, Linus Walleij, linux-kernel, linux-gpio, Rob Herring,
	Mark Brown, Frank Rowand, linux-arm-kernel

Hi Stephen,

> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
> 
> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
> 
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
> 

Excellent. It was about time this happened.

> Stephen Boyd (3):
>  of: Support parsing phandle argument lists through a nexus node
>  of: unittest: Add phandle remapping test
>  gpio: Support gpio nexus dt bindings
> 
> drivers/gpio/gpiolib-of.c                   |   5 +-
> drivers/of/base.c                           | 146 ++++++++++++++++++++++++++++
> drivers/of/unittest-data/testcases.dts      |  11 +++
> drivers/of/unittest-data/tests-phandle.dtsi |  24 +++++
> drivers/of/unittest.c                       | 124 +++++++++++++++++++++++
> include/linux/of.h                          |  14 +++
> 6 files changed, 322 insertions(+), 2 deletions(-)
> 
> -- 
> 2.10.0.297.gf6727b0
> 

Comments inline…

Regards

— Pantelis


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 12:47   ` Pantelis Antoniou
  0 siblings, 0 replies; 23+ messages in thread
From: Pantelis Antoniou @ 2016-11-24 12:47 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Herring, Frank Rowand, linux-arm-kernel, linux-kernel,
	devicetree, linux-gpio, Linus Walleij, Mark Brown

Hi Stephen,

> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
> 
> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
> 
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
> 

Excellent. It was about time this happened.

> Stephen Boyd (3):
>  of: Support parsing phandle argument lists through a nexus node
>  of: unittest: Add phandle remapping test
>  gpio: Support gpio nexus dt bindings
> 
> drivers/gpio/gpiolib-of.c                   |   5 +-
> drivers/of/base.c                           | 146 ++++++++++++++++++++++++++++
> drivers/of/unittest-data/testcases.dts      |  11 +++
> drivers/of/unittest-data/tests-phandle.dtsi |  24 +++++
> drivers/of/unittest.c                       | 124 +++++++++++++++++++++++
> include/linux/of.h                          |  14 +++
> 6 files changed, 322 insertions(+), 2 deletions(-)
> 
> -- 
> 2.10.0.297.gf6727b0
> 

Comments inline…

Regards

— Pantelis

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

* [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 12:47   ` Pantelis Antoniou
  0 siblings, 0 replies; 23+ messages in thread
From: Pantelis Antoniou @ 2016-11-24 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
> 
> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
> 
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
> 

Excellent. It was about time this happened.

> Stephen Boyd (3):
>  of: Support parsing phandle argument lists through a nexus node
>  of: unittest: Add phandle remapping test
>  gpio: Support gpio nexus dt bindings
> 
> drivers/gpio/gpiolib-of.c                   |   5 +-
> drivers/of/base.c                           | 146 ++++++++++++++++++++++++++++
> drivers/of/unittest-data/testcases.dts      |  11 +++
> drivers/of/unittest-data/tests-phandle.dtsi |  24 +++++
> drivers/of/unittest.c                       | 124 +++++++++++++++++++++++
> include/linux/of.h                          |  14 +++
> 6 files changed, 322 insertions(+), 2 deletions(-)
> 
> -- 
> 2.10.0.297.gf6727b0
> 

Comments inline?

Regards

? Pantelis

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

* Re: [PATCH 0/2] OF phandle nexus support + GPIO nexus
  2016-11-24 10:25 ` Stephen Boyd
  (?)
@ 2016-11-24 15:27     ` Linus Walleij
  -1 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2016-11-24 15:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Herring, Frank Rowand,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA, Pantelis Antoniou, Mark Brown

On Thu, Nov 24, 2016 at 11:25 AM, Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
>
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
>
> Stephen Boyd (3):
>   of: Support parsing phandle argument lists through a nexus node
>   of: unittest: Add phandle remapping test
>   gpio: Support gpio nexus dt bindings

Looks perfectly reasonable to me. But it's mainly for the DT people to review
I guess.

I have no idea about the eventual merge path though, I guess it needs to
go through the OF tree with my ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 15:27     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2016-11-24 15:27 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Herring, Frank Rowand, linux-arm-kernel, linux-kernel,
	devicetree, linux-gpio, Pantelis Antoniou, Mark Brown

On Thu, Nov 24, 2016 at 11:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:

> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
>
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
>
> Stephen Boyd (3):
>   of: Support parsing phandle argument lists through a nexus node
>   of: unittest: Add phandle remapping test
>   gpio: Support gpio nexus dt bindings

Looks perfectly reasonable to me. But it's mainly for the DT people to review
I guess.

I have no idea about the eventual merge path though, I guess it needs to
go through the OF tree with my ACK.

Yours,
Linus Walleij

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

* [PATCH 0/2] OF phandle nexus support + GPIO nexus
@ 2016-11-24 15:27     ` Linus Walleij
  0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2016-11-24 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 24, 2016 at 11:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:

> This is one small chunk of work related to DT overlays for expansion
> boards. It would be good to have a way to expose #<list>-cells types of
> providers through a connector in a standard way. So we introduce a way
> to make "nexus" nodes for these types of properties to remap the consumer
> number space to the other side of the connector's number space. It's
> basically a copy of the interrupt nexus implementation, but without
> the address space matching design and interrupt-parent walking.
>
> The first patch implements a generic method to do this, and the second patch
> adds a unit test for it. The third patch is more of an example than anything
> else. It shows how we would modify frameworks to use the new API.
>
> Stephen Boyd (3):
>   of: Support parsing phandle argument lists through a nexus node
>   of: unittest: Add phandle remapping test
>   gpio: Support gpio nexus dt bindings

Looks perfectly reasonable to me. But it's mainly for the DT people to review
I guess.

I have no idea about the eventual merge path though, I guess it needs to
go through the OF tree with my ACK.

Yours,
Linus Walleij

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

* Re: [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
  2016-11-24 10:25   ` Stephen Boyd
@ 2016-11-24 16:05     ` Pantelis Antoniou
  -1 siblings, 0 replies; 23+ messages in thread
From: Pantelis Antoniou @ 2016-11-24 16:05 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Rob Herring, Frank Rowand, linux-arm-kernel,
	linux-kernel @ vger . kernel . org, devicetree, linux-gpio,
	Linus Walleij, Mark Brown

Hi Stephen,

> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
> 
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
> 
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
> 
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
> 
> 	soc {
> 		soc_gpio1: gpio-controller1 {
> 			#gpio-cells = <2>;
> 		};
> 
> 		soc_gpio2: gpio-controller2 {
> 			#gpio-cells = <2>;
> 		};
> 	};
> 
> 	connector: connector {
> 		#gpio-cells = <2>;
> 		gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
> 			   <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
> 			   <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
> 			   <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
> 		gpio-map-mask = <0xf 0x1>;
> 	};
> 
> 	expansion_device {
> 		reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
> 	};
> 
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
> 

Very good. My only point would be to elaborate a little bit on the
documentation part about how there might be different #list-cells values
pointed at, and how the lookup is performed in steps.

> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h |  14 +++++
> 2 files changed, 160 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
> EXPORT_SYMBOL(of_parse_phandle_with_args);
> 
> /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np:		pointer to a device tree node containing a list
> + * @list_name:	property name that contains a list
> + * @cells_name:	property name that specifies phandles' arguments count
> + * @index:	index of a phandle to parse out
> + * @out_args:	optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + *	#list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + *	#list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + * 	#list-cells = <1>;
> + * 	list-map = <0 &phandle2 3>,
> + * 		   <1 &phandle2 2>,
> + * 		   <2 &phandle1 5 1>;
> + *	list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + *	list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + * 			      "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> +				   const char *list_name,
> +				   const char *cells_name,
> +				   const char *map_name,
> +				   const char *mask_name,
> +				   int index, struct of_phandle_args *out_args)
> +{
> +	struct device_node *cur, *new = NULL;
> +	const __be32 *map, *mask, *tmp;
> +	const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
> +	__be32 initial_match_array[MAX_PHANDLE_ARGS];
> +	const __be32 *match_array = initial_match_array;
> +	int i, ret, map_len, match;
> +	u32 list_size, new_size;
> +
> +	if (index < 0)
> +		return -EINVAL;
> +
> +	ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
> +					   out_args);
> +	if (ret)
> +		return ret;
> +
> +	/* Get the #<list>-cells property */
> +	cur = out_args->np;
> +	ret = of_property_read_u32(cur, cells_name, &list_size);
> +	if (ret < 0)
> +		goto fail;
> +
> +	/* Precalculate the match array - this simplifies match loop */
> +	for (i = 0; i < list_size; i++)
> +		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
> +
> +	while (cur) {
> +		/* Get the <list>-map property */
> +		map = of_get_property(cur, map_name, &map_len);
> +		if (!map)
> +			return 0;
> +		map_len /= sizeof(u32);
> +
> +		/* Get the <list>-map-mask property (optional) */
> +		mask = of_get_property(cur, mask_name, NULL);
> +		if (!mask)
> +			mask = dummy_mask;
> +
> +		/* Iterate through <list>-map property */
> +		match = 0;
> +		while (map_len > (list_size + 1) && !match) {
> +			/* Compare specifiers */
> +			match = 1;
> +			for (i = 0; i < list_size; i++, map_len--)
> +				match &= !((match_array[i] ^ *map++) & mask[i]);
> +
> +			of_node_put(new);
> +			new = of_find_node_by_phandle(be32_to_cpup(map));
> +			map++;
> +			map_len--;
> +
> +			/* Check if not found */
> +			if (!new)
> +				goto fail;
> +
> +			if (!of_device_is_available(new))
> +				match = 0;
> +
> +			tmp = of_get_property(new, cells_name, NULL);
> +			if (!tmp)
> +				goto fail;
> +
> +			new_size = be32_to_cpu(*tmp);
> +
> +			/* Check for malformed properties */
> +			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
> +				goto fail;
> +			if (map_len < new_size)
> +				goto fail;
> +
> +			/* Move forward by new node's #<list>-cells amount */
> +			map += new_size;
> +			map_len -= new_size;
> +		}
> +		if (!match)
> +			goto fail;
> +
> +		/*
> +		 * Successfully parsed a <list>-map translation; copy new
> +		 * specifier into the out_args structure.
> +		 */
> +		match_array = map - new_size;
> +		for (i = 0; i < new_size; i++)
> +			out_args->args[i] = be32_to_cpup(map - new_size + i);
> +		out_args->args_count = list_size = new_size;
> +		/* Iterate again with new provider */
> +		out_args->np = new;
> +		of_node_put(cur);
> +		cur = new;
> +	}
> +fail:
> +	of_node_put(cur);
> +	of_node_put(new);
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(of_parse_phandle_with_args_map);
> +
> +/**
>  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
>  * @np:		pointer to a device tree node containing a list
>  * @list_name:	property name that contains a list
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d3a9c2e69001..65ff306403a2 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
> extern int of_parse_phandle_with_args(const struct device_node *np,
> 	const char *list_name, const char *cells_name, int index,
> 	struct of_phandle_args *out_args);
> +extern int of_parse_phandle_with_args_map(const struct device_node *np,
> +	const char *list_name, const char *cells_name, const char *map_name,
> +	const char *mask_name, int index, struct of_phandle_args *out_args);
> extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
> 	const char *list_name, int cells_count, int index,
> 	struct of_phandle_args *out_args);
> @@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
> 	return -ENOSYS;
> }
> 
> +static inline int of_parse_phandle_with_args_map(const struct device_node *np,
> +						 const char *list_name,
> +						 const char *cells_name,
> +						 const char *map_name,
> +						 const char *mask_name,
> +						 int index,
> +						 struct of_phandle_args *out_args)
> +{
> +	return -ENOSYS;
> +}
> +
> static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
> 	const char *list_name, int cells_count, int index,
> 	struct of_phandle_args *out_args)
> -- 
> 2.10.0.297.gf6727b0
> 


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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-11-24 16:05     ` Pantelis Antoniou
  0 siblings, 0 replies; 23+ messages in thread
From: Pantelis Antoniou @ 2016-11-24 16:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

> On Nov 24, 2016, at 12:25 , Stephen Boyd <stephen.boyd@linaro.org> wrote:
> 
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
> 
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
> 
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
> 
> 	soc {
> 		soc_gpio1: gpio-controller1 {
> 			#gpio-cells = <2>;
> 		};
> 
> 		soc_gpio2: gpio-controller2 {
> 			#gpio-cells = <2>;
> 		};
> 	};
> 
> 	connector: connector {
> 		#gpio-cells = <2>;
> 		gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
> 			   <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
> 			   <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
> 			   <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
> 		gpio-map-mask = <0xf 0x1>;
> 	};
> 
> 	expansion_device {
> 		reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
> 	};
> 
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
> 

Very good. My only point would be to elaborate a little bit on the
documentation part about how there might be different #list-cells values
pointed at, and how the lookup is performed in steps.

> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
> drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/of.h |  14 +++++
> 2 files changed, 160 insertions(+)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
> EXPORT_SYMBOL(of_parse_phandle_with_args);
> 
> /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np:		pointer to a device tree node containing a list
> + * @list_name:	property name that contains a list
> + * @cells_name:	property name that specifies phandles' arguments count
> + * @index:	index of a phandle to parse out
> + * @out_args:	optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + *	#list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + *	#list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + * 	#list-cells = <1>;
> + * 	list-map = <0 &phandle2 3>,
> + * 		   <1 &phandle2 2>,
> + * 		   <2 &phandle1 5 1>;
> + *	list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + *	list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + * 			      "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> +				   const char *list_name,
> +				   const char *cells_name,
> +				   const char *map_name,
> +				   const char *mask_name,
> +				   int index, struct of_phandle_args *out_args)
> +{
> +	struct device_node *cur, *new = NULL;
> +	const __be32 *map, *mask, *tmp;
> +	const __be32 dummy_mask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
> +	__be32 initial_match_array[MAX_PHANDLE_ARGS];
> +	const __be32 *match_array = initial_match_array;
> +	int i, ret, map_len, match;
> +	u32 list_size, new_size;
> +
> +	if (index < 0)
> +		return -EINVAL;
> +
> +	ret = __of_parse_phandle_with_args(np, list_name, cells_name, 0, index,
> +					   out_args);
> +	if (ret)
> +		return ret;
> +
> +	/* Get the #<list>-cells property */
> +	cur = out_args->np;
> +	ret = of_property_read_u32(cur, cells_name, &list_size);
> +	if (ret < 0)
> +		goto fail;
> +
> +	/* Precalculate the match array - this simplifies match loop */
> +	for (i = 0; i < list_size; i++)
> +		initial_match_array[i] = cpu_to_be32(out_args->args[i]);
> +
> +	while (cur) {
> +		/* Get the <list>-map property */
> +		map = of_get_property(cur, map_name, &map_len);
> +		if (!map)
> +			return 0;
> +		map_len /= sizeof(u32);
> +
> +		/* Get the <list>-map-mask property (optional) */
> +		mask = of_get_property(cur, mask_name, NULL);
> +		if (!mask)
> +			mask = dummy_mask;
> +
> +		/* Iterate through <list>-map property */
> +		match = 0;
> +		while (map_len > (list_size + 1) && !match) {
> +			/* Compare specifiers */
> +			match = 1;
> +			for (i = 0; i < list_size; i++, map_len--)
> +				match &= !((match_array[i] ^ *map++) & mask[i]);
> +
> +			of_node_put(new);
> +			new = of_find_node_by_phandle(be32_to_cpup(map));
> +			map++;
> +			map_len--;
> +
> +			/* Check if not found */
> +			if (!new)
> +				goto fail;
> +
> +			if (!of_device_is_available(new))
> +				match = 0;
> +
> +			tmp = of_get_property(new, cells_name, NULL);
> +			if (!tmp)
> +				goto fail;
> +
> +			new_size = be32_to_cpu(*tmp);
> +
> +			/* Check for malformed properties */
> +			if (WARN_ON(new_size > MAX_PHANDLE_ARGS))
> +				goto fail;
> +			if (map_len < new_size)
> +				goto fail;
> +
> +			/* Move forward by new node's #<list>-cells amount */
> +			map += new_size;
> +			map_len -= new_size;
> +		}
> +		if (!match)
> +			goto fail;
> +
> +		/*
> +		 * Successfully parsed a <list>-map translation; copy new
> +		 * specifier into the out_args structure.
> +		 */
> +		match_array = map - new_size;
> +		for (i = 0; i < new_size; i++)
> +			out_args->args[i] = be32_to_cpup(map - new_size + i);
> +		out_args->args_count = list_size = new_size;
> +		/* Iterate again with new provider */
> +		out_args->np = new;
> +		of_node_put(cur);
> +		cur = new;
> +	}
> +fail:
> +	of_node_put(cur);
> +	of_node_put(new);
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL(of_parse_phandle_with_args_map);
> +
> +/**
>  * of_parse_phandle_with_fixed_args() - Find a node pointed by phandle in a list
>  * @np:		pointer to a device tree node containing a list
>  * @list_name:	property name that contains a list
> diff --git a/include/linux/of.h b/include/linux/of.h
> index d3a9c2e69001..65ff306403a2 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -344,6 +344,9 @@ extern struct device_node *of_parse_phandle(const struct device_node *np,
> extern int of_parse_phandle_with_args(const struct device_node *np,
> 	const char *list_name, const char *cells_name, int index,
> 	struct of_phandle_args *out_args);
> +extern int of_parse_phandle_with_args_map(const struct device_node *np,
> +	const char *list_name, const char *cells_name, const char *map_name,
> +	const char *mask_name, int index, struct of_phandle_args *out_args);
> extern int of_parse_phandle_with_fixed_args(const struct device_node *np,
> 	const char *list_name, int cells_count, int index,
> 	struct of_phandle_args *out_args);
> @@ -738,6 +741,17 @@ static inline int of_parse_phandle_with_args(const struct device_node *np,
> 	return -ENOSYS;
> }
> 
> +static inline int of_parse_phandle_with_args_map(const struct device_node *np,
> +						 const char *list_name,
> +						 const char *cells_name,
> +						 const char *map_name,
> +						 const char *mask_name,
> +						 int index,
> +						 struct of_phandle_args *out_args)
> +{
> +	return -ENOSYS;
> +}
> +
> static inline int of_parse_phandle_with_fixed_args(const struct device_node *np,
> 	const char *list_name, int cells_count, int index,
> 	struct of_phandle_args *out_args)
> -- 
> 2.10.0.297.gf6727b0
> 

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

* Re: [PATCH 3/3] gpio: Support gpio nexus dt bindings
  2016-11-24 10:25   ` Stephen Boyd
  (?)
@ 2016-11-30 22:48   ` Rob Herring
  -1 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-11-30 22:48 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Frank Rowand, linux-arm-kernel, linux-kernel, devicetree,
	linux-gpio, Pantelis Antoniou, Linus Walleij, Mark Brown



On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> Now that we have nexus support in the OF core let's change the
> function call here that parses the phandle lists of gpios to use
> the nexus variant. This allows us to remap phandles and their
> arguments through any number of nexus nodes and end up with the
> actual gpio provider being used.
>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>
> TODO: Document gpio-map and gpio-map-mask in GPIO devicetree binding
>
>  drivers/gpio/gpiolib-of.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index ecad3f0e3b77..3117397c4c41 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -71,8 +71,9 @@ struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
>         struct gpio_desc *desc;
>         int ret;
>
> -       ret = of_parse_phandle_with_args(np, propname, "#gpio-cells", index,
> -                                        &gpiospec);
> +       ret = of_parse_phandle_with_args_map(np, propname, "#gpio-cells",
> +                                            "gpio-map", "gpio-map-mask",
> +                                            index, &gpiospec);
>         if (ret) {
>                 pr_debug("%s: can't parse '%s' property of node '%s[%d]'\n",
>                         __func__, propname, np->full_name, index);
> --
> 2.10.0.297.gf6727b0
>

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

* Re: [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
  2016-11-24 10:25   ` Stephen Boyd
  (?)
@ 2016-11-30 23:30     ` Rob Herring
  -1 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-11-30 23:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Frank Rowand, linux-arm-kernel, linux-kernel, devicetree,
	linux-gpio, Pantelis Antoniou, Linus Walleij, Mark Brown

On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
>
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
>
>         soc {
>                 soc_gpio1: gpio-controller1 {
>                         #gpio-cells = <2>;
>                 };
>
>                 soc_gpio2: gpio-controller2 {
>                         #gpio-cells = <2>;
>                 };
>         };
>
>         connector: connector {
>                 #gpio-cells = <2>;
>                 gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
>                            <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
>                            <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
>                            <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
>                 gpio-map-mask = <0xf 0x1>;

I think the common case is something more like this:

                 gpio-map = <0 0 &soc_gpio1 1 0>,
                            <1 0 &soc_gpio2 4 0>,
                            <2 0 &soc_gpio1 3 0>,
                            <3 0 &soc_gpio2 2 0>;
                 gpio-map-mask = <0xf 0>;

where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
&soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
combination or it will be specific to the daughterboard's usage.

Also, GPIO cells are pretty well standardized, but some cases may need
a translation function which I guess would be part of a connector
driver. I don't think that affects the binding nor needs to be solved
now, but just want to raise that possibility.

>         };
>
>         expansion_device {
>                 reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
>         };
>
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

GPIOs also are interrupts frequently, so we need to make sure
interrupt translation works too. It's a bit tricky as interrupt-map
depends on #address-cells and #interrupt-cells. I think we just set
the #address-cells to 0 on the connector node and it will be fine. We
may need the same pass thru of flags though.

>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  14 +++++
>  2 files changed, 160 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
>  EXPORT_SYMBOL(of_parse_phandle_with_args);
>
>  /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name:        property name that specifies phandles' arguments count
> + * @index:     index of a phandle to parse out
> + * @out_args:  optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + *     #list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + *     #list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + *     #list-cells = <1>;
> + *     list-map = <0 &phandle2 3>,
> + *                <1 &phandle2 2>,
> + *                <2 &phandle1 5 1>;
> + *     list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + *     list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + *                           "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> +                                  const char *list_name,
> +                                  const char *cells_name,
> +                                  const char *map_name,
> +                                  const char *mask_name,

Perhaps these 3 could be just a single base name (e.g. "gpio")?
Doesn't really buy much other than enforce we don't mix 'gpios' and
'gpio'. That could never happen. ;)

> +                                  int index, struct of_phandle_args *out_args)
> +{

Rob

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

* Re: [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-11-30 23:30     ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-11-30 23:30 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Frank Rowand, linux-arm-kernel, linux-kernel, devicetree,
	linux-gpio, Pantelis Antoniou, Linus Walleij, Mark Brown

On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
>
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
>
>         soc {
>                 soc_gpio1: gpio-controller1 {
>                         #gpio-cells = <2>;
>                 };
>
>                 soc_gpio2: gpio-controller2 {
>                         #gpio-cells = <2>;
>                 };
>         };
>
>         connector: connector {
>                 #gpio-cells = <2>;
>                 gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
>                            <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
>                            <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
>                            <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
>                 gpio-map-mask = <0xf 0x1>;

I think the common case is something more like this:

                 gpio-map = <0 0 &soc_gpio1 1 0>,
                            <1 0 &soc_gpio2 4 0>,
                            <2 0 &soc_gpio1 3 0>,
                            <3 0 &soc_gpio2 2 0>;
                 gpio-map-mask = <0xf 0>;

where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
&soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
combination or it will be specific to the daughterboard's usage.

Also, GPIO cells are pretty well standardized, but some cases may need
a translation function which I guess would be part of a connector
driver. I don't think that affects the binding nor needs to be solved
now, but just want to raise that possibility.

>         };
>
>         expansion_device {
>                 reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
>         };
>
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

GPIOs also are interrupts frequently, so we need to make sure
interrupt translation works too. It's a bit tricky as interrupt-map
depends on #address-cells and #interrupt-cells. I think we just set
the #address-cells to 0 on the connector node and it will be fine. We
may need the same pass thru of flags though.

>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  14 +++++
>  2 files changed, 160 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
>  EXPORT_SYMBOL(of_parse_phandle_with_args);
>
>  /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name:        property name that specifies phandles' arguments count
> + * @index:     index of a phandle to parse out
> + * @out_args:  optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + *     #list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + *     #list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + *     #list-cells = <1>;
> + *     list-map = <0 &phandle2 3>,
> + *                <1 &phandle2 2>,
> + *                <2 &phandle1 5 1>;
> + *     list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + *     list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + *                           "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> +                                  const char *list_name,
> +                                  const char *cells_name,
> +                                  const char *map_name,
> +                                  const char *mask_name,

Perhaps these 3 could be just a single base name (e.g. "gpio")?
Doesn't really buy much other than enforce we don't mix 'gpios' and
'gpio'. That could never happen. ;)

> +                                  int index, struct of_phandle_args *out_args)
> +{

Rob

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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-11-30 23:30     ` Rob Herring
  0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2016-11-30 23:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> Platforms like 96boards have a standardized connector/expansion
> slot that exposes signals like GPIOs to expansion boards in an
> SoC agnostic way. We'd like the DT overlays for the expansion
> boards to be written once without knowledge of the SoC on the
> other side of the connector. This avoids the unscalable
> combinatorial explosion of a different DT overlay for each
> expansion board and SoC pair.
>
> We need a way to describe the GPIOs routed through the connector
> in an SoC agnostic way. Let's introduce nexus property parsing
> into the OF core to do this. This is largely based on the
> interrupt nexus support we already have. This allows us to remap
> a phandle list in a consumer node (e.g. reset-gpios) through a
> connector in a generic way (e.g. via gpio-map). Do this in a
> generic routine so that we can remap any sort of variable length
> phandle list.
>
> Taking GPIOs as an example, the connector would be a GPIO nexus,
> supporting the remapping of a GPIO specifier space to multiple
> GPIO providers on the SoC. DT would look as shown below, where
> 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> expansion port where boards can be plugged in, and
> 'expansion_device' is a device on the expansion board.
>
>         soc {
>                 soc_gpio1: gpio-controller1 {
>                         #gpio-cells = <2>;
>                 };
>
>                 soc_gpio2: gpio-controller2 {
>                         #gpio-cells = <2>;
>                 };
>         };
>
>         connector: connector {
>                 #gpio-cells = <2>;
>                 gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
>                            <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
>                            <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
>                            <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
>                 gpio-map-mask = <0xf 0x1>;

I think the common case is something more like this:

                 gpio-map = <0 0 &soc_gpio1 1 0>,
                            <1 0 &soc_gpio2 4 0>,
                            <2 0 &soc_gpio1 3 0>,
                            <3 0 &soc_gpio2 2 0>;
                 gpio-map-mask = <0xf 0>;

where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
&soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
combination or it will be specific to the daughterboard's usage.

Also, GPIO cells are pretty well standardized, but some cases may need
a translation function which I guess would be part of a connector
driver. I don't think that affects the binding nor needs to be solved
now, but just want to raise that possibility.

>         };
>
>         expansion_device {
>                 reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
>         };
>
> The GPIO core would use of_parse_phandle_with_args_map() instead
> of of_parse_phandle_with_args() and arrive at the same type of
> result, a phandle and argument list. The difference is that the
> phandle and arguments will be remapped through the nexus node to
> the underlying SoC GPIO controller node. In the example above,
> we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.

GPIOs also are interrupts frequently, so we need to make sure
interrupt translation works too. It's a bit tricky as interrupt-map
depends on #address-cells and #interrupt-cells. I think we just set
the #address-cells to 0 on the connector node and it will be fine. We
may need the same pass thru of flags though.

>
> Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> ---
>  drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/of.h |  14 +++++
>  2 files changed, 160 insertions(+)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d687e6de24a0..693b73f33675 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
>  EXPORT_SYMBOL(of_parse_phandle_with_args);
>
>  /**
> + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> + * @np:                pointer to a device tree node containing a list
> + * @list_name: property name that contains a list
> + * @cells_name:        property name that specifies phandles' arguments count
> + * @index:     index of a phandle to parse out
> + * @out_args:  optional pointer to output arguments structure (will be filled)
> + *
> + * This function is useful to parse lists of phandles and their arguments.
> + * Returns 0 on success and fills out_args, on error returns appropriate
> + * errno value.
> + *
> + * Caller is responsible to call of_node_put() on the returned out_args->np
> + * pointer.
> + *
> + * Example:
> + *
> + * phandle1: node1 {
> + *     #list-cells = <2>;
> + * }
> + *
> + * phandle2: node2 {
> + *     #list-cells = <1>;
> + * }
> + *
> + * phandle3: node3 {
> + *     #list-cells = <1>;
> + *     list-map = <0 &phandle2 3>,
> + *                <1 &phandle2 2>,
> + *                <2 &phandle1 5 1>;
> + *     list-map-mask = <0x3>;
> + * };
> + *
> + * node4 {
> + *     list = <&phandle1 1 2 &phandle3 0>;
> + * }
> + *
> + * To get a device_node of the `node2' node you may call this:
> + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> + *                           "list-map-mask", 1, &args);
> + */
> +int of_parse_phandle_with_args_map(const struct device_node *np,
> +                                  const char *list_name,
> +                                  const char *cells_name,
> +                                  const char *map_name,
> +                                  const char *mask_name,

Perhaps these 3 could be just a single base name (e.g. "gpio")?
Doesn't really buy much other than enforce we don't mix 'gpios' and
'gpio'. That could never happen. ;)

> +                                  int index, struct of_phandle_args *out_args)
> +{

Rob

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

* Re: [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
  2016-11-30 23:30     ` Rob Herring
@ 2016-12-02  1:10       ` Stephen Boyd
  -1 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-12-02  1:10 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Linus Walleij, Pantelis Antoniou, linux-kernel,
	linux-gpio, Mark Brown, Frank Rowand, linux-arm-kernel

Quoting Rob Herring (2016-11-30 15:30:47)
> On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> > Platforms like 96boards have a standardized connector/expansion
> > slot that exposes signals like GPIOs to expansion boards in an
> > SoC agnostic way. We'd like the DT overlays for the expansion
> > boards to be written once without knowledge of the SoC on the
> > other side of the connector. This avoids the unscalable
> > combinatorial explosion of a different DT overlay for each
> > expansion board and SoC pair.
> >
> > We need a way to describe the GPIOs routed through the connector
> > in an SoC agnostic way. Let's introduce nexus property parsing
> > into the OF core to do this. This is largely based on the
> > interrupt nexus support we already have. This allows us to remap
> > a phandle list in a consumer node (e.g. reset-gpios) through a
> > connector in a generic way (e.g. via gpio-map). Do this in a
> > generic routine so that we can remap any sort of variable length
> > phandle list.
> >
> > Taking GPIOs as an example, the connector would be a GPIO nexus,
> > supporting the remapping of a GPIO specifier space to multiple
> > GPIO providers on the SoC. DT would look as shown below, where
> > 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> > expansion port where boards can be plugged in, and
> > 'expansion_device' is a device on the expansion board.
> >
> >         soc {
> >                 soc_gpio1: gpio-controller1 {
> >                         #gpio-cells = <2>;
> >                 };
> >
> >                 soc_gpio2: gpio-controller2 {
> >                         #gpio-cells = <2>;
> >                 };
> >         };
> >
> >         connector: connector {
> >                 #gpio-cells = <2>;
> >                 gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
> >                            <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
> >                            <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
> >                            <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
> >                 gpio-map-mask = <0xf 0x1>;
> 
> I think the common case is something more like this:
> 
>                  gpio-map = <0 0 &soc_gpio1 1 0>,
>                             <1 0 &soc_gpio2 4 0>,
>                             <2 0 &soc_gpio1 3 0>,
>                             <3 0 &soc_gpio2 2 0>;
>                  gpio-map-mask = <0xf 0>;
> 
> where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
> thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
> &soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
> combination or it will be specific to the daughterboard's usage.
> 
> Also, GPIO cells are pretty well standardized, but some cases may need
> a translation function which I guess would be part of a connector
> driver. I don't think that affects the binding nor needs to be solved
> now, but just want to raise that possibility.

Right. I think that translation function could be done with DT though.
For example, we could remap an ACTIVE_LOW flag to an ACTIVE_HIGH flag,
by matching the GPIO on active low and changing it to active high during
the remap. That seems like something we can solve now if it ever happens
by keeping the remapping scheme that interrupts does. Of course, if it
becomes more complicated this breaks down, but then we can always have
the connector driver do the more complicated stuff.

If we want to support pass through, perhaps we should introduce yet
another property to indicate which cells and maybe even which bits in
those cells should be passed through from one side to the other. That
way we can support a compressed scheme without requiring all the
combinations of gpios and flags to be listed out.

For example, gpio-map-pass-thru = <0x0 0xff> would mean that we should
pass through the second cell values that are the lower 8 bits. Map
matching would still be done with the map-mask property, but this
property would indicate which part of the specifier to mask out of the
other side when copying it over.

> 
> >         };
> >
> >         expansion_device {
> >                 reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
> >         };
> >
> > The GPIO core would use of_parse_phandle_with_args_map() instead
> > of of_parse_phandle_with_args() and arrive at the same type of
> > result, a phandle and argument list. The difference is that the
> > phandle and arguments will be remapped through the nexus node to
> > the underlying SoC GPIO controller node. In the example above,
> > we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> > to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
> 
> GPIOs also are interrupts frequently, so we need to make sure
> interrupt translation works too. It's a bit tricky as interrupt-map
> depends on #address-cells and #interrupt-cells. I think we just set
> the #address-cells to 0 on the connector node and it will be fine. We
> may need the same pass thru of flags though.

Right I think that should work but I haven't tested it so far.
Unfortunately, interrupt mapping doesn't have pass through support, so
we may want to add the same pass through mask property there and update
the interrupt mapping code too? I was trying to figure out how to make
the interrupt code and this function the same, but the whole
interrupt-parent scan made it feel unwieldy to the point where I gave
up.

> 
> >
> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> >  drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/of.h |  14 +++++
> >  2 files changed, 160 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index d687e6de24a0..693b73f33675 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
> >  EXPORT_SYMBOL(of_parse_phandle_with_args);
> >
> >  /**
> > + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> > + * @np:                pointer to a device tree node containing a list
> > + * @list_name: property name that contains a list
> > + * @cells_name:        property name that specifies phandles' arguments count
> > + * @index:     index of a phandle to parse out
> > + * @out_args:  optional pointer to output arguments structure (will be filled)
> > + *
> > + * This function is useful to parse lists of phandles and their arguments.
> > + * Returns 0 on success and fills out_args, on error returns appropriate
> > + * errno value.
> > + *
> > + * Caller is responsible to call of_node_put() on the returned out_args->np
> > + * pointer.
> > + *
> > + * Example:
> > + *
> > + * phandle1: node1 {
> > + *     #list-cells = <2>;
> > + * }
> > + *
> > + * phandle2: node2 {
> > + *     #list-cells = <1>;
> > + * }
> > + *
> > + * phandle3: node3 {
> > + *     #list-cells = <1>;
> > + *     list-map = <0 &phandle2 3>,
> > + *                <1 &phandle2 2>,
> > + *                <2 &phandle1 5 1>;
> > + *     list-map-mask = <0x3>;
> > + * };
> > + *
> > + * node4 {
> > + *     list = <&phandle1 1 2 &phandle3 0>;
> > + * }
> > + *
> > + * To get a device_node of the `node2' node you may call this:
> > + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> > + *                           "list-map-mask", 1, &args);
> > + */
> > +int of_parse_phandle_with_args_map(const struct device_node *np,
> > +                                  const char *list_name,
> > +                                  const char *cells_name,
> > +                                  const char *map_name,
> > +                                  const char *mask_name,
> 
> Perhaps these 3 could be just a single base name (e.g. "gpio")?
> Doesn't really buy much other than enforce we don't mix 'gpios' and
> 'gpio'. That could never happen. ;)
> 

I thought about that. I was worried that we wanted to support this API
being called in atomic context, but that seems like it can't possibly be
the case. So I'll have to allocate a string for each of those and free
them on exit. Should be ok.

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

* [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node
@ 2016-12-02  1:10       ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2016-12-02  1:10 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Rob Herring (2016-11-30 15:30:47)
> On Thu, Nov 24, 2016 at 4:25 AM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> > Platforms like 96boards have a standardized connector/expansion
> > slot that exposes signals like GPIOs to expansion boards in an
> > SoC agnostic way. We'd like the DT overlays for the expansion
> > boards to be written once without knowledge of the SoC on the
> > other side of the connector. This avoids the unscalable
> > combinatorial explosion of a different DT overlay for each
> > expansion board and SoC pair.
> >
> > We need a way to describe the GPIOs routed through the connector
> > in an SoC agnostic way. Let's introduce nexus property parsing
> > into the OF core to do this. This is largely based on the
> > interrupt nexus support we already have. This allows us to remap
> > a phandle list in a consumer node (e.g. reset-gpios) through a
> > connector in a generic way (e.g. via gpio-map). Do this in a
> > generic routine so that we can remap any sort of variable length
> > phandle list.
> >
> > Taking GPIOs as an example, the connector would be a GPIO nexus,
> > supporting the remapping of a GPIO specifier space to multiple
> > GPIO providers on the SoC. DT would look as shown below, where
> > 'soc_gpio1' and 'soc_gpio2' are inside the SoC, 'connector' is an
> > expansion port where boards can be plugged in, and
> > 'expansion_device' is a device on the expansion board.
> >
> >         soc {
> >                 soc_gpio1: gpio-controller1 {
> >                         #gpio-cells = <2>;
> >                 };
> >
> >                 soc_gpio2: gpio-controller2 {
> >                         #gpio-cells = <2>;
> >                 };
> >         };
> >
> >         connector: connector {
> >                 #gpio-cells = <2>;
> >                 gpio-map = <0 GPIO_ACTIVE_LOW &soc_gpio1 1 GPIO_ACTIVE_LOW>,
> >                            <1 GPIO_ACTIVE_LOW &soc_gpio2 4 GPIO_ACTIVE_LOW>,
> >                            <2 GPIO_ACTIVE_LOW &soc_gpio1 3 GPIO_ACTIVE_LOW>,
> >                            <3 GPIO_ACTIVE_LOW &soc_gpio2 2 GPIO_ACTIVE_LOW>;
> >                 gpio-map-mask = <0xf 0x1>;
> 
> I think the common case is something more like this:
> 
>                  gpio-map = <0 0 &soc_gpio1 1 0>,
>                             <1 0 &soc_gpio2 4 0>,
>                             <2 0 &soc_gpio1 3 0>,
>                             <3 0 &soc_gpio2 2 0>;
>                  gpio-map-mask = <0xf 0>;
> 
> where we want to pass the 2nd cell of the consumer (e.g. reset-gpios)
> thru. So here the GPIO_ACTIVE_LOW flag below needs to pass thru to
> &soc_gpio1. Otherwise, the gpio-map is has to enumerate every possible
> combination or it will be specific to the daughterboard's usage.
> 
> Also, GPIO cells are pretty well standardized, but some cases may need
> a translation function which I guess would be part of a connector
> driver. I don't think that affects the binding nor needs to be solved
> now, but just want to raise that possibility.

Right. I think that translation function could be done with DT though.
For example, we could remap an ACTIVE_LOW flag to an ACTIVE_HIGH flag,
by matching the GPIO on active low and changing it to active high during
the remap. That seems like something we can solve now if it ever happens
by keeping the remapping scheme that interrupts does. Of course, if it
becomes more complicated this breaks down, but then we can always have
the connector driver do the more complicated stuff.

If we want to support pass through, perhaps we should introduce yet
another property to indicate which cells and maybe even which bits in
those cells should be passed through from one side to the other. That
way we can support a compressed scheme without requiring all the
combinations of gpios and flags to be listed out.

For example, gpio-map-pass-thru = <0x0 0xff> would mean that we should
pass through the second cell values that are the lower 8 bits. Map
matching would still be done with the map-mask property, but this
property would indicate which part of the specifier to mask out of the
other side when copying it over.

> 
> >         };
> >
> >         expansion_device {
> >                 reset-gpios = <&connector 2 GPIO_ACTIVE_LOW>;
> >         };
> >
> > The GPIO core would use of_parse_phandle_with_args_map() instead
> > of of_parse_phandle_with_args() and arrive at the same type of
> > result, a phandle and argument list. The difference is that the
> > phandle and arguments will be remapped through the nexus node to
> > the underlying SoC GPIO controller node. In the example above,
> > we would remap 'reset-gpios' from <&connector 2 GPIO_ACTIVE_LOW>
> > to <&soc_gpio1 3 GPIO_ACTIVE_LOW>.
> 
> GPIOs also are interrupts frequently, so we need to make sure
> interrupt translation works too. It's a bit tricky as interrupt-map
> depends on #address-cells and #interrupt-cells. I think we just set
> the #address-cells to 0 on the connector node and it will be fine. We
> may need the same pass thru of flags though.

Right I think that should work but I haven't tested it so far.
Unfortunately, interrupt mapping doesn't have pass through support, so
we may want to add the same pass through mask property there and update
the interrupt mapping code too? I was trying to figure out how to make
the interrupt code and this function the same, but the whole
interrupt-parent scan made it feel unwieldy to the point where I gave
up.

> 
> >
> > Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Mark Brown <broonie@kernel.org>
> > Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
> > ---
> >  drivers/of/base.c  | 146 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/of.h |  14 +++++
> >  2 files changed, 160 insertions(+)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index d687e6de24a0..693b73f33675 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -1772,6 +1772,152 @@ int of_parse_phandle_with_args(const struct device_node *np, const char *list_na
> >  EXPORT_SYMBOL(of_parse_phandle_with_args);
> >
> >  /**
> > + * of_parse_phandle_with_args_map() - Find a node pointed by phandle in a list and remap it
> > + * @np:                pointer to a device tree node containing a list
> > + * @list_name: property name that contains a list
> > + * @cells_name:        property name that specifies phandles' arguments count
> > + * @index:     index of a phandle to parse out
> > + * @out_args:  optional pointer to output arguments structure (will be filled)
> > + *
> > + * This function is useful to parse lists of phandles and their arguments.
> > + * Returns 0 on success and fills out_args, on error returns appropriate
> > + * errno value.
> > + *
> > + * Caller is responsible to call of_node_put() on the returned out_args->np
> > + * pointer.
> > + *
> > + * Example:
> > + *
> > + * phandle1: node1 {
> > + *     #list-cells = <2>;
> > + * }
> > + *
> > + * phandle2: node2 {
> > + *     #list-cells = <1>;
> > + * }
> > + *
> > + * phandle3: node3 {
> > + *     #list-cells = <1>;
> > + *     list-map = <0 &phandle2 3>,
> > + *                <1 &phandle2 2>,
> > + *                <2 &phandle1 5 1>;
> > + *     list-map-mask = <0x3>;
> > + * };
> > + *
> > + * node4 {
> > + *     list = <&phandle1 1 2 &phandle3 0>;
> > + * }
> > + *
> > + * To get a device_node of the `node2' node you may call this:
> > + * of_parse_phandle_with_args(node4, "list", "#list-cells", "list-map",
> > + *                           "list-map-mask", 1, &args);
> > + */
> > +int of_parse_phandle_with_args_map(const struct device_node *np,
> > +                                  const char *list_name,
> > +                                  const char *cells_name,
> > +                                  const char *map_name,
> > +                                  const char *mask_name,
> 
> Perhaps these 3 could be just a single base name (e.g. "gpio")?
> Doesn't really buy much other than enforce we don't mix 'gpios' and
> 'gpio'. That could never happen. ;)
> 

I thought about that. I was worried that we wanted to support this API
being called in atomic context, but that seems like it can't possibly be
the case. So I'll have to allocate a string for each of those and free
them on exit. Should be ok.

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

end of thread, other threads:[~2016-12-02  1:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-24 10:25 [PATCH 0/2] OF phandle nexus support + GPIO nexus Stephen Boyd
2016-11-24 10:25 ` Stephen Boyd
2016-11-24 10:25 ` [PATCH 1/3] of: Support parsing phandle argument lists through a nexus node Stephen Boyd
2016-11-24 10:25   ` Stephen Boyd
2016-11-24 10:25   ` Stephen Boyd
2016-11-24 16:05   ` Pantelis Antoniou
2016-11-24 16:05     ` Pantelis Antoniou
2016-11-30 23:30   ` Rob Herring
2016-11-30 23:30     ` Rob Herring
2016-11-30 23:30     ` Rob Herring
2016-12-02  1:10     ` Stephen Boyd
2016-12-02  1:10       ` Stephen Boyd
2016-11-24 10:25 ` [PATCH 2/3] of: unittest: Add phandle remapping test Stephen Boyd
2016-11-24 10:25   ` Stephen Boyd
2016-11-24 10:25 ` [PATCH 3/3] gpio: Support gpio nexus dt bindings Stephen Boyd
2016-11-24 10:25   ` Stephen Boyd
2016-11-30 22:48   ` Rob Herring
2016-11-24 12:47 ` [PATCH 0/2] OF phandle nexus support + GPIO nexus Pantelis Antoniou
2016-11-24 12:47   ` Pantelis Antoniou
2016-11-24 12:47   ` Pantelis Antoniou
     [not found] ` <20161124102529.20212-1-stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2016-11-24 15:27   ` Linus Walleij
2016-11-24 15:27     ` Linus Walleij
2016-11-24 15:27     ` Linus Walleij

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.