From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp01.au.ibm.com (e23smtp01.au.ibm.com [202.81.31.143]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "e23smtp01.au.ibm.com", Issuer "GeoTrust SSL CA" (verified OK)) by ozlabs.org (Postfix) with ESMTPS id 9A647B70C2 for ; Mon, 28 Nov 2011 22:03:20 +1100 (EST) Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 28 Nov 2011 10:59:09 +1000 Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id pASAxqre3367140 for ; Mon, 28 Nov 2011 21:59:52 +1100 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id pASB3FBH027878 for ; Mon, 28 Nov 2011 22:03:15 +1100 Message-ID: <4ED36A6E.1090002@linux.vnet.ibm.com> Date: Mon, 28 Nov 2011 16:33:10 +0530 From: Deepthi Dharwar MIME-Version: 1.0 To: Benjamin Herrenschmidt Subject: Re: [RFC PATCH v2 3/4] cpuidle: (POWER) Enable cpuidle and directly call cpuidle_idle_call() for pSeries References: <20111117112815.9191.2322.stgit@localhost6.localdomain6> <20111117112852.9191.34256.stgit@localhost6.localdomain6> <1322435131.23348.17.camel@pasglop> In-Reply-To: <1322435131.23348.17.camel@pasglop> Content-Type: text/plain; charset=ISO-8859-1 Cc: linuxppc-dev@ozlabs.org, linux-pm@lists.linux-foundation.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 11/28/2011 04:35 AM, Benjamin Herrenschmidt wrote: > On Thu, 2011-11-17 at 16:58 +0530, Deepthi Dharwar wrote: >> This patch enables cpuidle for pSeries and cpuidle_idle_call() is >> directly called from the idle loop. As a result pseries_idle cpuidle >> driver registered with cpuidle subsystem comes into action. This patch >> also removes the routines pseries_shared_idle_sleep and >> pseries_dedicated_idle_sleep as they are now implemented as part of >> pseries_idle cpuidle driver. >> >> Signed-off-by: Deepthi Dharwar >> Signed-off-by: Trinabh Gupta >> Signed-off-by: Arun R Bharadwaj >> --- >> arch/powerpc/platforms/Kconfig | 6 ++ >> arch/powerpc/platforms/pseries/setup.c | 86 +------------------------------- >> include/linux/cpuidle.h | 2 - >> 3 files changed, 8 insertions(+), 86 deletions(-) >> >> diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig >> index e458872..0d2a028 100644 >> --- a/arch/powerpc/platforms/Kconfig >> +++ b/arch/powerpc/platforms/Kconfig >> @@ -211,6 +211,12 @@ config PPC_PASEMI_CPUFREQ >> >> endmenu >> >> +menu "CPUIdle driver" >> + >> +source "drivers/cpuidle/Kconfig" >> + >> +endmenu >> + >> config PPC601_SYNC_FIX >> bool "Workarounds for PPC601 bugs" >> depends on 6xx && (PPC_PREP || PPC_PMAC) >> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c >> index 9c6716a..f624e74 100644 >> --- a/arch/powerpc/platforms/pseries/setup.c >> +++ b/arch/powerpc/platforms/pseries/setup.c >> @@ -39,6 +39,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -74,9 +75,6 @@ EXPORT_SYMBOL(CMO_PageSize); >> >> int fwnmi_active; /* TRUE if an FWNMI handler is present */ >> >> -static void pseries_shared_idle_sleep(void); >> -static void pseries_dedicated_idle_sleep(void); >> - >> static struct device_node *pSeries_mpic_node; >> >> static void pSeries_show_cpuinfo(struct seq_file *m) >> @@ -374,18 +372,9 @@ static void __init pSeries_setup_arch(void) >> >> pSeries_nvram_init(); >> >> - /* Choose an idle loop */ >> if (firmware_has_feature(FW_FEATURE_SPLPAR)) { >> vpa_init(boot_cpuid); >> - if (get_lppaca()->shared_proc) { >> - printk(KERN_DEBUG "Using shared processor idle loop\n"); >> - ppc_md.power_save = pseries_shared_idle_sleep; >> - } else { >> - printk(KERN_DEBUG "Using dedicated idle loop\n"); >> - ppc_md.power_save = pseries_dedicated_idle_sleep; >> - } >> - } else { >> - printk(KERN_DEBUG "Using default idle loop\n"); >> + ppc_md.power_save = (void *)cpuidle_idle_call; >> } > > I very very much dislike that cast. You should not have to cast a > function pointer ... EVER. Yes, I ll fix this. This actually bought out a design flaw with the current pseries idle as mentioned by you in the next patch of the series. > >> if (firmware_has_feature(FW_FEATURE_LPAR)) >> @@ -586,77 +575,6 @@ static int __init pSeries_probe(void) >> return 1; >> } >> > > Cheers, > Ben. > > Regards, Deepthi