On Wed, 31 Jan 2024 14:26:02 +0100 Ludwig Kormann ludwig.kormann@ict42.de wrote:
Hi,
thanks for your feedback!
thanks for the quick reply!
Am 31.01.24 um 13:36 schrieb Andre Przywara:
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?
We began seeing this issue beginning in early 2023 and it seems to affect only a very small percentage of the units. We had to introduce this patch for our customers and wanted to also share it with the community.
Thanks for that, much appreciated.
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?
You're right, I must have mixed something up there.
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?
Yes, it's hard/impossible to find any reliable information on this. Our main reference have been the values in the DTs.
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?
Yes, the CPU dies before the voltage gets updated.
If you have evidence (those "newer batches"? A20 batches in 2024?) for that, what about 312 MHz? Does that work?
The batches are actually from 2022+. We went for 144MHz as it's the lowest of the "default" speeds, that also ensures we're "low enough" to (hopefully) never trigger the issue again. It seems like there's some variation in A20 production that triggers the issue and as we don't know any "official" voltage/frequency limits it's better to have some safety margin.
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.
In my opinion the impact / additional delay for the initial SPL phase should not be in a very relevant range actually, as it usually only takes a few hundred milliseconds.
Well, I heard in the past about users that care a lot about boot times, and were looking for ways to shave of a few dozen milliseconds from the boot. So a "few hundred ms" would probably upset them. And while I personally don't really care about this range either, there are a lot of A20 users out there, so I want to keep the disturbance as low as possible.
But you're right of course, this would force the lower value onto all A20 users.
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?
I will try to look into this and provide an update.
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?
This delay is required after switching the clock source.
See "A20 Reference manual v1.4" Page 50 / section "1.5.4.16. CPU/AHB/APB0 CLOCK RATIO(DEFAULT: 0X00010010)": "If the clock source is changed, at most to wait for 8 present running clock cycles."
Also these delays are already correctly beeing used later on in the same file e.g. within clock_set_pll1(...) on line 153:
Ah, very good, thanks for digging that out. Can you make this a separate patch, then? I am happy to merge this ASAP, and this wouldn't need to be held up on this discussion here.
Cheers, Andre.
/* Switch to 24MHz clock while changing PLL1 */ writel(AXI_DIV_1 << AXI_DIV_SHIFT | AHB_DIV_2 << AHB_DIV_SHIFT | APB0_DIV_1 << APB0_DIV_SHIFT | CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT, &ccm->cpu_ahb_apb0_cfg); sdelay(20);
kind regards
Ludwig
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