Re: [RFC PATCH] of: base: add support to get machine model name

[Arnd Bergmann]

On Thursday, November 17, 2016 2:08:30 PM CET Sudeep Holla wrote:
> On 17/11/16 13:50, Arnd Bergmann wrote:
> > On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:
> >> Currently platforms/drivers needing to get the machine model name are
> >> replicating the same snippet of code. In some case, the OF reference
> >> counting is either missing or incorrect.
> >>
> >> This patch adds support to read the machine model name either using
> >> the "model" or the "compatible" property in the device tree root node.
> >>
> >> Signed-off-by: Sudeep Holla 
> >
> > I like the idea. One small comment:
> >
> 
> Thanks. I prefer it as single patch but it can't be applied to any tree.
> Any suggestions on handling this patch to fix the warning in -next ?
> 
The patch that causes the warning is currently in the mmc tree, and I
don't think it would be good to have your entire patch in there too.

It's probably best to just fix the warning there now by adding another
open-coded copy of that function, and then apply your patch on top
for v4.11.

Arnd

Re: [RFC PATCH] of: base: add support to get machine model name

[Sudeep Holla]

On 17/11/16 14:13, Arnd Bergmann wrote:

On Thursday, November 17, 2016 2:08:30 PM CET Sudeep Holla wrote:

On 17/11/16 13:50, Arnd Bergmann wrote:

On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:

Currently platforms/drivers needing to get the machine model name are
replicating the same snippet of code. In some case, the OF reference
counting is either missing or incorrect.

This patch adds support to read the machine model name either using
the "model" or the "compatible" property in the device tree root node.

Signed-off-by: Sudeep Holla 


I like the idea. One small comment:



Thanks. I prefer it as single patch but it can't be applied to any tree.
Any suggestions on handling this patch to fix the warning in -next ?


The patch that causes the warning is currently in the mmc tree, and I
don't think it would be good to have your entire patch in there too.

It's probably best to just fix the warning there now by adding another
open-coded copy of that function, and then apply your patch on top
for v4.11.


Sure, that's much simpler to deal with for now.

--
Regards,
Sudeep

Re: [RFC PATCH] of: base: add support to get machine model name

[Sudeep Holla]

On 17/11/16 13:50, Arnd Bergmann wrote:

On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:

Currently platforms/drivers needing to get the machine model name are
replicating the same snippet of code. In some case, the OF reference
counting is either missing or incorrect.

This patch adds support to read the machine model name either using
the "model" or the "compatible" property in the device tree root node.

Signed-off-by: Sudeep Holla 


I like the idea. One small comment:



Thanks. I prefer it as single patch but it can't be applied to any tree.
Any suggestions on handling this patch to fix the warning in -next ?


