[PATCH 0/6] of: remove kbasename() from core

[PATCH 0/6] of: remove kbasename() from core

[frowand.list]

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

Struct device_node full_name no longer includes the full path
name.

Fix some broken overlay code that was not updated to reflect this.

Clean up the unittest changeset test that calls into this
overlay code.

kbasename(->full_name) is no longer needed, so remove kbasename()
from core devicetree code.

Removal of kbasename() is separated out into one patch per file
as much as possible to make bisection easier if a problem occurs.

Frank Rowand (6):
  of: unittest: clean up changeset test
  of: remove kbasename(of->full_name) from overlay related code
  of: remove kbasename(of->full_name) from base.c
  of: remove kbasename(of->full_name) from kobj.c
  of: remove kbasename(of->full_name) from of_reserved_mem.c
  of: remove kbasename(of->full_name) from platform.c

 drivers/of/base.c            |  4 ++--
 drivers/of/dynamic.c         | 21 +++++++++----------
 drivers/of/kobj.c            |  2 +-
 drivers/of/of_private.h      |  3 ++-
 drivers/of/of_reserved_mem.c |  4 +---
 drivers/of/overlay.c         |  8 ++------
 drivers/of/platform.c        |  2 +-
 drivers/of/resolver.c        |  5 +----
 drivers/of/unittest.c        | 48 +++++++++++++++++++++++++++++++++++---------
 9 files changed, 59 insertions(+), 38 deletions(-)

-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 1/6] of: unittest: clean up changeset test

[frowand.list]

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

In preparation for fixing __of_node_dup(), clean up the unittest
function that calls it.

Devicetree nodes created from a flattened device tree have a name
property.  Follow this convention for nodes added by a changeset.

For node added by changeset, remove incorrect initialization of
child node pointer.

Add an additional node pointer 'changeset' to more naturally reflect
where in the tree the changeset is added.

Make changeset add property error messages unique.

Add whitespace to break apart logic blocks.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---

checkpatch warnings for "line over 80 characters" are for unittest
code in a function with long lines as a normal style.


 drivers/of/unittest.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 7a9abaae874d..490bbee0cf87 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -562,42 +562,72 @@ static void __init of_unittest_property_copy(void)
 static void __init of_unittest_changeset(void)
 {
 #ifdef CONFIG_OF_DYNAMIC
-	struct property *ppadd, padd = { .name = "prop-add", .length = 0, .value = "" };
+	struct property *ppadd, padd = { .name = "prop-add", .length = 1, .value = "" };
+	struct property *ppname_n1,  pname_n1  = { .name = "name", .length = 3, .value = "n1"  };
+	struct property *ppname_n2,  pname_n2  = { .name = "name", .length = 3, .value = "n2"  };
+	struct property *ppname_n21, pname_n21 = { .name = "name", .length = 3, .value = "n21" };
 	struct property *ppupdate, pupdate = { .name = "prop-update", .length = 5, .value = "abcd" };
 	struct property *ppremove;
-	struct device_node *n1, *n2, *n21, *nremove, *parent, *np;
+	struct device_node *n1, *n2, *n21, *nchangeset, *nremove, *parent, *np;
 	struct of_changeset chgset;
 
 	n1 = __of_node_dup(NULL, "/testcase-data/changeset/n1");
 	unittest(n1, "testcase setup failure\n");
+
 	n2 = __of_node_dup(NULL, "/testcase-data/changeset/n2");
 	unittest(n2, "testcase setup failure\n");
+
 	n21 = __of_node_dup(NULL, "%s/%s", "/testcase-data/changeset/n2", "n21");
 	unittest(n21, "testcase setup failure %p\n", n21);
-	nremove = of_find_node_by_path("/testcase-data/changeset/node-remove");
+
+	nchangeset = of_find_node_by_path("/testcase-data/changeset");
+	nremove = of_get_child_by_name(nchangeset, "node-remove");
 	unittest(nremove, "testcase setup failure\n");
+
 	ppadd = __of_prop_dup(&padd, GFP_KERNEL);
 	unittest(ppadd, "testcase setup failure\n");
+
+	ppname_n1  = __of_prop_dup(&pname_n1, GFP_KERNEL);
+	unittest(ppname_n1, "testcase setup failure\n");
+
+	ppname_n2  = __of_prop_dup(&pname_n2, GFP_KERNEL);
+	unittest(ppname_n2, "testcase setup failure\n");
+
+	ppname_n21 = __of_prop_dup(&pname_n21, GFP_KERNEL);
+	unittest(ppname_n21, "testcase setup failure\n");
+
 	ppupdate = __of_prop_dup(&pupdate, GFP_KERNEL);
 	unittest(ppupdate, "testcase setup failure\n");
-	parent = nremove->parent;
+
+	parent = nchangeset;
 	n1->parent = parent;
 	n2->parent = parent;
 	n21->parent = n2;
-	n2->child = n21;
+
 	ppremove = of_find_property(parent, "prop-remove", NULL);
 	unittest(ppremove, "failed to find removal prop");
 
 	of_changeset_init(&chgset);
+
 	unittest(!of_changeset_attach_node(&chgset, n1), "fail attach n1\n");
+	unittest(!of_changeset_add_property(&chgset, n1, ppname_n1), "fail add prop name\n");
+
 	unittest(!of_changeset_attach_node(&chgset, n2), "fail attach n2\n");
+	unittest(!of_changeset_add_property(&chgset, n2, ppname_n2), "fail add prop name\n");
+
 	unittest(!of_changeset_detach_node(&chgset, nremove), "fail remove node\n");
+	unittest(!of_changeset_add_property(&chgset, n21, ppname_n21), "fail add prop name\n");
+
 	unittest(!of_changeset_attach_node(&chgset, n21), "fail attach n21\n");
-	unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop\n");
+
+	unittest(!of_changeset_add_property(&chgset, parent, ppadd), "fail add prop prop-add\n");
 	unittest(!of_changeset_update_property(&chgset, parent, ppupdate), "fail update prop\n");
 	unittest(!of_changeset_remove_property(&chgset, parent, ppremove), "fail remove prop\n");
+
 	unittest(!of_changeset_apply(&chgset), "apply failed\n");
 
+	of_node_put(nchangeset);
+
 	/* Make sure node names are constructed correctly */
 	unittest((np = of_find_node_by_path("/testcase-data/changeset/n2/n21")),
 		 "'%pOF' not added\n", n21);
-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 2/6] of: remove kbasename(of->full_name) from overlay related code

