Re: [PATCH] of: add helper for getting endpoint node with specific identifiers

Dear Rob Herring,

On Mon, 16 Feb 2015 11:47:24 -0600
Rob Herring [off-list ref] wrote:

On Mon, Feb 16, 2015 at 4:37 AM, Hyungwon Hwang
[off-list ref] wrote:

quoted

When there are multiple ports or multiple endpoints in a port, they
have to be distinguished by the value of reg property. It is
common. The drivers can get the specific endpoint in the specific
port via this function. Now the drivers have to implement this code
in themselves or have to force the order of dt nodes to get the
right node.

Signed-off-by: Hyungwon Hwang <redacted>
---
 drivers/of/base.c        | 34 ++++++++++++++++++++++++++++++++++
 include/linux/of_graph.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 36536b6..95ffe1c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c

@@ -2155,6 +2155,40 @@ struct device_node

*of_graph_get_next_endpoint(const struct device_node *parent,
EXPORT_SYMBOL(of_graph_get_next_endpoint);

 /**
+ * of_graph_get_endpoint_by_regs() - get endpoint node of specific
identifiers
+ * @parent: pointer to the parent device node
+ * @port_reg: identifier (value of reg property) of the parent
port node
+ * @reg: identifier (value of reg property) of the endpoint node
+ *
+ * Return: An 'endpoint' node pointer which is identified by reg
and at the same
+ * is the child of a port node identified by port_reg. reg and
port_reg are
+ * ignored when they are -1.
+ */
+struct device_node *of_graph_get_endpoint_by_regs(
+       const struct device_node *parent, int port_reg, int reg)
+{
+       struct of_endpoint endpoint;
+       struct device_node *node, *prev_node = NULL;
+
+       while (1) {
+               node = of_graph_get_next_endpoint(parent,
prev_node);
+               of_node_put(prev_node);
+               if (!node)
+                       break;
+
+               of_graph_parse_endpoint(node, &endpoint);

Seems like this all could be in a "for_each_of_graph_child_endpoint"
iterator. I've not looked, but I'd guess there are other similar
loops?

There is no other iterator which can be used here. So other codes which
call of_graph_get_next_endpoint() are also using loop. I thought
about adding that kinds of iterator newly. IMHO, I feel it is a little
excessive. So I prefer to using the loop.

quoted

+               if (((port_reg == -1) ||

What if of_graph_parse_endpoint failed?

It never fails and never return except 0. So there is no need to check
the return value of it.

quoted

+                       (endpoint.port == (unsigned int) port_reg))
&&
+                       ((reg == -1) || (endpoint.id == (unsigned
int) reg)))

Why the casts?

I just clarify that endpoint.{port/id} is unsigned int. But now it
seems unnecessary, and make harder to read the code. I will remove it.

quoted

+                       return node;
+
+               prev_node = node;
+       }
+
+       return NULL;
+}
+
+/**
  * of_graph_get_remote_port_parent() - get remote port's parent
node
  * @node: pointer to a local endpoint device_node
  *

diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h
index befef42..c119728 100644
--- a/include/linux/of_graph.h
+++ b/include/linux/of_graph.h

@@ -31,6 +31,8 @@ int of_graph_parse_endpoint(const struct

device_node *node, struct of_endpoint *endpoint);
 struct device_node *of_graph_get_next_endpoint(const struct
device_node *parent, struct device_node *previous);
+struct device_node *of_graph_get_endpoint_by_regs(
+               const struct device_node *parent, int port_reg, int
reg);

Empty version is needed.

Sorry. I forgot.

quoted

 struct device_node *of_graph_get_remote_port_parent(
                                        const struct device_node
*node); struct device_node *of_graph_get_remote_port(const struct
device_node *node); --
1.9.1

Best regards,
Hyungwon Hwang
--
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