[PATCH 1/1] sunxi: sun4i: Reduce cpu clock at SPL initialization to 144 MHz

[Andre Przywara]

On Wed, 31 Jan 2024 11:49:43 +0100
Ludwig Kormann <ludwig.kormann at ict42.de> wrote:

Hi Ludwig,

thanks for taking care and sending a patch, though I scratch my head about
this a bit. My main concern is why this would be an issue *now*, 11 years
after the A20's release, and with tons of boards out there in operation.
Also 144 MHz seem a somewhat drastic reduction?

> Up until now cpu clock gets initialized at 384 MHz, which is
> the highest supported cpu clock.

What do you mean with "highest supported"? Surely the A20 goes up to
960 MHz?
Also please note that 384 MHz is the PLL1 reset configuration, so it's not
something we came up with, but probably some safe value that Allwinner
burned into their chips.

> Recent A20 batches show an increased percentage of modules
> reacting very sensitive to operating conditions outside the
> specifications.

What are those specifications, exactly? Do you have any more reliable
data? The datasheet is very quiet on those conditions, it seems.
In particular, I couldn't find any official frequency/voltage
combinations, it seems like the values in the DTs are just passed on from
some BSP drop?

> The cpu dies very shortly after PLLs, core frequency or cpu
> voltage are missconfigured. E.g.:
> - uboot SPL selects 384 MHz as cpu clock which requires a cpu
>   voltage of at least 1.1 V.
> - Linux CPU Frequency scaling with most sun7i dts will reduce
>   cpu voltage down to 1.0 V.

How so? The mainline DT suggests 1.1V for anything above 312 MHz, and
even above 144 MHz for the BananaPi. Are you using any OPs that differ
from that?

> - When intiating a reboot or reset from linux the cpu voltage
>   may keep the 1.0 V configuration and the cpu dies during SPL
>   initialization.

Ah, so you mean we run (in Linux) on a 1.0V OP, probably at a very low CPU
frequency, and then the CPU cores reset, leaving the PMIC at 1.0V? And
then the SPL programs 384 MHz, which is too high, even for the brief period
until we program DCDC2 to 1.4V?
If you have evidence (those "newer batches"? A20 batches in 2024?) for
that, what about 312 MHz? Does that work?

> Therefore reduce cpu clock at uboot SPL initialization down
> to 144 MHz from 384 MHz.

I am bit concerned about slowing down the initial SPL phase that much, for
*all* A20 users. We run the DRAM init with that initial clock, even though
the voltage is already up at this point.

So if you see issues with those "newer batches" only(?), and since I
haven't heard about any issues about that before, can we make this a
Kconfig choice? We could make it simple, forcing K to 1, so we just need
to divide the frequency by 24 and shift by 8 to get to the register value?

> Signed-off-by: Ludwig Kormann <ludwig.kormann at ict42.de>
> ---
>  arch/arm/include/asm/arch-sunxi/clock_sun4i.h | 2 +-
>  arch/arm/mach-sunxi/clock_sun4i.c             | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> index 2cec91cb20..252c4c693e 100644
> --- a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
> @@ -141,7 +141,7 @@ struct sunxi_ccm_reg {
>  #define CCM_PLL1_CFG_SIG_DELT_PAT_EN_SHIFT	2
>  #define CCM_PLL1_CFG_FACTOR_M_SHIFT		0
>  
> -#define PLL1_CFG_DEFAULT	0xa1005000
> +#define PLL1_CFG_DEFAULT	0xa1004c01
>  
>  #if defined CONFIG_OLD_SUNXI_KERNEL_COMPAT && defined CONFIG_MACH_SUN5I
>  /*
> diff --git a/arch/arm/mach-sunxi/clock_sun4i.c b/arch/arm/mach-sunxi/clock_sun4i.c
> index 8f1d1b65f0..ac3b7a801f 100644
> --- a/arch/arm/mach-sunxi/clock_sun4i.c
> +++ b/arch/arm/mach-sunxi/clock_sun4i.c
> @@ -25,6 +25,7 @@ void clock_init_safe(void)
>  	       APB0_DIV_1 << APB0_DIV_SHIFT |
>  	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
>  	       &ccm->cpu_ahb_apb0_cfg);
> +	sdelay(20);

Where does that come from? Can you mention that in the commit message?

Cheers,
Andre

>  	writel(PLL1_CFG_DEFAULT, &ccm->pll1_cfg);
>  	sdelay(200);
>  	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
> @@ -32,6 +33,7 @@ void clock_init_safe(void)
>  	       APB0_DIV_1 << APB0_DIV_SHIFT |
>  	       CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT,
>  	       &ccm->cpu_ahb_apb0_cfg);
> +	sdelay(20);
>  #ifdef CONFIG_MACH_SUN7I
>  	setbits_le32(&ccm->ahb_gate0, 0x1 << AHB_GATE_OFFSET_DMA);
>  #endif