Message ID | 1672968487.2408577.1344967615049.JavaMail.root@advansee.com |
---|---|
State | Superseded |
Delegated to: | Stefano Babic |
Headers | show |
Dear Benoît Thébaudeau, > This fixes config_pll_clk(), which used 0x20 instead of 0x200 for > PLL4_CLOCK. > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > Cc: Stefano Babic <sbabic@denx.de> > --- > .../arch/arm/cpu/armv7/mx5/clock.c | 34 > +++++++++++++------- .../arch/arm/include/asm/arch-mx5/crm_regs.h | > 17 ++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) > > diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index 9b083c0..10843a4 > 100644 > --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c > @@ -36,7 +36,9 @@ enum pll_clocks { > PLL1_CLOCK = 0, > PLL2_CLOCK, > PLL3_CLOCK, > +#ifdef CONFIG_MX53 > PLL4_CLOCK, > +#endif > PLL_CLOCKS, > }; > > @@ -379,10 +381,10 @@ static u32 get_lp_apm(void) > u32 ret_val = 0; > u32 ccsr = __raw_readl(&mxc_ccm->ccsr); > > - if (((ccsr >> 9) & 1) == 0) > - ret_val = CONFIG_SYS_MX5_HCLK; > + if (ccsr & MXC_CCM_CCSR_LP_APM) > + ret_val = 32768 * 1024; > else > - ret_val = ((32768 * 1024)); > + ret_val = CONFIG_SYS_MX5_HCLK; > > return ret_val; > } > @@ -660,40 +662,50 @@ static int config_pll_clk(enum pll_clocks index, > struct pll_param *pll_param) switch (index) { > case PLL1_CLOCK: > /* Switch ARM to PLL2 clock */ > - __raw_writel(ccsr | 0x4, &mxc_ccm->ccsr); > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > + &mxc_ccm->ccsr); > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > pll_param->mfi, pll_param->mfn, > pll_param->mfd); > /* Switch back */ > - __raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr); > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > + &mxc_ccm->ccsr); Why use __raw_writel() instead of writel() ? Besides ... again, clrsetbits_le32() is a better bet. > break; > case PLL2_CLOCK: > /* Switch to pll2 bypass clock */ > - __raw_writel(ccsr | 0x2, &mxc_ccm->ccsr); > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL2_SW_CLK_SEL, > + &mxc_ccm->ccsr); > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > pll_param->mfi, pll_param->mfn, > pll_param->mfd); > /* Switch back */ > - __raw_writel(ccsr & ~0x2, &mxc_ccm->ccsr); > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL2_SW_CLK_SEL, > + &mxc_ccm->ccsr); > break; > case PLL3_CLOCK: > /* Switch to pll3 bypass clock */ > - __raw_writel(ccsr | 0x1, &mxc_ccm->ccsr); > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL3_SW_CLK_SEL, > + &mxc_ccm->ccsr); > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > pll_param->mfi, pll_param->mfn, > pll_param->mfd); > /* Switch back */ > - __raw_writel(ccsr & ~0x1, &mxc_ccm->ccsr); > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL3_SW_CLK_SEL, > + &mxc_ccm->ccsr); > break; > +#ifdef CONFIG_MX53 > case PLL4_CLOCK: > /* Switch to pll4 bypass clock */ > - __raw_writel(ccsr | 0x20, &mxc_ccm->ccsr); > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL4_SW_CLK_SEL, > + &mxc_ccm->ccsr); > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > pll_param->mfi, pll_param->mfn, > pll_param->mfd); > /* Switch back */ > - __raw_writel(ccsr & ~0x20, &mxc_ccm->ccsr); > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL4_SW_CLK_SEL, > + &mxc_ccm->ccsr); > break; > +#endif > default: > return -EINVAL; > } > diff --git u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h > u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h index > 4fd8dba..7c21351 100644 > --- u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h > +++ u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h > @@ -82,6 +82,23 @@ struct mxc_ccm_reg { > u32 cmeor; > }; > > +/* Define the bits in register CCSR */ > +#if defined(CONFIG_MX51) > +#define MXC_CCM_CCSR_LP_APM (0x1 << 9) > +#elif defined(CONFIG_MX53) > +#define MXC_CCM_CCSR_LP_APM (0x1 << 10) > +#define MXC_CCM_CCSR_PLL4_SW_CLK_SEL (0x1 << 9) > +#endif > +#define MXC_CCM_CCSR_STEP_SEL_OFFSET 7 > +#define MXC_CCM_CCSR_STEP_SEL_MASK (0x3 << 7) > +#define MXC_CCM_CCSR_PLL2_DIV_PODF_OFFSET 5 > +#define MXC_CCM_CCSR_PLL2_DIV_PODF_MASK (0x3 << 5) > +#define MXC_CCM_CCSR_PLL3_DIV_PODF_OFFSET 3 > +#define MXC_CCM_CCSR_PLL3_DIV_PODF_MASK (0x3 << 3) > +#define MXC_CCM_CCSR_PLL1_SW_CLK_SEL (0x1 << 2) > +#define MXC_CCM_CCSR_PLL2_SW_CLK_SEL (0x1 << 1) > +#define MXC_CCM_CCSR_PLL3_SW_CLK_SEL 0x1 > + > /* Define the bits in register CACRR */ > #define MXC_CCM_CACRR_ARM_PODF_OFFSET 0 > #define MXC_CCM_CACRR_ARM_PODF_MASK 0x7
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > > Dear Benoît Thébaudeau, > > > > > This fixes config_pll_clk(), which used 0x20 instead of 0x200 for > > > PLL4_CLOCK. > > > > > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > > Cc: Stefano Babic <sbabic@denx.de> > > > --- > > > > > > .../arch/arm/cpu/armv7/mx5/clock.c | 34 > > > > > > +++++++++++++------- .../arch/arm/include/asm/arch-mx5/crm_regs.h > > > > > > 17 ++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) > > > > > > diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > > > u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index > > > 9b083c0..10843a4 > > > 100644 > > > --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > > > +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c > > > @@ -36,7 +36,9 @@ enum pll_clocks { > > > > > > PLL1_CLOCK = 0, > > > PLL2_CLOCK, > > > PLL3_CLOCK, > > > > > > +#ifdef CONFIG_MX53 > > > > > > PLL4_CLOCK, > > > > > > +#endif > > > > > > PLL_CLOCKS, > > > > > > }; > > > > > > @@ -379,10 +381,10 @@ static u32 get_lp_apm(void) > > > > > > u32 ret_val = 0; > > > u32 ccsr = __raw_readl(&mxc_ccm->ccsr); > > > > > > - if (((ccsr >> 9) & 1) == 0) > > > - ret_val = CONFIG_SYS_MX5_HCLK; > > > + if (ccsr & MXC_CCM_CCSR_LP_APM) > > > + ret_val = 32768 * 1024; > > > > > > else > > > > > > - ret_val = ((32768 * 1024)); > > > + ret_val = CONFIG_SYS_MX5_HCLK; > > > > > > return ret_val; > > > > > > } > > > > > > @@ -660,40 +662,50 @@ static int config_pll_clk(enum pll_clocks > > > index, > > > struct pll_param *pll_param) switch (index) { > > > > > > case PLL1_CLOCK: > > > /* Switch ARM to PLL2 clock */ > > > > > > - __raw_writel(ccsr | 0x4, &mxc_ccm->ccsr); > > > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > > > + &mxc_ccm->ccsr); > > > > > > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > > > > > > pll_param->mfi, pll_param->mfn, > > > pll_param->mfd); > > > > > > /* Switch back */ > > > > > > - __raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr); > > > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > > > + &mxc_ccm->ccsr); > > > > Why use __raw_writel() instead of writel() ? Besides ... again, > > clrsetbits_le32() is a better bet. > > Sure, but it's again for consistency with the existing code. Existing code looks crappy, any reason to not fix it ;-) [...]
Dear Marek Vasut, > Dear Benoît Thébaudeau, > > > This fixes config_pll_clk(), which used 0x20 instead of 0x200 for > > PLL4_CLOCK. > > > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > Cc: Stefano Babic <sbabic@denx.de> > > --- > > .../arch/arm/cpu/armv7/mx5/clock.c | 34 > > +++++++++++++------- .../arch/arm/include/asm/arch-mx5/crm_regs.h > > | > > 17 ++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) > > > > diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > > u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index > > 9b083c0..10843a4 > > 100644 > > --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c > > +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c > > @@ -36,7 +36,9 @@ enum pll_clocks { > > PLL1_CLOCK = 0, > > PLL2_CLOCK, > > PLL3_CLOCK, > > +#ifdef CONFIG_MX53 > > PLL4_CLOCK, > > +#endif > > PLL_CLOCKS, > > }; > > > > @@ -379,10 +381,10 @@ static u32 get_lp_apm(void) > > u32 ret_val = 0; > > u32 ccsr = __raw_readl(&mxc_ccm->ccsr); > > > > - if (((ccsr >> 9) & 1) == 0) > > - ret_val = CONFIG_SYS_MX5_HCLK; > > + if (ccsr & MXC_CCM_CCSR_LP_APM) > > + ret_val = 32768 * 1024; > > else > > - ret_val = ((32768 * 1024)); > > + ret_val = CONFIG_SYS_MX5_HCLK; > > > > return ret_val; > > } > > @@ -660,40 +662,50 @@ static int config_pll_clk(enum pll_clocks > > index, > > struct pll_param *pll_param) switch (index) { > > case PLL1_CLOCK: > > /* Switch ARM to PLL2 clock */ > > - __raw_writel(ccsr | 0x4, &mxc_ccm->ccsr); > > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > > pll_param->mfi, pll_param->mfn, > > pll_param->mfd); > > /* Switch back */ > > - __raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr); > > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL1_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > Why use __raw_writel() instead of writel() ? Besides ... again, > clrsetbits_le32() is a better bet. Sure, but it's again for consistency with the existing code. > > break; > > case PLL2_CLOCK: > > /* Switch to pll2 bypass clock */ > > - __raw_writel(ccsr | 0x2, &mxc_ccm->ccsr); > > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL2_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > > pll_param->mfi, pll_param->mfn, > > pll_param->mfd); > > /* Switch back */ > > - __raw_writel(ccsr & ~0x2, &mxc_ccm->ccsr); > > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL2_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > break; > > case PLL3_CLOCK: > > /* Switch to pll3 bypass clock */ > > - __raw_writel(ccsr | 0x1, &mxc_ccm->ccsr); > > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL3_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > > pll_param->mfi, pll_param->mfn, > > pll_param->mfd); > > /* Switch back */ > > - __raw_writel(ccsr & ~0x1, &mxc_ccm->ccsr); > > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL3_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > break; > > +#ifdef CONFIG_MX53 > > case PLL4_CLOCK: > > /* Switch to pll4 bypass clock */ > > - __raw_writel(ccsr | 0x20, &mxc_ccm->ccsr); > > + __raw_writel(ccsr | MXC_CCM_CCSR_PLL4_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > CHANGE_PLL_SETTINGS(pll, pll_param->pd, > > pll_param->mfi, pll_param->mfn, > > pll_param->mfd); > > /* Switch back */ > > - __raw_writel(ccsr & ~0x20, &mxc_ccm->ccsr); > > + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL4_SW_CLK_SEL, > > + &mxc_ccm->ccsr); > > break; > > +#endif > > default: > > return -EINVAL; > > } > > diff --git > > u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h > > u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h index > > 4fd8dba..7c21351 100644 > > --- u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h > > +++ u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h > > @@ -82,6 +82,23 @@ struct mxc_ccm_reg { > > u32 cmeor; > > }; > > > > +/* Define the bits in register CCSR */ > > +#if defined(CONFIG_MX51) > > +#define MXC_CCM_CCSR_LP_APM (0x1 << 9) > > +#elif defined(CONFIG_MX53) > > +#define MXC_CCM_CCSR_LP_APM (0x1 << 10) > > +#define MXC_CCM_CCSR_PLL4_SW_CLK_SEL (0x1 << 9) > > +#endif > > +#define MXC_CCM_CCSR_STEP_SEL_OFFSET 7 > > +#define MXC_CCM_CCSR_STEP_SEL_MASK (0x3 << 7) > > +#define MXC_CCM_CCSR_PLL2_DIV_PODF_OFFSET 5 > > +#define MXC_CCM_CCSR_PLL2_DIV_PODF_MASK (0x3 << 5) > > +#define MXC_CCM_CCSR_PLL3_DIV_PODF_OFFSET 3 > > +#define MXC_CCM_CCSR_PLL3_DIV_PODF_MASK (0x3 << 3) > > +#define MXC_CCM_CCSR_PLL1_SW_CLK_SEL (0x1 << 2) > > +#define MXC_CCM_CCSR_PLL2_SW_CLK_SEL (0x1 << 1) > > +#define MXC_CCM_CCSR_PLL3_SW_CLK_SEL 0x1 > > + > > /* Define the bits in register CACRR */ > > #define MXC_CCM_CACRR_ARM_PODF_OFFSET 0 > > #define MXC_CCM_CACRR_ARM_PODF_MASK 0x7 > Best regards, Benoît
diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c index 9b083c0..10843a4 100644 --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/clock.c +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/clock.c @@ -36,7 +36,9 @@ enum pll_clocks { PLL1_CLOCK = 0, PLL2_CLOCK, PLL3_CLOCK, +#ifdef CONFIG_MX53 PLL4_CLOCK, +#endif PLL_CLOCKS, }; @@ -379,10 +381,10 @@ static u32 get_lp_apm(void) u32 ret_val = 0; u32 ccsr = __raw_readl(&mxc_ccm->ccsr); - if (((ccsr >> 9) & 1) == 0) - ret_val = CONFIG_SYS_MX5_HCLK; + if (ccsr & MXC_CCM_CCSR_LP_APM) + ret_val = 32768 * 1024; else - ret_val = ((32768 * 1024)); + ret_val = CONFIG_SYS_MX5_HCLK; return ret_val; } @@ -660,40 +662,50 @@ static int config_pll_clk(enum pll_clocks index, struct pll_param *pll_param) switch (index) { case PLL1_CLOCK: /* Switch ARM to PLL2 clock */ - __raw_writel(ccsr | 0x4, &mxc_ccm->ccsr); + __raw_writel(ccsr | MXC_CCM_CCSR_PLL1_SW_CLK_SEL, + &mxc_ccm->ccsr); CHANGE_PLL_SETTINGS(pll, pll_param->pd, pll_param->mfi, pll_param->mfn, pll_param->mfd); /* Switch back */ - __raw_writel(ccsr & ~0x4, &mxc_ccm->ccsr); + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL1_SW_CLK_SEL, + &mxc_ccm->ccsr); break; case PLL2_CLOCK: /* Switch to pll2 bypass clock */ - __raw_writel(ccsr | 0x2, &mxc_ccm->ccsr); + __raw_writel(ccsr | MXC_CCM_CCSR_PLL2_SW_CLK_SEL, + &mxc_ccm->ccsr); CHANGE_PLL_SETTINGS(pll, pll_param->pd, pll_param->mfi, pll_param->mfn, pll_param->mfd); /* Switch back */ - __raw_writel(ccsr & ~0x2, &mxc_ccm->ccsr); + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL2_SW_CLK_SEL, + &mxc_ccm->ccsr); break; case PLL3_CLOCK: /* Switch to pll3 bypass clock */ - __raw_writel(ccsr | 0x1, &mxc_ccm->ccsr); + __raw_writel(ccsr | MXC_CCM_CCSR_PLL3_SW_CLK_SEL, + &mxc_ccm->ccsr); CHANGE_PLL_SETTINGS(pll, pll_param->pd, pll_param->mfi, pll_param->mfn, pll_param->mfd); /* Switch back */ - __raw_writel(ccsr & ~0x1, &mxc_ccm->ccsr); + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL3_SW_CLK_SEL, + &mxc_ccm->ccsr); break; +#ifdef CONFIG_MX53 case PLL4_CLOCK: /* Switch to pll4 bypass clock */ - __raw_writel(ccsr | 0x20, &mxc_ccm->ccsr); + __raw_writel(ccsr | MXC_CCM_CCSR_PLL4_SW_CLK_SEL, + &mxc_ccm->ccsr); CHANGE_PLL_SETTINGS(pll, pll_param->pd, pll_param->mfi, pll_param->mfn, pll_param->mfd); /* Switch back */ - __raw_writel(ccsr & ~0x20, &mxc_ccm->ccsr); + __raw_writel(ccsr & ~MXC_CCM_CCSR_PLL4_SW_CLK_SEL, + &mxc_ccm->ccsr); break; +#endif default: return -EINVAL; } diff --git u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h index 4fd8dba..7c21351 100644 --- u-boot-4d3c95f.orig/arch/arm/include/asm/arch-mx5/crm_regs.h +++ u-boot-4d3c95f/arch/arm/include/asm/arch-mx5/crm_regs.h @@ -82,6 +82,23 @@ struct mxc_ccm_reg { u32 cmeor; }; +/* Define the bits in register CCSR */ +#if defined(CONFIG_MX51) +#define MXC_CCM_CCSR_LP_APM (0x1 << 9) +#elif defined(CONFIG_MX53) +#define MXC_CCM_CCSR_LP_APM (0x1 << 10) +#define MXC_CCM_CCSR_PLL4_SW_CLK_SEL (0x1 << 9) +#endif +#define MXC_CCM_CCSR_STEP_SEL_OFFSET 7 +#define MXC_CCM_CCSR_STEP_SEL_MASK (0x3 << 7) +#define MXC_CCM_CCSR_PLL2_DIV_PODF_OFFSET 5 +#define MXC_CCM_CCSR_PLL2_DIV_PODF_MASK (0x3 << 5) +#define MXC_CCM_CCSR_PLL3_DIV_PODF_OFFSET 3 +#define MXC_CCM_CCSR_PLL3_DIV_PODF_MASK (0x3 << 3) +#define MXC_CCM_CCSR_PLL1_SW_CLK_SEL (0x1 << 2) +#define MXC_CCM_CCSR_PLL2_SW_CLK_SEL (0x1 << 1) +#define MXC_CCM_CCSR_PLL3_SW_CLK_SEL 0x1 + /* Define the bits in register CACRR */ #define MXC_CCM_CACRR_ARM_PODF_OFFSET 0 #define MXC_CCM_CACRR_ARM_PODF_MASK 0x7
This fixes config_pll_clk(), which used 0x20 instead of 0x200 for PLL4_CLOCK. Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Cc: Stefano Babic <sbabic@denx.de> --- .../arch/arm/cpu/armv7/mx5/clock.c | 34 +++++++++++++------- .../arch/arm/include/asm/arch-mx5/crm_regs.h | 17 ++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-)