From: khilman@ti•com (Kevin Hilman)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 2/5] cpu_pm: call notifiers during suspend
Date: Thu, 08 Sep 2011 10:56:25 -0700 [thread overview]
Message-ID: <87zkiehq9i.fsf@ti.com> (raw)
In-Reply-To: <4E68E956.9010006@ti.com> (Santosh's message of "Thu, 08 Sep 2011 21:42:06 +0530")
Santosh <santosh.shilimkar@ti•com> writes:
> On Thursday 08 September 2011 07:31 PM, Kevin Hilman wrote:
>> Santosh<santosh.shilimkar@ti•com> writes:
>>
>>> On Thursday 08 September 2011 01:32 AM, Kevin Hilman wrote:
>>>> Santosh Shilimkar<santosh.shilimkar@ti•com> writes:
>>>>
>>>>> From: Colin Cross<ccross@android•com>
>>>>>
>>>>> Implements syscore_ops in cpu_pm to call the cpu and
>>>>> cpu cluster notifiers during suspend and resume,
>>>>> allowing drivers receiving the notifications to
>>>>> avoid implementing syscore_ops.
>>>>>
>>>>> Signed-off-by: Colin Cross<ccross@android•com>
>>>>> [santosh.shilimkar at ti.com: Rebased against 3.1-rc4]
>>>>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti•com>
>>>>
>>>> I don't think using syscore_ops is right here. The platform code should
>>>> decide where in its own suspend path the notifiers should be triggered.
>>>>
>>>> The reason is because while the syscore_ops run late in the suspend
>>>> path, they still run before some platform-specific decisions about the
>>>> low-power states are made. That means that any notifiers that need to
>>>> use information about the target low-power state (e.g. whether context
>>>> will be lost or not) cannot do so since that information has not yet
>>>> been decided until the platform_suspend_ops->enter() runs.
>>>>
>>> Initially I thought the same but in general S2R, platform doesn't
>>> support multiple states like CPUIDLE. On OMAP, we do have a debug
>>> option to choose the state but on real product, it's always the
>>> deepest supported state is used. So the driver saving the
>>> full context for S2R, should be fine.
>>>
>>> Ofcourse for CPUIDLE, the notifier call chain decisions are left
>>> with platform CPUIDLE drivers since there can be multiple low
>>> power states and the context save/restore has to be done based
>>> on low power states.
>>>
>>> The advantage with this is, the platform code is clean from the
>>> notfiers calls. CPUIDLE driver needs to call the different notifier
>>> events based on C-states and that perfectly works.
>>>
>>> I liked this simplification for the S2R. Down side is in S2R if you
>>> don't plan to hit deepest state, drivers end up saving full context
>>> which is fine I guess.
>>
>> That's not the downside I'm worried about.
>>
>> If you have a driver that has a notifier, presumably it has something it
>> wants to do to prepare for suspend *and* for idle, and you'd only want a
>> single notifier callback in the driver to be used for both. That
>> callback would look something like:
>>
>> start_preparing_for_suspend();
>>
>> if (next_state == OFF)
>> save_context();
>>
>> finish_preparing_for_suspend();
>>
>>
>> The problem with the current cpu_*_pm_enter() calls in syscore_ops is
>> that they happen before the next states are programmed, so during
>> suspend the 'if (next_state == off)' above would never be true, but
>> during idle it might be.
>>
> Point taken and now I agree with you.
>
> I am going to drop this patch unless and until somebody shouts at me.
> What that means is platform code need to take care of suspend case
> as well which is trivial change.
Agreed.
Thanks,
Kevin
next prev parent reply other threads:[~2011-09-08 17:56 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-03 14:39 [PATCH v2 0/5] CPU PM notifiers Santosh Shilimkar
2011-09-03 14:39 ` [PATCH v2 1/5] cpu_pm: Add cpu power management notifiers Santosh Shilimkar
2011-09-09 22:56 ` Andrew Morton
2011-09-10 4:02 ` Santosh
2011-09-10 9:31 ` Santosh
2011-09-12 5:02 ` Santosh
2011-09-13 5:42 ` Santosh
2011-09-03 14:39 ` [PATCH v2 2/5] cpu_pm: call notifiers during suspend Santosh Shilimkar
2011-09-07 20:02 ` Kevin Hilman
2011-09-08 5:16 ` Santosh
2011-09-08 14:01 ` Kevin Hilman
2011-09-08 16:12 ` Santosh
2011-09-08 17:56 ` Kevin Hilman [this message]
2011-09-08 18:04 ` Colin Cross
2011-09-08 20:51 ` Kevin Hilman
2011-09-09 6:27 ` Santosh
2011-09-03 14:39 ` [PATCH v2 3/5] ARM: gic: Use cpu pm notifiers to save gic state Santosh Shilimkar
2011-09-03 14:39 ` [PATCH v2 4/5] ARM: vfp: Use cpu pm notifiers to save vfp state Santosh Shilimkar
2011-09-03 14:39 ` [PATCH v2 5/5] ARM: gic: Allow gic arch extensions to provide irqchip flags Santosh Shilimkar
2011-09-06 2:34 ` [PATCH v2 0/5] CPU PM notifiers Shawn Guo
2011-09-06 5:17 ` Santosh
2011-09-09 18:00 ` Kevin Hilman
2011-09-10 5:53 ` Santosh
2011-09-16 4:50 ` Santosh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zkiehq9i.fsf@ti.com \
--to=khilman@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox