Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932923AbcLMLv1 (ORCPT ); Tue, 13 Dec 2016 06:51:27 -0500 Received: from mail-pg0-f65.google.com ([74.125.83.65]:33261 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752733AbcLMLvY (ORCPT ); Tue, 13 Dec 2016 06:51:24 -0500 Subject: Re: [PATCH v4 2/4] cpuidle:powernv: Add helper function to populate powernv idle states. To: "Gautham R. Shenoy" , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , "Rafael J. Wysocki" , Daniel Lezcano , Michael Neuling , Vaidyanathan Srinivasan , "Shreyas B. Prabhu" , Shilpasri G Bhat , Stewart Smith , "Oliver O'Halloran" References: <36f9cd2d944772d8e414a8240f9ec36eaec65ebd.1481288905.git.ego@linux.vnet.ibm.com> Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, devicetree@vger.kernel.org, Rob Herring , Mark Rutland From: Balbir Singh Message-ID: Date: Tue, 13 Dec 2016 22:51:04 +1100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <36f9cd2d944772d8e414a8240f9ec36eaec65ebd.1481288905.git.ego@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2763 Lines: 80 On 10/12/16 00:32, Gautham R. Shenoy wrote: > From: "Gautham R. Shenoy" > > In the current code for powernv_add_idle_states, there is a lot of code > duplication while initializing an idle state in powernv_states table. > > Add an inline helper function to populate the powernv_states[] table for > a given idle state. Invoke this for populating the "Nap", "Fastsleep" > and the stop states in powernv_add_idle_states. > > Signed-off-by: Gautham R. Shenoy > --- > drivers/cpuidle/cpuidle-powernv.c | 85 ++++++++++++++++++++++----------------- > include/linux/cpuidle.h | 1 + > 2 files changed, 50 insertions(+), 36 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c > index 7fe442c..db18af1 100644 > --- a/drivers/cpuidle/cpuidle-powernv.c > +++ b/drivers/cpuidle/cpuidle-powernv.c > @@ -167,6 +167,24 @@ static int powernv_cpuidle_driver_init(void) > return 0; > } > > +static inline void add_powernv_state(int index, const char *name, > + unsigned int flags, > + int (*idle_fn)(struct cpuidle_device *, > + struct cpuidle_driver *, > + int), > + unsigned int target_residency, > + unsigned int exit_latency, > + u64 psscr_val) > +{ > + strlcpy(powernv_states[index].name, name, CPUIDLE_NAME_LEN); > + strlcpy(powernv_states[index].desc, name, CPUIDLE_NAME_LEN); Do name and desc ever diverge? > + powernv_states[index].flags = flags; > + powernv_states[index].target_residency = target_residency; > + powernv_states[index].exit_latency = exit_latency; > + powernv_states[index].enter = idle_fn; Why not call it idle_fn instead of enter? > + stop_psscr_table[index] = psscr_val; > +} > + > static int powernv_add_idle_states(void) > { > struct device_node *power_mgt; > @@ -236,6 +254,7 @@ static int powernv_add_idle_states(void) > "ibm,cpu-idle-state-residency-ns", residency_ns, dt_idle_states); > > for (i = 0; i < dt_idle_states; i++) { > + unsigned int exit_latency, target_residency; > /* > * If an idle state has exit latency beyond > * POWERNV_THRESHOLD_LATENCY_NS then don't use it > @@ -243,28 +262,33 @@ static int powernv_add_idle_states(void) > */ > if (latency_ns[i] > POWERNV_THRESHOLD_LATENCY_NS) Ideally this should be called POWERNV_MAX_THRESHOLD_LATENCY_NS then > continue; > + /* > + * Firmware passes residency and latency values in ns. > + * cpuidle expects it in us. > + */ > + exit_latency = ((unsigned int)latency_ns[i]) / 1000; > + if (!rc) > + target_residency = residency_ns[i] / 1000; > + else > + target_residency = 0; Where do we get rc from? what does target_residency = 0 mean? Balbir Singh