From mboxrd@z Thu Jan 1 00:00:00 1970 From: Francesco Lavra Subject: Re: [RFC] cpuidle - remove the power_specified field in the driver Date: Sun, 18 Nov 2012 09:40:40 +0100 Message-ID: <50A89F08.7020307@gmail.com> References: <50831CA3.2020602@linaro.org> <1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f46.google.com ([74.125.83.46]:63077 "EHLO mail-ee0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751321Ab2KRIlx (ORCPT ); Sun, 18 Nov 2012 03:41:53 -0500 In-Reply-To: <1352752016-3136-1-git-send-email-daniel.lezcano@linaro.org> Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Daniel Lezcano Cc: linux-pm@vger.kernel.org, khilman@ti.com, deepthi@linux.vnet.ibm.com, g.trinabh@gmail.com, linaro-dev@lists.linaro.org, len.brown@intel.com, linux-kernel@vger.kernel.org, rjw@sisk.pl, jwerner@chromium.org, akpm@linux-foundation.org, snanda@chromium.org 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 > --- > 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 > + return drv->states[i].enter_dead(dev, i); -- Francesco