[U-Boot] [PATCH V5 05/31] imx: mx8m: add clock driver

Peng Fan van.freenix at gmail.com
Tue Jan 23 08:58:43 UTC 2018


Hi Stefano,

>> +enum clk_root_index {
>> +	MXC_ARM_CLK			= 0,
>> +	ARM_A53_CLK_ROOT		= 0,
>> +	ARM_M4_CLK_ROOT			= 1,
>> +	VPU_A53_CLK_ROOT		= 2,
>> +	GPU_CORE_CLK_ROOT		= 3,
>> +	GPU_SHADER_CLK_ROOT		= 4,
>> +	MAIN_AXI_CLK_ROOT		= 16,
>> +	ENET_AXI_CLK_ROOT		= 17,
>> +	NAND_USDHC_BUS_CLK_ROOT		= 18,
>> +	MIPI_CSI1_PHY_REF_CLK_ROOT	= 123,
[....]
>> +	MIPI_CSI1_ESC_CLK_ROOT		= 124,
>> +	MIPI_CSI2_CORE_CLK_ROOT		= 125,
>> +	MIPI_CSI2_PHY_REF_CLK_ROOT	= 126,
>> +	MIPI_CSI2_ESC_CLK_ROOT		= 127,
>> +	PCIE2_CTRL_CLK_ROOT		= 128,
>> +	PCIE2_PHY_CLK_ROOT		= 129,
>> +	PCIE2_AUX_CLK_ROOT		= 130,
>> +	ECSPI3_CLK_ROOT			= 131,
>> +	OLD_MIPI_DSI_ESC_RX_ROOT	= 132,
>> +	DISPLAY_HDMI_CLK_ROOT		= 133,
>> +	CLK_ROOT_MAX,
>> +};
>> +
>
>Just to help board maintainers: add a comment to this structure to
>reference to the manual where they are defined (Clock root table, table
>5.1). This explains that the defines here reflects the hardware as
>described in manual.

Yes. I'll add them.

