| Message ID | 1449788736-14609-1-git-send-email-mhei@heimpold.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On Friday, December 11, 2015 at 12:05:36 AM, Michael Heimpold wrote: > For some board designs, it might be useful to switch the DC-DC > clock source to something else rather the default 24 MHz, e.g. > for EMI reasons. Can you elaborate on this ? Also, is there gonna be a user for this? > For this, override the mxs_power_setup_dcdc_clocksource function > in your board support files. > > Example: > void mxs_power_setup_dcdc_clocksource(void) > { > mxs_power_select_dcdc_clocksource(POWER_MISC_FREQSEL_20MHZ); > } > > Signed-off-by: Michael Heimpold <mhei@heimpold.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Otavio Salvador <otavio@ossystems.com.br> > Cc: Fabio Estevam <fabio.estevam@freescale.com> > --- > > Changes in v2: > - use a weak function approach instead of ifdef'ery as suggested > by Marek Vasut > > arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 31 > +++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mxs/sys_proto.h > | 2 ++ > 2 files changed, 33 insertions(+) > > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 1972de8..64e215c > 100644 > --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > @@ -248,6 +248,36 @@ static void mxs_power_setup_5v_detect(void) > } > > /** > + * mxs_power_switch_dcdc_clocksource() - Switch PLL clock for DC-DC > converters + * > + * This function configures and then enables an alternative PLL clock > source + * for the DC-DC converters. > + */ > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel) > +{ > + struct mxs_power_regs *power_regs = > + (struct mxs_power_regs *)MXS_POWER_BASE; > + > + /* Select clocksource for DC-DC converters */ > + clrsetbits_le32(&power_regs->hw_power_misc, > + POWER_MISC_FREQSEL_MASK, > + freqsel); > + setbits_le32(&power_regs->hw_power_misc, > + POWER_MISC_SEL_PLLCLK); > +} > + > +/** > + * mxs_power_setup_dcdc_clocksource() Please make sure this is compatible with kerneldoc. > + * Normally, there is no need to switch DC-DC clocksource. However, boards > + * can implement this function when required. > + */ > +__weak void mxs_power_setup_dcdc_clocksource(void) > +{ > + debug("SPL: Using default DC-DC clocksource\n"); > +} > + > +/** > * mxs_src_power_init() - Preconfigure the power block > * > * This function configures reasonable values for the DC-DC control loop > @@ -872,6 +902,7 @@ static void mxs_power_configure_power_source(void) > > debug("SPL: Configuring power source\n"); > > + mxs_power_setup_dcdc_clocksource(); > mxs_src_power_init(); > > if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { > diff --git a/arch/arm/include/asm/arch-mxs/sys_proto.h > b/arch/arm/include/asm/arch-mxs/sys_proto.h index 20ff101..4160e43 100644 > --- a/arch/arm/include/asm/arch-mxs/sys_proto.h > +++ b/arch/arm/include/asm/arch-mxs/sys_proto.h > @@ -25,6 +25,8 @@ int mxsmmc_initialize(bd_t *bis, int id, int (*wp)(int), > int (*cd)(int)); void mxs_common_spl_init(const uint32_t arg, const > uint32_t *resptr, const iomux_cfg_t *iomux_setup, > const unsigned int iomux_size); > + > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel); This function does not need to be exported if it's __weak I believe.
Hi Marek, Am Saturday 12 December 2015, 02:42:47 schrieb Marek Vasut: > On Friday, December 11, 2015 at 12:05:36 AM, Michael Heimpold wrote: > > For some board designs, it might be useful to switch the DC-DC > > clock source to something else rather the default 24 MHz, e.g. > > for EMI reasons. > > Can you elaborate on this ? Also, is there gonna be a user for this? Sure. Every DC-DC switching frequency also generates some harmonics. On usual board design, this is not a huge problem, but on our I2SE Duckbill EnOcean devices, the EnOcean transceiver sits very close to the i.MX because of stacked PCBs. Our hardware guys measured, that the harmonics overlaps a little bit with the EnOcean spectrum. Changing the DC-DC frequency results in others harmonics, which do not interfere anymore. I already sent a patch for adding Duckbill support and I'm preparing a v2 which would include this topic, so this would be the first user of this feature. > > > For this, override the mxs_power_setup_dcdc_clocksource function > > in your board support files. > > > > Example: > > void mxs_power_setup_dcdc_clocksource(void) > > { > > > > mxs_power_select_dcdc_clocksource(POWER_MISC_FREQSEL_20MHZ); > > > > } > > > > Signed-off-by: Michael Heimpold <mhei@heimpold.de> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > --- > > > > Changes in v2: > > - use a weak function approach instead of ifdef'ery as suggested > > > > by Marek Vasut > > > > arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 31 > > > > +++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mxs/sys_proto.h > > > > | 2 ++ > > > > 2 files changed, 33 insertions(+) > > > > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 1972de8..64e215c > > 100644 > > --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > @@ -248,6 +248,36 @@ static void mxs_power_setup_5v_detect(void) > > > > } > > > > /** > > > > + * mxs_power_switch_dcdc_clocksource() - Switch PLL clock for DC-DC > > converters + * > > + * This function configures and then enables an alternative PLL clock > > source + * for the DC-DC converters. > > + */ > > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel) > > +{ > > + struct mxs_power_regs *power_regs = > > + (struct mxs_power_regs *)MXS_POWER_BASE; > > + > > + /* Select clocksource for DC-DC converters */ > > + clrsetbits_le32(&power_regs->hw_power_misc, > > + POWER_MISC_FREQSEL_MASK, > > + freqsel); > > + setbits_le32(&power_regs->hw_power_misc, > > + POWER_MISC_SEL_PLLCLK); > > +} > > + > > +/** > > + * mxs_power_setup_dcdc_clocksource() > > Please make sure this is compatible with kerneldoc. Ah, yes. Will do in v3. > > > + * Normally, there is no need to switch DC-DC clocksource. However, > > boards > > + * can implement this function when required. > > + */ > > +__weak void mxs_power_setup_dcdc_clocksource(void) > > +{ > > + debug("SPL: Using default DC-DC clocksource\n"); > > +} > > + > > +/** > > > > * mxs_src_power_init() - Preconfigure the power block > > * > > * This function configures reasonable values for the DC-DC control loop > > > > @@ -872,6 +902,7 @@ static void mxs_power_configure_power_source(void) > > > > debug("SPL: Configuring power source\n"); > > > > + mxs_power_setup_dcdc_clocksource(); > > > > mxs_src_power_init(); > > > > if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { > > > > diff --git a/arch/arm/include/asm/arch-mxs/sys_proto.h > > b/arch/arm/include/asm/arch-mxs/sys_proto.h index 20ff101..4160e43 100644 > > --- a/arch/arm/include/asm/arch-mxs/sys_proto.h > > +++ b/arch/arm/include/asm/arch-mxs/sys_proto.h > > @@ -25,6 +25,8 @@ int mxsmmc_initialize(bd_t *bis, int id, int (*wp)(int), > > int (*cd)(int)); void mxs_common_spl_init(const uint32_t arg, const > > uint32_t *resptr, const iomux_cfg_t *iomux_setup, > > > > const unsigned int iomux_size); > > > > + > > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel); > > This function does not need to be exported if it's __weak I believe. mxs_power_setup_dcdc_clocksource is the weak function, not mxs_power_select_dcdc_clocksource so I think this is needed. Regards, Michael
On Saturday, December 12, 2015 at 10:02:30 AM, Michael Heimpold wrote: > Hi Marek, Hi! > Am Saturday 12 December 2015, 02:42:47 schrieb Marek Vasut: > > On Friday, December 11, 2015 at 12:05:36 AM, Michael Heimpold wrote: > > > For some board designs, it might be useful to switch the DC-DC > > > clock source to something else rather the default 24 MHz, e.g. > > > for EMI reasons. > > > > Can you elaborate on this ? Also, is there gonna be a user for this? > > Sure. Every DC-DC switching frequency also generates some harmonics. > On usual board design, this is not a huge problem, but on our I2SE > Duckbill EnOcean devices, the EnOcean transceiver sits very close to the > i.MX because of stacked PCBs. Our hardware guys measured, that the > harmonics overlaps a little bit with the EnOcean spectrum. Changing > the DC-DC frequency results in others harmonics, which do not interfere > anymore. > I already sent a patch for adding Duckbill support and I'm preparing a v2 > which would include this topic, so this would be the first user of this > feature. Ah, thanks for the detailed explanation, got it. > > > For this, override the mxs_power_setup_dcdc_clocksource function > > > in your board support files. > > > > > > Example: > > > void mxs_power_setup_dcdc_clocksource(void) > > > { > > > > > > mxs_power_select_dcdc_clocksource(POWER_MISC_FREQSEL_20MHZ); > > > > > > } > > > > > > Signed-off-by: Michael Heimpold <mhei@heimpold.de> > > > Cc: Marek Vasut <marex@denx.de> > > > Cc: Otavio Salvador <otavio@ossystems.com.br> > > > Cc: Fabio Estevam <fabio.estevam@freescale.com> > > > --- > > > > > > Changes in v2: > > > - use a weak function approach instead of ifdef'ery as suggested > > > > > > by Marek Vasut > > > > > > arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 31 > > > > > > +++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mxs/sys_proto.h > > > > > > | 2 ++ > > > > > > 2 files changed, 33 insertions(+) > > > > > > diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > > b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 1972de8..64e215c > > > 100644 > > > --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > > +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c > > > @@ -248,6 +248,36 @@ static void mxs_power_setup_5v_detect(void) > > > > > > } > > > > > > /** > > > > > > + * mxs_power_switch_dcdc_clocksource() - Switch PLL clock for DC-DC > > > converters + * > > > + * This function configures and then enables an alternative PLL clock > > > source + * for the DC-DC converters. > > > + */ > > > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel) > > > +{ Looks like the comment and name of the function do not match here ("select" vs "switch"). > > > + struct mxs_power_regs *power_regs = > > > + (struct mxs_power_regs *)MXS_POWER_BASE; > > > + > > > + /* Select clocksource for DC-DC converters */ > > > + clrsetbits_le32(&power_regs->hw_power_misc, > > > + POWER_MISC_FREQSEL_MASK, > > > + freqsel); > > > + setbits_le32(&power_regs->hw_power_misc, > > > + POWER_MISC_SEL_PLLCLK); > > > +} > > > + > > > +/** > > > + * mxs_power_setup_dcdc_clocksource() > > > > Please make sure this is compatible with kerneldoc. > > Ah, yes. Will do in v3. > > > > + * Normally, there is no need to switch DC-DC clocksource. However, > > > boards > > > + * can implement this function when required. > > > + */ > > > +__weak void mxs_power_setup_dcdc_clocksource(void) > > > +{ > > > + debug("SPL: Using default DC-DC clocksource\n"); Shouldn't this call the mxs_power_select_dcdc_clocksource() then somehow ? > > > +} > > > + > > > +/** > > > > > > * mxs_src_power_init() - Preconfigure the power block > > > * > > > * This function configures reasonable values for the DC-DC control > > > loop > > > > > > @@ -872,6 +902,7 @@ static void mxs_power_configure_power_source(void) > > > > > > debug("SPL: Configuring power source\n"); > > > > > > + mxs_power_setup_dcdc_clocksource(); > > > > > > mxs_src_power_init(); > > > > > > if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { > > > > > > diff --git a/arch/arm/include/asm/arch-mxs/sys_proto.h > > > b/arch/arm/include/asm/arch-mxs/sys_proto.h index 20ff101..4160e43 > > > 100644 --- a/arch/arm/include/asm/arch-mxs/sys_proto.h > > > +++ b/arch/arm/include/asm/arch-mxs/sys_proto.h > > > @@ -25,6 +25,8 @@ int mxsmmc_initialize(bd_t *bis, int id, int > > > (*wp)(int), int (*cd)(int)); void mxs_common_spl_init(const uint32_t > > > arg, const uint32_t *resptr, const iomux_cfg_t *iomux_setup, > > > > > > const unsigned int iomux_size); > > > > > > + > > > +void mxs_power_select_dcdc_clocksource(uint32_t freqsel); > > > > This function does not need to be exported if it's __weak I believe. > > mxs_power_setup_dcdc_clocksource is the weak function, not > mxs_power_select_dcdc_clocksource so I think this is needed. So the user is supposed to call this function from the board code, right? Best regards, Marek Vasut
diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c index 1972de8..64e215c 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c +++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c @@ -248,6 +248,36 @@ static void mxs_power_setup_5v_detect(void) } /** + * mxs_power_switch_dcdc_clocksource() - Switch PLL clock for DC-DC converters + * + * This function configures and then enables an alternative PLL clock source + * for the DC-DC converters. + */ +void mxs_power_select_dcdc_clocksource(uint32_t freqsel) +{ + struct mxs_power_regs *power_regs = + (struct mxs_power_regs *)MXS_POWER_BASE; + + /* Select clocksource for DC-DC converters */ + clrsetbits_le32(&power_regs->hw_power_misc, + POWER_MISC_FREQSEL_MASK, + freqsel); + setbits_le32(&power_regs->hw_power_misc, + POWER_MISC_SEL_PLLCLK); +} + +/** + * mxs_power_setup_dcdc_clocksource() + * + * Normally, there is no need to switch DC-DC clocksource. However, boards + * can implement this function when required. + */ +__weak void mxs_power_setup_dcdc_clocksource(void) +{ + debug("SPL: Using default DC-DC clocksource\n"); +} + +/** * mxs_src_power_init() - Preconfigure the power block * * This function configures reasonable values for the DC-DC control loop @@ -872,6 +902,7 @@ static void mxs_power_configure_power_source(void) debug("SPL: Configuring power source\n"); + mxs_power_setup_dcdc_clocksource(); mxs_src_power_init(); if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) { diff --git a/arch/arm/include/asm/arch-mxs/sys_proto.h b/arch/arm/include/asm/arch-mxs/sys_proto.h index 20ff101..4160e43 100644 --- a/arch/arm/include/asm/arch-mxs/sys_proto.h +++ b/arch/arm/include/asm/arch-mxs/sys_proto.h @@ -25,6 +25,8 @@ int mxsmmc_initialize(bd_t *bis, int id, int (*wp)(int), int (*cd)(int)); void mxs_common_spl_init(const uint32_t arg, const uint32_t *resptr, const iomux_cfg_t *iomux_setup, const unsigned int iomux_size); + +void mxs_power_select_dcdc_clocksource(uint32_t freqsel); #endif struct mxs_pair {
For some board designs, it might be useful to switch the DC-DC clock source to something else rather the default 24 MHz, e.g. for EMI reasons. For this, override the mxs_power_setup_dcdc_clocksource function in your board support files. Example: void mxs_power_setup_dcdc_clocksource(void) { mxs_power_select_dcdc_clocksource(POWER_MISC_FREQSEL_20MHZ); } Signed-off-by: Michael Heimpold <mhei@heimpold.de> Cc: Marek Vasut <marex@denx.de> Cc: Otavio Salvador <otavio@ossystems.com.br> Cc: Fabio Estevam <fabio.estevam@freescale.com> --- Changes in v2: - use a weak function approach instead of ifdef'ery as suggested by Marek Vasut arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 31 +++++++++++++++++++++++++++++ arch/arm/include/asm/arch-mxs/sys_proto.h | 2 ++ 2 files changed, 33 insertions(+)