Skip to contentSkip to conversation

QE Pin Multiplexing API

RFC PATCH In review Anton Vorontsov 2 participants 6 messages Sep 25, 2008 – Oct 13, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover QE Pin Multiplexing API

Here is the sketch, only for RFC. It is tested to work, but there could be silly thinkos anyway. I'll review the changes once again later...

Anyhow... David, is this approach looks ok? Or I'm hopeless..? ;-)

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#1 lore raw

PATCH 1/2 OF: new helper: of_parse_phandles_with_args()

Factored out of of_get_gpio(). Will be used by the QE pin multiplexing functions (they need to parse the gpios = <> too).

---
 drivers/of/base.c  |  111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/gpio.c  |   77 +++++++----------------------------
 include/linux/of.h |    3 +
 3 files changed, 130 insertions(+), 61 deletions(-)
drivers/of/base.c +111 -0
index ad8ac1a..51e0fce 100644
@@ -473,3 +473,114 @@ int of_modalias_node(struct device_node *node, char *modalias, int len)
 }
 EXPORT_SYMBOL_GPL(of_modalias_node);
 
+/**
+ * of_parse_phandles_with_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
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_node:	pointer to device_node struct pointer (will be filled)
+ * @out_args:	pointer to arguments pointer (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_node and out_args, on error returns
+ * appropriate errno value.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ * 	#list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ * 	#list-cells = <1>;
+ * }
+ *
+ * node3 {
+ * 	list = <&phandle1 1 2 &phandle2 3>;
+ * }
+ *
+ * To get a device_node of the `node2' you may call this:
+ * of_parse_phandles_with_args(node3, "list", "#list-cells", 2, &node2, &args);
+ */
+int of_parse_phandles_with_args(struct device_node *np, const char *list_name,
+				const char *cells_name, int index,
+				struct device_node **out_node,
+				const void **out_args)
+{
+	int ret = -EINVAL;
+	const u32 *list;
+	u32 list_cells;
+	int size;
+	int i;
+	int cur_index = 0;
+	struct device_node *node;
+	const void *args;
+
+	list = of_get_property(np, list_name, &size);
+	if (!list) {
+		ret = -ENOENT;
+		goto err0;
+	}
+	list_cells = size / sizeof(u32);
+
+	for (i = 0; i < list_cells; cur_index++) {
+		const u32 *cells;
+		const phandle *phandle;
+
+		phandle = list + i;
+		args = phandle + 1;
+
+		/* one cell hole in the list = <>; */
+		if (!*phandle) {
+			if (cur_index == index)
+				return -ENOENT;
+			i++;
+			continue;
+		}
+
+		node = of_find_node_by_phandle(*phandle);
+		if (!node) {
+			pr_debug("%s: could not find phandle\n",
+				 np->full_name);
+			goto err0;
+		}
+
+		cells = of_get_property(node, cells_name, &size);
+		if (!cells || size != sizeof(*cells)) {
+			pr_debug("%s: could not get %s for %s\n",
+				 np->full_name, cells_name, node->full_name);
+			goto err1;
+		}
+
+		/* Next phandle is at phandle cells + #cells */
+		i += sizeof(*phandle) / sizeof(u32) + *cells;
+		if (i >= list_cells + 1) {
+			pr_debug("%s: insufficient arguments length\n",
+				 np->full_name);
+			goto err1;
+		}
+
+		if (cur_index == index)
+			break;
+
+		node = NULL;
+		of_node_put(node);
+	}
+
+	if (!node) {
+		ret = -ENOENT;
+		goto err0;
+	}
+
+	*out_node = node;
+	*out_args = args;
+
+	return 0;
+err1:
+	of_node_put(node);
+err0:
+	pr_debug("%s exited with status %d\n", __func__, ret);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(of_parse_phandles_with_args);
drivers/of/gpio.c +17 -62

95 lines not on this page · raw message

include/linux/of.h +3 -0
index 79886ad..e2488f5 100644
@@ -71,5 +71,8 @@ extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+extern int of_parse_phandles_with_args(struct device_node *np,
+	const char *list_name, const char *cells_name, int index,
+	struct device_node **out_node, const void **out_args);
 
 #endif /* _LINUX_OF_H */
Signature · 1 line
1.5.6.3
#2 lore raw

Re: [PATCH 1/2] OF: new helper: of_parse_phandles_with_args()

drivers/of/base.c int of_modalias_node(struct device_node *node, char *modalias, int len) · line 532

Quoted 7 lines
drivers/of/base.c +6 -0
+	for (i = 0; i < list_cells; cur_index++) {
+		const u32 *cells;
+		const phandle *phandle;
+
+		phandle = list + i;
+		args = phandle + 1;

Rather than incrementing i, I would just use a running pointer "list" and drop "i" totally. Not big deal tho.

drivers/of/base.c int of_modalias_node(struct device_node *node, char *modalias, int len) · line 540

Quoted 7 lines
drivers/of/base.c +7 -0
+		/* one cell hole in the list = <>; */
+		if (!*phandle) {
+			if (cur_index == index)
+				return -ENOENT;
+			i++;
+			continue;
+		}

I don't totally understand the above. The 0 phandle terminates the list or is just an empty slot in it ? In the later case, it might be more readable to use goto to skip over down to the normal if (cur_index == index) break; and let it return via the normal if (!node) return -ENOENT out of the loop.

Appart from that it's good and I'm fine with putting it in if you respin despite being a bit late mostly because it's me who is later reviewing there :-)

Cheers,
Ben.

#3 lore raw

PATCH v2 1/2 OF: new helper: of_parse_phandles_with_args()

The helper is factored out of of_get_gpio(). Will be used by the QE pin multiplexing functions (they need to parse the gpios = <> too).

---

On Fri, Oct 10, 2008 at 02:37:29PM +1100, Benjamin Herrenschmidt wrote:
[...]

Quoted 2 lines

Rather than incrementing i, I would just use a running pointer "list" and drop "i" totally. Not big deal tho.

Done.

Quoted 10 lines

Review

Quoted 7 lines
(quoted diff) +7 -0
+		/* one cell hole in the list = <>; */
+		if (!*phandle) {
+			if (cur_index == index)
+				return -ENOENT;
+			i++;
+			continue;
+		}

I don't totally understand the above. The 0 phandle terminates the list or is just an empty slot in it ?

An empty slot case.

Quoted 4 lines

In the later case, it might be more
readable to use goto to skip over down to the normal if (cur_index == index) break; and let it return via the normal if (!node) return -ENOENT out of the loop.

Agree, done.

Quoted 3 lines

Appart from that it's good and I'm fine with putting it in if you respin despite being a bit late mostly because it's me who is later reviewing there :-)

Much appreciated. Thanks for the comments.

 drivers/of/base.c  |  109 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/of/gpio.c  |   79 +++++++++-----------------------------
 include/linux/of.h |    3 +
 3 files changed, 130 insertions(+), 61 deletions(-)
drivers/of/base.c +109 -0
index ad8ac1a..517da97 100644
@@ -473,3 +473,112 @@ int of_modalias_node(struct device_node *node, char *modalias, int len)
 }
 EXPORT_SYMBOL_GPL(of_modalias_node);
 
+/**
+ * of_parse_phandles_with_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
+ * @cells_name:	property name that specifies phandles' arguments count
+ * @index:	index of a phandle to parse out
+ * @out_node:	pointer to device_node struct pointer (will be filled)
+ * @out_args:	pointer to arguments pointer (will be filled)
+ *
+ * This function is useful to parse lists of phandles and their arguments.
+ * Returns 0 on success and fills out_node and out_args, on error returns
+ * appropriate errno value.
+ *
+ * Example:
+ *
+ * phandle1: node1 {
+ * 	#list-cells = <2>;
+ * }
+ *
+ * phandle2: node2 {
+ * 	#list-cells = <1>;
+ * }
+ *
+ * node3 {
+ * 	list = <&phandle1 1 2 &phandle2 3>;
+ * }
+ *
+ * To get a device_node of the `node2' node you may call this:
+ * of_parse_phandles_with_args(node3, "list", "#list-cells", 2, &node2, &args);
+ */
+int of_parse_phandles_with_args(struct device_node *np, const char *list_name,
+				const char *cells_name, int index,
+				struct device_node **out_node,
+				const void **out_args)
+{
+	int ret = -EINVAL;
+	const u32 *list;
+	const u32 *list_end;
+	int size;
+	int cur_index = 0;
+	struct device_node *node = NULL;
+	const void *args;
+
+	list = of_get_property(np, list_name, &size);
+	if (!list) {
+		ret = -ENOENT;
+		goto err0;
+	}
+	list_end = list + size / sizeof(*list);
+
+	while (list < list_end) {
+		const u32 *cells;
+		const phandle *phandle;
+
+		phandle = list;
+		args = list + 1;
+
+		/* one cell hole in the list = <>; */
+		if (!*phandle) {
+			list++;
+			goto next;
+		}
+
+		node = of_find_node_by_phandle(*phandle);
+		if (!node) {
+			pr_debug("%s: could not find phandle\n",
+				 np->full_name);
+			goto err0;
+		}
+
+		cells = of_get_property(node, cells_name, &size);
+		if (!cells || size != sizeof(*cells)) {
+			pr_debug("%s: could not get %s for %s\n",
+				 np->full_name, cells_name, node->full_name);
+			goto err1;
+		}
+
+		/* Next phandle is at offset of one phandle cell + #cells */
+		list += 1 + *cells;
+		if (list > list_end) {
+			pr_debug("%s: insufficient arguments length\n",
+				 np->full_name);
+			goto err1;
+		}
+next:
+		if (cur_index == index)
+			break;
+
+		of_node_put(node);
+		node = NULL;
+		cur_index++;
+	}
+
+	if (!node) {
+		ret = -ENOENT;
+		goto err0;
+	}
+
+	*out_node = node;
+	*out_args = args;
+
+	return 0;
+err1:
+	of_node_put(node);
+err0:
+	pr_debug("%s failed with status %d\n", __func__, ret);
+	return ret;
+}
+EXPORT_SYMBOL(of_parse_phandles_with_args);
drivers/of/gpio.c +19 -62

97 lines not on this page · raw message

include/linux/of.h +3 -0
index 79886ad..e2488f5 100644
@@ -71,5 +71,8 @@ extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
 extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+extern int of_parse_phandles_with_args(struct device_node *np,
+	const char *list_name, const char *cells_name, int index,
+	struct device_node **out_node, const void **out_args);
 
 #endif /* _LINUX_OF_H */
Signature · 1 line
1.5.6.3
#4 lore raw

Re: [PATCH 1/2 v2] OF: new helper: of_parse_phandles_with_args()

Quoted 8 lines · Anton Vorontsov

The helper is factored out of of_get_gpio(). Will be used by the QE pin multiplexing functions (they need to parse the gpios = <> too).

---

drivers/of/base.c +2 -0
+	while (list < list_end) {
+		const u32 *cells;

drivers/of/base.c int of_modalias_node(struct device_node *node, char *modalias, int len) · line 537

Quoted 10 lines
drivers/of/base.c +10 -0
+		const phandle *phandle;
+
+		phandle = list;
+		args = list + 1;
+
+		/* one cell hole in the list = <>; */
+		if (!*phandle) {
+			list++;
+			goto next;
+		}

Allright. I'll merge it. It might have been nicer to write here

		phandle = list++;
		args = list;

		if (!*phandle)
			goto next;

		.../...

		list += *cells;

But it's not big deal, I'll merge it as-is and if you feel like it you can aways submit a cleanup later. In fact you can even completely drop "args" I think.

Cheers,
Ben.

#5 lore raw

PATCH 2/2 powerpc/QE: implement QE Pin Multiplexing API

In reply to Anton Vorontsov

Today the API is still based on a fact that QE gpio controllers are registered. If they aren't, the API won't work (gracefully though).

There is one caveat though: if anybody occupied the node->data before us, or overwrote it, then bad things will happen. Luckily this is all in the platform code that we fully control, so this should never happen.

I could implement more checks (for example we could create a list of successfully registered QE controllers, and compare the node->data in the qe_pin_get()), but this is unneeded if nobody is going to do silly things behind our back.

---

p.s. I should probably change qe_pin_get() to allocate and return struct qe_pin instead of accepting already allocated one.

 arch/powerpc/include/asm/qe.h     |   20 +++++
 arch/powerpc/sysdev/qe_lib/gpio.c |  167 +++++++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+), 0 deletions(-)
arch/powerpc/include/asm/qe.h +20 -0
index edee15d..3f15b2f 100644
@@ -112,6 +112,26 @@ extern int par_io_config_pin(u8 port, u8 pin, int dir, int open_drain,
 			     int assignment, int has_irq);
 extern int par_io_data_set(u8 port, u8 pin, u8 val);
 
+/*
+ * Pin multiplexing functions.
+ */
+struct qe_gpio_chip;
+
+struct qe_pin {
+	/*
+	 * The qe_gpio_chip name is unfortunate, we should change that to
+	 * something like qe_pio_controller. Someday.
+	 */
+	struct qe_gpio_chip *controller;
+	int num;
+};
+
+extern int qe_pin_get(struct device_node *np, int index,
+		      struct qe_pin *qe_pin);
+extern void qe_pin_put(struct qe_pin *qe_pin);
+extern void qe_pin_set_gpio(struct qe_pin *qe_pin);
+extern void qe_pin_set_dedicated(struct qe_pin *pin);
+
 /* QE internal API */
 int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32 cmd_input);
 enum qe_clock qe_clock_source(const char *source);
arch/powerpc/sysdev/qe_lib/gpio.c +167 -0

187 lines not on this page · raw message

Signature · 1 line
1.5.6.3