+int of_machine_get_model_name(const char **model)
+{
+   int error;
+   struct device_node *root;
+
+   root = of_find_node_by_path("/");
+   if (!root)
+   return -EINVAL;


The global of_root variable points ot this already, and is defined
in the same file, so I think we can just skip the lookup.



Ah right, will fix it.

--
Regards,
Sudeep

Re: [RFC PATCH] of: base: add support to get machine model name

[Arnd Bergmann]

On Thursday, November 17, 2016 11:50:50 AM CET Sudeep Holla wrote:
> Currently platforms/drivers needing to get the machine model name are
> replicating the same snippet of code. In some case, the OF reference
> counting is either missing or incorrect.
> 
> This patch adds support to read the machine model name either using
> the "model" or the "compatible" property in the device tree root node.
> 
> Signed-off-by: Sudeep Holla 

I like the idea. One small comment:

> +int of_machine_get_model_name(const char **model)
> +{
> + int error;
> + struct device_node *root;
> +
> + root = of_find_node_by_path("/");
> + if (!root)
> + return -EINVAL;

The global of_root variable points ot this already, and is defined
in the same file, so I think we can just skip the lookup.

Arnd

[RFC PATCH] of: base: add support to get machine model name

[Sudeep Holla]

Currently platforms/drivers needing to get the machine model name are
replicating the same snippet of code. In some case, the OF reference
counting is either missing or incorrect.

This patch adds support to read the machine model name either using
the "model" or the "compatible" property in the device tree root node.

Signed-off-by: Sudeep Holla 
---
 arch/arm/mach-imx/cpu.c   |  4 +---
 arch/arm/mach-mxs/mach-mxs.c  |  3 +--
 arch/mips/cavium-octeon/setup.c   | 12 ++--
 arch/mips/generic/proc.c  | 15 +++
 arch/sh/boards/of-generic.c   |  6 +-
 drivers/of/base.c | 34 ++
 drivers/soc/fsl/guts.c|  3 +--
 drivers/soc/renesas/renesas-soc.c |  4 +---
 include/linux/of.h|  6 ++
 9 files changed, 50 insertions(+), 37 deletions(-)

Hi,

While trying to fix a simple build warning(as below) in -next for fsl/guts.c,
I came across this code duplication in multiple places.

WARNING: modpost: Found 1 section mismatch(es).
To see full details build your kernel with:
'make CONFIG_DEBUG_SECTION_MISMATCH=y'

With CONFIG_DEBUG_SECTION_MISMATCH enabled, the details are reported:

WARNING: vmlinux.o(.text+0x55d014): Section mismatch in reference from the
function fsl_guts_probe() to the function
.init.text:of_flat_dt_get_machine_name()
The function fsl_guts_probe() references
the function __init of_flat_dt_get_machine_name().
This is often because fsl_guts_probe lacks a __init
annotation or the annotation of of_flat_dt_get_machine_name is wrong.

I can split the patch if needed if people are OK with the idea.

Regards,
Sudeep

diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index b3347d32349f..846f40008752 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -85,9 +85,7 @@ struct device * __init imx_soc_device_init(void)

soc_dev_attr->family = "Freescale i.MX";

-   root = of_find_node_by_path("/");
-   ret = of_property_read_string(root, "model", _dev_attr->machine);
-   of_node_put(root);
+   ret = of_machine_get_model_name(_dev_attr->machine);
if (ret)
goto free_soc;

diff --git a/arch/arm/mach-mxs/mach-mxs.c b/arch/arm/mach-mxs/mach-mxs.c
index e4f21086b42b..ed9af3a894f0 100644
--- a/arch/arm/mach-mxs/mach-mxs.c
+++ b/arch/arm/mach-mxs/mach-mxs.c
@@ -391,8 +391,7 @@ static void __init mxs_machine_init(void)
if (!soc_dev_attr)
return;

-   root = of_find_node_by_path("/");
-   ret = of_property_read_string(root, "model", _dev_attr->machine);
+   ret = of_machine_get_model_name(_dev_attr->machine);
if (ret)
return;

diff --git a/arch/mips/cavium-octeon/setup.c b/arch/mips/cavium-octeon/setup.c
index 9a2db1c013d9..2e2b1b5befa4 100644
--- a/arch/mips/cavium-octeon/setup.c
+++ b/arch/mips/cavium-octeon/setup.c
@@ -498,16 +498,8 @@ static void __init init_octeon_system_type(void)
char const *board_type;

board_type = cvmx_board_type_to_string(octeon_bootinfo->board_type);
-   if (board_type == NULL) {
-   struct device_node *root;
-   int ret;
-
-   root = of_find_node_by_path("/");
-   ret = of_property_read_string(root, "model", _type);
-   of_node_put(root);
-   if (ret)
-   board_type = "Unsupported Board";
-   }
+   if (!board_type && of_machine_get_model_name(_type))
+   board_type = "Unsupported Board";

snprintf(octeon_system_type, sizeof(octeon_system_type), "%s (%s)",
 board_type, octeon_model_get_string(read_c0_prid()));
diff --git a/arch/mips/generic/proc.c b/arch/mips/generic/proc.c
index 42b33250a4a2..f7fc067bf908 100644
--- a/arch/mips/generic/proc.c
+++ b/arch/mips/generic/proc.c
@@ -10,20 +10,11 @@

 #include 

-#include 
-
 const char *get_system_type(void)
 {
const char *str;
-   int err;
-
-   err = of_property_read_string(of_root, "model", );
-   if (!err)
-   return str;
-
-   err = of_property_read_string_index(of_root, "compatible", 0, );
-   if (!err)
-   return str;

-   return "Unknown";
+   if (of_machine_get_model_name())
+   return "Unknown";
+   return str;
 }
diff --git a/arch/sh/boards/of-generic.c b/arch/sh/boards/of-generic.c
index 1fb6d5714bae..938a14499298 100644
--- a/arch/sh/boards/of-generic.c
+++ b/arch/sh/boards/of-generic.c
@@ -135,11 +135,7 @@ static void __init sh_of_setup(char **cmdline_p)
board_time_init = sh_of_time_init;

sh_mv.mv_name = "Unknown SH model";
-   root = of_find_node_by_path("/");
-   if (root) {
-   of_property_read_string(root, "model", _mv.mv_name);
-   of_node_put(root);
-   }
+   of_machine_get_model_name(_mv.mv_name);

sh_of_smp_probe();
 }
diff --git a/drivers/of/base.c