On Wed, 31 Jan 2024 11:49:43 +0100 Ludwig Kormann ludwig.kormann@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@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