[PATCH 00/12] of: Make drivers/of/resolver.c more readable

[PATCH 00/12] of: Make drivers/of/resolver.c more readable

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

drivers/of/resolve.c is a bit difficult to read.  Clean it up so
that review of future overlay related patches will be easier.

Most of the patches are intended to be reformatting, with no functional
change.  Patches that are expected to have a functional change are:

  Remove excessive printks to reduce clutter.
  Update structure of code to be clearer, also remove BUG_ON()
    Any functional change would reflect undefined behavior on bad overlay.
    Some error message text modified.
    BUG_ON() removed.
  Add back an error message, restructured

The patches are grouped into sets of changes that are intended
to be easy to verify correctness through simple inspection.

Some of the individual patches have checkpatch warnings or errors.
But after all patches are applied, the number of errors and
warnings from running checkpatch against the entire file are
reduced to two line size warnings.

These patches are only tested via the unit tests. I do not have
expansion boards to test with real hardware.

changes from rfc to v1:
  - Remove fewer one line comments
  - Add more extensive header comment to of_resolve_phandles()
    to explain the how and why of resolving phandles
  - Update patch header comments
  - Incorporated patch "Remove braces around single line blocks"
    into the previous patch in the series


Frank Rowand (12):
  of: Remove comments that state the obvious, to reduce clutter
  of: Remove excessive printks to reduce clutter.
  of: Convert comparisons to zero or NULL to logical expressions
  of: Rename functions to more accurately reflect what they do
  of: Remove prefix "__of_" from local function names
  of: Rename variables to better reflect purpose or follow convention
  of: Update structure of code to be clearer, also remove BUG_ON()
  of: Remove redundant size check
  of: Update comments to reflect changes and increase clarity
  of: Add back an error message, restructured
  of: Move setting of pointer to beside test for non-null
  of: Remove unused variable overlay_symbols

 drivers/of/resolver.c | 364 ++++++++++++++++++++++----------------------------
 1 file changed, 156 insertions(+), 208 deletions(-)

-- 
1.9.1

[PATCH 00/12] of: Make drivers/of/resolver.c more readable

[frowand.list-Re5JQEeQqe8AvxtiuMwx3w]

From: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>

drivers/of/resolve.c is a bit difficult to read.  Clean it up so
that review of future overlay related patches will be easier.

Most of the patches are intended to be reformatting, with no functional
change.  Patches that are expected to have a functional change are:

  Remove excessive printks to reduce clutter.
  Update structure of code to be clearer, also remove BUG_ON()
    Any functional change would reflect undefined behavior on bad overlay.
    Some error message text modified.
    BUG_ON() removed.
  Add back an error message, restructured

The patches are grouped into sets of changes that are intended
to be easy to verify correctness through simple inspection.

Some of the individual patches have checkpatch warnings or errors.
But after all patches are applied, the number of errors and
warnings from running checkpatch against the entire file are
reduced to two line size warnings.

These patches are only tested via the unit tests. I do not have
expansion boards to test with real hardware.

changes from rfc to v1:
  - Remove fewer one line comments
  - Add more extensive header comment to of_resolve_phandles()
    to explain the how and why of resolving phandles
  - Update patch header comments
  - Incorporated patch "Remove braces around single line blocks"
    into the previous patch in the series


Frank Rowand (12):
  of: Remove comments that state the obvious, to reduce clutter
  of: Remove excessive printks to reduce clutter.
  of: Convert comparisons to zero or NULL to logical expressions
  of: Rename functions to more accurately reflect what they do
  of: Remove prefix "__of_" from local function names
  of: Rename variables to better reflect purpose or follow convention
  of: Update structure of code to be clearer, also remove BUG_ON()
  of: Remove redundant size check
  of: Update comments to reflect changes and increase clarity
  of: Add back an error message, restructured
  of: Move setting of pointer to beside test for non-null
  of: Remove unused variable overlay_symbols

 drivers/of/resolver.c | 364 ++++++++++++++++++++++----------------------------
 1 file changed, 156 insertions(+), 208 deletions(-)

-- 
1.9.1

--
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

[PATCH 01/12] of: Remove comments that state the obvious, to reduce clutter

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove comments that report what is obvious from the code.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 31 ++-----------------------------
 1 file changed, 2 insertions(+), 29 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 46325d6394cf..4ff0220d7aa2 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -36,7 +36,6 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
 	if (node == NULL)
 		return NULL;
 
-	/* check */
 	if (of_node_cmp(node->full_name, full_name) == 0)
 		return of_node_get(node);
 
@@ -60,7 +59,6 @@ static phandle of_get_tree_max_phandle(void)
 	phandle phandle;
 	unsigned long flags;
 
