[PATCH] powerpc/4xx: DTS: Add Add'l SDRAM0 Compatible and Interrupt Info

[PATCH] powerpc/4xx: DTS: Add Add'l SDRAM0 Compatible and Interrupt Info

[Grant Erickson]

Added additional information for type and compatibility strings and
interrupt information to the SDRAM0 memory-controller device tree
nodes for AMCC PowerPC 405EX[r]-based boards to facilitate binding
with the new "ibm,sdram-4xx-ddr2" EDAC memory controller adapter driver.

Signed-off-by: Grant Erickson <gerickson@nuovations.com>
---
As support in the associated EDAC adapter driver is added over time,
similar changes will/should be made to the DTS files for boards
leveraging realizations of this "ibm,sdram-4xx-ddr2" controller,
including the 440SP, 440SPe, 460EX, 460GT and 460SX.

 arch/powerpc/boot/dts/haleakala.dts |   11 ++++++++++-
 arch/powerpc/boot/dts/kilauea.dts   |   11 ++++++++++-
 arch/powerpc/boot/dts/makalu.dts    |   11 ++++++++++-
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/haleakala.dts b/arch/powerpc/boot/dts/haleakala.dts
index 513bc43..e45ce7e 100644
--- a/arch/powerpc/boot/dts/haleakala.dts
+++ b/arch/powerpc/boot/dts/haleakala.dts
@@ -89,8 +89,17 @@
 		clock-frequency = <0>; /* Filled in by U-Boot */
 
 		SDRAM0: memory-controller {
-			compatible = "ibm,sdram-405exr";
+			device_type = "memory-controller";
+			compatible = "ibm,sdram-405exr", "ibm,sdram-4xx-ddr2";
 			dcr-reg = <0x010 0x002>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&SDRAM0>;
+			interrupts = <0x0 0x1>;
+			interrupt-map = </* ECCDED Error */ 0x0 &UIC2 0x5 0x4
+					 /* ECCSEC Error */ 0x1 &UIC2 0x6 0x4>;
+			interrupt-map-mask = <0xffffffff>;
 		};
 
 		MAL0: mcmal {
diff --git a/arch/powerpc/boot/dts/kilauea.dts b/arch/powerpc/boot/dts/kilauea.dts
index dececc4..d492ead 100644
--- a/arch/powerpc/boot/dts/kilauea.dts
+++ b/arch/powerpc/boot/dts/kilauea.dts
@@ -90,8 +90,17 @@
 		clock-frequency = <0>; /* Filled in by U-Boot */
 
 		SDRAM0: memory-controller {
-			compatible = "ibm,sdram-405ex";
+			device_type = "memory-controller";
+			compatible = "ibm,sdram-405ex", "ibm,sdram-4xx-ddr2";
 			dcr-reg = <0x010 0x002>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&SDRAM0>;
+			interrupts = <0x0 0x1>;
+			interrupt-map = </* ECCDED Error */ 0x0 &UIC2 0x5 0x4
+					 /* ECCSEC Error */ 0x1 &UIC2 0x6 0x4>;
+			interrupt-map-mask = <0xffffffff>;
 		};
 
 		MAL0: mcmal {
diff --git a/arch/powerpc/boot/dts/makalu.dts b/arch/powerpc/boot/dts/makalu.dts
index 945508c..52b9f32 100644
--- a/arch/powerpc/boot/dts/makalu.dts
+++ b/arch/powerpc/boot/dts/makalu.dts
@@ -90,8 +90,17 @@
 		clock-frequency = <0>; /* Filled in by U-Boot */
 
 		SDRAM0: memory-controller {
-			compatible = "ibm,sdram-405ex";
+			device_type = "memory-controller";
+			compatible = "ibm,sdram-405ex", "ibm,sdram-4xx-ddr2";
 			dcr-reg = <0x010 0x002>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&SDRAM0>;
+			interrupts = <0x0 0x1>;
+			interrupt-map = </* ECCDED Error */ 0x0 &UIC2 0x5 0x4
+					 /* ECCSEC Error */ 0x1 &UIC2 0x6 0x4>;
+			interrupt-map-mask = <0xffffffff>;
 		};
 
 		MAL0: mcmal {
-- 
1.6.0.4

Re: [PATCH] powerpc/4xx: DTS: Add Add'l SDRAM0 Compatible and Interrupt Info

[David Gibson]

On Wed, Dec 17, 2008 at 11:56:07AM -0800, Grant Erickson wrote:
> Added additional information for type and compatibility strings and
> interrupt information to the SDRAM0 memory-controller device tree
> nodes for AMCC PowerPC 405EX[r]-based boards to facilitate binding
> with the new "ibm,sdram-4xx-ddr2" EDAC memory controller adapter driver.
> 
> Signed-off-by: Grant Erickson <gerickson@nuovations.com>
> ---
> As support in the associated EDAC adapter driver is added over time,
> similar changes will/should be made to the DTS files for boards
> leveraging realizations of this "ibm,sdram-4xx-ddr2" controller,
> including the 440SP, 440SPe, 460EX, 460GT and 460SX.
> 
>  arch/powerpc/boot/dts/haleakala.dts |   11 ++++++++++-
>  arch/powerpc/boot/dts/kilauea.dts   |   11 ++++++++++-
>  arch/powerpc/boot/dts/makalu.dts    |   11 ++++++++++-
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/haleakala.dts b/arch/powerpc/boot/dts/haleakala.dts
> index 513bc43..e45ce7e 100644
> --- a/arch/powerpc/boot/dts/haleakala.dts
> +++ b/arch/powerpc/boot/dts/haleakala.dts
> @@ -89,8 +89,17 @@
>  		clock-frequency = <0>; /* Filled in by U-Boot */
>  
>  		SDRAM0: memory-controller {
> -			compatible = "ibm,sdram-405exr";
> +			device_type = "memory-controller";

This should not have a device_type.

> +			compatible = "ibm,sdram-405exr", "ibm,sdram-4xx-ddr2";
>  			dcr-reg = <0x010 0x002>;
> +			#address-cells = <0>;
> +			#size-cells = <0>;

This seems odd.  These should only be present if the node does, or at
least can, have subnodes - I don't see that it would.

> +			#interrupt-cells = <1>;
> +			interrupt-parent = <&SDRAM0>;
> +			interrupts = <0x0 0x1>;
> +			interrupt-map = </* ECCDED Error */ 0x0 &UIC2 0x5 0x4
> +					 /* ECCSEC Error */ 0x1 &UIC2 0x6 0x4>;
> +			interrupt-map-mask = <0xffffffff>;
>  		};

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] powerpc/4xx: DTS: Add Add'l SDRAM0 Compatible and Interrupt Info

[Grant Erickson]

On 12/17/08 3:46 PM, David Gibson wrote:
> On Wed, Dec 17, 2008 at 11:56:07AM -0800, Grant Erickson wrote:
>> Added additional information for type and compatibility strings and
>> interrupt information to the SDRAM0 memory-controller device tree
>> nodes for AMCC PowerPC 405EX[r]-based boards to facilitate binding
>> with the new "ibm,sdram-4xx-ddr2" EDAC memory controller adapter driver.
>> 
>> Signed-off-by: Grant Erickson <gerickson@nuovations.com>
>> ---
>> As support in the associated EDAC adapter driver is added over time,
>> similar changes will/should be made to the DTS files for boards
>> leveraging realizations of this "ibm,sdram-4xx-ddr2" controller,
>> including the 440SP, 440SPe, 460EX, 460GT and 460SX.
>> 
>>  arch/powerpc/boot/dts/haleakala.dts |   11 ++++++++++-
>>  arch/powerpc/boot/dts/kilauea.dts   |   11 ++++++++++-
>>  arch/powerpc/boot/dts/makalu.dts    |   11 ++++++++++-
>>  3 files changed, 30 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/powerpc/boot/dts/haleakala.dts
>> b/arch/powerpc/boot/dts/haleakala.dts
>> index 513bc43..e45ce7e 100644
>> --- a/arch/powerpc/boot/dts/haleakala.dts
>> +++ b/arch/powerpc/boot/dts/haleakala.dts
>> @@ -89,8 +89,17 @@
>> clock-frequency = <0>; /* Filled in by U-Boot */
>>  
>> SDRAM0: memory-controller {
>> -   compatible = "ibm,sdram-405exr";
>> +   device_type = "memory-controller";
> 
> This should not have a device_type.

I'm still growing my device tree expertise. Can you elaborate on why SDRAM0
shouldn't be described generically as a "memory-controller" device in the
same way the EMAC0 is generically described as a "network" device? A URL to
said elaboration would be sufficient.

>> +   compatible = "ibm,sdram-405exr", "ibm,sdram-4xx-ddr2";
>> dcr-reg = <0x010 0x002>;
>> +   #address-cells = <0>;
>> +   #size-cells = <0>;
> 
> This seems odd.  These should only be present if the node does, or at
> least can, have subnodes - I don't see that it would.

See above qualifier; will remove.

Thanks for the prompt review!

Regards,

Grant

Re: [PATCH] powerpc/4xx: DTS: Add Add'l SDRAM0 Compatible and Interrupt Info

[David Gibson]

On Wed, Dec 17, 2008 at 04:09:05PM -0800, Grant Erickson wrote:
> On 12/17/08 3:46 PM, David Gibson wrote:
> > On Wed, Dec 17, 2008 at 11:56:07AM -0800, Grant Erickson wrote:
> >> Added additional information for type and compatibility strings and
> >> interrupt information to the SDRAM0 memory-controller device tree
> >> nodes for AMCC PowerPC 405EX[r]-based boards to facilitate binding
> >> with the new "ibm,sdram-4xx-ddr2" EDAC memory controller adapter driver.
> >> 
> >> Signed-off-by: Grant Erickson <gerickson@nuovations.com>
> >> ---
> >> As support in the associated EDAC adapter driver is added over time,
> >> similar changes will/should be made to the DTS files for boards
> >> leveraging realizations of this "ibm,sdram-4xx-ddr2" controller,
> >> including the 440SP, 440SPe, 460EX, 460GT and 460SX.
> >> 
> >>  arch/powerpc/boot/dts/haleakala.dts |   11 ++++++++++-
> >>  arch/powerpc/boot/dts/kilauea.dts   |   11 ++++++++++-
> >>  arch/powerpc/boot/dts/makalu.dts    |   11 ++++++++++-
> >>  3 files changed, 30 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/boot/dts/haleakala.dts
> >> b/arch/powerpc/boot/dts/haleakala.dts
> >> index 513bc43..e45ce7e 100644
> >> --- a/arch/powerpc/boot/dts/haleakala.dts
> >> +++ b/arch/powerpc/boot/dts/haleakala.dts
> >> @@ -89,8 +89,17 @@
> >> clock-frequency = <0>; /* Filled in by U-Boot */
> >>  
> >> SDRAM0: memory-controller {
> >> -   compatible = "ibm,sdram-405exr";
> >> +   device_type = "memory-controller";
> > 
> > This should not have a device_type.
> 
> I'm still growing my device tree expertise. Can you elaborate on why SDRAM0
> shouldn't be described generically as a "memory-controller" device in the
> same way the EMAC0 is generically described as a "network" device? A URL to
> said elaboration would be sufficient.

Because "device_type" isn't just a generic description of the device.
"device_type" is used in actual OF implementations to specify what OF
method interface the device supports.  Since with a flattened tree
there is no method interface, device_type is not appropriate.  For
long-established device_type values, like "network" we keep it in
because there's software that expects to see it, but unless there is a
defined and supported OF method interface binding, device_type should
not be added for anything new.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson