On Thu, Jul 07, 2011 at 10:20:47PM +0100, Colin Cross wrote:
adding Rafael, since he was interested in cpu_pm notifiers.
On Thu, Jul 7, 2011 at 8:50 AM, Lorenzo Pieralisi lorenzo.pieralisi@arm.com wrote:
This patch adds notifiers to manage low-power entry/exit in a platform independent manner through a series of callbacks. The goal is to enhance CPU specific notifiers with a different notifier chain that executes callbacks defined to put the system into low-power states (C-state). The callback must be executed with IRQ disabled and caches still up and running, which in particular means that spinlocks implemented as ldrex/strex are still usable on ARM.
The callbacks are a means to achieve common idle code, where the platform_pm_enter()/exit() functions trigger the actions required to enter/exit low-power states (PCU, clock tree and power domain programming) for a specific platform.
Within the common idle code for ARM, the callbacks executed upon platform_pm_enter/exit run with a virtual mapping cloned from init_mm which means that the virtual address space is still accessible.
The notifier is passed a (void *) argument, that in the context of common idle code is meant to define cpu and cluster states in order to allow the platform specific callback to handle power down/up actions accordingly.
Can you explain how this is different from the cpu_pm notifiers, besides the name? Would they get called at a different point in the idle path? Could they be the same notifier list as the cpu_pm notifiers, but with different enum values?
That is what I did in my previous version, meaning adding an enum to avoid duplicating it, then I decided to change the code to two independent chains.
For the patchset to be generic I have to have a way to call into platform code to do every action needed to program power control unit registers or send firmware commands to put che CPU/Cluster in the required power state. The actions taken are different from cpu_pm and I thought that avoid sharing the same notifier chain would be better.
I just pass a (void *) to platform code through the notifier as a way to decode the required cpu and cluster states that have to be hit.
I call platform_pm_enter before cleaning and invalidating/disabling caching, which might not be proper because some platforms have a fixed time frame to enter low power from the moment power control is programmed and this does not go hand in hand with the variable number of dirty cache lines which can imply different timing required to clean/invalidate them.
If this is purely for platform idle ops, maybe something more like the struct platform_suspend_ops would be more appropriate?
I will look into that, that was planned, I have to check/test if and how I can adapt it to the patchset and if that is proper.
Rafael wanted cpu_pm moved to somewhere outside of ARM, but these platform notifiers sound specific to an ARM common idle implementation, in which case you might need to find another place to put them besides cpu_pm.c.
I understand. It would be nice if we came up with a solution to tackle both requirements in one single place.
Signed-off-by: Lorenzo Pieralisi lorenzo.pieralisi@arm.com
arch/arm/include/asm/cpu_pm.h | 15 +++++++ arch/arm/kernel/cpu_pm.c | 92 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/cpu_pm.h b/arch/arm/include/asm/cpu_pm.h index b4bb715..19b8106 100644 --- a/arch/arm/include/asm/cpu_pm.h +++ b/arch/arm/include/asm/cpu_pm.h @@ -42,8 +42,21 @@ enum cpu_pm_event {
<snip>
int cpu_pm_register_notifier(struct notifier_block *nb); int cpu_pm_unregister_notifier(struct notifier_block *nb); +int platform_pm_register_notifier(struct notifier_block *nb); +int platform__pm_unregister_notifier(struct notifier_block *nb);
Extra underscore
Gah.., will fix it.
Thanks, Lorenzo