| Message ID | 1229543767-1617-1-git-send-email-gerickson@nuovations.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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>; > };
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
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.
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 {
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(-)