Hi Andre,
thanks for your feedback!
Am 31.01.24 um 13:36 schrieb Andre Przywara:
On Wed, 31 Jan 2024 11:49:43 +0100
Ludwig Kormann <ludwig.korm...@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.
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.
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.korm...@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:
/* 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
