[U-Boot] [PATCH 02/24] sun6i: Restrict some register initialization to Allwinner A31 SoC

[Siarhei Siamashka]

On Sun, 20 Nov 2016 14:56:56 +0000
Andre Przywara <andre.przywara at arm.com> wrote:

> These days many Allwinner SoCs use clock_sun6i.c, although out of them
> only the (original sun6i) A31 has a second MBUS clock register.
> Also setting up the PRCM PLL_CTLR1 register to provide the proper voltage
> seems to be an A31-only feature as well.
> So restrict the initialization to this SoC only to avoid writing bogus
> values to (undefined) registers in other chips.
> 
> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> Reviewed-by: Alexander Graf <agraf at suse.de>
> Reviewed-by: Chen-Yu Tsai <wens at csie.org>
> ---
>  arch/arm/mach-sunxi/clock_sun6i.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/mach-sunxi/clock_sun6i.c b/arch/arm/mach-sunxi/clock_sun6i.c
> index ed8cd9b..382fa94 100644
> --- a/arch/arm/mach-sunxi/clock_sun6i.c
> +++ b/arch/arm/mach-sunxi/clock_sun6i.c
> @@ -21,6 +21,8 @@ void clock_init_safe(void)
>  {
>  	struct sunxi_ccm_reg * const ccm =
>  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> +#ifdef CONFIG_MACH_SUN6I
>  	struct sunxi_prcm_reg * const prcm =
>  		(struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
>  
> @@ -31,6 +33,7 @@ void clock_init_safe(void)
>  		PRCM_PLL_CTRL_LDO_DIGITAL_EN | PRCM_PLL_CTRL_LDO_ANALOG_EN |
>  		PRCM_PLL_CTRL_EXT_OSC_EN | PRCM_PLL_CTRL_LDO_OUT_L(1140));
>  	clrbits_le32(&prcm->pll_ctrl1, PRCM_PLL_CTRL_LDO_KEY_MASK);
> +#endif

PRCM is generally poorly documented, with its description sometimes
entirely missing from the Allwinner manuals (while it exists in the
hardware). But many SoC variants are sharing the same features and need
the same code. I can confirm that this code chunk is needed on my A31s
tablet (otherwise the system does not boot), so it was not entirely
useless.

What about the other SoC variants? For example, I can see that the
A23 manual documents this register in a roughly the same way as A31
(the PLLVDD voltage settings, etc.). But I don't have any A23 hardware
to test. Basically, are we sure that we will not break A23 support
with this commit?

If I understand it correctly, you primarily wanted to disable this
code on A64. But disabling it everywhere other than A31 may be a
bit too broad.

>  
>  	clock_set_pll1(408000000);
>  
> @@ -41,7 +44,9 @@ void clock_init_safe(void)
>  	writel(AHB1_ABP1_DIV_DEFAULT, &ccm->ahb1_apb1_div);
>  
>  	writel(MBUS_CLK_DEFAULT, &ccm->mbus0_clk_cfg);
> +#ifdef CONFIG_MACH_SUN6I
>  	writel(MBUS_CLK_DEFAULT, &ccm->mbus1_clk_cfg);
> +#endif

We can change this to:

       if (IS_ENABLED(CONFIG_MACH_SUN6I))
           writel(MBUS_CLK_DEFAULT, &ccm->mbus1_clk_cfg);

This saves one line of code and also looks a bit less ugly.

-- 
Best regards,
Siarhei Siamashka