Re: [U-Boot] [PATCH v2 4/8] imx: mx6: ccm: Change the clock settings for i.MX6QP
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 +++
Re: [U-Boot] [PATCH v2 4/8] imx: mx6: ccm: Change the clock settings for i.MX6QP
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. + 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
[U-Boot] [PATCH v2 4/8] imx: mx6: ccm: Change the clock settings for i.MX6QP
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) + 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