[U-Boot] [PATCH v2 4/8] imx: mx6: ccm: Change the clock settings for i.MX6QP
Peng Fan
b51431 at freescale.com
Mon Jun 29 03:50:10 CEST 2015
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 <B37916 at freescale.com>
>> Signed-off-by: Peng Fan <Peng.Fan at 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: sbabic at denx.de
>=====================================================================
Regards,
Peng.
--
More information about the U-Boot
mailing list