Re: [PATCH 5/5 v3] OMAP3630: PM: Erratum i583: disable coreoff if < ES1.2

[Nishanth Menon <nm@ti.com>]

Kevin Hilman had written, on 12/13/2010 09:42 PM, the following:
> Nishanth Menon <nm@ti.com> writes:
> 
>> Vishwanath Sripathy had written, on 12/13/2010 08:58 AM, the following:
>> [...]
>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-
>>>> omap2/pm34xx.c
>>>> index ba3c0d6..da12a56 100644
>>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>>> @@ -932,8 +932,15 @@ void omap3_pm_off_mode_enable(int enable)
>>>>   #endif
>>>>
>>>>          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) {
>>>> +                       pwrst->next_state = PWRDM_POWER_RET;
>>>> +                       pr_err("%s: cannot enable Core OFF due to
>>> i583\n",
>>>> +                               __func__);
>>> 		You probably need to throw up this warning only if state
>>> == PWRDM_POWER_OFF. Otherwise this code looks fine to me.
>> Thanks for the review. added it. will post a v4 later today if no one
>> cribs with this approach. I will retain the logic in sram_idle as well
>> as a backup measure.
> 
> This logic doesn't belong in SRAM idle.  To handle the idle case, you
> should also disable the 'valid' bit for any C-state that has CORE off (I
> think there's only one.)
Apologies, but I dont think I get your point. Do you intend to state 
that we dynamically add the C7 state in cpuidle34xx.c if this condition 
is met?
I agree that this additional check in sram_idle should be removed, but 
as long as I handle it in omap3_pm_off_mode_enable where the next states 
are configured, is'nt that enough or am I missing something?

-- 
Regards,
Nishanth Menon