[PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Matthew McClintock]

The first global-utilities node might not contain the rstcr
property, so we should search all the nodes

Signed-off-by: Matthew McClintock <msm@freescale.com>
---
 arch/powerpc/sysdev/fsl_soc.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index b91f7ac..e2c8e47 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -378,17 +378,19 @@ static __be32 __iomem *rstcr;
 static int __init setup_rstcr(void)
 {
 	struct device_node *np;
-	np = of_find_node_by_name(NULL, "global-utilities");
-	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
-		rstcr = of_iomap(np, 0) + 0xb0;
-		if (!rstcr)
-			printk (KERN_EMERG "Error: reset control register "
-					"not mapped!\n");
-	} else if (ppc_md.restart == fsl_rstcr_restart)
+
+	for_each_node_by_name(np, "global-utilities") {
+		if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
+			rstcr = of_iomap(np, 0) + 0xb0;
+			if (!rstcr)
+				printk (KERN_EMERG "Error: reset control "
+						"register not mapped!\n");
+		}
+	}
+
+	if (!rstcr && ppc_md.restart == fsl_rstcr_restart)
 		printk(KERN_ERR "No RSTCR register, warm reboot won't work\n");
 
-	if (np)
-		of_node_put(np);
 	return 0;
 }
 
-- 
1.6.6.1

Re: [PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Timur Tabi]

> <msm@freescale.com> wrote:

> +
> + =A0 =A0 =A0 for_each_node_by_name(np, "global-utilities") {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((of_get_property(np, "fsl,has-rstcr", N=
ULL))) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rstcr =3D of_iomap(np, 0) +=
 0xb0;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!rstcr)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printk (KER=
N_EMERG "Error: reset control "

I'm not sure KERN_EMERG is warranted for this kind of error.

> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 "register not mapped!\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }

So if a node has an fsl,rstcr property, but the of_iomap() fails, we
jump to the next global-utilities node?  Perhaps you need a 'break'
after the printk()?

> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 if (!rstcr && ppc_md.restart =3D=3D fsl_rstcr_restart)

Wouldn't it make more sense to assign fsl_rstcr_restart to
ppc_md.restart only if we find a valid fsl,has-rstcr property?

--=20
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Matthew McClintock]

On Aug 28, 2010, at 5:34 PM, Timur Tabi wrote:

>> <msm@freescale.com> wrote:
>=20
>> +
>> +       for_each_node_by_name(np, "global-utilities") {
>> +               if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
>> +                       rstcr =3D of_iomap(np, 0) + 0xb0;
>> +                       if (!rstcr)
>> +                               printk (KERN_EMERG "Error: reset =
control "
>=20
> I'm not sure KERN_EMERG is warranted for this kind of error.

I'm not sure either - I left it as it was before.

>=20
>> +                                               "register not =
mapped!\n");
>> +               }
>=20
> So if a node has an fsl,rstcr property, but the of_iomap() fails, we
> jump to the next global-utilities node?  Perhaps you need a 'break'
> after the printk()?

Or potentially a continue to be more robust? Or would two (or more) =
"has-rstcr" nodes be wrong?

>=20
>> +       }
>> +
>> +       if (!rstcr && ppc_md.restart =3D=3D fsl_rstcr_restart)
>=20
> Wouldn't it make more sense to assign fsl_rstcr_restart to
> ppc_md.restart only if we find a valid fsl,has-rstcr property?

Again I'm not entirely sure, I left this as it was before. Is there =
another way to reset the board if the rstcr node was not found =
correctly?

-M

>=20
> --=20
> Timur Tabi
> Linux kernel developer at Freescale
>=20

Re: [PATCH] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Timur Tabi]

On Tue, Aug 31, 2010 at 11:26 AM, Matthew McClintock <msm@freescale.com> wr=
ote:

>> I'm not sure KERN_EMERG is warranted for this kind of error.
>
> I'm not sure either - I left it as it was before.

My vote is to change it to KERN_ERR, but it's your call.

>> So if a node has an fsl,rstcr property, but the of_iomap() fails, we
>> jump to the next global-utilities node? =A0Perhaps you need a 'break'
>> after the printk()?
>
> Or potentially a continue to be more robust? Or would two (or more) "has-=
rstcr" nodes be wrong?

My point is that if the of_iomap() call fails, looking for another
global-utilities node is not the right course of action.  of_iomap()
failing is a serious error that should result in an immediate exit.

>> Wouldn't it make more sense to assign fsl_rstcr_restart to
>> ppc_md.restart only if we find a valid fsl,has-rstcr property?
>
> Again I'm not entirely sure, I left this as it was before. Is there anoth=
er way to reset the board if the rstcr node was not found correctly?

Not on our 85xx boards.  On 83xx, there's mpc83xx_restart().  I just
don't like "ppc_md.restart =3D=3D fsl_rstcr_restart", because it assumes
that the define_machine() entry is incorrect.

--=20
Timur Tabi
Linux kernel developer at Freescale

[PATCH v2] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Matthew McClintock]

The first global-utilities node might not contain the rstcr
property, so we should search all the nodes

Signed-off-by: Matthew McClintock <msm@freescale.com>
---
-Changed KERN_EMERG to KERN_ERR
-Break if we do not find rstcr mapped
-Restore of_put_node that was dropped

 arch/powerpc/sysdev/fsl_soc.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index b91f7ac..6c67d9e 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -378,17 +378,23 @@ static __be32 __iomem *rstcr;
 static int __init setup_rstcr(void)
 {
 	struct device_node *np;
-	np = of_find_node_by_name(NULL, "global-utilities");
-	if ((np && of_get_property(np, "fsl,has-rstcr", NULL))) {
-		rstcr = of_iomap(np, 0) + 0xb0;
-		if (!rstcr)
-			printk (KERN_EMERG "Error: reset control register "
-					"not mapped!\n");
-	} else if (ppc_md.restart == fsl_rstcr_restart)
+
+	for_each_node_by_name(np, "global-utilities") {
+		if ((of_get_property(np, "fsl,has-rstcr", NULL))) {
+			rstcr = of_iomap(np, 0) + 0xb0;
+			if (!rstcr)
+				printk (KERN_ERR "Error: reset control "
+						"register not mapped!\n");
+			break;
+		}
+	}
+
+	if (!rstcr && ppc_md.restart == fsl_rstcr_restart)
 		printk(KERN_ERR "No RSTCR register, warm reboot won't work\n");
 
 	if (np)
 		of_node_put(np);
+
 	return 0;
 }
 
-- 
1.6.6.1

Re: [PATCH v2] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Timur Tabi]

Matthew McClintock wrote:
> The first global-utilities node might not contain the rstcr
> property, so we should search all the nodes
> 
> Signed-off-by: Matthew McClintock <msm@freescale.com>

Acked-by: Timur Tabi <timur@freescale.com>

Re: [PATCH v2] powerpc/fsl_soc: Search all global-utilities nodes for rstccr

[Kumar Gala]

On Aug 31, 2010, at 5:44 PM, Matthew McClintock wrote:

> The first global-utilities node might not contain the rstcr
> property, so we should search all the nodes
> 
> Signed-off-by: Matthew McClintock <msm@freescale.com>
> ---
> -Changed KERN_EMERG to KERN_ERR
> -Break if we do not find rstcr mapped
> -Restore of_put_node that was dropped
> 
> arch/powerpc/sysdev/fsl_soc.c |   20 +++++++++++++-------
> 1 files changed, 13 insertions(+), 7 deletions(-)

applied to next

- k