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
