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....@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot