[PATCH 1/1] omap3: Save and restore CM_AUTOIDLE_PLL across off mode

[Premi, Sanjeev]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Saturday, March 05, 2011 4:16 AM
> To: Premi, Sanjeev
> Cc: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH 1/1] omap3: Save and restore CM_AUTOIDLE_PLL across
> off mode
> 
> Hello Sanjeev,
> 
> On Thu, 10 Feb 2011, Sanjeev Premi wrote:
> 
> > As per commit "bb33cc58", ROM code is expected to restore
> > context related to CORE domain. As part of this change,
> > CM_AUTOIDLE_PLL is neither saved nor restored.
> 
> ... by Linux.
[sp] Yes.

> 
> > This results in loosing the value of AUTO_PERIPH_DPLL.
> 
> A few questions:
> 
> - Is ROM code supposed to restore this register?
[sp] Going by the description of the commit message I referenced, it
     It appears to be the case. I am yet to get official confirmation
     from the ROM code team (usually takes quite long); but all
     experiments (i put them in patch 0/1) suggest so.
> 
> - Is there a documented list of which registers/bitfields the ROM code
> will and won't restore?
[sp] No that I am aware of; but yes I have already requested for same
     since this issue was found.
> 
> - Are the entire contents of the register lost, or just AUTO_PERIPH_DPLL?
[sp] Just AUTO_PERIPH_DPLL.

> 
> > The concern of setting the AUTOIDLE flag before the DPLL
> > is locked seems valid. Hence, the restoration of this
> > register is delayed until after the context of PER
> > domain is restored.
> 
> Could you explain a little more about this?  Is there a hardware
> limitation where AUTO_PERIPH_DPLL shouldn't be set unless the DPLL is
> locked?
[sp] This is based on my understanding (and observation) that DPLL may
     take longer to lock after resume. Now if the clock goes to AUTOIDLE
     before it is locked; I am not sure what would be the consequences.
> 
> >
> > The patch has been verified on OMAP3EVM by checking value
> > of CM_AUTOIDLE_PLL register across the suspend/resume
> > sequence (using devmem) with flags sleep_while_idle and
> > enable_off_mode set to 1.
> >
> > Signed-off-by: Sanjeev Premi <premi at ti.com>
> > ---

[sp] 
[snip]...[snip]

> >
> > +/**
> > + * Restore the contents of CM_AUTOIDLE_PLL register.
> > + *
> > + * The implementation below restores AUTO_CORE_DPLL as 'good'
> redundancy.
> 
> I don't understand this part.  Are the entire contents of the register
> lost, or just the AUTO_PERIPH_DPLL field?
> 

[sp] As put above, only AUTO_PERIPH_DPLL; but this is based on observation
     not a *real specification*; I chose to set the AUTO_CORE_DPLL bit as
     redundancy.

~sanjeev

> > + */
> > +static void pll_mod_restore_autoidle(void)
> > +{
> > +	u32 ctx = stored_cm_autoidle_pll();
> > +	u32 val = omap2_cm_read_mod_reg(PLL_MOD, CM_AUTOIDLE);
> > +
> > +	if (ctx & OMAP3430_AUTO_CORE_DPLL_MASK)
> > +		val |= ctx & OMAP3430_AUTO_CORE_DPLL_MASK;
> > +
> > +	if (ctx & OMAP3430_AUTO_PERIPH_DPLL_MASK)
> > +		val |= ctx & OMAP3430_AUTO_PERIPH_DPLL_MASK;
> > +
> > +	omap2_cm_write_mod_reg(val, PLL_MOD, CM_AUTOIDLE);
> > +}
> > +
[snip]...[snip]