[U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64"

[Andre Przywara]

Hi,

On 05/09/16 05:48, Siarhei Siamashka wrote:
> On Mon,  5 Sep 2016 01:32:39 +0100
> Andre Przywara <andre.przywara at arm.com> wrote:
> 
>> Now that we don't use SRAM C for the SPL stack anymore, there is no
>> need to clock down AHB1 to 100 MHz.
> 
> It's just another way to say it, but we are not clocking the AHB1
> down, but rather keeping it at a failsafe default. If I understand it
> correctly, reclocking the AHB1 to 200 MHz early in the SPL code is
> not really necessary for the v2016.09 release and does not fix anything.

Yes, this patch is indeed somewhat optional. It was just a logical
consequence of the other patch being reverted.

> Moreover this revert affects USB FEL boot, which currently uses 8K
> of SRAM C as a backup storage.

Ah, that may explain the FEL issues I was trying to track down on the
weekend.

> Yes, we can also move the FEL backup
> storage to SRAM A2, but the real question is whether we really want
> to have it this way in the long run.
> 
> Is it really needed to reclock AHB1 early in the SPL? Can't ATF or
> U-Boot proper do this a bit later?

Good point. My current ATF branch at home sets up all the basic clocks
to their desired values, so indeed U-Boot wouldn't need to touch the
clocks anymore (also I think it shouldn't care).
In my current setup ATF is expected to run after the SPL, but before the
U-Boot proper.

> Also it's best to unmap the SRAM C
> from the CPU address space at the same time as the AHB1 is reclocked
> to 200 MHz. So that nobody can accidentally access it.

Yes, that sounds reasonable.

> There is a
> special "bootmode" bit, which configures the SRAM C mapping:
> 
>     https://irclog.whitequark.org/linux-sunxi/2016-06-29#16863309;
> 
> Writing 0x00000000 to 0x01c00004 hides the SRAM C from the CPU. And
> writing 0x01000000 there enables it again. Some SRAM C experiments
> can be done with the sunxi-fel tool:
> 
> $ sunxi-fel hex 0x18000 64
> 00018000: 32 a6 21 9a da f7 16 d7 7c 59 e9 b3 db a2 5f 9e  2.!.....|Y...._.
> 00018010: d2 0c 54 b8 79 7e 7a ff 7f 0d b5 e7 96 27 34 c8  ..T.y~z......'4.
> 00018020: c7 d1 66 f8 4b dc a6 cb d5 ba e3 ce 26 18 49 c4  ..f.K.......&.I.
> 00018030: 4f f6 3f a9 56 f7 92 d4 50 b6 7f e6 73 e8 e5 69  O.?.V...P...s..i
> 
> $ sunxi-fel fill 0x18000 64 0xCC && sunxi-fel hex 0x18000 64
> 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 
> $ sunxi-fel writel 0x01c00004 0x00000000 && sunxi-fel hex 0x18000 64
> 00018000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 00018030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> 
> $ sunxi-fel writel 0x01c00004 0x01000000 && sunxi-fel hex 0x18000 64
> 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
> 
> The commands above first show the uninitialized garbage in the SRAM C,
> then initialize it to 0xCC, then disable the SRAM C access from the
> CPU (it is then read as all zeros), then enable it back (it is read
> as 0xCC again).

Nice one! Didn't know about that. I will do some experiments tonight.

>> Keeping it at the recommended 200 MHz allows faster peripherals.
> 
> I usually prefer to see some performance numbers in such commit
> messages ;) Not that I doubt that something becomes faster, but it
> is always interesting to know how big is the real practical effect.
> 
>> This reverts commit 5bc88cc2be3a962005b6e5768e06ca8f6ffcb88d.
>>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> 
> We are not fixing any bugs with this revert. Or do we?

Probably not. Let me give it actually a try without this patch and then
we can drop this one.

Thanks,
Andre.

>> ---
>>  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> index be9fcfd..f7e93b0 100644
>> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h
>> @@ -230,12 +230,7 @@ struct sunxi_ccm_reg {
>>  #define CCM_PLL5_TUN_INIT_FREQ(x)	(((x) & 0x7f) << 16)
>>  #define CCM_PLL5_TUN_INIT_FREQ_MASK	CCM_PLL5_TUN_INIT_FREQ(0x7f)
>>  
>> -#if defined(CONFIG_MACH_SUN50I)
>> -/* AHB1=100MHz failsafe setup from the FEL mode, usable with PMIC defaults */
>> -#define AHB1_ABP1_DIV_DEFAULT		0x00003190 /* AHB1=PLL6/6,APB1=AHB1/2 */
>> -#else
>>  #define AHB1_ABP1_DIV_DEFAULT		0x00003180 /* AHB1=PLL6/3,APB1=AHB1/2 */
>> -#endif
>>  
>>  #define AXI_GATE_OFFSET_DRAM		0
>>  
>