[PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Scott Wood]

The reg property in fsl soc nodes should be removed.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/sysdev/fsl_soc.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 3ace747..7502e03 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -54,10 +54,18 @@ phys_addr_t get_immrbase(void)
 	soc = of_find_node_by_type(NULL, "soc");
 	if (soc) {
 		int size;
-		const void *prop = of_get_property(soc, "reg", &size);
+		u32 naddr;
+		const u32 *prop = of_get_property(soc, "#address-cells", &size);
+
+		if (prop && size == 4)
+			naddr = *prop;
+		else
+			naddr = 2;
+
+		prop = of_get_property(soc, "ranges", &size);
+		if (prop)
+			immrbase = of_translate_address(soc, prop + naddr);
 
-		if (prop)
-			immrbase = of_translate_address(soc, prop);
 		of_node_put(soc);
 	}
 
-- 
1.5.3

[PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Scott Wood]

The reg property in fsl soc nodes should be removed.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/sysdev/fsl_soc.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 3ace747..7502e03 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -54,10 +54,18 @@ phys_addr_t get_immrbase(void)
 	soc = of_find_node_by_type(NULL, "soc");
 	if (soc) {
 		int size;
-		const void *prop = of_get_property(soc, "reg", &size);
+		u32 naddr;
+		const u32 *prop = of_get_property(soc, "#address-cells", &size);
+
+		if (prop && size == 4)
+			naddr = *prop;
+		else
+			naddr = 2;
+
+		prop = of_get_property(soc, "ranges", &size);
+		if (prop && size == 12)
+			immrbase = of_translate_address(soc, prop + naddr);
 
-		if (prop)
-			immrbase = of_translate_address(soc, prop);
 		of_node_put(soc);
 	}
 
-- 
1.5.3

Re: [PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Kumar Gala]

On Jan 14, 2008, at 10:29 AM, Scott Wood wrote:

> The reg property in fsl soc nodes should be removed.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/sysdev/fsl_soc.c |   14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> fsl_soc.c
> index 3ace747..7502e03 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -54,10 +54,18 @@ phys_addr_t get_immrbase(void)
> 	soc = of_find_node_by_type(NULL, "soc");
> 	if (soc) {
> 		int size;
> -		const void *prop = of_get_property(soc, "reg", &size);
> +		u32 naddr;
> +		const u32 *prop = of_get_property(soc, "#address-cells", &size);
> +
> +		if (prop && size == 4)
> +			naddr = *prop;
> +		else
> +			naddr = 2;

Why default to two?

>
> +
> +		prop = of_get_property(soc, "ranges", &size);
> +		if (prop && size == 12)
> +			immrbase = of_translate_address(soc, prop + naddr);
>
> -		if (prop)
> -			immrbase = of_translate_address(soc, prop);

why not make your code an else case if we don't have reg?

>
> 		of_node_put(soc);
> 	}

or something like, than we don't have to worry about adjust anything,  
and if you don't have any children its kinda a pointless device tree :)

	if (soc) {
		struct device_node *child = of_get_next_child(soc, NULL);
		if (child) {
			const void *prop = of_get_property(soc, "ranges", NULL);
			if (prop)
				immrbase = of_translate_address(child, prop);
			of_node_put(child);
		}
		of_node_put(soc);
	}

- k

Re: [PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Scott Wood]

