[U-Boot] [PATCH v2] ARM: mxs: allow boards to select DC-DC switching clock source

[Marek Vasut]

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 at heimpold.de>
> > > Cc: Marek Vasut <marex at denx.de>
> > > Cc: Otavio Salvador <otavio at ossystems.com.br>
> > > Cc: Fabio Estevam <fabio.estevam at 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