On Sun, 20 Nov 2016 14:56:56 +0000 Andre Przywara andre.przywara@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@arm.com Reviewed-by: Alexander Graf agraf@suse.de Reviewed-by: Chen-Yu Tsai wens@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.