Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1508630 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 05/13] of: Rename functions to more accurately reflect what they do frowand.list@gmail.com - 2016-10-25 23:10 +0200
    Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable Frank Rowand <frowand.list@gmail.com> - 2016-10-25 23:10 +0200
    [RFC PATCH 09/13] of: Remove redundant size check frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 03/13] of: Remove braces around single line blocks. frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 08/13] of: Update structure of code, remove BUG_ON() frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 07/13] of: Rename variables to better reflect purpose or follow convention frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 01/13] of: Remove comments that state the obvious frowand.list@gmail.com - 2016-10-25 23:10 +0200
      Re: [RFC PATCH 01/13] of: Remove comments that state the obvious Joe Perches <joe@perches.com> - 2016-10-25 23:40 +0200
    [RFC PATCH 04/13] of: Convert comparisons to zero or NULL to simplify logical expressions frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 12/13] of: Move setting of pointer to beside test for non-null frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 10/13] of: Update comments to reflect changes and increase clarity frowand.list@gmail.com - 2016-10-25 23:10 +0200
    [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter frowand.list@gmail.com - 2016-10-25 23:10 +0200

#1508630 — [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable

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 comments that state the obvious, to reduce clutter
  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.


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

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

-- 
1.9.1

[toc] | [next] | [standalone]

#1508631 — [RFC PATCH 05/13] of: Rename functions to more accurately reflect what they do

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

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

[toc] | [prev] | [next] | [standalone]

#1508633

On 10/25/16 13:58, 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.

< snip >

Hi Pantelis,

Can you test this patch series on some real hardware?

Thanks,

Frank

[toc] | [prev] | [next] | [standalone]

#1508634 — [RFC PATCH 09/13] of: Remove redundant size check

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

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

[toc] | [prev] | [next] | [standalone]

#1508635 — [RFC PATCH 03/13] of: Remove braces around single line blocks.

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

The single line blocks were created by previous patches in the series.

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

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 93a7ca0bf98c..c61ba99a1792 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -115,9 +115,8 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 	int err = 0;
 
 	propval = kmalloc(rprop->length, GFP_KERNEL);
-	if (!propval) {
+	if (!propval)
 		return -ENOMEM;
-	}
 	memcpy(propval, rprop->value, rprop->length);
 
 	propend = propval + rprop->length;
@@ -141,14 +140,12 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 
 		*s++ = '\0';
 		err = kstrtoint(s, 10, &offset);
-		if (err != 0) {
+		if (err != 0)
 			goto err_fail;
-		}
 
 		refnode = __of_find_node_by_full_name(node, nodestr);
-		if (!refnode) {
+		if (!refnode)
 			continue;
-		}
 
 		for_each_property_of_node(refnode, sprop) {
 			if (of_prop_cmp(sprop->name, propstr) == 0)
@@ -207,9 +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) {
+		if ((rprop->length % 4) != 0 || rprop->length == 0)
 			return -EINVAL;
-		}
 		count = rprop->length / sizeof(__be32);
 
 		for_each_property_of_node(target, sprop) {
@@ -217,16 +213,13 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 				break;
 		}
 
-		if (sprop == NULL) {
+		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) {
+			if (off >= sprop->length || (off + 4) > sprop->length)
 				return -EINVAL;
-			}
 
 			if (phandle_delta) {
 				phandle = be32_to_cpu(*(__be32 *)(sprop->value + off));
@@ -242,9 +235,8 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 			if (__of_node_name_cmp(child, childtarget) == 0)
 				break;
 
-		if (!childtarget) {
+		if (!childtarget)
 			return -EINVAL;
-		}
 
 		err = __of_adjust_tree_phandle_references(child, childtarget,
 				phandle_delta);
@@ -342,9 +334,8 @@ int of_resolve_phandles(struct device_node *resolve)
 
 		err = of_property_read_string(root_sym,
 				rprop->name, &refpath);
-		if (err != 0) {
+		if (err != 0)
 			goto out;
-		}
 
 		refnode = of_find_node_by_path(refpath);
 		if (!refnode) {
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1508639 — [RFC PATCH 08/13] of: Update structure of code, remove BUG_ON()

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

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

[toc] | [prev] | [next] | [standalone]

#1508640 — [RFC PATCH 07/13] of: Rename variables to better reflect purpose or follow convention

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

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

[toc] | [prev] | [next] | [standalone]

#1508641 — [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names

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

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

[toc] | [prev] | [next] | [standalone]

#1508643 — [RFC PATCH 01/13] of: Remove comments that state the obvious

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

Remove comments that state the obvious, to reduce clutter

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

[toc] | [prev] | [next] | [standalone]

#1508674 — Re: [RFC PATCH 01/13] of: Remove comments that state the obvious

On Tue, 2016-10-25 at 13:58 -0700, frowand.list@gmail.com wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Remove comments that state the obvious, to reduce clutter

Some of these removals might be overly aggressive.

> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
[]
> @@ -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",ld

This kmalloc/memcpy could use kmemdup instead.

It doesn't really need the pr_err either as kmalloc and/or
kmemdup get a generic OOM message.

[toc] | [prev] | [next] | [standalone]

#1508644 — [RFC PATCH 04/13] of: Convert comparisons to zero or NULL to simplify logical expressions

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

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

[toc] | [prev] | [next] | [standalone]

#1508645 — [RFC PATCH 12/13] of: Move setting of pointer to beside test for non-null

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

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 664c97e1ecb4..3f7cf569c7ea 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -305,8 +305,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;
@@ -319,6 +317,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

[toc] | [prev] | [next] | [standalone]

#1508647 — [RFC PATCH 10/13] of: Update comments to reflect changes and increase clarity

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

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

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 76c09cb57eae..4e6df385118b 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)
 {
@@ -118,6 +112,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 +172,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 +224,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 +250,24 @@ 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.
+ *
+ * @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

[toc] | [prev] | [next] | [standalone]

#1508648 — [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter

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

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

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 4ff0220d7aa2..93a7ca0bf98c 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -116,8 +116,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 
 	propval = kmalloc(rprop->length, GFP_KERNEL);
 	if (!propval) {
-		pr_err("%s: Could not copy value of '%s'\n",
-				__func__, rprop->name);
 		return -ENOMEM;
 	}
 	memcpy(propval, rprop->value, rprop->length);
@@ -129,8 +127,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,8 +135,6 @@ 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;
 		}
@@ -148,15 +142,11 @@ static int __of_adjust_phandle_ref(struct device_node *node,
 		*s++ = '\0';
 		err = kstrtoint(s, 10, &offset);
 		if (err != 0) {
-			pr_err("%s: Could get offset '%s'\n",
-				__func__, (char *)rprop->value);
 			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);
 			continue;
 		}
 
@@ -167,8 +157,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;
 		}
@@ -220,8 +208,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 			continue;
 
 		if ((rprop->length % 4) != 0 || rprop->length == 0) {
-			pr_err("%s: Illegal property (size) '%s' @%s\n",
-					__func__, rprop->name, node->full_name);
 			return -EINVAL;
 		}
 		count = rprop->length / sizeof(__be32);
@@ -232,8 +218,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 		}
 
 		if (sprop == NULL) {
-			pr_err("%s: Could not find target property '%s' @%s\n",
-					__func__, rprop->name, node->full_name);
 			return -EINVAL;
 		}
 
@@ -241,9 +225,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 			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);
 				return -EINVAL;
 			}
 
@@ -262,8 +243,6 @@ static int __of_adjust_tree_phandle_references(struct device_node *node,
 				break;
 
 		if (!childtarget) {
-			pr_err("%s: Could not find target child '%s' @%s\n",
-					__func__, child->name, node->full_name);
 			return -EINVAL;
 		}
 
@@ -364,15 +343,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);
 			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 +355,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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web