| Message ID | 20240201133014.578083-1-ludwig.kormann@ict42.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] sunxi: sun4i: Introduce KConfig option SPL_SYS_CLK_FREQ | expand |
On Thu, Feb 01, 2024 at 02:30:14PM +0100, Ludwig Kormann wrote: > This option can be used to modify the initial SPL > CPU clock frequency. > > This follows an earlier discussion regarding A20 > CPUs dying after reboot in SPL initialization due to > incompatible CPU clock frequency and core voltage. [1] > > First attempt was to update PLL1_CFG_DEFAULT to a fixed > lower frequency (144MHz), which fixed the observed issue > but might not suit all A20 users. A KConfig option > should be the better solution. > > [1] > https://lists.denx.de/pipermail/u-boot/2024-January/544897.html > > Signed-off-by: Ludwig Kormann <ludwig.kormann@ict42.de> [snip] > +if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I > +config SPL_SYS_CLK_FREQ > + int "sunxi SPL CPU clock frequency" > + default 384 > + ---help--- > + A static value for the sunxi SPL CPU frequency, must be a multiple of 24. > +endif This should really just be "depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I" and then "help".
On Thu, 1 Feb 2024 14:30:14 +0100 Ludwig Kormann <ludwig.kormann@ict42.de> wrote: Hi, > This option can be used to modify the initial SPL > CPU clock frequency. > > This follows an earlier discussion regarding A20 > CPUs dying after reboot in SPL initialization due to > incompatible CPU clock frequency and core voltage. [1] > > First attempt was to update PLL1_CFG_DEFAULT to a fixed > lower frequency (144MHz), which fixed the observed issue > but might not suit all A20 users. A KConfig option > should be the better solution. Can you please reword this to include more vital information directly in the commit message? We had mailing list archives vanish in the past. Please mention that our default frequency of 384 MHz does not work at every voltage, and after just a core reset the PMIC might still use a previously programmed lower voltage, that will hang the SPL, on some batches of chips. > [1] > https://lists.denx.de/pipermail/u-boot/2024-January/544897.html Please use the Link: tag: Link: https://lists.denx.de/.... > > Signed-off-by: Ludwig Kormann <ludwig.kormann@ict42.de> > --- > arch/arm/include/asm/arch-sunxi/clock_sun4i.h | 4 ++++ > arch/arm/mach-sunxi/Kconfig | 8 ++++++++ > arch/arm/mach-sunxi/clock_sun4i.c | 11 ++++++++++- > 3 files changed, 22 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..11b350824e 100644 > --- a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h > +++ b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h > @@ -134,12 +134,16 @@ struct sunxi_ccm_reg { > #define CCM_PLL1_CFG_PLL4_EXCH_SHIFT 25 > #define CCM_PLL1_CFG_BIAS_CUR_SHIFT 20 > #define CCM_PLL1_CFG_DIVP_SHIFT 16 > +#define CCM_PLL1_CFG_DIVP_MASK (0x3 << CCM_PLL1_CFG_DIVP_SHIFT) > #define CCM_PLL1_CFG_LCK_TMR_SHIFT 13 > #define CCM_PLL1_CFG_FACTOR_N_SHIFT 8 > +#define CCM_PLL1_CFG_FACTOR_N_MASK (0x1f << CCM_PLL1_CFG_FACTOR_N_SHIFT) > #define CCM_PLL1_CFG_FACTOR_K_SHIFT 4 > +#define CCM_PLL1_CFG_FACTOR_K_MASK (0x3 << CCM_PLL1_CFG_FACTOR_K_SHIFT) > #define CCM_PLL1_CFG_SIG_DELT_PAT_IN_SHIFT 3 > #define CCM_PLL1_CFG_SIG_DELT_PAT_EN_SHIFT 2 > #define CCM_PLL1_CFG_FACTOR_M_SHIFT 0 > +#define CCM_PLL1_CFG_FACTOR_M_MASK (0x3 << CCM_PLL1_CFG_FACTOR_M_SHIFT) > > #define PLL1_CFG_DEFAULT 0xa1005000 > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index fe89aec6b9..85e3a26855 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -705,6 +705,14 @@ config SYS_CLK_FREQ > default 1008000000 if MACH_SUN50I_H616 > default 1008000000 if MACH_SUN8I_R528 > > +if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I As Tom already mentioned, please use "depends on". > +config SPL_SYS_CLK_FREQ > + int "sunxi SPL CPU clock frequency" "sunxi" is redundant here. Please state that this is the early SPL frequency, since we switch to SYS_CLK_FREQ in the SPL still, only later. > + default 384 > + ---help--- > + A static value for the sunxi SPL CPU frequency, must be a multiple of 24. Minus sunxi, plus early, as above. Also please mention the safe frequency (144 MHz) here, as a recommendation if people experience problems. > +endif > + > config SYS_CONFIG_NAME > default "suniv" if MACH_SUNIV > default "sun4i" if MACH_SUN4I > diff --git a/arch/arm/mach-sunxi/clock_sun4i.c b/arch/arm/mach-sunxi/clock_sun4i.c > index 8f1d1b65f0..04623c1d09 100644 > --- a/arch/arm/mach-sunxi/clock_sun4i.c > +++ b/arch/arm/mach-sunxi/clock_sun4i.c > @@ -25,7 +25,16 @@ void clock_init_safe(void) > APB0_DIV_1 << APB0_DIV_SHIFT | > CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT, > &ccm->cpu_ahb_apb0_cfg); > - writel(PLL1_CFG_DEFAULT, &ccm->pll1_cfg); > + writel((PLL1_CFG_DEFAULT & > + ~(CCM_PLL1_CFG_FACTOR_N_MASK | > + CCM_PLL1_CFG_FACTOR_K_MASK | > + CCM_PLL1_CFG_FACTOR_M_MASK | > + CCM_PLL1_CFG_DIVP_MASK)) | > + (CONFIG_SPL_SYS_CLK_FREQ / 24) << CCM_PLL1_CFG_FACTOR_N_SHIFT | > + 0 << CCM_PLL1_CFG_FACTOR_K_SHIFT | > + 0 << CCM_PLL1_CFG_FACTOR_M_SHIFT | > + 0 << CCM_PLL1_CFG_DIVP_SHIFT, > + &ccm->pll1_cfg); There is already the PLL1_CFG macro later in that same file, can you reuse that, maybe? Otherwise this is indeed what I had in mind, thanks for providing the patch! Cheers, Andre > sdelay(200); > writel(AXI_DIV_1 << AXI_DIV_SHIFT | > AHB_DIV_2 << AHB_DIV_SHIFT |
diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h index 2cec91cb20..11b350824e 100644 --- a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h +++ b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h @@ -134,12 +134,16 @@ struct sunxi_ccm_reg { #define CCM_PLL1_CFG_PLL4_EXCH_SHIFT 25 #define CCM_PLL1_CFG_BIAS_CUR_SHIFT 20 #define CCM_PLL1_CFG_DIVP_SHIFT 16 +#define CCM_PLL1_CFG_DIVP_MASK (0x3 << CCM_PLL1_CFG_DIVP_SHIFT) #define CCM_PLL1_CFG_LCK_TMR_SHIFT 13 #define CCM_PLL1_CFG_FACTOR_N_SHIFT 8 +#define CCM_PLL1_CFG_FACTOR_N_MASK (0x1f << CCM_PLL1_CFG_FACTOR_N_SHIFT) #define CCM_PLL1_CFG_FACTOR_K_SHIFT 4 +#define CCM_PLL1_CFG_FACTOR_K_MASK (0x3 << CCM_PLL1_CFG_FACTOR_K_SHIFT) #define CCM_PLL1_CFG_SIG_DELT_PAT_IN_SHIFT 3 #define CCM_PLL1_CFG_SIG_DELT_PAT_EN_SHIFT 2 #define CCM_PLL1_CFG_FACTOR_M_SHIFT 0 +#define CCM_PLL1_CFG_FACTOR_M_MASK (0x3 << CCM_PLL1_CFG_FACTOR_M_SHIFT) #define PLL1_CFG_DEFAULT 0xa1005000 diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index fe89aec6b9..85e3a26855 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -705,6 +705,14 @@ config SYS_CLK_FREQ default 1008000000 if MACH_SUN50I_H616 default 1008000000 if MACH_SUN8I_R528 +if MACH_SUN4I || MACH_SUN5I || MACH_SUN7I +config SPL_SYS_CLK_FREQ + int "sunxi SPL CPU clock frequency" + default 384 + ---help--- + A static value for the sunxi SPL CPU frequency, must be a multiple of 24. +endif + config SYS_CONFIG_NAME default "suniv" if MACH_SUNIV default "sun4i" if MACH_SUN4I diff --git a/arch/arm/mach-sunxi/clock_sun4i.c b/arch/arm/mach-sunxi/clock_sun4i.c index 8f1d1b65f0..04623c1d09 100644 --- a/arch/arm/mach-sunxi/clock_sun4i.c +++ b/arch/arm/mach-sunxi/clock_sun4i.c @@ -25,7 +25,16 @@ void clock_init_safe(void) APB0_DIV_1 << APB0_DIV_SHIFT | CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT, &ccm->cpu_ahb_apb0_cfg); - writel(PLL1_CFG_DEFAULT, &ccm->pll1_cfg); + writel((PLL1_CFG_DEFAULT & + ~(CCM_PLL1_CFG_FACTOR_N_MASK | + CCM_PLL1_CFG_FACTOR_K_MASK | + CCM_PLL1_CFG_FACTOR_M_MASK | + CCM_PLL1_CFG_DIVP_MASK)) | + (CONFIG_SPL_SYS_CLK_FREQ / 24) << CCM_PLL1_CFG_FACTOR_N_SHIFT | + 0 << CCM_PLL1_CFG_FACTOR_K_SHIFT | + 0 << CCM_PLL1_CFG_FACTOR_M_SHIFT | + 0 << CCM_PLL1_CFG_DIVP_SHIFT, + &ccm->pll1_cfg); sdelay(200); writel(AXI_DIV_1 << AXI_DIV_SHIFT | AHB_DIV_2 << AHB_DIV_SHIFT |
This option can be used to modify the initial SPL CPU clock frequency. This follows an earlier discussion regarding A20 CPUs dying after reboot in SPL initialization due to incompatible CPU clock frequency and core voltage. [1] First attempt was to update PLL1_CFG_DEFAULT to a fixed lower frequency (144MHz), which fixed the observed issue but might not suit all A20 users. A KConfig option should be the better solution. [1] https://lists.denx.de/pipermail/u-boot/2024-January/544897.html Signed-off-by: Ludwig Kormann <ludwig.kormann@ict42.de> --- arch/arm/include/asm/arch-sunxi/clock_sun4i.h | 4 ++++ arch/arm/mach-sunxi/Kconfig | 8 ++++++++ arch/arm/mach-sunxi/clock_sun4i.c | 11 ++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-)