[2/8] of: Drop full path from Sparc PDT full_name

Message ID 20181107223151.14991-3-robh@kernel.org
State New
Headers show
Series
  • sparc: OF name and device_type rework
Related show

Commit Message

Rob Herring Nov. 7, 2018, 10:31 p.m.
Now that there are no more users of path_component_name outside of
the PDT code and users of device_node.full_name are converted to use
"%pOF" printf specifier, we can align Sparc with other arches and store
just the base node name and unit address in full_name. This makes
path_component_name redundant, so it can be removed.

As full_name is used by printf specifiers, set it as early as possible.

Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>

---
 drivers/of/pdt.c   | 26 +++-----------------------
 include/linux/of.h |  1 -
 2 files changed, 3 insertions(+), 24 deletions(-)

-- 
2.19.1

Comments

David Miller Nov. 9, 2018, 3:11 a.m. | #1
From: Rob Herring <robh@kernel.org>

Date: Wed,  7 Nov 2018 16:31:45 -0600

> @@ -32,24 +32,7 @@ unsigned int of_pdt_unique_id __initdata;

>  

>  static char * __init of_pdt_build_full_name(struct device_node *dp)

>  {

> -	int len, ourlen, plen;

> -	char *n;

> -

> -	dp->path_component_name = build_path_component(dp);

> -

> -	plen = strlen(dp->parent->full_name);

> -	ourlen = strlen(dp->path_component_name);

> -	len = ourlen + plen + 2;

> -

> -	n = prom_early_alloc(len);

> -	strcpy(n, dp->parent->full_name);

> -	if (!of_node_is_root(dp->parent)) {

> -		strcpy(n + plen, "/");

> -		plen++;

> -	}

> -	strcpy(n + plen, dp->path_component_name);

> -

> -	return n;

> +	return build_path_component(dp);

>  }

>  

>  #else /* CONFIG_SPARC */


Hmmm, shouldn't this be doing what the non-SPARC version of
of_pdt_build_full_name() does which is to return the component string
appended to the parent's full_name?

You're just returning the component string here.
Rob Herring Nov. 9, 2018, 8:30 p.m. | #2
On Thu, Nov 8, 2018 at 9:11 PM David Miller <davem@davemloft.net> wrote:
>

> From: Rob Herring <robh@kernel.org>

> Date: Wed,  7 Nov 2018 16:31:45 -0600

>

> > @@ -32,24 +32,7 @@ unsigned int of_pdt_unique_id __initdata;

> >

> >  static char * __init of_pdt_build_full_name(struct device_node *dp)

> >  {

> > -     int len, ourlen, plen;

> > -     char *n;

> > -

> > -     dp->path_component_name = build_path_component(dp);

> > -

> > -     plen = strlen(dp->parent->full_name);

> > -     ourlen = strlen(dp->path_component_name);

> > -     len = ourlen + plen + 2;

> > -

> > -     n = prom_early_alloc(len);

> > -     strcpy(n, dp->parent->full_name);

> > -     if (!of_node_is_root(dp->parent)) {

> > -             strcpy(n + plen, "/");

> > -             plen++;

> > -     }

> > -     strcpy(n + plen, dp->path_component_name);

> > -

> > -     return n;

> > +     return build_path_component(dp);

> >  }

> >

> >  #else /* CONFIG_SPARC */

>

> Hmmm, shouldn't this be doing what the non-SPARC version of

> of_pdt_build_full_name() does which is to return the component string

> appended to the parent's full_name?

>

> You're just returning the component string here.


That is the intent. With this change we stop storing the full path for
every node. Everywhere that needs the full path, generates it with the
%pOF printf specifier. Other than users in arch/sparc converted in
this series all the users of full_name have been converted already or
use kbasename(full_name) so they work either way. BTW, I've found a
couple more, so I'll be sending you a v2.

And of_pdt_build_full_name for !SPARC does need to be converted too,
but I'll do that separately.

Rob
David Miller Nov. 10, 2018, 4:36 a.m. | #3
From: Rob Herring <robh@kernel.org>

Date: Fri, 9 Nov 2018 14:30:01 -0600

> That is the intent. With this change we stop storing the full path for

> every node. Everywhere that needs the full path, generates it with the

> %pOF printf specifier. Other than users in arch/sparc converted in

> this series all the users of full_name have been converted already or

> use kbasename(full_name) so they work either way. BTW, I've found a

> couple more, so I'll be sending you a v2.

> 

> And of_pdt_build_full_name for !SPARC does need to be converted too,

> but I'll do that separately.


I would think that, for correctness, you'd need to adjust both functions
at the same time, no?

Patch

diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
index 013e65de074a..40321b6b82e2 100644
--- a/drivers/of/pdt.c
+++ b/drivers/of/pdt.c
@@ -32,24 +32,7 @@  unsigned int of_pdt_unique_id __initdata;
 
 static char * __init of_pdt_build_full_name(struct device_node *dp)
 {
-	int len, ourlen, plen;
-	char *n;
-
-	dp->path_component_name = build_path_component(dp);
-
-	plen = strlen(dp->parent->full_name);
-	ourlen = strlen(dp->path_component_name);
-	len = ourlen + plen + 2;
-
-	n = prom_early_alloc(len);
-	strcpy(n, dp->parent->full_name);
-	if (!of_node_is_root(dp->parent)) {
-		strcpy(n + plen, "/");
-		plen++;
-	}
-	strcpy(n + plen, dp->path_component_name);
-
-	return n;
+	return build_path_component(dp);
 }
 
 #else /* CONFIG_SPARC */
@@ -181,6 +164,8 @@  static struct device_node * __init of_pdt_create_node(phandle node,
 
 	dp->properties = of_pdt_build_prop_list(node);
 
+	dp->full_name = of_pdt_build_full_name(dp);
+
 	irq_trans_init(dp);
 
 	return dp;
@@ -204,8 +189,6 @@  static struct device_node * __init of_pdt_build_tree(struct device_node *parent,
 			ret = dp;
 		prev_sibling = dp;
 
-		dp->full_name = of_pdt_build_full_name(dp);
-
 		dp->child = of_pdt_build_tree(dp, of_pdt_prom_ops->getchild(node));
 
 		if (of_pdt_build_more)
@@ -228,9 +211,6 @@  void __init of_pdt_build_devicetree(phandle root_node, struct of_pdt_ops *ops)
 	of_pdt_prom_ops = ops;
 
 	of_root = of_pdt_create_node(root_node, NULL);
-#if defined(CONFIG_SPARC)
-	of_root->path_component_name = "";
-#endif
 	of_root->full_name = "/";
 
 	of_root->child = of_pdt_build_tree(of_root,
diff --git a/include/linux/of.h b/include/linux/of.h
index a5aee3c438ad..0fe5bef81a7e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -66,7 +66,6 @@  struct device_node {
 	unsigned long _flags;
 	void	*data;
 #if defined(CONFIG_SPARC)
-	const char *path_component_name;
 	unsigned int unique_id;
 	struct of_irq_controller *irq_trans;
 #endif