Hi Stefano, On Sat, Jun 27, 2015 at 06:44:25PM +0200, Stefano Babic wrote: >Hi Peng, > >On 11/06/2015 12:30, Peng Fan wrote: >> Since i.MX6QP changes some CCM registers, so modify the clocks settings to >> follow the hardware changes. >> >> In c files, use runtime check and discard #ifdef. >> >> A new CONFIG_MX6QP is introduced here and is used for the CCM difference, >> only used in header files for different bits. >> At default CONFIG_MX6Q is enabled along with the CONFIG_MX6QP. >> >> Signed-off-by: Ye.Li <b37...@freescale.com> >> Signed-off-by: Peng Fan <peng....@freescale.com> >> --- >> >> Changes v2: >> 1. Remove #ifdef, but use runtime check >> 2. A few bit definitions are introduced in c files, because to other >> platforms >> the macro will make compilation fail, also there are no other places >> refer >> the bit macro definitions. >> >> arch/arm/cpu/armv7/mx6/clock.c | 33 >> ++++++++++++++++++++++---------- >> arch/arm/cpu/armv7/mx6/soc.c | 5 ++++- >> arch/arm/include/asm/arch-mx6/crm_regs.h | 33 >> +++++++++++++++++--------------- >> include/configs/mx6_common.h | 3 +++ >> 4 files changed, 48 insertions(+), 26 deletions(-) >> >> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c >> index ae99945..0d862b2 100644 >> --- a/arch/arm/cpu/armv7/mx6/clock.c >> +++ b/arch/arm/cpu/armv7/mx6/clock.c >> @@ -323,10 +323,13 @@ static u32 get_ipg_per_clk(void) >> u32 reg, perclk_podf; >> >> reg = __raw_readl(&imx_ccm->cscmr1); >> -#if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6SX)) >> - if (reg & MXC_CCM_CSCMR1_PER_CLK_SEL_MASK) >> - return MXC_HCLK; /* OSC 24Mhz */ >> -#endif >> + if (is_cpu_type(MXC_CPU_MX6SL) || is_cpu_type(MXC_CPU_MX6SX) || >> + is_mx6dqp()) { >> +#define MXC_CCM_CSCMR1_PER_CLK_SEL_MASK (1 << 6) > >I am missing why the define is set here and dropped from crm_regs.h. >This makes the defines split between code (clock.c) and header >(crm_regs.h), and make harder to read and to find them. > >I am also missing if the goal to have runtime checked is really reached. >The MX6QuadPlus is (please correct me if I am wrong) pin compatible with >Quad. If it is, we lose the possibility to have a single binary for all >SOC variants that a board can mount. > >This is more evident for the code you surround with #ifdef CONFIG_MX6QP >- I mean, it is fully ok if you add new defines to crm_regs.h: they do >not conflict with the old ones. But if you redefine some of them, the >SOC must be decided at compile time. Having a single binary (of course, >for SOC variants where it is possible) is a feture we get with big >efforts and we should not remove it.
Will move the macros to crm_regs.h. > > > >> + if (reg & MXC_CCM_CSCMR1_PER_CLK_SEL_MASK) >> + return MXC_HCLK; /* OSC 24Mhz */ >> + } >> + >> perclk_podf = reg & MXC_CCM_CSCMR1_PERCLK_PODF_MASK; >> >> return get_ipg_clk() / (perclk_podf + 1); >> @@ -337,10 +340,14 @@ static u32 get_uart_clk(void) >> u32 reg, uart_podf; >> u32 freq = decode_pll(PLL_USBOTG, MXC_HCLK) / 6; /* static divider */ >> reg = __raw_readl(&imx_ccm->cscdr1); >> -#if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6SX)) >> - if (reg & MXC_CCM_CSCDR1_UART_CLK_SEL) >> - freq = MXC_HCLK; >> -#endif >> + >> + if (is_cpu_type(MXC_CPU_MX6SL) || is_cpu_type(MXC_CPU_MX6SX) || >> + is_mx6dqp()) { >> +#define MXC_CCM_CSCDR1_UART_CLK_SEL (1 << 6) >> + if (reg & MXC_CCM_CSCDR1_UART_CLK_SEL) >> + freq = MXC_HCLK; >> + } >> + >> reg &= MXC_CCM_CSCDR1_UART_CLK_PODF_MASK; >> uart_podf = reg >> MXC_CCM_CSCDR1_UART_CLK_PODF_OFFSET; >> >> @@ -352,8 +359,14 @@ static u32 get_cspi_clk(void) >> u32 reg, cspi_podf; >> >> reg = __raw_readl(&imx_ccm->cscdr2); >> - reg &= MXC_CCM_CSCDR2_ECSPI_CLK_PODF_MASK; >> - cspi_podf = reg >> MXC_CCM_CSCDR2_ECSPI_CLK_PODF_OFFSET; >> + cspi_podf = (reg & MXC_CCM_CSCDR2_ECSPI_CLK_PODF_MASK) >> >> + MXC_CCM_CSCDR2_ECSPI_CLK_PODF_OFFSET; >> + >> + if (is_mx6dqp()) { >> +#define MXC_CCM_CSCDR2_ECSPI_CLK_SEL_MASK (0x1 << 18) >> + if (reg & MXC_CCM_CSCDR2_ECSPI_CLK_SEL_MASK) >> + return MXC_HCLK / (cspi_podf + 1); >> + } >> >> return decode_pll(PLL_USBOTG, MXC_HCLK) / (8 * (cspi_podf + 1)); >> } >> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c >> index 29de624..bcfa2f6 100644 >> --- a/arch/arm/cpu/armv7/mx6/soc.c >> +++ b/arch/arm/cpu/armv7/mx6/soc.c >> @@ -335,9 +335,12 @@ static void set_ahb_rate(u32 val) >> static void clear_mmdc_ch_mask(void) >> { >> struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; >> + u32 reg; >> + reg = readl(&mxc_ccm->ccdr); >> >> /* Clear MMDC channel mask */ >> - writel(0, &mxc_ccm->ccdr); >> + reg &= ~(MXC_CCM_CCDR_MMDC_CH1_HS_MASK | MXC_CCM_CCDR_MMDC_CH0_HS_MASK); >> + writel(reg, &mxc_ccm->ccdr); >> } >> >> static void init_bandgap(void) >> diff --git a/arch/arm/include/asm/arch-mx6/crm_regs.h >> b/arch/arm/include/asm/arch-mx6/crm_regs.h >> index 887d048..2ff1005 100644 >> --- a/arch/arm/include/asm/arch-mx6/crm_regs.h >> +++ b/arch/arm/include/asm/arch-mx6/crm_regs.h >> @@ -113,7 +113,7 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CCR_WB_COUNT_MASK 0x7 >> #define MXC_CCM_CCR_WB_COUNT_OFFSET (1 << 16) >> #define MXC_CCM_CCR_COSC_EN (1 << 12) >> -#ifdef CONFIG_MX6SX >> +#if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6QP)) >> #define MXC_CCM_CCR_OSCNT_MASK 0x7F >> #else >> #define MXC_CCM_CCR_OSCNT_MASK 0xFF >> @@ -123,6 +123,9 @@ struct mxc_ccm_reg { >> /* Define the bits in register CCDR */ >> #define MXC_CCM_CCDR_MMDC_CH1_HS_MASK (1 << 16) >> #define MXC_CCM_CCDR_MMDC_CH0_HS_MASK (1 << 17) >> +#ifdef CONFIG_MX6QP >> +#define MXC_CCM_CCDR_MMDC_CH1_AXI_ROOT_CG (1 << 18) >> +#endif >> >> /* Define the bits in register CSR */ >> #define MXC_CCM_CSR_COSC_READY (1 << 5) >> @@ -196,7 +199,11 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CBCMR_GPU3D_CORE_CLK_SEL_MASK (0x3 << 4) >> #define MXC_CCM_CBCMR_GPU3D_CORE_CLK_SEL_OFFSET 4 >> #ifndef CONFIG_MX6SX >> +#ifdef CONFIG_MX6QP >> +#define MXC_CCM_CBCMR_PRE_CLK_SEL (1 << 1) >> +#else >> #define MXC_CCM_CBCMR_GPU3D_AXI_CLK_SEL (1 << 1) >> +#endif >> #define MXC_CCM_CBCMR_GPU2D_AXI_CLK_SEL (1 << 0) >> #endif >> >> @@ -230,7 +237,6 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CSCMR1_QSPI1_CLK_SEL_OFFSET 7 >> #endif >> #if (defined(CONFIG_MX6SL) || defined(CONFIG_MX6SX)) >> -#define MXC_CCM_CSCMR1_PER_CLK_SEL_MASK (1 << 6) >> #define MXC_CCM_CSCMR1_PER_CLK_SEL_OFFSET 6 >> #endif >> #define MXC_CCM_CSCMR1_PERCLK_PODF_MASK 0x3F >> @@ -244,15 +250,12 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CSCMR2_ESAI_PRE_SEL_OFFSET 19 >> #define MXC_CCM_CSCMR2_LDB_DI1_IPU_DIV (1 << 11) >> #define MXC_CCM_CSCMR2_LDB_DI0_IPU_DIV (1 << 10) >> -#ifdef CONFIG_MX6SX >> +#if (defined(CONFIG_MX6SX) || defined(CONFIG_MX6QP)) >> #define MXC_CCM_CSCMR2_CAN_CLK_SEL_MASK (0x3 << 8) >> #define MXC_CCM_CSCMR2_CAN_CLK_SEL_OFFSET 8 >> +#endif >> #define MXC_CCM_CSCMR2_CAN_CLK_PODF_MASK (0x3F << 2) >> #define MXC_CCM_CSCMR2_CAN_CLK_PODF_OFFSET 2 >> -#else >> -#define MXC_CCM_CSCMR2_CAN_CLK_SEL_MASK (0x3F << 2) >> -#define MXC_CCM_CSCMR2_CAN_CLK_SEL_OFFSET 2 >> -#endif >> >> /* Define the bits in register CSCDR1 */ >> #ifndef CONFIG_MX6SX >> @@ -273,15 +276,7 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CSCDR1_USBOH3_CLK_PODF_OFFSET 6 >> #define MXC_CCM_CSCDR1_USBOH3_CLK_PODF_MASK (0x3 << 6) >> #endif >> -#ifdef CONFIG_MX6SL >> -#define MXC_CCM_CSCDR1_UART_CLK_PODF_MASK 0x1F >> -#define MXC_CCM_CSCDR1_UART_CLK_SEL (1 << 6) >> -#else >> #define MXC_CCM_CSCDR1_UART_CLK_PODF_MASK 0x3F >> -#ifdef CONFIG_MX6SX >> -#define MXC_CCM_CSCDR1_UART_CLK_SEL (1 << 6) >> -#endif >> -#endif >> #define MXC_CCM_CSCDR1_UART_CLK_PODF_OFFSET 0 >> >> /* Define the bits in register CS1CDR */ >> @@ -316,10 +311,17 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CS2CDR_ENFC_CLK_PRED_MASK (0x7 << 18) >> #define MXC_CCM_CS2CDR_ENFC_CLK_PRED_OFFSET 18 >> #define MXC_CCM_CS2CDR_ENFC_CLK_PRED(v) (((v) & 0x7) << >> 18) >> + >> +#ifdef CONFIG_MX6QP >> +#define MXC_CCM_CS2CDR_ENFC_CLK_SEL_MASK (0x7 << 15) >> +#define MXC_CCM_CS2CDR_ENFC_CLK_SEL_OFFSET 15 >> +#define MXC_CCM_CS2CDR_ENFC_CLK_SEL(v) (((v) & 0x7) << >> 15) >> +#else >> #define MXC_CCM_CS2CDR_ENFC_CLK_SEL_MASK (0x3 << 16) >> #define MXC_CCM_CS2CDR_ENFC_CLK_SEL_OFFSET 16 >> #define MXC_CCM_CS2CDR_ENFC_CLK_SEL(v) (((v) & 0x3) << >> 16) >> #endif >> +#endif > >This is an example about my concerns I tried to explain above. > >Best regards, >Stefano Babic > > > >-- >===================================================================== >DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de >===================================================================== Regards, Peng. -- _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot