diff mbox

[v5,6/7] of: Implement simplified graph binding for single port devices

Message ID 1393522540-22887-7-git-send-email-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Zabel Feb. 27, 2014, 5:35 p.m. UTC
For simple devices with only one port, it can be made implicit.
The endpoint node can be a direct child of the device node.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/of/base.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Comments

Tomi Valkeinen March 4, 2014, 9:06 a.m. UTC | #1
On 27/02/14 19:35, Philipp Zabel wrote:
> For simple devices with only one port, it can be made implicit.
> The endpoint node can be a direct child of the device node.

<snip>

> @@ -2105,9 +2112,11 @@ struct device_node *of_graph_get_remote_port_parent(
>  	/* Get remote endpoint node. */
>  	np = of_parse_phandle(node, "remote-endpoint", 0);
>  
> -	/* Walk 3 levels up only if there is 'ports' node. */
> +	/* Walk 3 levels up only if there is 'ports' node */
>  	for (depth = 3; depth && np; depth--) {
>  		np = of_get_next_parent(np);
> +		if (depth == 3 && of_node_cmp(np->name, "port"))
> +			break;
>  		if (depth == 2 && of_node_cmp(np->name, "ports"))
>  			break;
>  	}

This function becomes a bit funny. Would it be clearer just to do
something like:

	np = of_parse_phandle(node, "remote-endpoint", 0);

	np = of_get_next_parent(np);
	if (of_node_cmp(np->name, "port") != 0)
		return np;

	np = of_get_next_parent(np);
	if (of_node_cmp(np->name, "ports") != 0)
		return np;

	np = of_get_next_parent(np);
	return np;

 Tomi
Philipp Zabel March 4, 2014, 10:04 a.m. UTC | #2
Am Dienstag, den 04.03.2014, 11:06 +0200 schrieb Tomi Valkeinen:
> On 27/02/14 19:35, Philipp Zabel wrote:
> > For simple devices with only one port, it can be made implicit.
> > The endpoint node can be a direct child of the device node.
> 
> <snip>
> 
> > @@ -2105,9 +2112,11 @@ struct device_node *of_graph_get_remote_port_parent(
> >  	/* Get remote endpoint node. */
> >  	np = of_parse_phandle(node, "remote-endpoint", 0);
> >  
> > -	/* Walk 3 levels up only if there is 'ports' node. */
> > +	/* Walk 3 levels up only if there is 'ports' node */
> >  	for (depth = 3; depth && np; depth--) {
> >  		np = of_get_next_parent(np);
> > +		if (depth == 3 && of_node_cmp(np->name, "port"))
> > +			break;
> >  		if (depth == 2 && of_node_cmp(np->name, "ports"))
> >  			break;
> >  	}
> 
> This function becomes a bit funny. Would it be clearer just to do
> something like:
> 
> 	np = of_parse_phandle(node, "remote-endpoint", 0);
> 
> 	np = of_get_next_parent(np);
> 	if (of_node_cmp(np->name, "port") != 0)
> 		return np;
> 
> 	np = of_get_next_parent(np);
> 	if (of_node_cmp(np->name, "ports") != 0)
> 		return np;
> 
> 	np = of_get_next_parent(np);
> 	return np;

I'm not sure if this was initially crafted to reduce the number of
function calls, but rolled out it certainly is easier to read. I'll
change this as you suggest.

thanks
Philipp

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ba3cfca..7d9c62b 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2037,8 +2037,13 @@  struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 		struct device_node *node;
 		/*
 		 * It's the first call, we have to find a port subnode
-		 * within this node or within an optional 'ports' node.
+		 * within this node or within an optional 'ports' node,
+		 * or at least a single endpoint.
 		 */
+		endpoint = of_get_child_by_name(parent, "endpoint");
+		if (endpoint)
+			return endpoint;
+
 		node = of_get_child_by_name(parent, "ports");
 		if (node)
 			parent = node;
@@ -2049,8 +2054,6 @@  struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 			/* Found a port, get an endpoint. */
 			endpoint = of_get_next_child(port, NULL);
 			of_node_put(port);
-		} else {
-			endpoint = NULL;
 		}
 
 		if (!endpoint)
@@ -2065,6 +2068,10 @@  struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
 	if (WARN_ONCE(!port, "%s(): endpoint has no parent node\n",
 		      __func__))
 		return NULL;
+	if (port == parent) {
+		of_node_put(port);
+		return NULL;
+	}
 
 	/* Avoid dropping prev node refcount to 0. */
 	of_node_get(prev);
@@ -2105,9 +2112,11 @@  struct device_node *of_graph_get_remote_port_parent(
 	/* Get remote endpoint node. */
 	np = of_parse_phandle(node, "remote-endpoint", 0);
 
-	/* Walk 3 levels up only if there is 'ports' node. */
+	/* Walk 3 levels up only if there is 'ports' node */
 	for (depth = 3; depth && np; depth--) {
 		np = of_get_next_parent(np);
+		if (depth == 3 && of_node_cmp(np->name, "port"))
+			break;
 		if (depth == 2 && of_node_cmp(np->name, "ports"))
 			break;
 	}
@@ -2130,6 +2139,11 @@  struct device_node *of_graph_get_remote_port(const struct device_node *node)
 	np = of_parse_phandle(node, "remote-endpoint", 0);
 	if (!np)
 		return NULL;
-	return of_get_next_parent(np);
+	np = of_get_next_parent(np);
+	if (of_node_cmp(np->name, "port")) {
+		of_node_put(np);
+		return NULL;
+	}
+	return np;
 }
 EXPORT_SYMBOL(of_graph_get_remote_port);