| Message ID | 20181107223151.14991-3-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | sparc: OF name and device_type rework | expand |
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.
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
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?
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
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