[PATCH 0/4] Removing full paths from DT full_name

[PATCH 0/4] Removing full paths from DT full_name

[Rob Herring]

This series is the last steps to remove storing the full path for every 
DT node. Instead, we can create full path strings dynamically as needed 
with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of 
remaining direct users of full_name after this series. I don't believe 
there should be any functional impact for those users with the change to 
only the node name (+unit-address). The majority are for struct 
resource.name. This should only affect /proc/iomem display.

Patches 1 and 2 can be applied now for 4.14. For patches 3 and 4, my 
target is 4.15 after all the dependencies have been merged.

PPC folks, Please test! The PPC parts are untested. A git branch with 
all the dependencies is here[1].

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt-printf

Rob Herring (4):
  powerpc: pseries: vio: match parent nodes with of_find_node_by_path
  powerpc: pseries: remove dlpar_attach_node dependency on full path
  powerpc: pseries: only store the device node basename in full_name
  of/fdt: only store the device node basename in full_name

 arch/powerpc/platforms/pseries/dlpar.c       | 26 +++--------
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  2 +-
 arch/powerpc/platforms/pseries/mobility.c    |  2 +-
 arch/powerpc/platforms/pseries/pseries.h     |  2 +-
 arch/powerpc/platforms/pseries/reconfig.c    |  2 +-
 arch/powerpc/platforms/pseries/vio.c         |  4 +-
 drivers/of/fdt.c                             | 69 +++++-----------------------
 7 files changed, 23 insertions(+), 84 deletions(-)

-- 
2.11.0

[PATCH 1/4] powerpc: pseries: vio: match parent nodes with of_find_node_by_path

[Rob Herring]