On Mon, Jan 14, 2008 at 08:37:27PM -0600, Kumar Gala wrote:
> >diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> >fsl_soc.c
> >index 3ace747..7502e03 100644
> >--- a/arch/powerpc/sysdev/fsl_soc.c
> >+++ b/arch/powerpc/sysdev/fsl_soc.c
> >@@ -54,10 +54,18 @@ phys_addr_t get_immrbase(void)
> >	soc = of_find_node_by_type(NULL, "soc");
> >	if (soc) {
> >		int size;
> >-		const void *prop = of_get_property(soc, "reg", &size);
> >+		u32 naddr;
> >+		const u32 *prop = of_get_property(soc, "#address-cells", 
> >&size);
> >+
> >+		if (prop && size == 4)
> >+			naddr = *prop;
> >+		else
> >+			naddr = 2;
> 
> Why default to two?

Because that's what the OF spec says the default is?

> >+		prop = of_get_property(soc, "ranges", &size);
> >+		if (prop && size == 12)
> >+			immrbase = of_translate_address(soc, prop + naddr);

Grr, I thought I removed the size == 12 check...

> >-		if (prop)
> >-			immrbase = of_translate_address(soc, prop);
> 
> why not make your code an else case if we don't have reg?

Why?

> or something like, than we don't have to worry about adjust anything,  
> and if you don't have any children its kinda a pointless device tree :)

It's not pointless, it's just incomplete.

> 	if (soc) {
> 		struct device_node *child = of_get_next_child(soc, NULL);
> 		if (child) {
> 			const void *prop = of_get_property(soc, "ranges", 
> 			NULL);
> 			if (prop)
> 				immrbase = of_translate_address(child, prop);
> 			of_node_put(child);
> 		}
> 		of_node_put(soc);
> 	}

Why go out of our way to fail on a childless soc node?

-Scott

Re: [PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Kumar Gala]

On Jan 15, 2008, at 10:40 AM, Scott Wood wrote:

> On Mon, Jan 14, 2008 at 08:37:27PM -0600, Kumar Gala wrote:
>>> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/
>>> fsl_soc.c
>>> index 3ace747..7502e03 100644
>>> --- a/arch/powerpc/sysdev/fsl_soc.c
>>> +++ b/arch/powerpc/sysdev/fsl_soc.c
>>> @@ -54,10 +54,18 @@ phys_addr_t get_immrbase(void)
>>> 	soc = of_find_node_by_type(NULL, "soc");
>>> 	if (soc) {
>>> 		int size;
>>> -		const void *prop = of_get_property(soc, "reg", &size);
>>> +		u32 naddr;
>>> +		const u32 *prop = of_get_property(soc, "#address-cells",
>>> &size);
>>> +
>>> +		if (prop && size == 4)
>>> +			naddr = *prop;
>>> +		else
>>> +			naddr = 2;
>>
>> Why default to two?
>
> Because that's what the OF spec says the default is?

fair.

>>> +		prop = of_get_property(soc, "ranges", &size);
>>> +		if (prop && size == 12)
>>> +			immrbase = of_translate_address(soc, prop + naddr);
>
> Grr, I thought I removed the size == 12 check...
>
>>> -		if (prop)
>>> -			immrbase = of_translate_address(soc, prop);
>>
>> why not make your code an else case if we don't have reg?
>
> Why?

I agree (had to think about it a bit more).

>> or something like, than we don't have to worry about adjust anything,
>> and if you don't have any children its kinda a pointless device  
>> tree :)
>
> It's not pointless, it's just incomplete.
>
>> 	if (soc) {
>> 		struct device_node *child = of_get_next_child(soc, NULL);
>> 		if (child) {
>> 			const void *prop = of_get_property(soc, "ranges",
>> 			NULL);
>> 			if (prop)
>> 				immrbase = of_translate_address(child, prop);
>> 			of_node_put(child);
>> 		}
>> 		of_node_put(soc);
>> 	}
>
> Why go out of our way to fail on a childless soc node?

do we see any case in which we'd have a childless soc node?

I'm just concerned about make sure this works for all the various  
cases of #address-cells and #size-cells.

- k

Re: [PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Scott Wood]

Kumar Gala wrote:
>> Why go out of our way to fail on a childless soc node?
> 
> do we see any case in which we'd have a childless soc node?

It's possible that it could be used to communicate immrbase and soc 
type, but nothing else.  Not overly likely, but possible.

> I'm just concerned about make sure this works for all the various cases 
> of #address-cells and #size-cells.

It should.  We check the soc's #address-cells to skip the child bus 
address, and of_translate_address should take care of looking up the 
parent #address-cells.  The size portion of the ranges is ignored.

-Scott

Re: [PATCH] fsl_soc: Fix get_immrbase() to use ranges, rather than reg.

[Kumar Gala]

On Mon, 14 Jan 2008, Scott Wood wrote:

> The reg property in fsl soc nodes should be removed.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
>  arch/powerpc/sysdev/fsl_soc.c |   14 +++++++++++---
>  1 files changed, 11 insertions(+), 3 deletions(-)

applied.  (dropped the size == 12 check).


- k