Re: [RFC PATCH v2 4/4] cpuidle: (POWER) Handle power_save=off

[Deepthi Dharwar <deepthi@linux.vnet.ibm.com>]

On 11/28/2011 04:37 AM, Benjamin Herrenschmidt wrote:

> On Thu, 2011-11-17 at 16:59 +0530, Deepthi Dharwar wrote:
>> This patch makes pseries_idle_driver not to be registered when
>> power_save=off kernel boot option is specified. The
>> boot_option_idle_override variable used here is similar to
>> its usage on x86.
> 
> Quick Q. With your changes, the CPU will never get into idle at all
> until cpuidle initializes and the driver loads.
> 
> That means not only much later in the boot process, but potentially
> never if the distro has the driver as a module and fails to load it, or
> similar.
> 
> Can't that be an issue ? Shouldn't we keep at least one of the basic
> idle functions as a fallback ?
> 


On an LPAR if cpuidle is disabled, ppc_md.power_save is still set to
cpuidle_idle_call by default here. This would result in calling of
cpuidle_idle_call repeatedly, only for the call to return -ENODEV. The
default idle is never executed.
This would be a major design flaw. No fallback idle routine.

We propose to fix this by checking the return value of
ppc_md.power_save() call from void to int.
Right now return value is void, but if we change this to int, this
would solve two problems. One being removing the cast to a function
pointer in the prev patch and this design flaw stated above.

So by checking the return value of ppc_md.power_save(), we can invoke
the default idle on failure. But my only concern is about the effects of
changing the ppc_md.power_save() to return int on other powerpc
architectures. Would it be a good idea to change the return type to int
which would help us flag an error and fallback to default idle?

> Cheers,

> Ben.
> 
> 


Regards,
Deepthi