[PATCH] of: Make of_find_node_by_path() traverse /aliases for relative paths.

[PATCH] of: Make of_find_node_by_path() traverse /aliases for relative paths.

[David Daney]

Currently all paths passed to of_find_node_by_path() must begin with a
'/', indicating a full path to the desired node.

Augment the look-up code so that if a path does *not* begin with '/',
the path is used as the name of an /aliases property.  The value of
this alias is then used as the full node path to be found.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 drivers/of/base.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 710b53b..e63da9c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -340,7 +340,10 @@ EXPORT_SYMBOL(of_get_next_child);
 
 /**
  *	of_find_node_by_path - Find a node matching a full OF path
- *	@path:	The full path to match
+ *	@path: Either the full path to match, or if the path does not
+ *	       start with '/', the name of a property of the /aliases
+ *	       node (an alias).  In the case of an alias, the node
+ *	       matching the alias' value will be returned.
  *
  *	Returns a node pointer with refcount incremented, use
  *	of_node_put() on it when done.
@@ -348,13 +351,32 @@ EXPORT_SYMBOL(of_get_next_child);
 struct device_node *of_find_node_by_path(const char *path)
 {
 	struct device_node *np = allnodes;
+	struct device_node *aliases = NULL;
 
 	read_lock(&devtree_lock);
+
+	if (path[0] != '/') {
+		aliases = of_find_node_by_path("/aliases");
+		if (!aliases)
+			goto out;
+		path = of_get_property(aliases, path, NULL);
+		/*
+		 * The alias must begin with '/', otherwise we could
+		 * get stuck in an endless loop and blow out the
+		 * stack.
+		 */
+		if (!path || path[0] != '/')
+			goto out;
+	}
+
 	for (; np; np = np->allnext) {
 		if (np->full_name && (of_node_cmp(np->full_name, path) == 0)
 		    && of_node_get(np))
 			break;
 	}
+out:
+	if (aliases)
+		of_node_put(aliases);
 	read_unlock(&devtree_lock);
 	return np;
 }
-- 
1.7.2.3

Re: [PATCH] of: Make of_find_node_by_path() traverse /aliases for relative paths.

[Grant Likely]

On Tue, Mar 1, 2011 at 4:15 PM, David Daney <ddaney-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org> wrote:
> Currently all paths passed to of_find_node_by_path() must begin with a
> '/', indicating a full path to the desired node.
>
> Augment the look-up code so that if a path does *not* begin with '/',
> the path is used as the name of an /aliases property.  The value of
> this alias is then used as the full node path to be found.

Nice, thanks David!

However, there is one more use case that this patch needs to handle.
Right now it will support:

"/full/path/to/node"
and
"alias"

but in OF terms it is also valid to specify:

"alias/relative/path/to/child"

How I would handle this is to split on the first '/', use the front
half to look up the path to the alias, and then concatenate the alias
path with the child node relative path before doing the full lookup.
Although that might not be the best way since it requires doing a copy
to a temporary buffer.

Alternately, it could be implemented with a multi test loop that does
something like:

for (; np; np = np->allnext) {
        int len = strlen(alias_path);
        if (np->full_name &&
            (strncmp(np->full_name, alias_path, len) == 0) &&
            (np->full_name[len] == '/') &&
            (strncmp(&np->full_name[len+1], relative_path,
strlen(relative_path)) == 0)) {
                of_node_get(np);
                goto out;
        }
}


[...]
> +
> +       if (path[0] != '/') {
> +               aliases = of_find_node_by_path("/aliases");
> +               if (!aliases)
> +                       goto out;
> +               path = of_get_property(aliases, path, NULL);
> +               /*
> +                * The alias must begin with '/', otherwise we could
> +                * get stuck in an endless loop and blow out the
> +                * stack.
> +                */

I don't think this is true (the endless loop part).  I don't see how
this would enter into a recursive set of calls other than the call
of_find_node_by_path("/aliases") which is hard coded with a '/' in the
first character.  The test for path[0] != '/' looks unnecessary, or am
I missing something?

> +               if (!path || path[0] != '/')
> +                       goto out;
> +       }
> +
>        for (; np; np = np->allnext) {
>                if (np->full_name && (of_node_cmp(np->full_name, path) == 0)
>                    && of_node_get(np))
>                        break;
>        }
> +out:
> +       if (aliases)
> +               of_node_put(aliases);

Since there is only one error path that needs to do this
of_node_put(), I'd put it right into the error path and save one test.

g.