[frowand.list]

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

Struct device_node full_name no longer includes the full path
name.  The overlay node creation code was not modified to
reflect this change.  Fix the node names generate by overlay
code to contain only the basename.

Unittests call an overlay internal function to create new nodes.
Fix up these calls to provide basename only instead of the full
path.

Fixes: a7e4cfb0a7ca ("of/fdt: only store the device node basename
in full_name")

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 drivers/of/dynamic.c    | 21 ++++++++++-----------
 drivers/of/of_private.h |  3 ++-
 drivers/of/overlay.c    |  8 ++------
 drivers/of/resolver.c   |  5 +----
 drivers/of/unittest.c   |  6 +++---
 5 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 7bb33d22b4e2..f4f8ed9b5454 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -383,25 +383,24 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
 
 /**
  * __of_node_dup() - Duplicate or create an empty device node dynamically.
- * @fmt: Format string (plus vargs) for new full name of the device node
+ * @np:		if not NULL, contains properties to be duplicated in new node
+ * @full_name:	string value to be duplicated into new node's full_name field
  *
- * Create an device tree node, either by duplicating an empty node or by allocating
- * an empty one suitable for further modification.  The node data are
- * dynamically allocated and all the node flags have the OF_DYNAMIC &
- * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of
- * memory error.
+ * Create a device tree node, optionally duplicating the properties of
+ * another node.  The node data are dynamically allocated and all the node
+ * flags have the OF_DYNAMIC & OF_DETACHED bits set.
+ *
+ * Returns the newly allocated node or NULL on out of memory error.
  */
-struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...)
+struct device_node *__of_node_dup(const struct device_node *np,
+				  const char *full_name)
 {
-	va_list vargs;
 	struct device_node *node;
 
 	node = kzalloc(sizeof(*node), GFP_KERNEL);
 	if (!node)
 		return NULL;
-	va_start(vargs, fmt);
-	node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs);
-	va_end(vargs);
+	node->full_name = kstrdup(full_name, GFP_KERNEL);
 	if (!node->full_name) {
 		kfree(node);
 		return NULL;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0c609e7d0334..26bb31beb03e 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -104,7 +104,8 @@ extern void *__unflatten_device_tree(const void *blob,
  * own the devtree lock or work on detached trees only.
  */
 struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags);
-__printf(2, 3) struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...);
+struct device_node *__of_node_dup(const struct device_node *np,
+				  const char *full_name);
 
 struct device_node *__of_find_node_by_path(struct device_node *parent,
 						const char *path);
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 3397d7642958..b9df55e0a656 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -323,19 +323,15 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
 static int add_changeset_node(struct overlay_changeset *ovcs,
 		struct device_node *target_node, struct device_node *node)
 {
-	const char *node_kbasename;
 	struct device_node *tchild;
 	int ret = 0;
 
-	node_kbasename = kbasename(node->full_name);
-
 	for_each_child_of_node(target_node, tchild)
-		if (!of_node_cmp(node_kbasename, kbasename(tchild->full_name)))
+		if (!of_node_cmp(node->full_name, tchild->full_name))
 			break;
 
 	if (!tchild) {
-		tchild = __of_node_dup(node, "%pOF/%s",
-				       target_node, node_kbasename);
+		tchild = __of_node_dup(node, node->full_name);
 		if (!tchild)
 			return -ENOMEM;
 
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 740d19bde601..0e0969f58216 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -137,10 +137,7 @@ static int update_usages_of_a_phandle_reference(struct device_node *overlay,
 static int node_name_cmp(const struct device_node *dn1,
 		const struct device_node *dn2)
 {
-	const char *n1 = kbasename(dn1->full_name);
-	const char *n2 = kbasename(dn2->full_name);
-
-	return of_node_cmp(n1, n2);
+	return of_node_cmp(dn1->full_name, dn2->full_name);
 }
 
 /*
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 490bbee0cf87..acf233c34ef7 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -571,13 +571,13 @@ static void __init of_unittest_changeset(void)
 	struct device_node *n1, *n2, *n21, *nchangeset, *nremove, *parent, *np;
 	struct of_changeset chgset;
 
-	n1 = __of_node_dup(NULL, "/testcase-data/changeset/n1");
+	n1 = __of_node_dup(NULL, "n1");
 	unittest(n1, "testcase setup failure\n");
 
-	n2 = __of_node_dup(NULL, "/testcase-data/changeset/n2");
+	n2 = __of_node_dup(NULL, "n2");
 	unittest(n2, "testcase setup failure\n");
 
-	n21 = __of_node_dup(NULL, "%s/%s", "/testcase-data/changeset/n2", "n21");
+	n21 = __of_node_dup(NULL, "n21");
 	unittest(n21, "testcase setup failure %p\n", n21);
 
 	nchangeset = of_find_node_by_path("/testcase-data/changeset");
-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 3/6] of: remove kbasename(of->full_name) from base.c

[frowand.list]

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

struct device_node full_name has been changed to include the
basename instead of the full path.  kbasename() is no longer
needed to extract the basename from full_name.

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

diff --git a/drivers/of/base.c b/drivers/of/base.c
index ad28de96e13f..926d08b8674d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -691,8 +691,8 @@ struct device_node *__of_find_node_by_path(struct device_node *parent,
 		return NULL;
 
 	__for_each_child_of_node(parent, child) {
-		const char *name = kbasename(child->full_name);
-		if (strncmp(path, name, len) == 0 && (strlen(name) == len))
+		if (strncmp(path, child->full_name, len) == 0 &&
+		    (strlen(child->full_name) == len))
 			return child;
 	}
 	return NULL;
-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 4/6] of: remove kbasename(of->full_name) from kobj.c

[frowand.list]

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

struct device_node full_name has been changed to include the
basename instead of the full path.  kbasename() is no longer
needed to extract the basename from full_name.

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

diff --git a/drivers/of/kobj.c b/drivers/of/kobj.c
index 7a0a18980b98..b0aecea35706 100644
--- a/drivers/of/kobj.c
+++ b/drivers/of/kobj.c
@@ -128,7 +128,7 @@ int __of_attach_node_sysfs(struct device_node *np)
 		name = safe_name(&of_kset->kobj, "base");
 		parent = NULL;
 	} else {
-		name = safe_name(&np->parent->kobj, kbasename(np->full_name));
+		name = safe_name(&np->parent->kobj, np->full_name);
 		parent = &np->parent->kobj;
 	}
 	if (!name)
-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 5/6] of: remove kbasename(of->full_name) from of_reserved_mem.c

[frowand.list]

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

struct device_node full_name has been changed to include the
basename instead of the full path.  kbasename() is no longer
needed to extract the basename from full_name.

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

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 9a4f4246231d..e770cab4b95b 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -405,15 +405,13 @@ void of_reserved_mem_device_release(struct device *dev)
  */
 struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
 {
-	const char *name;
 	int i;
 
 	if (!np->full_name)
 		return NULL;
 
-	name = kbasename(np->full_name);
 	for (i = 0; i < reserved_mem_count; i++)
-		if (!strcmp(reserved_mem[i].name, name))
+		if (!strcmp(reserved_mem[i].name, np->full_name))
 			return &reserved_mem[i];
 
 	return NULL;
-- 
Frank Rowand <frank.rowand@sony.com>

[PATCH 6/6] of: remove kbasename(of->full_name) from platform.c

[frowand.list]

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

struct device_node full_name has been changed to include the
basename instead of the full path.  kbasename() is no longer
needed to extract the basename from full_name.

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

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index c00d81dfac0b..17116f5ce6ba 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -94,7 +94,7 @@ static void of_device_make_bus_id(struct device *dev)
 
 		/* format arguments only used if dev_name() resolves to NULL */
 		dev_set_name(dev, dev_name(dev) ? "%s:%s" : "%s",
-			     kbasename(node->full_name), dev_name(dev));
+			     node->full_name, dev_name(dev));
 		node = node->parent;
 	}
 }
-- 
Frank Rowand <frank.rowand@sony.com>

Re: [PATCH 0/6] of: remove kbasename() from core

[Geert Uytterhoeven]

Hi Frank,

On Mon, Feb 26, 2018 at 3:22 AM,  <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@sony.com>
>
> Struct device_node full_name no longer includes the full path
> name.

Is this now also true on systems with real Open Firmware?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/6] of: remove kbasename() from core

[Frank Rowand]

On 02/26/18 01:03, Geert Uytterhoeven wrote:
> Hi Frank,
> 
> On Mon, Feb 26, 2018 at 3:22 AM,  <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> Struct device_node full_name no longer includes the full path
>> name.
> 
> Is this now also true on systems with real Open Firmware?

Thanks, good point.  I can not remove the kbasename() calls.

Will re-spin, as a much smaller patch to deal with the
overlay issues.

-Frank