Nishant, > -----Original Message----- > From: Nishanth Menon [mailto:nm@xxxxxx] > Sent: Monday, December 13, 2010 7:35 PM > To: Vishwanath Sripathy > Cc: linux-omap; Eduardo Valentin; Kevin Hilman; Tony Lindgren > Subject: Re: [PATCH 5/5 v3] OMAP3630: PM: Erratum i583: disable > coreoff if < ES1.2 > > Vishwanath Sripathy had written, on 12/13/2010 07:54 AM, the > following: > > Nishant, > > > >> -----Original Message----- > >> From: Nishanth Menon [mailto:nm@xxxxxx] > >> Sent: Monday, December 13, 2010 7:13 PM > >> To: Vishwanath Sripathy > >> Cc: linux-omap; Eduardo Valentin; Kevin Hilman; Tony Lindgren > >> Subject: Re: [PATCH 5/5 v3] OMAP3630: PM: Erratum i583: disable > >> coreoff if < ES1.2 > >> > >> Vishwanath Sripathy had written, on 12/13/2010 07:35 AM, the > >> following: > >>> Nishant, > >>> > >>>> -----Original Message----- > >>>> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > >>>> owner@xxxxxxxxxxxxxxx] On Behalf Of Nishanth Menon > >>>> Sent: Friday, December 03, 2010 10:34 PM > >>>> To: linux-omap > >>>> Cc: Eduardo Valentin; Kevin Hilman; Tony Lindgren; Nishanth > Menon > >>>> Subject: [PATCH 5/5 v3] OMAP3630: PM: Erratum i583: disable > >> coreoff if > >>>> < ES1.2 > >>>> > >>>> From: Eduardo Valentin <eduardo.valentin@xxxxxxxxx> > >>>> > >>>> Limitation i583: Self_Refresh Exit issue after OFF mode > >>>> > >>>> Issue: > >>>> When device is waking up from OFF mode, then SDRC state > machine > >>>> sends > >>>> inappropriate sequence violating JEDEC standards. > >>>> > >>>> Impact: > >>>> OMAP3630 < ES1.2 is impacted as follows depending on the > >> platform: > >>>> CS0: for 38.4MHz as internal sysclk, DDR content seen to be > stable, > >>>> while > >>>> for all other sysclk frequencies, varied levels of instability > >>>> seen based on varied parameters. > >>>> CS1: impacted > >>>> > >>>> This patch takes option #3 as recommended by the Silicon > erratum: > >>>> Avoid core power domain transitioning to OFF mode. Power > >> consumption > >>>> impact is expected in this case. > >>>> To do this, we route OFF requests to RET request on the impacted > >>>> revisions > >>>> of silicon. > >>>> > >>>> Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx> > >>>> Cc: Tony Lindgren <tony@xxxxxxxxxxx> > >>>> > >>>> [nm@xxxxxx: rebased the code to 2.6.37-rc2- short circuit code > >> changed > >>>> a bit] > >>>> Signed-off-by: Nishanth Menon <nm@xxxxxx> > >>>> Signed-off-by: Eduardo Valentin <eduardo.valentin@xxxxxxxxx> > >>>> --- > >>>> v3: no functional change in erratum wa implementation, just > >> registration > >>>> of > >>>> erratum is collated to a single cpu detection and version > check > >>>> v2: https://patchwork.kernel.org/patch/365262/ > >>>> rebased to this patch series instead of depending on hs > changes > >>>> fix typo for macro definition > >>>> v1: http://marc.info/?l=linux-omap&m=129013173425266&w=2 > >>>> arch/arm/mach-omap2/pm34xx.c | 14 ++++++++++++++ > >>>> 1 files changed, 14 insertions(+), 0 deletions(-) > >>>> > >>>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach- > >>>> omap2/pm34xx.c > >>>> index b49e02b..ba3c0d6 100644 > >>>> --- a/arch/arm/mach-omap2/pm34xx.c > >>>> +++ b/arch/arm/mach-omap2/pm34xx.c > >>>> @@ -56,6 +56,7 @@ > >>>> #define OMAP343X_CONTROL_REG_VALUE_OFFSET 0xc8 > >>>> > >>>> #define RTA_ERRATUM_i608 (1 << 0) > >>>> +#define SDRC_WAKEUP_ERRATUM_i583 (1 << 1) > >>>> static u16 pm34xx_errata; > >>>> #define IS_PM34XX_ERRATUM(id) (pm34xx_errata & (id)) > >>>> > >>>> @@ -406,6 +407,17 @@ void omap_sram_idle(void) > >>>> } > >>>> > >>>> /* CORE */ > >>>> + /* > >>>> + * Erratum i583: implementation for ES rev < Es1.2 on > 3630 > >>>> + * We cannot enable OFF mode in a stable form for > previous > >>>> + * revisions, transition instead to RET > >>>> + */ > >>>> + if (IS_PM34XX_ERRATUM(SDRC_WAKEUP_ERRATUM_i583) > && > >>>> + (core_next_state == PWRDM_POWER_OFF)) > { > >>>> + pwrdm_set_next_pwrst(core_pwrdm, > >>>> PWRDM_POWER_RET); > >>>> + core_next_state = PWRDM_POWER_RET; > >>>> + } > >>> Since next_state in pwrst_list (for core) is not updated, this is > > throwing > >>> up an error "Powerdomain (core_pwrdm) didn't enter target state > 0" > >> when > >>> you off mode is enabled for ES1.1 or lesser (in suspend path). It's > > not > >>> really true that Core has not entered target state. It has entered > >>> Retention state which is the actual target state set in > > omap_sram_idle. > >>> However it did not enter the state that was passed by > >> omap3_pm_suspend. Is > >>> this expected behavior? > >> we could go both ways on this - this patch will(as you noticed) > indicate > >> that the transition failed on <ES1.2, or we could make it entirely > >> transparent(by modifying the the pwrst_list - claim we achieved off, > >> while not really hitting off - I personally dont think that is correct. > > The point I am making is that you cannot distinguish between genuine > off > > /retention failure since this message is thrown for both pass and fail. > > May be an additional trace message indicating that system entered > > retention instead of off (for ES<1.2) will be useful. > hmm... good point there. > two issues here: > a) omap3_pm_suspend should probably state which state was achieved > as > well in the error message (trivial fix). > b) how do we notify users that it was due to > SDRC_WAKEUP_ERRATUM_i583 > that core-off was denied. -> do this in omap3_pm_suspend(when user > attempts actual OFF) OR omap3_pm_off_mode_enable(when user > attempts to > enable OFF mode)? > > Any suggestions to allow the same uImage boot on all silicon + allow > cpu_idle + suspend paths not to spew pr_info messages(aka cant add > prints in sram_idle)? I vote for denying off mode for Core (for ES<1.2) in omap3_pm_off_mode_enable and throw up a message saying that Core off is not enabled. Then we will not get this failure message in suspend path since pwrst_list will have the right state. Vishwa > > -- > Regards, > Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html