[PATCH 0/3] ARM: dts: SDRAM and L2 cache EDAC for Armada SoCs

[PATCH 0/3] ARM: dts: SDRAM and L2 cache EDAC for Armada SoCs

[Chris Packham]

This series was waiting for the armada_xp edac driver to be accepted.
Now that it has the relevant nodes can be added to the Armada SoCs. So
that boards can use the EDAC driver if they have the hardware support.

The db-xc3-24g4xg.dts board doesn't have an ECC chip for it's DDR but it
can use the L2 cache parity and ecc support.

Chris Packham (3):
  ARM: dts: armada-xp: enable L2 cache parity and ecc on db-xc3-24g4xg
  ARM: dts: mvebu: add sdram controller node to Armada-38x
  ARM: dts: armada-xp: add label to sdram-controller node

 arch/arm/boot/dts/armada-38x.dtsi             | 5 +++++
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi     | 2 +-
 arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
 arch/arm/boot/dts/armada-xp.dtsi              | 2 +-
 4 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.23.0

[PATCH 1/3] ARM: dts: armada-xp: enable L2 cache parity and ecc on db-xc3-24g4xg

[Chris Packham]

Enable L2 cache parity and ECC on the db-xc3-24g4xg board so that cache
operations are protected and errors can be flagged to the EDAC
subsystem.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
index df048050615f..4ec0ae01b61d 100644
--- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
+++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts
@@ -33,6 +33,11 @@
 	};
 };
 
