Nishanth Menon <nm@xxxxxx> writes: > Kevin Hilman had written, on 12/13/2010 09:42 PM, the following: >> Nishanth Menon <nm@xxxxxx> 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? Yes. More precisely, dynamically set the 'valid' bit of C7 based on this condition. > 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? Setting the next states only sets the default states, but CPUidle changes them. Looking closer at omap3_pm_off_mode_enable() though, it already calls into CPUidle and disables the valid bit for any states that have *either* MPU or core off. You'll probably just need to extend this approach to disable only CORE off state(s). Kevin -- 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