[U-Boot] [PATCH V2 04/23] imx: mx8m: add clock driver
Fabio Estevam
festevam at gmail.com
Sat Dec 16 17:33:15 UTC 2017
On Mon, Dec 4, 2017 at 2:31 AM, Peng Fan <peng.fan at nxp.com> wrote:
> + 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("Not supported\n");
Please improve the error message. "Not supported" is simply too vague.
Also, you use this same error message in several parts of the driver.
Please distinguish between them so that it can make it easier for
people debugging issues to know where exactly the error message comes
from.
> +
> +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;
> + default:
> + printf("Not supported\n");
Ditto.
> + case SYSTEM_PLL2_50M_CLK:
> + case SYSTEM_PLL1_40M_CLK:
> + pll_clke = SSCG_PLL_DIV20_CLKE_MASK;
> + div = 20;
> + break;
> + default:
> + printf("Not supported\n");
Ditto.
> +static u32 get_root_src_clk(enum clk_root_src root_src)
> +{
> + switch (root_src) {
> + case OSC_25M_CLK:
> + return 25000000u;
Is this 'u' in the end really needed? It doesn't seem so.
> +#ifdef CONFIG_MXC_OCOTP
> +void enable_ocotp_clk(unsigned char enable)
> +{
> + clock_enable(CCGR_OCOTP, !!enable);
Not a big fan of a function called clock_enabled() that can be used to
enable or disable a clock.
I prefer the kernel style: clock_enable() and clock_disable().
> +}
> +#endif
> +
> +int enable_i2c_clk(unsigned char enable, unsigned int i2c_num)
> +{
> + /* 0 - 3 is valid i2c num */
> + if (i2c_num > 3)
> + return -EINVAL;
> +
> + clock_enable(CCGR_I2C1 + i2c_num, !!enable);
> +
> + return 0;
> +}
> +
> +unsigned int mxc_get_clock(enum clk_root_index clk)
> +{
> + u32 val;
> +
> + if (clk >= CLK_ROOT_MAX)
> + return 0;
> +
> + if (clk == MXC_ARM_CLK)
> + return get_root_clk(ARM_A53_CLK_ROOT);
> +
> + if (clk == MXC_IPG_CLK) {
> + clock_get_target_val(IPG_CLK_ROOT, &val);
> + val = val & 0x3;
> + return get_root_clk(AHB_CLK_ROOT) / (val + 1);
> + }
> +
> + return get_root_clk(clk);
> +}
> +
> +u32 imx_get_uartclk(void)
> +{
> + return mxc_get_clock(UART1_CLK_ROOT);
> +}
> +
> +void mxs_set_lcdclk(u32 base_addr, u32 freq)
> +{
> + /*
> + * LCDIF_PIXEL_CLK: select 800MHz root clock,
> + * select pre divider 8, output is 100 MHz
> + */
> + clock_set_target_val(LCDIF_PIXEL_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(4) |
> + CLK_ROOT_PRE_DIV(CLK_ROOT_PRE_DIV8));
> +}
> +
> +void init_wdog_clk(void)
> +{
> + clock_enable(CCGR_WDOG1, 0);
> + clock_enable(CCGR_WDOG2, 0);
> + clock_enable(CCGR_WDOG3, 0);
> + clock_set_target_val(WDOG_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(0));
> + clock_set_target_val(WDOG_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(0));
> + clock_set_target_val(WDOG_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(0));
> + clock_enable(CCGR_WDOG1, 1);
> + clock_enable(CCGR_WDOG2, 1);
> + clock_enable(CCGR_WDOG3, 1);
> +}
> +
> +void init_usb_clk(void)
> +{
> + if (!is_usb_boot()) {
> + clock_enable(CCGR_USB_CTRL1, 0);
> + clock_enable(CCGR_USB_CTRL2, 0);
> + clock_enable(CCGR_USB_PHY1, 0);
> + clock_enable(CCGR_USB_PHY2, 0);
> + /* 500M */
500 MHz
> + clock_set_target_val(USB_BUS_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(1));
> + /* 100M */
100 MHz
> + clock_set_target_val(USB_CORE_REF_CLK_ROOT, CLK_ROOT_ON |
> + CLK_ROOT_SOURCE_SEL(1));
> + /* 100M */
100 MHz again?
> +#ifdef CONFIG_SPL_BUILD
> +void dram_pll_init(void)
> +{
> + struct src *src = (struct src *)SRC_BASE_ADDR;
> + void __iomem *pll_control_reg = &ana_pll->dram_pll_cfg0;
> + u32 pwdn_mask = 0;
> + u32 pll_clke = 0;
> + u32 bypass1 = 0;
> + u32 bypass2 = 0;
> +
> + setbits_le32(GPC_BASE_ADDR + 0xEC, BIT(7));
> + setbits_le32(GPC_BASE_ADDR + 0xF8, BIT(5));
> +
> + pwdn_mask = SSCG_PLL_PD_MASK;
> + pll_clke = SSCG_PLL_DRAM_PLL_CLKE_MASK;
> + bypass1 = SSCG_PLL_BYPASS1_MASK;
> + bypass2 = SSCG_PLL_BYPASS2_MASK;
> +
> + /* Enable DDR1 and DDR2 domain */
> + writel(SRC_DDR1_ENABLE_MASK, &src->ddr1_rcr);
> + writel(SRC_DDR1_ENABLE_MASK, &src->ddr2_rcr);
> +
> + /* Clear power down bit */
> + clrbits_le32(pll_control_reg, pwdn_mask);
> + /* Eanble ARM_PLL/SYS_PLL */
> + setbits_le32(pll_control_reg, pll_clke);
> +
> + /* Clear bypass */
> + clrbits_le32(pll_control_reg, bypass1);
> + __udelay(100);
> + clrbits_le32(pll_control_reg, bypass2);
> + /* Wait until lock */
> + while (!(readl(pll_control_reg) & SSCG_PLL_LOCK_MASK))
> + ;
No timeout? If bad things happens we can get stuck here forever.
> +}
> +
> +int frac_pll_init(u32 pll, enum frac_pll_out_val val)
> +{
> + void __iomem *pll_cfg0, __iomem *pll_cfg1;
> + u32 val_cfg0, val_cfg1;
> +
> + switch (pll) {
> + case ANATOP_ARM_PLL:
> + pll_cfg0 = &ana_pll->arm_pll_cfg0;
> + pll_cfg1 = &ana_pll->arm_pll_cfg1;
> +
> + if (val == FRAC_PLL_OUT_1000M)
> + val_cfg1 = FRAC_PLL_INT_DIV_CTL_VAL(49);
> + else
> + val_cfg1 = FRAC_PLL_INT_DIV_CTL_VAL(79);
> + val_cfg0 = FRAC_PLL_CLKE_MASK | FRAC_PLL_REFCLK_SEL_OSC_25M |
> + FRAC_PLL_LOCK_SEL_MASK | FRAC_PLL_NEWDIV_VAL_MASK |
> + FRAC_PLL_REFCLK_DIV_VAL(4) |
> + FRAC_PLL_OUTPUT_DIV_VAL(0);
> + break;
> + default:
> + return -1;
-EINVAL, please.
> + }
> +
> + /* bypass the clock */
> + setbits_le32(pll_cfg0, FRAC_PLL_BYPASS_MASK);
> + /* Set the value */
> + writel(val_cfg1, pll_cfg1);
> + writel(val_cfg0 | FRAC_PLL_BYPASS_MASK, pll_cfg0);
> + val_cfg0 = readl(pll_cfg0);
> + /* unbypass the clock */
> + clrbits_le32(pll_cfg0, FRAC_PLL_BYPASS_MASK);
> + while (!(readl(pll_cfg0) & FRAC_PLL_LOCK_MASK))
> + ;
Same here.
> + case ANATOP_SYSTEM_PLL3:
> + pll_cfg0 = &ana_pll->sys_pll3_cfg0;
> + pll_cfg1 = &ana_pll->sys_pll3_cfg1;
> + pll_cfg2 = &ana_pll->sys_pll3_cfg2;
> + /* 800MHz */
> + val_cfg2 = SSCG_PLL_FEEDBACK_DIV_F1_VAL(3) |
> + SSCG_PLL_FEEDBACK_DIV_F2_VAL(3);
> + val_cfg1 = 0;
> + val_cfg0 = SSCG_PLL_PLL3_CLKE_MASK | SSCG_PLL_LOCK_SEL_MASK |
> + SSCG_PLL_REFCLK_SEL_OSC_25M;
> + break;
> + default:
> + return -1;
-EINVAL
> + }
> +
> + /*bypass*/
> + setbits_le32(pll_cfg0, bypass1_mask | bypass2_mask);
> + /* set value */
> + writel(val_cfg2, pll_cfg2);
> + writel(val_cfg1, pll_cfg1);
> + /*unbypass1 and wait 70us */
> + writel(val_cfg0 | bypass2_mask, pll_cfg1);
> +
> + __udelay(70);
> +
> + /* unbypass2 and wait lock */
> + writel(val_cfg0, pll_cfg1);
> + while (!(readl(pll_cfg0) & SSCG_PLL_LOCK_MASK))
> + ;
Timeout please.
> +#endif
> +
> +/*
> + * Dump some clockes.
Typo: clocks
More information about the U-Boot
mailing list