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

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Mon,  5 Sep 2016 01:32:39 +0100
Andre Przywara <andre.przywara@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.

Moreover this revert affects USB FEL boot, which currently uses 8K
of SRAM C as a backup storage. 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? 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. 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).

> 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@arm.com>

We are not fixing any bugs with this revert. Or do we?

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

-- 
Best regards,
Siarhei Siamashka