-	/* now search recursively */
 	raw_spin_lock_irqsave(&devtree_lock, flags);
 	phandle = 0;
 	for_each_of_allnodes(node) {
@@ -75,8 +73,6 @@ static phandle of_get_tree_max_phandle(void)
 
 /*
  * Adjust a subtree's phandle values by a given delta.
- * Makes sure not to just adjust the device node's phandle value,
- * but modify the phandle properties values as well.
  */
 static void __of_adjust_tree_phandles(struct device_node *node,
 		int phandle_delta)
@@ -85,32 +81,25 @@ static void __of_adjust_tree_phandles(struct device_node *node,
 	struct property *prop;
 	phandle phandle;
 
-	/* first adjust the node's phandle direct value */
 	if (node->phandle != 0 && node->phandle != OF_PHANDLE_ILLEGAL)
 		node->phandle += phandle_delta;
 
-	/* now adjust phandle & linux,phandle values */
 	for_each_property_of_node(node, prop) {
 
-		/* only look for these two */
 		if (of_prop_cmp(prop->name, "phandle") != 0 &&
 		    of_prop_cmp(prop->name, "linux,phandle") != 0)
 			continue;
 
-		/* must be big enough */
 		if (prop->length < 4)
 			continue;
 
-		/* read phandle value */
 		phandle = be32_to_cpup(prop->value);
-		if (phandle == OF_PHANDLE_ILLEGAL)	/* unresolved */
+		if (phandle == OF_PHANDLE_ILLEGAL)
 			continue;
 
-		/* adjust */
 		*(uint32_t *)prop->value = cpu_to_be32(node->phandle);
 	}
 
-	/* now do the children recursively */
 	for_each_child_of_node(node, child)
 		__of_adjust_tree_phandles(child, phandle_delta);
 }
@@ -125,7 +114,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 	int offset, propcurlen;
 	int err = 0;
 
-	/* make a copy */
 	propval = kmalloc(rprop->length, GFP_KERNEL);
 	if (!propval) {
 		pr_err("%s: Could not copy value of '%s'\n",
@@ -165,7 +153,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 			goto err_fail;
 		}
 
-		/* look into the resolve node for the full path */
 		refnode = __of_find_node_by_full_name(node, nodestr);
 		if (!refnode) {
 			pr_warn("%s: Could not find refnode '%s'\n",
@@ -173,7 +160,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 			continue;
 		}
 
-		/* now find the property */
 		for_each_property_of_node(refnode, sprop) {
 			if (of_prop_cmp(sprop->name, propstr) == 0)
 				break;
@@ -240,7 +226,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 		}
 		count = rprop->length / sizeof(__be32);
 
-		/* now find the target property */
 		for_each_property_of_node(target, sprop) {
 			if (of_prop_cmp(sprop->name, rprop->name) == 0)
 				break;
@@ -254,7 +239,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 
 		for (i = 0; i < count; i++) {
 			off = be32_to_cpu(((__be32 *)rprop->value)[i]);
-			/* make sure the offset doesn't overstep (even wrap) */
 			if (off >= sprop->length ||
 					(off + 4) > sprop->length) {
 				pr_err("%s: Illegal property '%s' @%s\n",
@@ -264,7 +248,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 			}
 
 			if (phandle_delta) {
-				/* adjust */
 				phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
 				phandle += phandle_delta;
 				*(__be32 *)(sprop->value + off) = cpu_to_be32(phandle);
@@ -320,22 +303,18 @@ int of_resolve_phandles(struct device_node *resolve)
 	if (resolve && !of_node_check_flag(resolve, OF_DETACHED))
 		pr_err("%s: node %s not detached\n", __func__,
 			 resolve->full_name);
-	/* the resolve node must exist, and be detached */
 	if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
 		return -EINVAL;
 
-	/* first we need to adjust the phandles */
 	phandle_delta = of_get_tree_max_phandle() + 1;
 	__of_adjust_tree_phandles(resolve, phandle_delta);
 
-	/* locate the local fixups */
 	childroot = NULL;
 	for_each_child_of_node(resolve, childroot)
 		if (of_node_cmp(childroot->name, "__local_fixups__") == 0)
 			break;
 
 	if (childroot != NULL) {
-		/* resolve root is guaranteed to be the '/' */
 		err = __of_adjust_tree_phandle_references(childroot,
 				resolve, 0);
 		if (err != 0)
@@ -349,10 +328,8 @@ int of_resolve_phandles(struct device_node *resolve)
 	resolve_sym = NULL;
 	resolve_fix = NULL;
 
-	/* this may fail (if no fixups are required) */
 	root_sym = of_find_node_by_path("/__symbols__");
 
-	/* locate the symbols & fixups nodes on resolve */
 	for_each_child_of_node(resolve, child) {
 
 		if (!resolve_sym &&
@@ -363,18 +340,15 @@ int of_resolve_phandles(struct device_node *resolve)
 				of_node_cmp(child->name, "__fixups__") == 0)
 			resolve_fix = child;
 
-		/* both found, don't bother anymore */
 		if (resolve_sym && resolve_fix)
 			break;
 	}
 
-	/* we do allow for the case where no fixups are needed */
 	if (!resolve_fix) {
-		err = 0;	/* no error */
+		err = 0;
 		goto out;
 	}
 
-	/* we need to fixup, but no root symbols... */
 	if (!root_sym) {
 		pr_err("%s: no symbols in root of device tree.\n", __func__);
 		err = -EINVAL;
@@ -415,7 +389,6 @@ int of_resolve_phandles(struct device_node *resolve)
 	}
 
 out:
-	/* NULL is handled by of_node_put as NOP */
 	of_node_put(root_sym);
 
 	return err;
-- 
1.9.1

[PATCH 02/12] of: Remove excessive printks to reduce clutter.

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove extra debug and error printks.  A single pr_err() will
be added at the end of this series to replace many of these
error messages.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 53 ++++++++-------------------------------------------
 1 file changed, 8 insertions(+), 45 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 4ff0220d7aa2..c61ba99a1792 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -115,11 +115,8 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 	int err = 0;
 
 	propval = kmalloc(rprop->length, GFP_KERNEL);
-	if (!propval) {
-		pr_err("%s: Could not copy value of '%s'\n",
-				__func__, rprop->name);
+	if (!propval)
 		return -ENOMEM;
-	}
 	memcpy(propval, rprop->value, rprop->length);
 
 	propend = propval + rprop->length;
@@ -129,8 +126,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 		nodestr = propcur;
 		s = strchr(propcur, ':');
 		if (!s) {
-			pr_err("%s: Illegal symbol entry '%s' (1)\n",
-				__func__, propcur);
 			err = -EINVAL;
 			goto err_fail;
 		}
@@ -139,26 +134,18 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 		propstr = s;
 		s = strchr(s, ':');
 		if (!s) {
-			pr_err("%s: Illegal symbol entry '%s' (2)\n",
-				__func__, (char *)rprop->value);
 			err = -EINVAL;
 			goto err_fail;
 		}
 
 		*s++ = '\0';
 		err = kstrtoint(s, 10, &offset);
-		if (err != 0) {
-			pr_err("%s: Could get offset '%s'\n",
-				__func__, (char *)rprop->value);
+		if (err != 0)
 			goto err_fail;
-		}
 
 		refnode = __of_find_node_by_full_name(node, nodestr);
-		if (!refnode) {
-			pr_warn("%s: Could not find refnode '%s'\n",
-				__func__, (char *)rprop->value);
+		if (!refnode)
 			continue;
-		}
 
 		for_each_property_of_node(refnode, sprop) {
 			if (of_prop_cmp(sprop->name, propstr) == 0)
@@ -167,8 +154,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 		of_node_put(refnode);
 
 		if (!sprop) {
-			pr_err("%s: Could not find property '%s'\n",
-				__func__, (char *)rprop->value);
 			err = -ENOENT;
 			goto err_fail;
 		}
@@ -219,11 +204,8 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 		    of_prop_cmp(rprop->name, "linux,phandle") == 0)
 			continue;
 
-		if ((rprop->length % 4) != 0 || rprop->length == 0) {
-			pr_err("%s: Illegal property (size) '%s' @%s\n",
-					__func__, rprop->name, node->full_name);
+		if ((rprop->length % 4) != 0 || rprop->length == 0)
 			return -EINVAL;
-		}
 		count = rprop->length / sizeof(__be32);
 
 		for_each_property_of_node(target, sprop) {
@@ -231,21 +213,13 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 				break;
 		}
 
-		if (sprop == NULL) {
-			pr_err("%s: Could not find target property '%s' @%s\n",
-					__func__, rprop->name, node->full_name);
+		if (sprop == NULL)
 			return -EINVAL;
-		}
 
 		for (i = 0; i < count; i++) {
 			off = be32_to_cpu(((__be32 *)rprop->value)[i]);
-			if (off >= sprop->length ||
-					(off + 4) > sprop->length) {
-				pr_err("%s: Illegal property '%s' @%s\n",
-						__func__, rprop->name,
-						node->full_name);
+			if (off >= sprop->length || (off + 4) > sprop->length)
 				return -EINVAL;
-			}
 
 			if (phandle_delta) {
 				phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
@@ -261,11 +235,8 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 			if (__of_node_name_cmp(child, childtarget) == 0)
 				break;
 
-		if (!childtarget) {
-			pr_err("%s: Could not find target child '%s' @%s\n",
-					__func__, child->name, node->full_name);
+		if (!childtarget)
 			return -EINVAL;
-		}
 
 		err = __of_adjust_tree_phandle_references(child, childtarget,
 				phandle_delta);
@@ -363,16 +334,11 @@ int of_resolve_phandles(struct device_node *resolve)
 
 		err = of_property_read_string(root_sym,
 				rprop->name, &refpath);
-		if (err != 0) {
-			pr_err("%s: Could not find symbol '%s'\n",
-					__func__, rprop->name);
+		if (err != 0)
 			goto out;
-		}
 
 		refnode = of_find_node_by_path(refpath);
 		if (!refnode) {
-			pr_err("%s: Could not find node by path '%s'\n",
-					__func__, refpath);
 			err = -ENOENT;
 			goto out;
 		}
@@ -380,9 +346,6 @@ int of_resolve_phandles(struct device_node *resolve)
 		phandle = refnode->phandle;
 		of_node_put(refnode);
 
-		pr_debug("%s: %s phandle is 0x%08x\n",
-				__func__, rprop->name, phandle);
-
 		err = __of_adjust_phandle_ref(resolve, rprop, phandle);
 		if (err)
 			break;
-- 
1.9.1

[PATCH 03/12] of: Convert comparisons to zero or NULL to logical expressions

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Convert comparisons to zero or NULL to logical expressions.  A
small number of such comparisons remain where they provide more
clarity of the numeric nature of a variable.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index c61ba99a1792..31fd3800787a 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -33,10 +33,10 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
 {
 	struct device_node *child, *found;
 
-	if (node == NULL)
+	if (!node)
 		return NULL;
 
-	if (of_node_cmp(node->full_name, full_name) == 0)
+	if (!of_node_cmp(node->full_name, full_name))
 		return of_node_get(node);
 
 	for_each_child_of_node(node, child) {
@@ -86,8 +86,8 @@ static void __of_adjust_tree_phandles(struct device_node *node,
 
 	for_each_property_of_node(node, prop) {
 
-		if (of_prop_cmp(prop->name, "phandle") != 0 &&
-		    of_prop_cmp(prop->name, "linux,phandle") != 0)
+		if (of_prop_cmp(prop->name, "phandle") &&
+		    of_prop_cmp(prop->name, "linux,phandle"))
 			continue;
 
 		if (prop->length < 4)
@@ -140,7 +140,7 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 
 		*s++ = '\0';
 		err = kstrtoint(s, 10, &offset);
-		if (err != 0)
+		if (err)
 			goto err_fail;
 
 		refnode = __of_find_node_by_full_name(node, nodestr);
@@ -148,7 +148,7 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 			continue;
 
 		for_each_property_of_node(refnode, sprop) {
-			if (of_prop_cmp(sprop->name, propstr) == 0)
+			if (!of_prop_cmp(sprop->name, propstr))
 				break;
 		}
 		of_node_put(refnode);
@@ -193,15 +193,15 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 	unsigned int off;
 	phandle phandle;
 
-	if (node == NULL)
+	if (!node)
 		return 0;
 
 	for_each_property_of_node(node, rprop) {
 
 		/* skip properties added automatically */
-		if (of_prop_cmp(rprop->name, "name") == 0 ||
-		    of_prop_cmp(rprop->name, "phandle") == 0 ||
-		    of_prop_cmp(rprop->name, "linux,phandle") == 0)
+		if (!of_prop_cmp(rprop->name, "name") ||
+		    !of_prop_cmp(rprop->name, "phandle") ||
+		    !of_prop_cmp(rprop->name, "linux,phandle"))
 			continue;
 
 		if ((rprop->length % 4) != 0 || rprop->length == 0)
@@ -209,11 +209,11 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 		count = rprop->length / sizeof(__be32);
 
 		for_each_property_of_node(target, sprop) {
-			if (of_prop_cmp(sprop->name, rprop->name) == 0)
+			if (!of_prop_cmp(sprop->name, rprop->name))
 				break;
 		}
 
-		if (sprop == NULL)
+		if (!sprop)
 			return -EINVAL;
 
 		for (i = 0; i < count; i++) {
@@ -232,7 +232,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 	for_each_child_of_node(node, child) {
 
 		for_each_child_of_node(target, childtarget)
-			if (__of_node_name_cmp(child, childtarget) == 0)
+			if (!__of_node_name_cmp(child, childtarget))
 				break;
 
 		if (!childtarget)
@@ -240,7 +240,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 
 		err = __of_adjust_tree_phandle_references(child, childtarget,
 				phandle_delta);
-		if (err != 0)
+		if (err)
 			return err;
 	}
 
@@ -282,13 +282,13 @@ int of_resolve_phandles(struct device_node *resolve)
 
 	childroot = NULL;
 	for_each_child_of_node(resolve, childroot)
-		if (of_node_cmp(childroot->name, "__local_fixups__") == 0)
+		if (!of_node_cmp(childroot->name, "__local_fixups__"))
 			break;
 
 	if (childroot != NULL) {
 		err = __of_adjust_tree_phandle_references(childroot,
 				resolve, 0);
-		if (err != 0)
+		if (err)
 			return err;
 
 		BUG_ON(__of_adjust_tree_phandle_references(childroot,
@@ -303,12 +303,10 @@ int of_resolve_phandles(struct device_node *resolve)
 
 	for_each_child_of_node(resolve, child) {
 
-		if (!resolve_sym &&
-				of_node_cmp(child->name, "__symbols__") == 0)
+		if (!resolve_sym && !of_node_cmp(child->name, "__symbols__"))
 			resolve_sym = child;
 
-		if (!resolve_fix &&
-				of_node_cmp(child->name, "__fixups__") == 0)
+		if (!resolve_fix && !of_node_cmp(child->name, "__fixups__"))
 			resolve_fix = child;
 
 		if (resolve_sym && resolve_fix)
@@ -329,12 +327,12 @@ int of_resolve_phandles(struct device_node *resolve)
 	for_each_property_of_node(resolve_fix, rprop) {
 
 		/* skip properties added automatically */
-		if (of_prop_cmp(rprop->name, "name") == 0)
+		if (!of_prop_cmp(rprop->name, "name"))
 			continue;
 
 		err = of_property_read_string(root_sym,
 				rprop->name, &refpath);
-		if (err != 0)
+		if (err)
 			goto out;
 
 		refnode = of_find_node_by_path(refpath);
-- 
1.9.1

[PATCH 04/12] of: Rename functions to more accurately reflect what they do

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Some function names are misleading or do not provide a good
sense of what they do.  Rename the functions to ne more
informative.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 31fd3800787a..3d123b612789 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -53,7 +53,7 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
 /*
  * Find live tree's maximum phandle value.
  */
-static phandle of_get_tree_max_phandle(void)
+static phandle live_tree_max_phandle(void)
 {
 	struct device_node *node;
 	phandle phandle;
@@ -74,7 +74,7 @@ static phandle of_get_tree_max_phandle(void)
 /*
  * Adjust a subtree's phandle values by a given delta.
  */
-static void __of_adjust_tree_phandles(struct device_node *node,
+static void adjust_overlay_phandles(struct device_node *node,
 		int phandle_delta)
 {
 	struct device_node *child;
@@ -101,10 +101,10 @@ static void __of_adjust_tree_phandles(struct device_node *node,
 	}
 
 	for_each_child_of_node(node, child)
-		__of_adjust_tree_phandles(child, phandle_delta);
+		adjust_overlay_phandles(child, phandle_delta);
 }
 
-static int __of_adjust_phandle_ref(struct device_node *node,
+static int update_usages_of_a_phandle_reference(struct device_node *node,
 		struct property *rprop, int value)
 {
 	phandle phandle;
@@ -184,7 +184,7 @@ static int __of_node_name_cmp(const struct device_node *dn1,
  * Does not take any devtree locks so make sure you call this on a tree
  * which is at the detached state.
  */
-static int __of_adjust_tree_phandle_references(struct device_node *node,
+static int adjust_local_phandle_references(struct device_node *node,
 		struct device_node *target, int phandle_delta)
 {
 	struct device_node *child, *childtarget;
@@ -238,7 +238,7 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 		if (!childtarget)
 			return -EINVAL;
 
-		err = __of_adjust_tree_phandle_references(child, childtarget,
+		err = adjust_local_phandle_references(child, childtarget,
 				phandle_delta);
 		if (err)
 			return err;
@@ -277,8 +277,8 @@ int of_resolve_phandles(struct device_node *resolve)
 	if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
 		return -EINVAL;
 
-	phandle_delta = of_get_tree_max_phandle() + 1;
-	__of_adjust_tree_phandles(resolve, phandle_delta);
+	phandle_delta = live_tree_max_phandle() + 1;
+	adjust_overlay_phandles(resolve, phandle_delta);
 
 	childroot = NULL;
 	for_each_child_of_node(resolve, childroot)
@@ -286,12 +286,12 @@ int of_resolve_phandles(struct device_node *resolve)
 			break;
 
 	if (childroot != NULL) {
-		err = __of_adjust_tree_phandle_references(childroot,
+		err = adjust_local_phandle_references(childroot,
 				resolve, 0);
 		if (err)
 			return err;
 
-		BUG_ON(__of_adjust_tree_phandle_references(childroot,
+		BUG_ON(adjust_local_phandle_references(childroot,
 				resolve, phandle_delta));
 	}
 
@@ -344,7 +344,7 @@ int of_resolve_phandles(struct device_node *resolve)
 		phandle = refnode->phandle;
 		of_node_put(refnode);
 
-		err = __of_adjust_phandle_ref(resolve, rprop, phandle);
+		err = update_usages_of_a_phandle_reference(resolve, rprop, phandle);
 		if (err)
 			break;
 	}
-- 
1.9.1

[PATCH 05/12] of: Remove prefix "__of_" from local function names

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove "__of_" prefix from local function names.  The pattern of
a leading "__" is used in drivers/of/ to signify a function that
must be called with a lock held.  These functions do not fit
that pattern.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 3d123b612789..0ce38aa0ed3c 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -28,7 +28,7 @@
  * Find a node with the give full name by recursively following any of
  * the child node links.
  */
-static struct device_node *__of_find_node_by_full_name(struct device_node *node,
+static struct device_node *find_node_by_full_name(struct device_node *node,
 		const char *full_name)
 {
 	struct device_node *child, *found;
@@ -40,7 +40,7 @@ static struct device_node *__of_find_node_by_full_name(struct device_node *node,
 		return of_node_get(node);
 
 	for_each_child_of_node(node, child) {
-		found = __of_find_node_by_full_name(child, full_name);
+		found = find_node_by_full_name(child, full_name);
 		if (found != NULL) {
 			of_node_put(child);
 			return found;
@@ -143,7 +143,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
 		if (err)
 			goto err_fail;
 
-		refnode = __of_find_node_by_full_name(node, nodestr);
+		refnode = find_node_by_full_name(node, nodestr);
 		if (!refnode)
 			continue;
 
@@ -168,7 +168,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
 }
 
 /* compare nodes taking into account that 'name' strips out the @ part */
-static int __of_node_name_cmp(const struct device_node *dn1,
+static int node_name_cmp(const struct device_node *dn1,
 		const struct device_node *dn2)
 {
 	const char *n1 = strrchr(dn1->full_name, '/') ? : "/";
@@ -232,7 +232,7 @@ static int adjust_local_phandle_references(struct device_node *node,
 	for_each_child_of_node(node, child) {
 
 		for_each_child_of_node(target, childtarget)
-			if (!__of_node_name_cmp(child, childtarget))
+			if (!node_name_cmp(child, childtarget))
 				break;
 
 		if (!childtarget)
-- 
1.9.1

[PATCH 06/12] of: Rename variables to better reflect purpose or follow convention

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Rename variables to better reflect what their purpose is.  As a side
effect, this reduces the need for some of the comments previously
removed in this series.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 172 +++++++++++++++++++++++++-------------------------
 1 file changed, 85 insertions(+), 87 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 0ce38aa0ed3c..0778747cdd58 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -74,17 +74,17 @@ static phandle live_tree_max_phandle(void)
 /*
  * Adjust a subtree's phandle values by a given delta.
  */
-static void adjust_overlay_phandles(struct device_node *node,
+static void adjust_overlay_phandles(struct device_node *overlay,
 		int phandle_delta)
 {
 	struct device_node *child;
 	struct property *prop;
 	phandle phandle;
 
-	if (node->phandle != 0 && node->phandle != OF_PHANDLE_ILLEGAL)
-		node->phandle += phandle_delta;
+	if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
+		overlay->phandle += phandle_delta;
 
-	for_each_property_of_node(node, prop) {
+	for_each_property_of_node(overlay, prop) {
 
 		if (of_prop_cmp(prop->name, "phandle") &&
 		    of_prop_cmp(prop->name, "linux,phandle"))
@@ -97,41 +97,40 @@ static void adjust_overlay_phandles(struct device_node *node,
 		if (phandle == OF_PHANDLE_ILLEGAL)
 			continue;
 
-		*(uint32_t *)prop->value = cpu_to_be32(node->phandle);
+		*(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
 	}
 
-	for_each_child_of_node(node, child)
+	for_each_child_of_node(overlay, child)
 		adjust_overlay_phandles(child, phandle_delta);
 }
 
-static int update_usages_of_a_phandle_reference(struct device_node *node,
-		struct property *rprop, int value)
+static int update_usages_of_a_phandle_reference(struct device_node *overlay,
+		struct property *prop_fixup, phandle phandle)
 {
-	phandle phandle;
 	struct device_node *refnode;
-	struct property *sprop;
-	char *propval, *propcur, *propend, *nodestr, *propstr, *s;
-	int offset, propcurlen;
+	struct property *prop;
+	char *value, *cur, *end, *node_path, *prop_name, *s;
+	int offset, len;
 	int err = 0;
 
-	propval = kmalloc(rprop->length, GFP_KERNEL);
-	if (!propval)
+	value = kmalloc(prop_fixup->length, GFP_KERNEL);
+	if (!value)
 		return -ENOMEM;
-	memcpy(propval, rprop->value, rprop->length);
+	memcpy(value, prop_fixup->value, prop_fixup->length);
 
-	propend = propval + rprop->length;
-	for (propcur = propval; propcur < propend; propcur += propcurlen + 1) {
-		propcurlen = strlen(propcur);
+	end = value + prop_fixup->length;
+	for (cur = value; cur < end; cur += len + 1) {
+		len = strlen(cur);
 
-		nodestr = propcur;
-		s = strchr(propcur, ':');
+		node_path = cur;
+		s = strchr(cur, ':');
 		if (!s) {
 			err = -EINVAL;
 			goto err_fail;
 		}
 		*s++ = '\0';
 
-		propstr = s;
+		prop_name = s;
 		s = strchr(s, ':');
 		if (!s) {
 			err = -EINVAL;
@@ -143,27 +142,26 @@ static int update_usages_of_a_phandle_reference(struct device_node *node,
 		if (err)
 			goto err_fail;
 
-		refnode = find_node_by_full_name(node, nodestr);
+		refnode = find_node_by_full_name(overlay, node_path);
 		if (!refnode)
 			continue;
 
-		for_each_property_of_node(refnode, sprop) {
-			if (!of_prop_cmp(sprop->name, propstr))
+		for_each_property_of_node(refnode, prop) {
+			if (!of_prop_cmp(prop->name, prop_name))
 				break;
 		}
 		of_node_put(refnode);
 
-		if (!sprop) {
+		if (!prop) {
 			err = -ENOENT;
 			goto err_fail;
 		}
 
-		phandle = value;
-		*(__be32 *)(sprop->value + offset) = cpu_to_be32(phandle);
+		*(__be32 *)(prop->value + offset) = cpu_to_be32(phandle);
 	}
 
 err_fail:
-	kfree(propval);
+	kfree(value);
 	return err;
 }
 
@@ -184,61 +182,61 @@ static int node_name_cmp(const struct device_node *dn1,
  * Does not take any devtree locks so make sure you call this on a tree
  * which is at the detached state.
  */
-static int adjust_local_phandle_references(struct device_node *node,
-		struct device_node *target, int phandle_delta)
+static int adjust_local_phandle_references(struct device_node *local_fixups,
+		struct device_node *overlay, int phandle_delta)
 {
-	struct device_node *child, *childtarget;
-	struct property *rprop, *sprop;
+	struct device_node *child, *overlay_child;
+	struct property *prop_fix, *prop;
 	int err, i, count;
 	unsigned int off;
 	phandle phandle;
 
-	if (!node)
+	if (!local_fixups)
 		return 0;
 
-	for_each_property_of_node(node, rprop) {
+	for_each_property_of_node(local_fixups, prop_fix) {
 
 		/* skip properties added automatically */
-		if (!of_prop_cmp(rprop->name, "name") ||
-		    !of_prop_cmp(rprop->name, "phandle") ||
-		    !of_prop_cmp(rprop->name, "linux,phandle"))
+		if (!of_prop_cmp(prop_fix->name, "name") ||
+		    !of_prop_cmp(prop_fix->name, "phandle") ||
+		    !of_prop_cmp(prop_fix->name, "linux,phandle"))
 			continue;
 
-		if ((rprop->length % 4) != 0 || rprop->length == 0)
+		if ((prop_fix->length % 4) != 0 || prop_fix->length == 0)
 			return -EINVAL;
-		count = rprop->length / sizeof(__be32);
+		count = prop_fix->length / sizeof(__be32);
 
-		for_each_property_of_node(target, sprop) {
-			if (!of_prop_cmp(sprop->name, rprop->name))
+		for_each_property_of_node(overlay, prop) {
+			if (!of_prop_cmp(prop->name, prop_fix->name))
 				break;
 		}
 
-		if (!sprop)
+		if (!prop)
 			return -EINVAL;
 
 		for (i = 0; i < count; i++) {
-			off = be32_to_cpu(((__be32 *)rprop->value)[i]);
-			if (off >= sprop->length || (off + 4) > sprop->length)
+			off = be32_to_cpu(((__be32 *)prop_fix->value)[i]);
+			if (off >= prop->length || (off + 4) > prop->length)
 				return -EINVAL;
 
 			if (phandle_delta) {
-				phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
+				phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
 				phandle += phandle_delta;
-				*(__be32 *)(sprop->value + off) = cpu_to_be32(phandle);
+				*(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
 			}
 		}
 	}
 
-	for_each_child_of_node(node, child) {
+	for_each_child_of_node(local_fixups, child) {
 
-		for_each_child_of_node(target, childtarget)
-			if (!node_name_cmp(child, childtarget))
+		for_each_child_of_node(overlay, overlay_child)
+			if (!node_name_cmp(child, overlay_child))
 				break;
 
-		if (!childtarget)
+		if (!overlay_child)
 			return -EINVAL;
 
-		err = adjust_local_phandle_references(child, childtarget,
+		err = adjust_local_phandle_references(child, overlay_child,
 				phandle_delta);
 		if (err)
 			return err;
@@ -260,78 +258,78 @@ static int adjust_local_phandle_references(struct device_node *node,
  * are fit to be inserted or operate upon the live tree.
  * Returns 0 on success or a negative error value on error.
  */
-int of_resolve_phandles(struct device_node *resolve)
+int of_resolve_phandles(struct device_node *overlay)
 {
-	struct device_node *child, *childroot, *refnode;
-	struct device_node *root_sym, *resolve_sym, *resolve_fix;
-	struct property *rprop;
+	struct device_node *child, *local_fixups, *refnode;
+	struct device_node *tree_symbols, *overlay_symbols, *overlay_fixups;
+	struct property *prop;
 	const char *refpath;
 	phandle phandle, phandle_delta;
 	int err;
 
-	if (!resolve)
-		pr_err("%s: null node\n", __func__);
-	if (resolve && !of_node_check_flag(resolve, OF_DETACHED))
+	if (!overlay)
+		pr_err("%s: null overlay\n", __func__);
+	if (overlay && !of_node_check_flag(overlay, OF_DETACHED))
 		pr_err("%s: node %s not detached\n", __func__,
-			 resolve->full_name);
-	if (!resolve || !of_node_check_flag(resolve, OF_DETACHED))
+			 overlay->full_name);
+	if (!overlay || !of_node_check_flag(overlay, OF_DETACHED))
 		return -EINVAL;
 
 	phandle_delta = live_tree_max_phandle() + 1;
-	adjust_overlay_phandles(resolve, phandle_delta);
+	adjust_overlay_phandles(overlay, phandle_delta);
 
-	childroot = NULL;
-	for_each_child_of_node(resolve, childroot)
-		if (!of_node_cmp(childroot->name, "__local_fixups__"))
+	local_fixups = NULL;
+	for_each_child_of_node(overlay, local_fixups)
+		if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
 			break;
 
-	if (childroot != NULL) {
-		err = adjust_local_phandle_references(childroot,
-				resolve, 0);
+	if (local_fixups != NULL) {
+		err = adjust_local_phandle_references(local_fixups,
+				overlay, 0);
 		if (err)
 			return err;
 
-		BUG_ON(adjust_local_phandle_references(childroot,
-				resolve, phandle_delta));
+		BUG_ON(adjust_local_phandle_references(local_fixups,
+				overlay, phandle_delta));
 	}
 
-	root_sym = NULL;
-	resolve_sym = NULL;
-	resolve_fix = NULL;
+	tree_symbols = NULL;
+	overlay_symbols = NULL;
+	overlay_fixups = NULL;
 
-	root_sym = of_find_node_by_path("/__symbols__");
+	tree_symbols = of_find_node_by_path("/__symbols__");
 
-	for_each_child_of_node(resolve, child) {
+	for_each_child_of_node(overlay, child) {
 
-		if (!resolve_sym && !of_node_cmp(child->name, "__symbols__"))
-			resolve_sym = child;
+		if (!overlay_symbols && !of_node_cmp(child->name, "__symbols__"))
+			overlay_symbols = child;
 
-		if (!resolve_fix && !of_node_cmp(child->name, "__fixups__"))
-			resolve_fix = child;
+		if (!overlay_fixups && !of_node_cmp(child->name, "__fixups__"))
+			overlay_fixups = child;
 
-		if (resolve_sym && resolve_fix)
+		if (overlay_symbols && overlay_fixups)
 			break;
 	}
 
-	if (!resolve_fix) {
+	if (!overlay_fixups) {
 		err = 0;
 		goto out;
 	}
 
-	if (!root_sym) {
+	if (!tree_symbols) {
 		pr_err("%s: no symbols in root of device tree.\n", __func__);
 		err = -EINVAL;
 		goto out;
 	}
 
-	for_each_property_of_node(resolve_fix, rprop) {
+	for_each_property_of_node(overlay_fixups, prop) {
 
 		/* skip properties added automatically */
-		if (!of_prop_cmp(rprop->name, "name"))
+		if (!of_prop_cmp(prop->name, "name"))
 			continue;
 
-		err = of_property_read_string(root_sym,
-				rprop->name, &refpath);
+		err = of_property_read_string(tree_symbols,
+				prop->name, &refpath);
 		if (err)
 			goto out;
 
@@ -344,13 +342,13 @@ int of_resolve_phandles(struct device_node *resolve)
 		phandle = refnode->phandle;
 		of_node_put(refnode);
 
-		err = update_usages_of_a_phandle_reference(resolve, rprop, phandle);
+		err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
 		if (err)
 			break;
 	}
 
 out:
-	of_node_put(root_sym);
+	of_node_put(tree_symbols);
 
 	return err;
 }
-- 
1.9.1

[PATCH 07/12] of: Update structure of code to be clearer, also remove BUG_ON()

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove BUG_ON(), which is frowned upon and not needed here.
Restructure to remove some excessive complexity.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 48 +++++++++++++++++-------------------------------
 1 file changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 0778747cdd58..708daca1d522 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -136,8 +136,8 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
 			err = -EINVAL;
 			goto err_fail;
 		}
-
 		*s++ = '\0';
+
 		err = kstrtoint(s, 10, &offset);
 		if (err)
 			goto err_fail;
@@ -219,11 +219,9 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 			if (off >= prop->length || (off + 4) > prop->length)
 				return -EINVAL;
 
-			if (phandle_delta) {
-				phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
-				phandle += phandle_delta;
-				*(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
-			}
+			phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
+			phandle += phandle_delta;
+			*(__be32 *)(prop->value + off) = cpu_to_be32(phandle);
 		}
 	}
 
@@ -267,48 +265,36 @@ int of_resolve_phandles(struct device_node *overlay)
 	phandle phandle, phandle_delta;
 	int err;
 
-	if (!overlay)
-		pr_err("%s: null overlay\n", __func__);
-	if (overlay && !of_node_check_flag(overlay, OF_DETACHED))
-		pr_err("%s: node %s not detached\n", __func__,
-			 overlay->full_name);
-	if (!overlay || !of_node_check_flag(overlay, OF_DETACHED))
+	if (!overlay) {
+		pr_err("null overlay\n");
+		return -EINVAL;
+	}
+	if (!of_node_check_flag(overlay, OF_DETACHED)) {
+		pr_err("overlay not detached\n");
 		return -EINVAL;
+	}
 
 	phandle_delta = live_tree_max_phandle() + 1;
 	adjust_overlay_phandles(overlay, phandle_delta);
 
-	local_fixups = NULL;
 	for_each_child_of_node(overlay, local_fixups)
 		if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
 			break;
 
-	if (local_fixups != NULL) {
-		err = adjust_local_phandle_references(local_fixups,
-				overlay, 0);
-		if (err)
-			return err;
+	err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
+	if (err)
+		return err;
 
-		BUG_ON(adjust_local_phandle_references(local_fixups,
-				overlay, phandle_delta));
-	}
-
-	tree_symbols = NULL;
 	overlay_symbols = NULL;
 	overlay_fixups = NULL;
 
 	tree_symbols = of_find_node_by_path("/__symbols__");
 
 	for_each_child_of_node(overlay, child) {
-
-		if (!overlay_symbols && !of_node_cmp(child->name, "__symbols__"))
+		if (!of_node_cmp(child->name, "__symbols__"))
 			overlay_symbols = child;
-
-		if (!overlay_fixups && !of_node_cmp(child->name, "__fixups__"))
+		if (!of_node_cmp(child->name, "__fixups__"))
 			overlay_fixups = child;
-
-		if (overlay_symbols && overlay_fixups)
-			break;
 	}
 
 	if (!overlay_fixups) {
@@ -317,7 +303,7 @@ int of_resolve_phandles(struct device_node *overlay)
 	}
 
 	if (!tree_symbols) {
-		pr_err("%s: no symbols in root of device tree.\n", __func__);
+		pr_err("no symbols in root of device tree.\n");
 		err = -EINVAL;
 		goto out;
 	}
-- 
1.9.1

[PATCH 08/12] of: Remove redundant size check

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove a redundant check of buffer size.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 708daca1d522..76c09cb57eae 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -216,7 +216,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 
 		for (i = 0; i < count; i++) {
 			off = be32_to_cpu(((__be32 *)prop_fix->value)[i]);
-			if (off >= prop->length || (off + 4) > prop->length)
+			if ((off + 4) > prop->length)
 				return -EINVAL;
 
 			phandle = be32_to_cpu(*(__be32 *)(prop->value + off));
-- 
1.9.1

[PATCH 09/12] of: Update comments to reflect changes and increase clarity

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Update comments to better explain what functions are doing.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 66 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 76c09cb57eae..f842dbd1585c 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -50,9 +50,6 @@ static struct device_node *find_node_by_full_name(struct device_node *node,
 	return NULL;
 }
 
-/*
- * Find live tree's maximum phandle value.
- */
 static phandle live_tree_max_phandle(void)
 {
 	struct device_node *node;
@@ -71,9 +68,6 @@ static phandle live_tree_max_phandle(void)
 	return phandle;
 }
 
-/*
- * Adjust a subtree's phandle values by a given delta.
- */
 static void adjust_overlay_phandles(struct device_node *overlay,
 		int phandle_delta)
 {
@@ -81,9 +75,11 @@ static void adjust_overlay_phandles(struct device_node *overlay,
 	struct property *prop;
 	phandle phandle;
 
+	/* adjust node's phandle in node */
 	if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
 		overlay->phandle += phandle_delta;
 
+	/* copy adjusted phandle into *phandle properties */
 	for_each_property_of_node(overlay, prop) {
 
 		if (of_prop_cmp(prop->name, "phandle") &&
@@ -118,6 +114,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
 		return -ENOMEM;
 	memcpy(value, prop_fixup->value, prop_fixup->length);
 
+	/* prop_fixup contains a list of tuples of path:property_name:offset */
 	end = value + prop_fixup->length;
 	for (cur = value; cur < end; cur += len + 1) {
 		len = strlen(cur);
@@ -177,10 +174,14 @@ static int node_name_cmp(const struct device_node *dn1,
 
 /*
  * Adjust the local phandle references by the given phandle delta.
- * Assumes the existances of a __local_fixups__ node at the root.
- * Assumes that __of_verify_tree_phandle_references has been called.
- * Does not take any devtree locks so make sure you call this on a tree
- * which is at the detached state.
+ *
+ * Subtree @local_fixups, which is overlay node __local_fixups__,
+ * mirrors the fragment node structure at the root of the overlay.
+ *
+ * For each property in the fragments that contains a phandle reference,
+ * @local_fixups has a property of the same name that contains a list
+ * of offsets of the phandle reference(s) within the respective property
+ * value(s).  The values at these offsets will be fixed up.
  */
 static int adjust_local_phandle_references(struct device_node *local_fixups,
 		struct device_node *overlay, int phandle_delta)
@@ -225,6 +226,13 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 		}
 	}
 
+	/*
+	 * These nested loops recurse down two subtrees in parallel, where the
+	 * node names in the two subtrees match.
+	 *
+	 * The roots of the subtrees are the overlay's __local_fixups__ node
+	 * and the overlay's root node.
+	 */
 	for_each_child_of_node(local_fixups, child) {
 
 		for_each_child_of_node(overlay, overlay_child)
@@ -244,17 +252,37 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 }
 
 /**
- * of_resolve	- Resolve the given node against the live tree.
+ * of_resolve_phandles - Relocate and resolve overlay against live tree
+ *
+ * @overlay:	Pointer to devicetree overlay to relocate and resolve
+ *
+ * Modify (relocate) values of local phandles in @overlay to a range that
+ * does not conflict with the live expanded devicetree.  Update references
+ * to the local phandles in @overlay.  Update (resolve) phandle references
+ * in @overlay that refer to the live expanded devicetree.
+ *
+ * Phandle values in the live tree are in the range of
+ * 1 .. live_tree_max_phandle().  The range of phandle values in the overlay
+ * also begin with at 1.  Adjust the phandle values in the overlay to begin
+ * at live_tree_max_phandle() + 1.  Update references to the phandles to
+ * the adjusted phandle values.
+ *
+ * The name of each property in the "__fixups__" node in the overlay matches
+ * the name of a symbol (a label) in the live tree.  The values of each
+ * property in the "__fixups__" node is a list of the property values in the
+ * overlay that need to be updated to contain the phandle reference
+ * corresponding to that symbol in the live tree.  Update the references in
+ * the overlay with the phandle values in the live tree.
+ *
+ * @overlay must be detached.
  *
- * @resolve:	Node to resolve
+ * Resolving and applying @overlay to the live expanded devicetree must be
+ * protected by a mechanism to ensure that multiple overlays are processed
+ * in a single threaded manner so that multiple overlays will not relocate
+ * phandles to overlapping ranges.  The mechanism to enforce this is not
+ * yet implemented.
  *
- * Perform dynamic Device Tree resolution against the live tree
- * to the given node to resolve. This depends on the live tree
- * having a __symbols__ node, and the resolve node the __fixups__ &
- * __local_fixups__ nodes (if needed).
- * The result of the operation is a resolve node that it's contents
- * are fit to be inserted or operate upon the live tree.
- * Returns 0 on success or a negative error value on error.
+ * Return: %0 on success or a negative error value on error.
  */
 int of_resolve_phandles(struct device_node *overlay)
 {
-- 
1.9.1

[PATCH 10/12] of: Add back an error message, restructured

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Add a single pr_err() to cover a range of errors that were reported
by several pr_err() that were removed earlier in this series.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index f842dbd1585c..eb78010c21a3 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -293,13 +293,17 @@ int of_resolve_phandles(struct device_node *overlay)
 	phandle phandle, phandle_delta;
 	int err;
 
+	tree_symbols = NULL;
+
 	if (!overlay) {
 		pr_err("null overlay\n");
-		return -EINVAL;
+		err = -EINVAL;
+		goto err_out;
 	}
 	if (!of_node_check_flag(overlay, OF_DETACHED)) {
 		pr_err("overlay not detached\n");
-		return -EINVAL;
+		err = -EINVAL;
+		goto err_out;
 	}
 
 	phandle_delta = live_tree_max_phandle() + 1;
@@ -311,7 +315,7 @@ int of_resolve_phandles(struct device_node *overlay)
 
 	err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
 	if (err)
-		return err;
+		goto err_out;
 
 	overlay_symbols = NULL;
 	overlay_fixups = NULL;
@@ -333,7 +337,7 @@ int of_resolve_phandles(struct device_node *overlay)
 	if (!tree_symbols) {
 		pr_err("no symbols in root of device tree.\n");
 		err = -EINVAL;
-		goto out;
+		goto err_out;
 	}
 
 	for_each_property_of_node(overlay_fixups, prop) {
@@ -345,12 +349,12 @@ int of_resolve_phandles(struct device_node *overlay)
 		err = of_property_read_string(tree_symbols,
 				prop->name, &refpath);
 		if (err)
-			goto out;
+			goto err_out;
 
 		refnode = of_find_node_by_path(refpath);
 		if (!refnode) {
 			err = -ENOENT;
-			goto out;
+			goto err_out;
 		}
 
 		phandle = refnode->phandle;
@@ -361,6 +365,8 @@ int of_resolve_phandles(struct device_node *overlay)
 			break;
 	}
 
+err_out:
+	pr_err("overlay phandle fixup failed: %d\n", err);
 out:
 	of_node_put(tree_symbols);
 
-- 
1.9.1

[PATCH 11/12] of: Move setting of pointer to beside test for non-null

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Value of pointer was calculated in an earlier block than
where it was used.  Move it down into the block where it
is used, immediately before where is is checked to be valid.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index eb78010c21a3..53353cc8f2bb 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -320,8 +320,6 @@ int of_resolve_phandles(struct device_node *overlay)
 	overlay_symbols = NULL;
 	overlay_fixups = NULL;
 
-	tree_symbols = of_find_node_by_path("/__symbols__");
-
 	for_each_child_of_node(overlay, child) {
 		if (!of_node_cmp(child->name, "__symbols__"))
 			overlay_symbols = child;
@@ -334,6 +332,7 @@ int of_resolve_phandles(struct device_node *overlay)
 		goto out;
 	}
 
+	tree_symbols = of_find_node_by_path("/__symbols__");
 	if (!tree_symbols) {
 		pr_err("no symbols in root of device tree.\n");
 		err = -EINVAL;
-- 
1.9.1

[PATCH 12/12] of: Remove unused variable overlay_symbols

[frowand.list]

From: Frank Rowand <frank.rowand@am.sony.com>

Remove unused pointer to node "__symbols__".

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
 drivers/of/resolver.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 53353cc8f2bb..783bd09463b5 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -287,7 +287,7 @@ static int adjust_local_phandle_references(struct device_node *local_fixups,
 int of_resolve_phandles(struct device_node *overlay)
 {
 	struct device_node *child, *local_fixups, *refnode;
-	struct device_node *tree_symbols, *overlay_symbols, *overlay_fixups;
+	struct device_node *tree_symbols, *overlay_fixups;
 	struct property *prop;
 	const char *refpath;
 	phandle phandle, phandle_delta;
@@ -317,12 +317,9 @@ int of_resolve_phandles(struct device_node *overlay)
 	if (err)
 		goto err_out;
 
-	overlay_symbols = NULL;
 	overlay_fixups = NULL;
 
 	for_each_child_of_node(overlay, child) {
-		if (!of_node_cmp(child->name, "__symbols__"))
-			overlay_symbols = child;
 		if (!of_node_cmp(child->name, "__fixups__"))
 			overlay_fixups = child;
 	}
-- 
1.9.1

Re: [PATCH 00/12] of: Make drivers/of/resolver.c more readable

[Rob Herring]

On Fri, Oct 28, 2016 at 11:26:20PM -0700, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> drivers/of/resolve.c is a bit difficult to read.  Clean it up so
> that review of future overlay related patches will be easier.
> 
> Most of the patches are intended to be reformatting, with no functional
> change.  Patches that are expected to have a functional change are:
> 
>   Remove excessive printks to reduce clutter.
>   Update structure of code to be clearer, also remove BUG_ON()
>     Any functional change would reflect undefined behavior on bad overlay.
>     Some error message text modified.
>     BUG_ON() removed.
>   Add back an error message, restructured
> 
> The patches are grouped into sets of changes that are intended
> to be easy to verify correctness through simple inspection.
> 
> Some of the individual patches have checkpatch warnings or errors.
> But after all patches are applied, the number of errors and
> warnings from running checkpatch against the entire file are
> reduced to two line size warnings.
> 
> These patches are only tested via the unit tests. I do not have
> expansion boards to test with real hardware.
> 
> changes from rfc to v1:
>   - Remove fewer one line comments
>   - Add more extensive header comment to of_resolve_phandles()
>     to explain the how and why of resolving phandles
>   - Update patch header comments
>   - Incorporated patch "Remove braces around single line blocks"
>     into the previous patch in the series
> 
> 
> Frank Rowand (12):
>   of: Remove comments that state the obvious, to reduce clutter
>   of: Remove excessive printks to reduce clutter.
>   of: Convert comparisons to zero or NULL to logical expressions
>   of: Rename functions to more accurately reflect what they do
>   of: Remove prefix "__of_" from local function names
>   of: Rename variables to better reflect purpose or follow convention
>   of: Update structure of code to be clearer, also remove BUG_ON()
>   of: Remove redundant size check
>   of: Update comments to reflect changes and increase clarity
>   of: Add back an error message, restructured
>   of: Move setting of pointer to beside test for non-null
>   of: Remove unused variable overlay_symbols

Series applied.

Rob