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

From: Daniel Lezcano
Date: Sun Nov 18 2012 - 04:17:41 EST


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/