From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753062Ab1K1LDV (ORCPT ); Mon, 28 Nov 2011 06:03:21 -0500 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:50425 "EHLO e23smtp07.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752947Ab1K1LDT (ORCPT ); Mon, 28 Nov 2011 06:03:19 -0500 Message-ID: <4ED36A6E.1090002@linux.vnet.ibm.com> Date: Mon, 28 Nov 2011 16:33:10 +0530 From: Deepthi Dharwar User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.23) Gecko/20110922 Thunderbird/3.1.15 MIME-Version: 1.0 Newsgroups: gmane.linux.power-management.general,gmane.linux.ports.ppc64.devel,gmane.linux.kernel To: Benjamin Herrenschmidt CC: linuxppc-dev@ozlabs.org, linux-pm@lists.linux-foundation.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org 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 Content-Transfer-Encoding: 7bit x-cbid: 11112801-0260-0000-0000-0000001EED32 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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