>
>> +enum clk_root_src {
>> +	OSC_25M_CLK,
>> +	ARM_PLL_CLK,
>> +	DRAM_PLL1_CLK,
>> +	VIDEO_PLL2_CLK,
>> +	VPU_PLL_CLK,
>> +	GPU_PLL_CLK,
>> +	SYSTEM_PLL1_800M_CLK,
>> +	SYSTEM_PLL1_400M_CLK,
>> +	SYSTEM_PLL1_266M_CLK,
>> +	SYSTEM_PLL1_200M_CLK,
>> +	SYSTEM_PLL1_160M_CLK,
>> +	SYSTEM_PLL1_133M_CLK,
>> +	SYSTEM_PLL1_100M_CLK,
>> +	SYSTEM_PLL1_80M_CLK,
>> +	SYSTEM_PLL1_40M_CLK,
>> +	SYSTEM_PLL2_1000M_CLK,
>> +	SYSTEM_PLL2_500M_CLK,
>> +	SYSTEM_PLL2_333M_CLK,
>> +	SYSTEM_PLL2_250M_CLK,
>> +	SYSTEM_PLL2_200M_CLK,
>> +	SYSTEM_PLL2_166M_CLK,
>> +	SYSTEM_PLL2_125M_CLK,
>> +	SYSTEM_PLL2_100M_CLK,
>> +	SYSTEM_PLL2_50M_CLK,
>> +	SYSTEM_PLL3_CLK,
>> +	AUDIO_PLL1_CLK,
>> +	AUDIO_PLL2_CLK,
>> +	VIDEO_PLL_CLK,
>> +	OSC_32K_CLK,
>> +	EXT_CLK_1,
>> +	EXT_CLK_2,
>> +	EXT_CLK_3,
>> +	EXT_CLK_4,
>> +	OSC_27M_CLK,
>> +};
>> +
>> +/* CCGR index */
>> +enum clk_ccgr_index {
>> +	CCGR_DVFS = 0,
>> +	CCGR_ANAMIX = 1,
>> +	CCGR_CPU = 2,
>> +	CCGR_CSU = 4,
>> +	CCGR_DRAM1 = 5,
>> +	CCGR_DRAM2_OBSOLETE = 6,
>> +	CCGR_ECSPI1 = 7,
>> +	CCGR_ECSPI2 = 8,
>> +	CCGR_ECSPI3 = 9,
>> +	CCGR_ENET1 = 10,
>> +	CCGR_GPIO1 = 11,
>> +	CCGR_GPIO2 = 12,
>> +	CCGR_GPIO3 = 13,
>> +	CCGR_GPIO4 = 14,
>> +	CCGR_GPIO5 = 15,
>> +	CCGR_GPT1 = 16,
>> +	CCGR_GPT2 = 17,
>> +	CCGR_GPT3 = 18,
>> +	CCGR_GPT4 = 19,
>> +	CCGR_GPT5 = 20,
>> +	CCGR_GPT6 = 21,
>> +	CCGR_HS = 22,
>> +	CCGR_I2C1 = 23,
>> +	CCGR_I2C2 = 24,
>> +	CCGR_I2C3 = 25,
>> +	CCGR_I2C4 = 26,
>> +	CCGR_IOMUX = 27,
>> +	CCGR_IOMUX1 = 28,
>> +	CCGR_IOMUX2 = 29,
>> +	CCGR_IOMUX3 = 30,
>> +	CCGR_IOMUX4 = 31,
>> +	CCGR_M4 = 32,
>> +	CCGR_MU = 33,
>> +	CCGR_OCOTP = 34,
>> +	CCGR_OCRAM = 35,
>> +	CCGR_OCRAM_S = 36,
>> +	CCGR_PCIE = 37,
>> +	CCGR_PERFMON1 = 38,
>> +	CCGR_PERFMON2 = 39,
>> +	CCGR_PWM1 = 40,
>> +	CCGR_PWM2 = 41,
>> +	CCGR_PWM3 = 42,
>> +	CCGR_PWM4 = 43,
>> +	CCGR_QOS = 44,
>> +	CCGR_DISMIX = 45,
>> +	CCGR_MEGAMIX = 46,
>> +	CCGR_QSPI = 47,
>> +	CCGR_RAWNAND = 48,
>> +	CCGR_RDC = 49,
>> +	CCGR_ROM = 50,
>> +	CCGR_SAI1 = 51,
>> +	CCGR_SAI2 = 52,
>> +	CCGR_SAI3 = 53,
>> +	CCGR_SAI4 = 54,
>> +	CCGR_SAI5 = 55,
>> +	CCGR_SAI6 = 56,
>> +	CCGR_SCTR = 57,
>> +	CCGR_SDMA1 = 58,
>> +	CCGR_SDMA2 = 59,
>> +	CCGR_SEC_DEBUG = 60,
>> +	CCGR_SEMA1 = 61,
>> +	CCGR_SEMA2 = 62,
>> +	CCGR_SIM_DISPLAY = 63,
>> +	CCGR_SIM_ENET = 64,
>> +	CCGR_SIM_M = 65,
>> +	CCGR_SIM_MAIN = 66,
>> +	CCGR_SIM_S = 67,
>> +	CCGR_SIM_WAKEUP = 68,
>> +	CCGR_SIM_USB = 69,
>> +	CCGR_SIM_VPU = 70,
>> +	CCGR_SNVS = 71,
>> +	CCGR_TRACE = 72,
>> +	CCGR_UART1 = 73,
>> +	CCGR_UART2 = 74,
>> +	CCGR_UART3 = 75,
>> +	CCGR_UART4 = 76,
>> +	CCGR_USB_CTRL1 = 77,
>> +	CCGR_USB_CTRL2 = 78,
>> +	CCGR_USB_PHY1 = 79,
>> +	CCGR_USB_PHY2 = 80,
>> +	CCGR_USDHC1 = 81,
>> +	CCGR_USDHC2 = 82,
>> +	CCGR_WDOG1 = 83,
>> +	CCGR_WDOG2 = 84,
>> +	CCGR_WDOG3 = 85,
>> +	CCGR_VA53 = 86,
>> +	CCGR_GPU = 87,
>> +	CCGR_HEVC = 88,
>> +	CCGR_AVC = 89,
>> +	CCGR_VP9 = 90,
>> +	CCGR_HEVC_INTER = 91,
>> +	CCGR_GIC = 92,
>> +	CCGR_DISPLAY = 93,
>> +	CCGR_HDMI = 94,
>> +	CCGR_HDMI_PHY = 95,
>> +	CCGR_XTAL = 96,
>> +	CCGR_PLL = 97,
>> +	CCGR_TSENSOR = 98,
>> +	CCGR_VPU_DEC = 99,
>> +	CCGR_PCIE2 = 100,
>> +	CCGR_MIPI_CSI1 = 101,
>> +	CCGR_MIPI_CSI2 = 102,
>> +	CCGR_MAX,
>> +};
>> +
>> +/* src index */
>> +enum clk_src_index {
>> +	CLK_SRC_CKIL_SYNC_REQ = 0,
>> +	CLK_SRC_ARM_PLL_EN = 1,
>> +	CLK_SRC_GPU_PLL_EN = 2,
>> +	CLK_SRC_VPU_PLL_EN = 3,
>> +	CLK_SRC_DRAM_PLL_EN = 4,
>> +	CLK_SRC_SYSTEM_PLL1_EN = 5,
>> +	CLK_SRC_SYSTEM_PLL2_EN = 6,
>> +	CLK_SRC_SYSTEM_PLL3_EN = 7,
>> +	CLK_SRC_AUDIO_PLL1_EN = 8,
>> +	CLK_SRC_AUDIO_PLL2_EN = 9,
>> +	CLK_SRC_VIDEO_PLL1_EN = 10,
>> +	CLK_SRC_VIDEO_PLL2_EN = 11,
>> +	CLK_SRC_ARM_PLL = 12,
>> +	CLK_SRC_GPU_PLL = 13,
>> +	CLK_SRC_VPU_PLL = 14,
>> +	CLK_SRC_DRAM_PLL = 15,
>> +	CLK_SRC_SYSTEM_PLL1_800M = 16,
>> +	CLK_SRC_SYSTEM_PLL1_400M = 17,
>> +	CLK_SRC_SYSTEM_PLL1_266M = 18,
>> +	CLK_SRC_SYSTEM_PLL1_200M = 19,
>> +	CLK_SRC_SYSTEM_PLL1_160M = 20,
>> +	CLK_SRC_SYSTEM_PLL1_133M = 21,
>> +	CLK_SRC_SYSTEM_PLL1_100M = 22,
>> +	CLK_SRC_SYSTEM_PLL1_80M = 23,
>> +	CLK_SRC_SYSTEM_PLL1_40M = 24,
>> +	CLK_SRC_SYSTEM_PLL2_1000M = 25,
>> +	CLK_SRC_SYSTEM_PLL2_500M = 26,
>> +	CLK_SRC_SYSTEM_PLL2_333M = 27,
>> +	CLK_SRC_SYSTEM_PLL2_250M = 28,
>> +	CLK_SRC_SYSTEM_PLL2_200M = 29,
>> +	CLK_SRC_SYSTEM_PLL2_166M = 30,
>> +	CLK_SRC_SYSTEM_PLL2_125M = 31,
>> +	CLK_SRC_SYSTEM_PLL2_100M = 32,
>> +	CLK_SRC_SYSTEM_PLL2_50M = 33,
>> +	CLK_SRC_SYSTEM_PLL3 = 34,
>> +	CLK_SRC_AUDIO_PLL1 = 35,
>> +	CLK_SRC_AUDIO_PLL2 = 36,
>> +	CLK_SRC_VIDEO_PLL1 = 37,
>> +	CLK_SRC_VIDEO_PLL2 = 38,
>> +	CLK_SRC_OSC_25M = 39,
>> +	CLK_SRC_OSC_27M = 40,
>> +};
>
>
>ok, fine

