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

Started byfrowand.list@gmail.com
First post2016-10-25 23:10 +0200
Last post2016-10-27 18:10 +0200
Articles 8 on this page of 28 — 5 participants

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
      Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2016-10-27 16:00 +0200
      Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable Rob Herring <robh+dt@kernel.org> - 2016-10-27 16:40 +0200
        Re: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:40 +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
      Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 16:20 +0200
        Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:40 +0200
          Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 19:00 +0200
            Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Frank Rowand <frowand.list@gmail.com> - 2016-10-27 20:30 +0200
              Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 22:30 +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
      Re: [RFC PATCH 01/13] of: Remove comments that state the obvious Rob Herring <robh+dt@kernel.org> - 2016-10-27 16:50 +0200
        Re: [RFC PATCH 01/13] of: Remove comments that state the obvious Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:10 +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
      Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter Pantelis Antoniou <pantelis.antoniou@konsulko.com> - 2016-10-27 16:00 +0200
        Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:20 +0200
      Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter Rob Herring <robh+dt@kernel.org> - 2016-10-27 17:00 +0200
        Re: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:10 +0200

Page 2 of 2 — ← Prev page 1 [2]


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

Fromfrowand.list@gmail.com
Date2016-10-25 23:10 +0200
Subject[RFC PATCH 04/13] of: Convert comparisons to zero or NULL to simplify logical expressions
Message-ID<swh8S-2vP-59@gated-at.bofh.it>
In reply to#1508630
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

Fromfrowand.list@gmail.com
Date2016-10-25 23:10 +0200
Subject[RFC PATCH 12/13] of: Move setting of pointer to beside test for non-null
Message-ID<swh8S-2vP-51@gated-at.bofh.it>
In reply to#1508630
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

Fromfrowand.list@gmail.com
Date2016-10-25 23:10 +0200
Subject[RFC PATCH 10/13] of: Update comments to reflect changes and increase clarity
Message-ID<swh8S-2vP-63@gated-at.bofh.it>
In reply to#1508630
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

Fromfrowand.list@gmail.com
Date2016-10-25 23:10 +0200
Subject[RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
Message-ID<swh8T-2vP-65@gated-at.bofh.it>
In reply to#1508630
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] | [next] | [standalone]


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

FromPantelis Antoniou <pantelis.antoniou@konsulko.com>
Date2016-10-27 16:00 +0200
SubjectRe: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
Message-ID<swTnR-2Bn-85@gated-at.bofh.it>
In reply to#1508648
Hi Rob, Frank,

> On Oct 27, 2016, at 15:21 , Rob Herring <robh+dt@kernel.org> wrote:
> 
> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Maybe some should be debug?
> 

Yes, please do not get rid of them completely.
Leave them at least as debug level so that if there’s a problem
there’s a way to figure out why something happened.

>> 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;
>>        }
> 
> I would remove the brackets in this patch rather than separately.
> 
>>        memcpy(propval, rprop->value, rprop->length);


Regards

— Pantelis

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


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

FromFrank Rowand <frowand.list@gmail.com>
Date2016-10-27 18:20 +0200
SubjectRe: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
Message-ID<swVzj-4eL-11@gated-at.bofh.it>
In reply to#1510178
On 10/27/16 06:51, Pantelis Antoniou wrote:
> Hi Rob, Frank,
> 
>> On Oct 27, 2016, at 15:21 , Rob Herring <robh+dt@kernel.org> wrote:
>>
>> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> Maybe some should be debug?
>>
> 
> Yes, please do not get rid of them completely.
> Leave them at least as debug level so that if there’s a problem
> there’s a way to figure out why something happened.

After patch "Add back an error message, restructured" is applied,
a lot of the messages return, but hopefully keeping readability.
Note that the one message added back covers a number of error
locations.

Are there any additional key messages that you think I missed in
the add back an error message patch?

Keep in mind that many of the debug messages address malformed
dtb, which would be a bug in dtc.  It made sense for these to
exist while dtc was being modified, but now that you have
created and tested the dtc changes, I think most of those
debug messages no longer make sense for mainline code.


>>> 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;
>>>        }
>>
>> I would remove the brackets in this patch rather than separately.
>>
>>>        memcpy(propval, rprop->value, rprop->length);
> 
> 
> Regards
> 
> — Pantelis
> 
> 

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


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

FromRob Herring <robh+dt@kernel.org>
Date2016-10-27 17:00 +0200
SubjectRe: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
Message-ID<swTnR-2Bn-87@gated-at.bofh.it>
In reply to#1508648
On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>

Maybe some should be debug?

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

I would remove the brackets in this patch rather than separately.

>         memcpy(propval, rprop->value, rprop->length);

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


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

FromFrank Rowand <frowand.list@gmail.com>
Date2016-10-27 18:10 +0200
SubjectRe: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
Message-ID<swVpE-49R-31@gated-at.bofh.it>
In reply to#1510343
On 10/27/16 05:21, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> Maybe some should be debug?
> 
>> 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;
>>         }
> 
> I would remove the brackets in this patch rather than separately.
> 
>>         memcpy(propval, rprop->value, rprop->length);
> .
> 

OK, I will collapse the "remove braces" patch into this patch.

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web