diff mbox

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

Message ID 1416154613-8506-3-git-send-email-hdegoede@redhat.com
State Superseded
Delegated to: Ian Campbell
Headers show

Commit Message

Hans de Goede Nov. 16, 2014, 4:16 p.m. UTC 
The A31s only has one dram channel, so do not bother with trying to initalize
a second channel.

Signed-off-by: Hans de Goede <hdegoede@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(-)

Comments

Ian Campbell Nov. 17, 2014, 7 p.m. UTC | #1 
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"

> a second channel.
> 
> Signed-off-by: Hans de Goede <hdegoede@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?

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

Ian.
Hans de Goede Nov. 18, 2014, 12:34 p.m. UTC | #2 
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@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.

Regards,

Hans
Ian Campbell Nov. 18, 2014, 2:08 p.m. UTC | #3 
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@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.
diff mbox

Patch

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_sys_init();
 
 	mctl_dll_init(0, &para);
-	mctl_dll_init(1, &para);
+	if (para.chan == 2)
+		mctl_dll_init(1, &para);
 
 	setbits_le32(&mctl_com->ccr,
 		     MCTL_CCR_MASTER_CLK_EN |
@@ -383,7 +388,9 @@  unsigned long sunxi_dram_init(void)
 		     MCTL_CCR_CH1_CLK_EN);
 
 	mctl_channel_init(0, &para);
-	mctl_channel_init(1, &para);
+	if (para.chan == 2)
+		mctl_channel_init(1, &para);
+
 	mctl_com_init(&para);
 	mctl_port_cfg();