[U-Boot] [PATCH 3/5] sun6i: dram: Do not try to initialize a second dram chan on A31s

[Ian Campbell]

On Tue, 2014-11-18 at 13:34 +0100, Hans de Goede wrote:
> Hi,
> 
> On 11/17/2014 08:00 PM, Ian Campbell wrote:
> > On Sun, 2014-11-16 at 17:16 +0100, Hans de Goede wrote:
> >> The A31s only has one dram channel, so do not bother with trying to initalize
> > 
> > "initialize"
> 
> Fixed in my local tree.
> 
> >> a second channel.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> >> ---
> >>  arch/arm/cpu/armv7/sunxi/Makefile     |  2 +-
> >>  arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 11 +++++++++--
> >>  2 files changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
> >> index 3b6ae47..1337b60 100644
> >> --- a/arch/arm/cpu/armv7/sunxi/Makefile
> >> +++ b/arch/arm/cpu/armv7/sunxi/Makefile
> >> @@ -10,6 +10,7 @@
> >>  obj-y	+= timer.o
> >>  obj-y	+= board.o
> >>  obj-y	+= clock.o
> >> +obj-y	+= cpu_info.o
> >>  obj-y	+= pinmux.o
> >>  obj-$(CONFIG_MACH_SUN6I)	+= prcm.o
> >>  obj-$(CONFIG_MACH_SUN8I)	+= prcm.o
> >> @@ -21,7 +22,6 @@ obj-$(CONFIG_MACH_SUN7I)	+= clock_sun4i.o
> >>  obj-$(CONFIG_MACH_SUN8I)	+= clock_sun6i.o
> >>  
> >>  ifndef CONFIG_SPL_BUILD
> >> -obj-y	+= cpu_info.o
> >>  ifdef CONFIG_ARMV7_PSCI
> >>  obj-y	+= psci.o
> >>  endif
> >> diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> >> index 30439dc..2ac0b58 100644
> >> --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> >> +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c
> >> @@ -372,10 +372,15 @@ unsigned long sunxi_dram_init(void)
> >>  		.rows = 16,
> >>  	};
> >>  
> >> +	/* A31s only has one channel */
> >> +	if (sunxi_get_ss_bonding_id() == SUNXI_SS_BOND_ID_A31S)
> >> +		para.chan = 1;
> > 
> > mctl_channel_init seems to contain some auto detection code, I suppose
> > that doesn't work in this case for some reason? Or is this just an
> > optimisation? In which case is the benefit just quicker to boot?
> 
> My assumption was that it would not work, as the A31s has only one
> channel, or so the datasheets claim. But it turned out it does work,
> so they may be using the same die in a different package, I'll go
> and mail Allwinner and see if they are willing to disclose anything
> about this (knowing it is the same die would be useful).
> 
> So in the end this is only an optimization.
> 
> > 
> >> +
> >>  	mctl_sys_init();
> >>  
> >>  	mctl_dll_init(0, &para);
> >> -	mctl_dll_init(1, &para);
> >> +	if (para.chan == 2)
> >> +		mctl_dll_init(1, &para);
> > 
> > Both this an the next one are basically unrolled for-loops over
> > 0-para.chan now. I suppose it doesn't really matter.
> 
> True, note that in mctl_com_init() we've:
> 
>         if (para->chan == 1) {
>                 /* Shutdown channel 1 */
> 		...
> 	}
> 
> So I would prefer to keep this as is (rather then turn it into a loop),
> for consistency.

OK.

(IMHO that shutdown should be pulled up to before/after the loop, but
lets not block these patches on that.)

Ian.