> -----Original Message----- > From: Nishanth Menon [mailto:nm@xxxxxx] > Sent: Monday, December 13, 2010 8:14 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 > > > -----Original Message----- > > Vishwanath Sripathy had written, on 12/13/2010 08:25 AM, the > following: > > [...] > > >>>>>> + 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. > > Keep in mind - if we disable it in omap3_pm_off_mode_enable - we > will > > deny OFF wholesale if I understand the logic right- not just core-off - > > I kind of think that is extreme. > > I take that back, we could do something like the following instead: > diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach- > omap2/pm34xx.c > index ba3c0d6..74842f1 100644 > --- a/arch/arm/mach-omap2/pm34xx.c > +++ b/arch/arm/mach-omap2/pm34xx.c > @@ -933,7 +933,14 @@ void omap3_pm_off_mode_enable(int enable) > > list_for_each_entry(pwrst, &pwrst_list, node) { > pwrst->next_state = state; > - omap_set_pwrdm_state(pwrst->pwrdm, state); > + if (IS_PM34XX_ERRATUM(SDRC_WAKEUP_ERRATUM_i583) > && > + pwrst->pwrdm == core_pwrdm) { > + omap_set_pwrdm_state(pwrst->pwrdm, > PWRDM_POWER_RET); You need to update pwrst->next_state as well. Otherwise suspend will still throw the same error. I just sent the code in other email. Vishwa > + pr_err("%s: cannot enable Core OFF due to i583\n", > + __func__); > + } else { > + omap_set_pwrdm_state(pwrst->pwrdm, state); > + } > } > } > > Will wait to see if there are additional opinions on this approach -if > none, > Will post a v4 for this patch alone. > 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