+&L2 {
+	arm,parity-enable;
+	marvell,ecc-enable;
+};
+
 &devbus_bootcs {
 	status = "okay";
 
-- 
2.23.0

[PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x

[Chris Packham]

The Armada-38x uses an SDRAM controller that is compatible with the
Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
is 32/16). The SDRAM controller registers are the same between the two
SoCs.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 3f4bb44d85f0..e038abc0c6b4 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -103,6 +103,11 @@
 			#size-cells = <1>;
 			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
 
+			sdramc: sdramc@1400 {
+				compatible = "marvell,armada-xp-sdram-controller";
+				reg = <0x1400 0x500>;
+			};
+
 			L2: cache-controller@8000 {
 				compatible = "arm,pl310-cache";
 				reg = <0x8000 0x1000>;
-- 
2.23.0

[PATCH 3/3] ARM: dts: armada-xp: add label to sdram-controller node

[Chris Packham]

Add the label "sdramc" to the sdram-controller nodes for the Armada-XP
and 98dx3236 SoCs.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +-
 arch/arm/boot/dts/armada-xp.dtsi          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index 267d0c178e55..654648b05c7c 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -90,7 +90,7 @@
 		};
 
 		internal-regs {
-			sdramc@1400 {
+			sdramc: sdramc@1400 {
 				compatible = "marvell,armada-xp-sdram-controller";
 				reg = <0x1400 0x500>;
 			};
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index ee15c77d3689..6c19984d668e 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -36,7 +36,7 @@
 		};
 
 		internal-regs {
-			sdramc@1400 {
+			sdramc: sdramc@1400 {
 				compatible = "marvell,armada-xp-sdram-controller";
 				reg = <0x1400 0x500>;
 			};
-- 
2.23.0

Re: [PATCH 0/3] ARM: dts: SDRAM and L2 cache EDAC for Armada SoCs

[Gregory CLEMENT]

Hi Chris,

> This series was waiting for the armada_xp edac driver to be accepted.
> Now that it has the relevant nodes can be added to the Armada SoCs. So
> that boards can use the EDAC driver if they have the hardware support.
>
> The db-xc3-24g4xg.dts board doesn't have an ECC chip for it's DDR but it
> can use the L2 cache parity and ecc support.
>
> Chris Packham (3):
>   ARM: dts: armada-xp: enable L2 cache parity and ecc on db-xc3-24g4xg
>   ARM: dts: mvebu: add sdram controller node to Armada-38x
>   ARM: dts: armada-xp: add label to sdram-controller node

Series applied on mvebu/dt

Thanks,

Gregory


>
>  arch/arm/boot/dts/armada-38x.dtsi             | 5 +++++
>  arch/arm/boot/dts/armada-xp-98dx3236.dtsi     | 2 +-
>  arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
>  arch/arm/boot/dts/armada-xp.dtsi              | 2 +-
>  4 files changed, 12 insertions(+), 2 deletions(-)
>
> -- 
> 2.23.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Gregory Clement, Bootlin
Embedded Linux and Kernel engineering
http://bootlin.com

[PATCH 0/3] EDAC: support for Armada 38x and 98dx3236 SoCs

[Chris Packham]

This series applies on top of Jan Lubbe's "EDAC drivers for Armada XP L2 and
DDR" series[1].

The Armada 38x as well as the 98dx3236 and similar switch chips with integrated
CPUs use the same SDRAM controller block as the Armada XP. The key difference
is the width of the DDR interface.

[1] - https://marc.info/?l=linux-edac&m=151030475715706&w=2

Chris Packham (3):
  ARM: dts: enable L2 cache parity and ecc on db-xc3-24g4xg board
  ARM: dts: mvebu: add sdram controller node to Armada-38x
  EDAC: armada_xp: Add support for more SoCs

 arch/arm/boot/dts/armada-38x.dtsi             | 5 +++++
 arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 5 +++++
 drivers/edac/armada_xp_edac.c                 | 5 +++++
 3 files changed, 15 insertions(+)

-- 
2.15.1

[PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x

[Chris Packham]

The Armada-38x uses an SDRAM controller that is compatible with the
Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
is 32/16). The SDRAM controller registers are the same between the two
SoCs.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 00ff549d4e39..6d34c5ec178f 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -138,6 +138,11 @@
 			#size-cells = <1>;
 			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
 
+			sdramc@1400 {
+				compatible = "marvell,armada-xp-sdram-controller";
+				reg = <0x1400 0x500>;
+			};
+
 			L2: cache-controller@8000 {
 				compatible = "arm,pl310-cache";
 				reg = <0x8000 0x1000>;
-- 
2.15.1

Re: [PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x

[Gregory CLEMENT]

Hi Chris,
 
 On mar., janv. 09 2018, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:

> The Armada-38x uses an SDRAM controller that is compatible with the
> Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
> is 32/16). The SDRAM controller registers are the same between the two
> SoCs.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index 00ff549d4e39..6d34c5ec178f 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -138,6 +138,11 @@
>  			#size-cells = <1>;
>  			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
>  
> +			sdramc@1400 {

Could you add a label? Thanks to this it would be possible to
enable/disable it at board level in a esay way.

> +				compatible = "marvell,armada-xp-sdram-controller";
> +				reg = <0x1400 0x500>;

What about adding status = "disabled" ?

Thanks to this we can enable it at board level only if we really want
it, it would avoid nasty regression on boards that don't need it, if an
issue occurs. Unless you are sure that it is completely safe to enable
it for everyone.

Thanks,

Gregory


> +			};
> +
>  			L2: cache-controller@8000 {
>  				compatible = "arm,pl310-cache";
>  				reg = <0x8000 0x1000>;
> -- 
> 2.15.1
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x

[Chris Packham]

On 10/01/18 21:31, Gregory CLEMENT wrote:
> Hi Chris,
>   
>   On mar., janv. 09 2018, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
> 
>> The Armada-38x uses an SDRAM controller that is compatible with the
>> Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
>> is 32/16). The SDRAM controller registers are the same between the two
>> SoCs.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
>> index 00ff549d4e39..6d34c5ec178f 100644
>> --- a/arch/arm/boot/dts/armada-38x.dtsi
>> +++ b/arch/arm/boot/dts/armada-38x.dtsi
>> @@ -138,6 +138,11 @@
>>   			#size-cells = <1>;
>>   			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
>>   
>> +			sdramc@1400 {
> 
> Could you add a label? Thanks to this it would be possible to
> enable/disable it at board level in a esay way.
> 

Sure. Any suggestions for a name better than "sdramc:"?

It's probably worth adding the same label to armada-xp.dtsi and 
armada-xp-98dx3236.dtsi.

>> +				compatible = "marvell,armada-xp-sdram-controller";
>> +				reg = <0x1400 0x500>;
> 
> What about adding status = "disabled" ?
> 
> Thanks to this we can enable it at board level only if we really want
> it, it would avoid nasty regression on boards that don't need it, if an
> issue occurs. Unless you are sure that it is completely safe to enable
> it for everyone.

The EDAC driver (which is default n) will not probe the device if ECC 
has not been enabled so that should be safe.

Other than the EDAC driver the only other code that looks at this is in 
arch/arm/mach-mvebu/pm.c and it almost seems like an omission that this 
code is not active on armada-38x. The armada-38x platforms I have access 
to don't use suspend/resume so I can't verify this.

Re: [PATCH 2/3] ARM: dts: mvebu: add sdram controller node to Armada-38x

[Gregory CLEMENT]

Hi Chris,
 
 On mer., janv. 10 2018, Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:

> On 10/01/18 21:31, Gregory CLEMENT wrote:
>> Hi Chris,
>>   
>>   On mar., janv. 09 2018, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote:
>> 
>>> The Armada-38x uses an SDRAM controller that is compatible with the
>>> Armada-XP. The key difference is the width of the bus (XP is 64/32, 38x
>>> is 32/16). The SDRAM controller registers are the same between the two
>>> SoCs.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   arch/arm/boot/dts/armada-38x.dtsi | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
>>> index 00ff549d4e39..6d34c5ec178f 100644
>>> --- a/arch/arm/boot/dts/armada-38x.dtsi
>>> +++ b/arch/arm/boot/dts/armada-38x.dtsi
>>> @@ -138,6 +138,11 @@
>>>   			#size-cells = <1>;
>>>   			ranges = <0 MBUS_ID(0xf0, 0x01) 0 0x100000>;
>>>   
>>> +			sdramc@1400 {
>> 
>> Could you add a label? Thanks to this it would be possible to
>> enable/disable it at board level in a esay way.
>> 
>
> Sure. Any suggestions for a name better than "sdramc:"?

For me sdramc: is fine.

>
> It's probably worth adding the same label to armada-xp.dtsi and 
> armada-xp-98dx3236.dtsi.

Right.

>
>>> +				compatible = "marvell,armada-xp-sdram-controller";
>>> +				reg = <0x1400 0x500>;
>> 
>> What about adding status = "disabled" ?
>> 
>> Thanks to this we can enable it at board level only if we really want
>> it, it would avoid nasty regression on boards that don't need it, if an
>> issue occurs. Unless you are sure that it is completely safe to enable
>> it for everyone.
>
> The EDAC driver (which is default n) will not probe the device if ECC 
> has not been enabled so that should be safe.

OK in this case no need to disable it by default.

Thanks,

Gregory

>
> Other than the EDAC driver the only other code that looks at this is in 
> arch/arm/mach-mvebu/pm.c and it almost seems like an omission that this 
> code is not active on armada-38x. The armada-38x platforms I have access 
> to don't use suspend/resume so I can't verify this.
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com