I could not get your point (:

>
>> +
>> +enum root_pre_div {
>> +	CLK_ROOT_PRE_DIV1 = 0,
>> +	CLK_ROOT_PRE_DIV2,
>> +	CLK_ROOT_PRE_DIV3,
>> +	CLK_ROOT_PRE_DIV4,
>> +	CLK_ROOT_PRE_DIV5,
>> +	CLK_ROOT_PRE_DIV6,
>> +	CLK_ROOT_PRE_DIV7,
>> +	CLK_ROOT_PRE_DIV8,
>> +};
>> +
>> +enum root_post_div {
>> +	CLK_ROOT_POST_DIV1 = 0,
>> +	CLK_ROOT_POST_DIV2,
>> +	CLK_ROOT_POST_DIV3,
>> +	CLK_ROOT_POST_DIV4,
>> +	CLK_ROOT_POST_DIV5,
>> +	CLK_ROOT_POST_DIV6,
>> +	CLK_ROOT_POST_DIV7,
>> +	CLK_ROOT_POST_DIV8,
>> +	CLK_ROOT_POST_DIV9,
>> +	CLK_ROOT_POST_DIV10,
>> +	CLK_ROOT_POST_DIV11,
>> +	CLK_ROOT_POST_DIV12,
>> +	CLK_ROOT_POST_DIV13,
>> +	CLK_ROOT_POST_DIV14,
>> +	CLK_ROOT_POST_DIV15,
>> +	CLK_ROOT_POST_DIV16,
>> +	CLK_ROOT_POST_DIV17,
>> +	CLK_ROOT_POST_DIV18,
>> +	CLK_ROOT_POST_DIV19,
>> +	CLK_ROOT_POST_DIV20,
>> +	CLK_ROOT_POST_DIV21,
>> +	CLK_ROOT_POST_DIV22,
>> +	CLK_ROOT_POST_DIV23,
>> +	CLK_ROOT_POST_DIV24,
>> +	CLK_ROOT_POST_DIV25,
>> +	CLK_ROOT_POST_DIV26,
>> +	CLK_ROOT_POST_DIV27,
>> +	CLK_ROOT_POST_DIV28,
>> +	CLK_ROOT_POST_DIV29,
>> +	CLK_ROOT_POST_DIV30,
>> +	CLK_ROOT_POST_DIV31,
>> +	CLK_ROOT_POST_DIV32,
>> +	CLK_ROOT_POST_DIV33,
>> +	CLK_ROOT_POST_DIV34,
>> +	CLK_ROOT_POST_DIV35,
>> +	CLK_ROOT_POST_DIV36,
>> +	CLK_ROOT_POST_DIV37,
>> +	CLK_ROOT_POST_DIV38,
>> +	CLK_ROOT_POST_DIV39,
>> +	CLK_ROOT_POST_DIV40,
>> +	CLK_ROOT_POST_DIV41,
>> +	CLK_ROOT_POST_DIV42,
>> +	CLK_ROOT_POST_DIV43,
>> +	CLK_ROOT_POST_DIV44,
>> +	CLK_ROOT_POST_DIV45,
>> +	CLK_ROOT_POST_DIV46,
>> +	CLK_ROOT_POST_DIV47,
>> +	CLK_ROOT_POST_DIV48,
>> +	CLK_ROOT_POST_DIV49,
>> +	CLK_ROOT_POST_DIV50,
>> +	CLK_ROOT_POST_DIV51,
>> +	CLK_ROOT_POST_DIV52,
>> +	CLK_ROOT_POST_DIV53,
>> +	CLK_ROOT_POST_DIV54,
>> +	CLK_ROOT_POST_DIV55,
>> +	CLK_ROOT_POST_DIV56,
>> +	CLK_ROOT_POST_DIV57,
>> +	CLK_ROOT_POST_DIV58,
>> +	CLK_ROOT_POST_DIV59,
>> +	CLK_ROOT_POST_DIV60,
>> +	CLK_ROOT_POST_DIV61,
>> +	CLK_ROOT_POST_DIV62,
>> +	CLK_ROOT_POST_DIV63,
>> +	CLK_ROOT_POST_DIV64,
>> +};
>
>I am asking myself which is the information carry out by these two
>previous enum. It does not make a big sense IMHO.  Why do we need a
>CLK_ROOT_POST_DIVX to store the value (x-1) ?

`#define CLK_ROOT_POST_DIV(X) ((x) - 1)` is cleaner. I'll clean the patch.

>
>> +
>> +
>> +enum enet_freq {
>> +	ENET_25MHZ = 0,
>> +	ENET_50MHZ,
>> +	ENET_125MHZ,
>> +};
>
>Soon or later we should move this enum where it belongs to (FEC), we
>duplicate this for all SOCs.

Agree.

>
>> +
>> +enum frac_pll_out_val {
>> +	FRAC_PLL_OUT_1000M,
>> +	FRAC_PLL_OUT_1600M,
>> +};
>> +
>> +u32 imx_get_fecclk(void);
>> +u32 imx_get_uartclk(void);
>> +int clock_init(void);
>> +void init_clk_usdhc(u32 index);
>> +void init_uart_clk(u32 index);
>> +void init_wdog_clk(void);
>> +unsigned int mxc_get_clock(enum clk_root_index clk);
>> +int clock_enable(enum clk_ccgr_index index, bool enable);
>> +int clock_root_enabled(enum clk_root_index clock_id);
>> +int clock_root_cfg(enum clk_root_index clock_id, enum root_pre_div pre_div,
>> +		   enum root_post_div post_div, enum clk_root_src clock_src);
>> +int clock_set_target_val(enum clk_root_index clock_id, u32 val);
>> +int clock_get_target_val(enum clk_root_index clock_id, u32 *val);
>> +int clock_get_prediv(enum clk_root_index clock_id, enum root_pre_div *pre_div);
>> +int clock_get_postdiv(enum clk_root_index clock_id,
>> +		      enum root_post_div *post_div);
>> +int clock_get_src(enum clk_root_index clock_id, enum clk_root_src *p_clock_src);
>> +void mxs_set_lcdclk(u32 base_addr, u32 freq);
>> +int set_clk_qspi(void);
>> +void enable_ocotp_clk(unsigned char enable);
>> +int enable_i2c_clk(unsigned char enable, unsigned int i2c_num);
>> +int set_clk_enet(enum enet_freq type);
>
>Wow ! We export a very large API, much more as on other i.MXes. I am
>bothering if we really need this, that is if a board maintainer (or
>driver, of course) need all of them. Some of them are just between
>clock.c and clock_slice.c. We maybe could add a clock_priv.h that is
>shared between internal modules in mx8m, but are not exported globally
>because they are not needed. Goal is to have the cleanest interface for
>new MX8M boards that we can have.

Agree.

>
>> +#endif
>> diff --git a/arch/arm/mach-imx/mx8m/Makefile b/arch/arm/mach-imx/mx8m/Makefile
>> new file mode 100644
>> index 0000000000..05f38842f0
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/mx8m/Makefile
>> @@ -0,0 +1,7 @@
>> +#
>> +# Copyright 2017 NXP
>> +#
>> +# SPDX-License-Identifier:	GPL-2.0+
>> +#
>> +
>> +obj-y += clock.o clock_slice.o
>> diff --git a/arch/arm/mach-imx/mx8m/clock.c b/arch/arm/mach-imx/mx8m/clock.c
>> new file mode 100644
>> index 0000000000..c56ba99d5c
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/mx8m/clock.c
>> @@ -0,0 +1,795 @@
>> +/*
>> + * Copyright 2017 NXP
>> + *
>> + * Peng Fan <peng.fan at nxp.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch/clock.h>
>> +#include <asm/arch/imx-regs.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/sys_proto.h>
>> +#include <errno.h>
>> +#include <linux/iopoll.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static struct anamix_pll *ana_pll = (struct anamix_pll *)ANATOP_BASE_ADDR;
>> +
>> +static u32 decode_frac_pll(enum clk_root_src frac_pll)
>> +{
>> +	u32 pll_cfg0, pll_cfg1, pllout;
>> +	u32 pll_refclk_sel, pll_refclk;
>> +	u32 divr_val, divq_val, divf_val, divff, divfi;
>> +	u32 pllout_div_shift, pllout_div_mask, pllout_div;
>> +
>> +	switch (frac_pll) {
>> +	case ARM_PLL_CLK:
>> +		pll_cfg0 = readl(&ana_pll->arm_pll_cfg0);
>> +		pll_cfg1 = readl(&ana_pll->arm_pll_cfg1);
>> +		pllout_div_shift = HW_FRAC_ARM_PLL_DIV_SHIFT;
>> +		pllout_div_mask = HW_FRAC_ARM_PLL_DIV_MASK;
>> +		break;
>> +	default:
>> +		printf("Frac PLL %d not supporte\n", frac_pll);
>> +		return 0;
>> +	}
>> +
>> +	pllout_div = readl(&ana_pll->frac_pllout_div_cfg);
>> +	pllout_div = (pllout_div & pllout_div_mask) >> pllout_div_shift;
>> +
>> +	/* Power down */
>> +	if (pll_cfg0 & FRAC_PLL_PD_MASK)
>> +		return 0;
>> +
>> +	/* output not enabled */
>> +	if ((pll_cfg0 & FRAC_PLL_CLKE_MASK) == 0)
>> +		return 0;
>> +
>> +	pll_refclk_sel = pll_cfg0 & FRAC_PLL_REFCLK_SEL_MASK;
>> +
>> +	if (pll_refclk_sel == FRAC_PLL_REFCLK_SEL_OSC_25M)
>> +		pll_refclk = 25000000u;
>> +	else if (pll_refclk_sel == FRAC_PLL_REFCLK_SEL_OSC_27M)
>> +		pll_refclk = 27000000u;
>> +	else if (pll_refclk_sel == FRAC_PLL_REFCLK_SEL_HDMI_PHY_27M)
>> +		pll_refclk = 27000000u;
>> +	else
>> +		pll_refclk = 0;
>> +
>> +	if (pll_cfg0 & FRAC_PLL_BYPASS_MASK)
>> +		return pll_refclk;
>> +
>> +	divr_val = (pll_cfg0 & FRAC_PLL_REFCLK_DIV_VAL_MASK) >>
>> +		FRAC_PLL_REFCLK_DIV_VAL_SHIFT;
>> +	divq_val = pll_cfg0 & FRAC_PLL_OUTPUT_DIV_VAL_MASK;
>> +
>> +	divff = (pll_cfg1 & FRAC_PLL_FRAC_DIV_CTL_MASK) >>
>> +		FRAC_PLL_FRAC_DIV_CTL_SHIFT;
>> +	divfi = pll_cfg1 & FRAC_PLL_INT_DIV_CTL_MASK;
>> +
>> +	divf_val = 1 + divfi + divff / (1 << 24);
>> +
>> +	pllout = pll_refclk / (divr_val + 1) * 8 * divf_val /
>> +		((divq_val + 1) * 2);
>> +
>> +	return pllout / (pllout_div + 1);
>> +}
>> +
>> +static u32 decode_sscg_pll(enum clk_root_src sscg_pll)
>> +{
>> +	u32 pll_cfg0, pll_cfg1, pll_cfg2;
>> +	u32 pll_refclk_sel, pll_refclk;
>> +	u32 divr1, divr2, divf1, divf2, divq, div;
>> +	u32 sse;
>> +	u32 pll_clke;
>> +	u32 pllout_div_shift, pllout_div_mask, pllout_div;
>> +	u32 pllout;
>> +
>> +	switch (sscg_pll) {
>> +	case SYSTEM_PLL1_800M_CLK:
>> +	case SYSTEM_PLL1_400M_CLK:
>> +	case SYSTEM_PLL1_266M_CLK:
>> +	case SYSTEM_PLL1_200M_CLK:
>> +	case SYSTEM_PLL1_160M_CLK:
>> +	case SYSTEM_PLL1_133M_CLK:
>> +	case SYSTEM_PLL1_100M_CLK:
>> +	case SYSTEM_PLL1_80M_CLK:
>> +	case SYSTEM_PLL1_40M_CLK:
>> +		pll_cfg0 = readl(&ana_pll->sys_pll1_cfg0);
>> +		pll_cfg1 = readl(&ana_pll->sys_pll1_cfg1);
>> +		pll_cfg2 = readl(&ana_pll->sys_pll1_cfg2);
>> +		pllout_div_shift = HW_SSCG_SYSTEM_PLL1_DIV_SHIFT;
>> +		pllout_div_mask = HW_SSCG_SYSTEM_PLL1_DIV_MASK;
>> +		break;
>> +	case SYSTEM_PLL2_1000M_CLK:
>> +	case SYSTEM_PLL2_500M_CLK:
>> +	case SYSTEM_PLL2_333M_CLK:
>> +	case SYSTEM_PLL2_250M_CLK:
>> +	case SYSTEM_PLL2_200M_CLK:
>> +	case SYSTEM_PLL2_166M_CLK:
>> +	case SYSTEM_PLL2_125M_CLK:
>> +	case SYSTEM_PLL2_100M_CLK:
>> +	case SYSTEM_PLL2_50M_CLK:
>> +		pll_cfg0 = readl(&ana_pll->sys_pll2_cfg0);
>> +		pll_cfg1 = readl(&ana_pll->sys_pll2_cfg1);
>> +		pll_cfg2 = readl(&ana_pll->sys_pll2_cfg2);
>> +		pllout_div_shift = HW_SSCG_SYSTEM_PLL2_DIV_SHIFT;
>> +		pllout_div_mask = HW_SSCG_SYSTEM_PLL2_DIV_MASK;
>> +		break;
>> +	case SYSTEM_PLL3_CLK:
>> +		pll_cfg0 = readl(&ana_pll->sys_pll3_cfg0);
>> +		pll_cfg1 = readl(&ana_pll->sys_pll3_cfg1);
>> +		pll_cfg2 = readl(&ana_pll->sys_pll3_cfg2);
>> +		pllout_div_shift = HW_SSCG_SYSTEM_PLL3_DIV_SHIFT;
>> +		pllout_div_mask = HW_SSCG_SYSTEM_PLL3_DIV_MASK;
>> +		break;
>> +	case DRAM_PLL1_CLK:
>> +		pll_cfg0 = readl(&ana_pll->dram_pll_cfg0);
>> +		pll_cfg1 = readl(&ana_pll->dram_pll_cfg1);
>> +		pll_cfg2 = readl(&ana_pll->dram_pll_cfg2);
>> +		pllout_div_shift = HW_SSCG_DRAM_PLL_DIV_SHIFT;
>> +		pllout_div_mask = HW_SSCG_DRAM_PLL_DIV_MASK;
>> +		break;
>
>The code in the switch looks a bit clumsy. The same action is done in
>all cases.

There are many PLLs, they have different register address and mask/shift
,so use pll_cfg[0-3] and pllout_div_shift/mask for different PLLs with
switch/case logic.

I do not have better idea to clean them.

Thanks,
Peng.


More information about the U-Boot mailing list