On Fri, Nov 28, 2014 at 12:44:03AM +0000, Grant Likely wrote:
> On Thu, 27 Nov 2014 17:56:06 +0000
> , Leif Lindholm <[email protected]>
> wrote:
> > Update of_find_node_by_path():
> > 1) Rename function to of_find_node_opts_by_path(), adding an optional
> > pointer argument. Provide a static inline wrapper version of
> > of_find_node_by_path() which calls the new function with NULL as
> > the optional argument.
> > 2) Ignore any part of the path beyond and including the ':' separator.
> > 3) Set the new provided pointer argument to the beginning of the string
> > following the ':' separator.
> > 4: Add tests.
> >
> > Signed-off-by: Leif Lindholm <[email protected]>
> > ---
> > drivers/of/base.c | 21 +++++++++++++++++----
> > drivers/of/selftest.c | 12 ++++++++++++
> > include/linux/of.h | 14 +++++++++++++-
> > 3 files changed, 42 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/of/base.c b/drivers/of/base.c
> > index 3823edf..7f0e5f7 100644
> > --- a/drivers/of/base.c
> > +++ b/drivers/of/base.c
> > @@ -699,10 +699,15 @@ static struct device_node
> > *__of_find_node_by_path(struct device_node *parent,
> > {
> > struct device_node *child;
> > int len = strchrnul(path, '/') - path;
> > + int term;
> >
> > if (!len)
> > return NULL;
> >
> > + term = strchrnul(path, ':') - path;
> > + if (term < len)
> > + len = term;
> > +
> > __for_each_child_of_node(parent, child) {
> > const char *name = strrchr(child->full_name, '/');
> > if (WARN(!name, "malformed device_node %s\n", child->full_name))
> > @@ -715,11 +720,14 @@ static struct device_node
> > *__of_find_node_by_path(struct device_node *parent,
> > }
> >
> > /**
> > - * of_find_node_by_path - Find a node matching a full OF path
> > + * of_find_node_opts_by_path - Find a node matching a full OF path
> > * @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.
> > + * @opts: Address of a pointer into which to store the start of
> > + * an options string appended to the end of the path with
> > + * a ':' separator.
> > *
> > * Valid paths:
> > * /foo/bar Full path
> > @@ -729,19 +737,24 @@ static struct device_node
> > *__of_find_node_by_path(struct device_node *parent,
> > * Returns a node pointer with refcount incremented, use
> > * of_node_put() on it when done.
> > */
> > -struct device_node *of_find_node_by_path(const char *path)
> > +struct device_node *of_find_node_opts_by_path(const char *path, const char
> > **opts)
> > {
> > struct device_node *np = NULL;
> > struct property *pp;
> > unsigned long flags;
> > + char *separator;
> >
> > if (strcmp(path, "/") == 0)
> > return of_node_get(of_allnodes);
> >
> > + separator = strchr(path, ':');
> > + if (separator && opts)
> > + *opts = separator + 1;
> > +
>
> What about when there are no opts? Do we require the caller to make sure
> opts is NULL before calling the function (which sounds like a good
> source of bugs) or do we clear it on successful return?
>
> I think if opts is passed in, but there are no options, then it should
> set *opts = NULL.
Yeah, oops.
> There should be test cases for this also. Must set opts to NULL on
> successful return, and (I think) should leave opts alone on an
> unsuccessful search.
I would actually argue for always nuking the opts - since that could
(theoretically) prevent something working by accident in spite of
error conditions.
How about the below?
/
Leif
>From 2e1a44e539967d96366d074ae158092900e0c822 Mon Sep 17 00:00:00 2001
From: Leif Lindholm <[email protected]>
Date: Thu, 27 Nov 2014 09:24:31 +0000
Subject: [PATCH] of: add optional options parameter to of_find_node_by_path()
Update of_find_node_by_path():
1) Rename function to of_find_node_opts_by_path(), adding an optional
pointer argument. Provide a static inline wrapper version of
of_find_node_by_path() which calls the new function with NULL as
the optional argument.
2) Ignore any part of the path beyond and including the ':' separator.
3) Set the new provided pointer argument to the beginning of the string
following the ':' separator.
4: Add tests.
Signed-off-by: Leif Lindholm <[email protected]>
---
drivers/of/base.c | 21 +++++++++++++++++----
drivers/of/selftest.c | 17 +++++++++++++++++
include/linux/of.h | 14 +++++++++++++-
3 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 3823edf..99f0fd9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -699,10 +699,15 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
{
struct device_node *child;
int len = strchrnul(path, '/') - path;
+ int term;
if (!len)
return NULL;
+ term = strchrnul(path, ':') - path;
+ if (term < len)
+ len = term;
+
__for_each_child_of_node(parent, child) {
const char *name = strrchr(child->full_name, '/');
if (WARN(!name, "malformed device_node %s\n", child->full_name))
@@ -715,11 +720,14 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
}
/**
- * of_find_node_by_path - Find a node matching a full OF path
+ * of_find_node_opts_by_path - Find a node matching a full OF path
* @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.
+ * @opts: Address of a pointer into which to store the start of
+ * an options string appended to the end of the path with
+ * a ':' separator.
*
* Valid paths:
* /foo/bar Full path
@@ -729,19 +737,24 @@ static struct device_node *__of_find_node_by_path(struct
device_node *parent,
* Returns a node pointer with refcount incremented, use
* of_node_put() on it when done.
*/
-struct device_node *of_find_node_by_path(const char *path)
+struct device_node *of_find_node_opts_by_path(const char *path, const char
**opts)
{
struct device_node *np = NULL;
struct property *pp;
unsigned long flags;
+ char *separator;
if (strcmp(path, "/") == 0)
return of_node_get(of_allnodes);
+ separator = strchr(path, ':');
+ if (opts)
+ *opts = separator ? separator + 1 : NULL;
+
/* The path could begin with an alias */
if (*path != '/') {
char *p = strchrnul(path, '/');
- int len = p - path;
+ int len = separator ? separator - path : p - path;
/* of_aliases must not be NULL */
if (!of_aliases)
@@ -770,7 +783,7 @@ struct device_node *of_find_node_by_path(const char *path)
raw_spin_unlock_irqrestore(&devtree_lock, flags);
return np;
}
-EXPORT_SYMBOL(of_find_node_by_path);
+EXPORT_SYMBOL(of_find_node_opts_by_path);
/**
* of_find_node_by_name - Find a node by its "name" property
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index e2d79af..721b2a4 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -43,6 +43,7 @@ static bool selftest_live_tree;
static void __init of_selftest_find_node_by_name(void)
{
struct device_node *np;
+ const char *options;
np = of_find_node_by_path("/testcase-data");
selftest(np && !strcmp("/testcase-data", np->full_name),
@@ -83,6 +84,22 @@ static void __init of_selftest_find_node_by_name(void)
np = of_find_node_by_path("testcase-alias/missing-path");
selftest(!np, "non-existent alias with relative path returned node
%s\n", np->full_name);
of_node_put(np);
+
+ np = of_find_node_opts_by_path("/testcase-data:testoption", &options);
+ selftest(np && !strcmp("testoption", options),
+ "option path test failed\n");
+ of_node_put(np);
+
+ np = of_find_node_opts_by_path("testcase-alias:testaliasoption",
+ &options);
+ selftest(np && !strcmp("testaliasoption", options),
+ "option alias path test failed\n");
+ of_node_put(np);
+
+ options = "testoption";
+ np = of_find_node_opts_by_path("testcase-alias", &options);
+ selftest(np && !options, "option clearing test failed\n");
+ of_node_put(np);
}
static void __init of_selftest_dynamic(void)
diff --git a/include/linux/of.h b/include/linux/of.h
index 29f0adc..a36be70 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -228,7 +228,13 @@ extern struct device_node *of_find_matching_node_and_match(
const struct of_device_id *matches,
const struct of_device_id **match);
-extern struct device_node *of_find_node_by_path(const char *path);
+extern struct device_node *of_find_node_opts_by_path(const char *path,
+ const char **opts);
+static inline struct device_node *of_find_node_by_path(const char *path)
+{
+ return of_find_node_opts_by_path(path, NULL);
+}
+
extern struct device_node *of_find_node_by_phandle(phandle handle);
extern struct device_node *of_get_parent(const struct device_node *node);
extern struct device_node *of_get_next_parent(struct device_node *node);
@@ -385,6 +391,12 @@ static inline struct device_node
*of_find_node_by_path(const char *path)
return NULL;
}
+static inline struct device_node *of_find_node_opts_by_path(const char *path,
+ const char **opts)
+{
+ return NULL;
+}
+
static inline struct device_node *of_get_parent(const struct device_node *node)
{
return NULL;
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
