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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux