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

Hi Chris,
 
 On mer., janv. 10 2018, Chris Packham [off-list ref] wrote:

On 10/01/18 21:31, Gregory CLEMENT wrote:

quoted

Hi Chris,
  
  On mar., janv. 09 2018, Chris Packham [off-list ref] wrote:

quoted

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 at 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.

quoted

quoted

+				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