Re: [v4 PATCH 1/5]: cpuidle: Cleanup drivers/cpuidle/cpuidle.c
From: Arun R Bharadwaj <hidden>
Date: 2009-09-03 04:43:05
Also in:
lkml
* Peter Zijlstra [off-list ref] [2009-09-02 07:42:24]:
On Tue, 2009-09-01 at 17:08 +0530, Arun R Bharadwaj wrote:quoted
* Arun R Bharadwaj [off-list ref] [2009-09-01 17:07:04]: Cleanup drivers/cpuidle/cpuidle.c Cpuidle maintains a pm_idle_old void pointer because, currently in x86 there is no clean way of registering and unregistering a idle function.Right, and instead of fixing that, they build this cpuidle crap on top, instead of replacing the current crap with it.quoted
So remove pm_idle_old and leave the responsibility of maintaining the list of registered idle loops to the architecture specific code. If the architecture registers cpuidle_idle_call as its idle loop, only then this loop is called.OK, that's a start I guess. Best would be to replace all of pm_idle with cpuidle, which is what should have been done from the very start. If cpuidle cannot fully replace the pm_idle functionality, then it needs to fix that. But having two layers of idle functions is just silly. Looking at patch 2 and 3, you're making the same mistake on power, after those patches there are multiple ways of registering idle functions, one through some native interface and one through cpuidle, this strikes me as undesirable. If cpuidle is a good idle function manager, then it should be good enough to be the sole one, if its not, then why bother with it at all.
Okay, I'm giving this approach a shot now. i.e. trying to make cpuidle as _the_ sole idle function manager. This would mean doing away with pm_idle and ppc_md.power_save. And, cpuidle_idle_call() which is the main idle loop of cpuidle, present in drivers/cpuidle/cpuidle.c will have to be called from arch specific code of cpu_idle() Also this would mean enabling cpuidle for all platforms, even if the platform doesn't have multiple idle states. So suppose a platform doesnt have multiple states, it wouldn't want the bloated code of cpuidle governors, and would want just a simple cpuidle loop. --arun