Re: [RFC] cpuidle - remove the power_specified field in the driver

[Daniel Lezcano]

On 11/18/2012 09:40 AM, Francesco Lavra wrote:
> Hi,
> 
> On 11/12/2012 09:26 PM, Daniel Lezcano wrote:
>> This patch follows the discussion about reinitializing the power usage
>> when a C-state is added/removed.
>>
>>  https://lkml.org/lkml/2012/10/16/518
>>
>> We realized the power usage field is never filled and when it is
>> filled for tegra, the power_specified flag is not set making all these
>> values to be resetted when the driver is initialized with the set_power_state
>> function.
>>
>> Julius and I feel this is over-engineered and the power_specified
>> flag could be simply removed and continue assuming the states are
>> backward sorted.
>>
>> The menu governor select function is simplified as the power is ordered.
>> Actually the condition is always true with the current code.
>>
>> The cpuidle_play_dead function is also simplified by doing a reverse lookup
>> on the array.
>>
>> The set_power_states function is removed as it does no make sense anymore.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
>> ---
>>  drivers/cpuidle/cpuidle.c        |   17 ++++-------------
>>  drivers/cpuidle/driver.c         |   25 -------------------------
>>  drivers/cpuidle/governors/menu.c |    8 ++------
>>  include/linux/cpuidle.h          |    2 +-
>>  4 files changed, 7 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 711dd83..f983262 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
>>  {
>>  	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>>  	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>> -	int i, dead_state = -1;
>> -	int power_usage = -1;
>> +	int i;
>>  
>>  	if (!drv)
>>  		return -ENODEV;
>>  
>>  	/* Find lowest-power state that supports long-term idle */
>> -	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
>> -		struct cpuidle_state *s = &drv->states[i];
>> -
>> -		if (s->power_usage < power_usage && s->enter_dead) {
>> -			power_usage = s->power_usage;
>> -			dead_state = i;
>> -		}
>> -	}
>> -
>> -	if (dead_state != -1)
>> -		return drv->states[dead_state].enter_dead(dev, dead_state);
>> +	for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
>> +		if (drv->states[i].play_dead)
> 
> I guess you meant drv->states[i].enter_dead

Yep :)



-- 
 <http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/