In preparation to remove the full path from device_node.full_name, use
of_find_node_by_path instead of open coding with strcmp.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/platforms/pseries/vio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c
index 3201feb6d32b..28a9505e4919 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
 	 */
 	parent_node = of_get_parent(of_node);
 	if (parent_node) {
-		if (!strcmp(parent_node->full_name, "ibm,platform-facilities"))
+		if (parent_node == of_find_node_by_path("/ibm,platform-facilities"))
 			family = PFO;
-		else if (!strcmp(parent_node->full_name, "vdevice"))
+		else if (parent_node == of_find_node_by_path("/vdevice"))
 			family = VDEVICE;
 		else {
 			pr_warn("%s: parent(%pOF) of %s not recognized.\n",
-- 
2.11.0

[PATCH 2/4] powerpc: pseries: remove dlpar_attach_node dependency on full path

[Rob Herring]

In preparation to stop storing the full node path in full_name, remove the
dependency on full_name from dlpar_attach_node(). Callers of
dlpar_attach_node() already have the parent device_node, so just pass the
parent node into dlpar_attach_node instead of the path. This avoids doing
a lookup of the parent node by the path.

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/platforms/pseries/dlpar.c       | 6 ++----
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +-
 arch/powerpc/platforms/pseries/mobility.c    | 2 +-
 arch/powerpc/platforms/pseries/pseries.h     | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 80b84c9c8509..783f36364690 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -254,13 +254,11 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
 	return first_dn;
 }
 
-int dlpar_attach_node(struct device_node *dn)
+int dlpar_attach_node(struct device_node *dn, struct device_node *parent)
 {
 	int rc;
 
-	dn->parent = pseries_of_derive_parent(dn->full_name);
-	if (IS_ERR(dn->parent))
-		return PTR_ERR(dn->parent);
+	dn->parent = parent;
 
 	rc = of_attach_node(dn);
 	if (rc) {
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 0a93093fbcef..b357f1ae0b0a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -463,7 +463,7 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
 		return -EINVAL;
 	}
 
-	rc = dlpar_attach_node(dn);
+	rc = dlpar_attach_node(dn, parent);
 	if (rc) {
 		saved_rc = rc;
 		pr_warn("Failed to attach node %s, rc: %d, drc index: %x\n",
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 2da4851eff99..210ce632d63e 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -229,7 +229,7 @@ static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
 	if (!dn)
 		return -ENOENT;
 
-	rc = dlpar_attach_node(dn);
+	rc = dlpar_attach_node(dn, parent_dn);
 	if (rc)
 		dlpar_free_cc_nodes(dn);
 
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 1361a9db534b..4470a3194311 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -46,7 +46,7 @@ extern void dlpar_free_cc_nodes(struct device_node *);
 extern void dlpar_free_cc_property(struct property *);
 extern struct device_node *dlpar_configure_connector(__be32,
 						struct device_node *);
-extern int dlpar_attach_node(struct device_node *);
+extern int dlpar_attach_node(struct device_node *, struct device_node *);
 extern int dlpar_detach_node(struct device_node *);
 extern int dlpar_acquire_drc(u32 drc_index);
 extern int dlpar_release_drc(u32 drc_index);
-- 
2.11.0

[PATCH 3/4] powerpc: pseries: only store the device node basename in full_name

[Rob Herring]

With dependencies on full_name containing the entire device node path
removed, stop storing the full_name in nodes created by
dlpar_configure_connector() and pSeries_reconfig_add_node().

Signed-off-by: Rob Herring <robh@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/platforms/pseries/dlpar.c    | 20 ++++----------------
 arch/powerpc/platforms/pseries/reconfig.c |  2 +-
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index 783f36364690..8ab0be0706fd 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -75,24 +75,17 @@ static struct property *dlpar_parse_cc_property(struct cc_workarea *ccwa)
 	return prop;
 }
 
-static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa,
-					       const char *path)
+static struct device_node *dlpar_parse_cc_node(struct cc_workarea *ccwa)
 {
 	struct device_node *dn;
 	char *name;
 
-	/* If parent node path is "/" advance path to NULL terminator to
-	 * prevent double leading slashs in full_name.
-	 */
-	if (!path[1])
-		path++;
-
 	dn = kzalloc(sizeof(*dn), GFP_KERNEL);
 	if (!dn)
 		return NULL;
 
 	name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
-	dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
+	dn->full_name = kasprintf(GFP_KERNEL, "%s", name);
 	if (!dn->full_name) {
 		kfree(dn);
 		return NULL;
@@ -148,7 +141,6 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
 	struct property *last_property = NULL;
 	struct cc_workarea *ccwa;
 	char *data_buf;
-	const char *parent_path = parent->full_name;
 	int cc_token;
 	int rc = -1;
 
@@ -182,7 +174,7 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
 			break;
 
 		case NEXT_SIBLING:
-			dn = dlpar_parse_cc_node(ccwa, parent_path);
+			dn = dlpar_parse_cc_node(ccwa);
 			if (!dn)
 				goto cc_error;
 
@@ -192,10 +184,7 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
 			break;
 
 		case NEXT_CHILD:
-			if (first_dn)
-				parent_path = last_dn->full_name;
-
-			dn = dlpar_parse_cc_node(ccwa, parent_path);
+			dn = dlpar_parse_cc_node(ccwa);
 			if (!dn)
 				goto cc_error;
 
@@ -226,7 +215,6 @@ struct device_node *dlpar_configure_connector(__be32 drc_index,
 
 		case PREV_PARENT:
 			last_dn = last_dn->parent;
-			parent_path = last_dn->parent->full_name;
 			break;
 
 		case CALL_AGAIN:
diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index e5bf1e84047f..73e4063ad997 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -33,7 +33,7 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
 	if (!np)
 		goto out_err;
 
-	np->full_name = kstrdup(path, GFP_KERNEL);
+	np->full_name = kstrdup(kbasename(path), GFP_KERNEL);
 	if (!np->full_name)
 		goto out_err;
 
-- 
2.11.0

RE: [PATCH 3/4] powerpc: pseries: only store the device node basename in full_name

[David Laight]

From: Rob Herring
> Sent: 25 July 2017 22:44
> With dependencies on full_name containing the entire device node path
> removed, stop storing the full_name in nodes created by
> dlpar_configure_connector() and pSeries_reconfig_add_node().
...
>	dn =3D kzalloc(sizeof(*dn), GFP_KERNEL);
> 	if (!dn)
> 		return NULL;
>=20
> 	name =3D (char *)ccwa + be32_to_cpu(ccwa->name_offset);
> -	dn->full_name =3D kasprintf(GFP_KERNEL, "%s/%s", path, name);
> +	dn->full_name =3D kasprintf(GFP_KERNEL, "%s", name);

Isn't this just strdup()?
Perhaps you should rename full_name to name - since it is no longer 'full'?

Maybe you could do a single malloc() for both 'dn' and the name?

	David

Re: [PATCH 3/4] powerpc: pseries: only store the device node basename in full_name

[Rob Herring]

On Wed, Jul 26, 2017 at 5:07 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Rob Herring
>> Sent: 25 July 2017 22:44
>> With dependencies on full_name containing the entire device node path
>> removed, stop storing the full_name in nodes created by
>> dlpar_configure_connector() and pSeries_reconfig_add_node().
> ...
>>       dn = kzalloc(sizeof(*dn), GFP_KERNEL);
>>       if (!dn)
>>               return NULL;
>>
>>       name = (char *)ccwa + be32_to_cpu(ccwa->name_offset);
>> -     dn->full_name = kasprintf(GFP_KERNEL, "%s/%s", path, name);
>> +     dn->full_name = kasprintf(GFP_KERNEL, "%s", name);
>
> Isn't this just strdup()?

Yes, it can be simplified to that now.

> Perhaps you should rename full_name to name - since it is no longer 'full'?

Ideally, yes. However, we still have users in other places tree wide
(which should still work with the change) and I don't think it is
worth the additional churn. Also, we already have "name" as that is
the node name without the unit-address. I'd like to get rid of that,
but name is special in that it is exposed as a property too. Finally,
full_name is still the full path on Sparc.

> Maybe you could do a single malloc() for both 'dn' and the name?

Sure.

Rob

[PATCH 4/4] of/fdt: only store the device node basename in full_name

[Rob Herring]

With dependencies on a statically allocated full path name converted to
use %pOF format specifier, we can store just the basename of node, and
the unflattening of the FDT can be simplified.

This commit will affect the remaining users of full_name. After
analyzing these users, the remaining cases should only change some print
messages. The main users of full_name are providing a name for struct
resource. The resource names shouldn't be important other than providing
/proc/iomem names.

We no longer distinguish between pre and post 0x10 dtb formats as either
a full path or basename will work. However, less than 0x10 formats have
been broken since the conversion to use libfdt (and no one has cared).
The conversion of the unflattening code to be non-recursive also broke
pre 0x10 formats as the populate_node function would return 0 in that
case.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 drivers/of/fdt.c | 69 +++++++++-----------------------------------------------
 1 file changed, 11 insertions(+), 58 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ce30c9a588a4..27c535af0be8 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -266,74 +266,32 @@ static void populate_properties(const void *blob,
 		*pprev = NULL;
 }
 
-static unsigned int populate_node(const void *blob,
-				  int offset,
-				  void **mem,
-				  struct device_node *dad,
-				  unsigned int fpsize,
-				  struct device_node **pnp,
-				  bool dryrun)
+static bool populate_node(const void *blob,
+			  int offset,
+			  void **mem,
+			  struct device_node *dad,
+			  struct device_node **pnp,
+			  bool dryrun)
 {
 	struct device_node *np;
 	const char *pathp;
 	unsigned int l, allocl;
-	int new_format = 0;
 
 	pathp = fdt_get_name(blob, offset, &l);
 	if (!pathp) {
 		*pnp = NULL;
-		return 0;
+		return false;
 	}
 
 	allocl = ++l;
 
-	/* version 0x10 has a more compact unit name here instead of the full
-	 * path. we accumulate the full path size using "fpsize", we'll rebuild
-	 * it later. We detect this because the first character of the name is
-	 * not '/'.
-	 */
-	if ((*pathp) != '/') {
-		new_format = 1;
-		if (fpsize == 0) {
-			/* root node: special case. fpsize accounts for path
-			 * plus terminating zero. root node only has '/', so
-			 * fpsize should be 2, but we want to avoid the first
-			 * level nodes to have two '/' so we use fpsize 1 here
-			 */
-			fpsize = 1;
-			allocl = 2;
-			l = 1;
-			pathp = "";
-		} else {
-			/* account for '/' and path size minus terminal 0
-			 * already in 'l'
-			 */
-			fpsize += l;
-			allocl = fpsize;
-		}
-	}
-
 	np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl,
 				__alignof__(struct device_node));
 	if (!dryrun) {
 		char *fn;
 		of_node_init(np);
 		np->full_name = fn = ((char *)np) + sizeof(*np);
-		if (new_format) {
-			/* rebuild full path for new format */
-			if (dad && dad->parent) {
-				strcpy(fn, dad->full_name);
-#ifdef DEBUG
-				if ((strlen(fn) + l + 1) != allocl) {
-					pr_debug("%s: p: %d, l: %d, a: %d\n",
-						pathp, (int)strlen(fn),
-						l, allocl);
-				}
-#endif
-				fn += strlen(fn);
-			}
-			*(fn++) = '/';
-		}
+
 		memcpy(fn, pathp, l);
 
 		if (dad != NULL) {
@@ -355,7 +313,7 @@ static unsigned int populate_node(const void *blob,
 	}
 
 	*pnp = np;
-	return fpsize;
+	return true;
 }
 
 static void reverse_nodes(struct device_node *parent)
@@ -399,7 +357,6 @@ static int unflatten_dt_nodes(const void *blob,
 	struct device_node *root;
 	int offset = 0, depth = 0, initial_depth = 0;
 #define FDT_MAX_DEPTH	64
-	unsigned int fpsizes[FDT_MAX_DEPTH];
 	struct device_node *nps[FDT_MAX_DEPTH];
 	void *base = mem;
 	bool dryrun = !base;
@@ -418,7 +375,6 @@ static int unflatten_dt_nodes(const void *blob,
 		depth = initial_depth = 1;
 
 	root = dad;
-	fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0;
 	nps[depth] = dad;
 
 	for (offset = 0;
@@ -427,11 +383,8 @@ static int unflatten_dt_nodes(const void *blob,
 		if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH))
 			continue;
 
-		fpsizes[depth+1] = populate_node(blob, offset, &mem,
-						 nps[depth],
-						 fpsizes[depth],
-						 &nps[depth+1], dryrun);
-		if (!fpsizes[depth+1])
+		if (!populate_node(blob, offset, &mem, nps[depth],
+				   &nps[depth+1], dryrun))
 			return mem - base;
 
 		if (!dryrun && nodepp && !*nodepp)
-- 
2.11.0

Re: [PATCH 4/4] of/fdt: only store the device node basename in full_name

[Michael Ellerman]

Rob Herring <robh@kernel.org> writes:

> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.
>
> We no longer distinguish between pre and post 0x10 dtb formats as either
> a full path or basename will work. However, less than 0x10 formats have
> been broken since the conversion to use libfdt (and no one has cared).

For the record - yes we did care. It broke booting with old versions of
kexec, and it was a royal P.I.T.# to debug :D

cheers

Re: [PATCH 4/4] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Wed, Jul 26, 2017 at 5:26 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> With dependencies on a statically allocated full path name converted to
>> use %pOF format specifier, we can store just the basename of node, and
>> the unflattening of the FDT can be simplified.
>>
>> This commit will affect the remaining users of full_name. After
>> analyzing these users, the remaining cases should only change some print
>> messages. The main users of full_name are providing a name for struct
>> resource. The resource names shouldn't be important other than providing
>> /proc/iomem names.
>>
>> We no longer distinguish between pre and post 0x10 dtb formats as either
>> a full path or basename will work. However, less than 0x10 formats have
>> been broken since the conversion to use libfdt (and no one has cared).
>
> For the record - yes we did care. It broke booting with old versions of
> kexec, and it was a royal P.I.T.# to debug :D

Sorry, I forgot about that one. I'll drop the statement. I had gone
back and looked and only found the issue on mpc8323 booting[1] which
was an issue with libfdt having more checks on the fdt. I proposed
some fixes, but never heard back on that.

Rob

[1] https://lkml.org/lkml/2015/6/10/820

Re: [PATCH 0/4] Removing full paths from DT full_name

[Frank Rowand]

Hi Rob,

On 07/25/17 14:44, Rob Herring wrote:
> This series is the last steps to remove storing the full path for every 
> DT node. Instead, we can create full path strings dynamically as needed 
> with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of 
> remaining direct users of full_name after this series. I don't believe 
> there should be any functional impact for those users with the change to 
> only the node name (+unit-address). The majority are for struct 
> resource.name. This should only affect /proc/iomem display.

I added a new dependency on full_name in:

  [PATCH v4 3/3] of: overlay: add overlay symbols to live device tree

You don't need to fix that -- I knew the removal of full_name was coming
and expected to adapt to that change when it came, so I'll take care of
it.  It will probably take me two or three weeks to get to it, but that
shouldn't be a big deal since the affected code is a new feature that
is not yet used.

-Frank

> 
> Patches 1 and 2 can be applied now for 4.14. For patches 3 and 4, my 
> target is 4.15 after all the dependencies have been merged.
> 
> PPC folks, Please test! The PPC parts are untested. A git branch with 
> all the dependencies is here[1].
> 
> Rob
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dt-printf
> 
> Rob Herring (4):
>   powerpc: pseries: vio: match parent nodes with of_find_node_by_path
>   powerpc: pseries: remove dlpar_attach_node dependency on full path
>   powerpc: pseries: only store the device node basename in full_name
>   of/fdt: only store the device node basename in full_name
> 
>  arch/powerpc/platforms/pseries/dlpar.c       | 26 +++--------
>  arch/powerpc/platforms/pseries/hotplug-cpu.c |  2 +-
>  arch/powerpc/platforms/pseries/mobility.c    |  2 +-
>  arch/powerpc/platforms/pseries/pseries.h     |  2 +-
>  arch/powerpc/platforms/pseries/reconfig.c    |  2 +-
>  arch/powerpc/platforms/pseries/vio.c         |  4 +-
>  drivers/of/fdt.c                             | 69 +++++-----------------------
>  7 files changed, 23 insertions(+), 84 deletions(-)
> 

Re: [PATCH 0/4] Removing full paths from DT full_name

[Rob Herring]

On Wed, Jul 26, 2017 at 9:20 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> Hi Rob,
>
> On 07/25/17 14:44, Rob Herring wrote:
>> This series is the last steps to remove storing the full path for every
>> DT node. Instead, we can create full path strings dynamically as needed
>> with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of
>> remaining direct users of full_name after this series. I don't believe
>> there should be any functional impact for those users with the change to
>> only the node name (+unit-address). The majority are for struct
>> resource.name. This should only affect /proc/iomem display.
>
> I added a new dependency on full_name in:
>
>   [PATCH v4 3/3] of: overlay: add overlay symbols to live device tree
>
> You don't need to fix that -- I knew the removal of full_name was coming
> and expected to adapt to that change when it came, so I'll take care of
> it.  It will probably take me two or three weeks to get to it, but that
> shouldn't be a big deal since the affected code is a new feature that
> is not yet used.

Indeed, I had fixed up the print statements, but missed that. Thanks
for the heads up.

Rob

Re: [PATCH 0/4] Removing full paths from DT full_name

[Rob Herring]

On Tue, Jul 25, 2017 at 4:44 PM, Rob Herring <robh@kernel.org> wrote:
> This series is the last steps to remove storing the full path for every
> DT node. Instead, we can create full path strings dynamically as needed
> with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of
> remaining direct users of full_name after this series. I don't believe
> there should be any functional impact for those users with the change to
> only the node name (+unit-address). The majority are for struct
> resource.name. This should only affect /proc/iomem display.
>
> Patches 1 and 2 can be applied now for 4.14. For patches 3 and 4, my
> target is 4.15 after all the dependencies have been merged.
>
> PPC folks, Please test! The PPC parts are untested. A git branch with
> all the dependencies is here[1].

PPC folks, any chance to test this?

Rob

Re: [PATCH 0/4] Removing full paths from DT full_name

[Michael Ellerman]

Rob Herring <robh@kernel.org> writes:

> On Tue, Jul 25, 2017 at 4:44 PM, Rob Herring <robh@kernel.org> wrote:
>> This series is the last steps to remove storing the full path for every
>> DT node. Instead, we can create full path strings dynamically as needed
>> with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of
>> remaining direct users of full_name after this series. I don't believe
>> there should be any functional impact for those users with the change to
>> only the node name (+unit-address). The majority are for struct
>> resource.name. This should only affect /proc/iomem display.
>>
>> Patches 1 and 2 can be applied now for 4.14. For patches 3 and 4, my
>> target is 4.15 after all the dependencies have been merged.
>>
>> PPC folks, Please test! The PPC parts are untested. A git branch with
>> all the dependencies is here[1].
>
> PPC folks, any chance to test this?

I got stuck on your %pOF conversion, which broke the vio.c code, because of:

-		if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
+		if (!strcmp(parent_node->full_name, "ibm,platform-facilities"))

But full_name hasn't been changed yet.

But patch 1 here should fix that, so I'll pull it all together and try
and get it tested.

cheers

Re: [PATCH 0/4] Removing full paths from DT full_name

[Rob Herring]

On Mon, Aug 7, 2017 at 9:21 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Tue, Jul 25, 2017 at 4:44 PM, Rob Herring <robh@kernel.org> wrote:
>>> This series is the last steps to remove storing the full path for every
>>> DT node. Instead, we can create full path strings dynamically as needed
>>> with printf %pOF specifiers (commit ce4fecf1fe15). There are a number of
>>> remaining direct users of full_name after this series. I don't believe
>>> there should be any functional impact for those users with the change to
>>> only the node name (+unit-address). The majority are for struct
>>> resource.name. This should only affect /proc/iomem display.
>>>
>>> Patches 1 and 2 can be applied now for 4.14. For patches 3 and 4, my
>>> target is 4.15 after all the dependencies have been merged.
>>>
>>> PPC folks, Please test! The PPC parts are untested. A git branch with
>>> all the dependencies is here[1].
>>
>> PPC folks, any chance to test this?
>
> I got stuck on your %pOF conversion, which broke the vio.c code, because of:
>
> -               if (!strcmp(parent_node->full_name, "/ibm,platform-facilities"))
> +               if (!strcmp(parent_node->full_name, "ibm,platform-facilities"))
>
> But full_name hasn't been changed yet.

Ah, those lines need to be dropped from the %pOF conversion. I'll send
a new version.

What I was originally considering here was just:

strcmp(kbasename(parent_node->full_name), "ibm,platform-facilities")

That would work for "/foo/ibm,platform-facilities" too, but IMO
validation of the DT is not the kernel's job (if it is, we're doing a
horrible job). But in the end here, I decided to keep the existing
full path matching as patch 1 does.

> But patch 1 here should fix that, so I'll pull it all together and try
> and get it tested.

Thanks.

Rob