Re: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
On 08/07/2013 05:11 AM, Scott Wood wrote: > On Wed, 2013-08-07 at 09:30 +1000, Benjamin Herrenschmidt wrote: >> On Tue, 2013-08-06 at 18:08 -0500, Scott Wood wrote: >>> Here's another example. get_lppaca() will only build on book3s -- and >>> yet we get requests for e500 code to use this file. >> >> Indeed, Besides there is already accessors afaik for lppaca that compile >> to nothing on E (and if not they would be trivial to add). > > I don't see such an accessor, but if there were, what would happen when > the caller goes on to dereference that nothing? > > There is an accessor for shared_proc specifically (in the spinlock code) > -- not that it would be much help on booke to just compile away that > check and always select one of the pseries state tables over the other. > > -Scott Thanks a lot Scott and Ben for the review. I have addressed the issues in V3 of this patch series which I have just posted out. Regards, Deepthi > > > ___ > Linuxppc-dev mailing list > linuxppc-...@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
On Wed, 2013-08-07 at 09:30 +1000, Benjamin Herrenschmidt wrote: > On Tue, 2013-08-06 at 18:08 -0500, Scott Wood wrote: > > Here's another example. get_lppaca() will only build on book3s -- and > > yet we get requests for e500 code to use this file. > > Indeed, Besides there is already accessors afaik for lppaca that compile > to nothing on E (and if not they would be trivial to add). I don't see such an accessor, but if there were, what would happen when the caller goes on to dereference that nothing? There is an accessor for shared_proc specifically (in the spinlock code) -- not that it would be much help on booke to just compile away that check and always select one of the pseries state tables over the other. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
On Tue, 2013-08-06 at 18:08 -0500, Scott Wood wrote: > Here's another example. get_lppaca() will only build on book3s -- and > yet we get requests for e500 code to use this file. Indeed, Besides there is already accessors afaik for lppaca that compile to nothing on E (and if not they would be trivial to add). Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
On Wed, 2013-07-31 at 08:29 +0530, Deepthi Dharwar wrote: > /* > - * pseries_idle_probe() > + * powerpc_idle_probe() > * Choose state table for shared versus dedicated partition > */ > -static int pseries_idle_probe(void) > +static int powerpc_idle_probe(void) > { > > +#ifndef PPC_POWERNV > if (!firmware_has_feature(FW_FEATURE_SPLPAR)) > return -ENODEV; > +#endif A bunch of ifdefs is not a good start for a file you're claiming is now generic for all powerpc. Certainly you shouldn't be calling pseries stuff based only on the absence of powernv. And do you not support building one kernel that supports both pseries and powernv? > if (cpuidle_disable != IDLE_NO_OVERRIDE) > return -ENODEV; > > if (max_idle_state == 0) { > - printk(KERN_DEBUG "pseries processor idle disabled.\n"); > + printk(KERN_DEBUG "powerpc processor idle disabled.\n"); > return -EPERM; > } > > +#ifdef PPC_POWERNV > + cpuidle_state_table = powernv_states; > +#else > if (get_lppaca()->shared_proc) Here's another example. get_lppaca() will only build on book3s -- and yet we get requests for e500 code to use this file. -Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
> > -static int pseries_cpuidle_add_cpu_notifier(struct notifier_block *n, > +static int powerpc_cpuidle_add_cpu_notifier(struct notifier_block *n, > unsigned long action, void *hcpu) > { > int hotcpu = (unsigned long)hcpu; > struct cpuidle_device *dev = > - per_cpu_ptr(pseries_cpuidle_devices, hotcpu); > + per_cpu_ptr(powerpc_cpuidle_devices, hotcpu); > > if (dev && cpuidle_get_driver()) { > switch (action) { > @@ -235,16 +270,16 @@ static int pseries_cpuidle_add_cpu_notifier(struct > notifier_block *n, > } > > static struct notifier_block setup_hotplug_notifier = { > - .notifier_call = pseries_cpuidle_add_cpu_notifier, > + .notifier_call = powerpc_cpuidle_add_cpu_notifier, > }; > I think Daniel means move the notifier to cpuidle framework, not just powerpc. And should be remove all about *device*. If the notifier handle using device, you can use "cpuidle_devices". - dongsheng > -static int __init pseries_processor_idle_init(void) > +static int __init powerpc_processor_idle_init(void) > { > int retval; > > - retval = pseries_idle_probe(); > + retval = powerpc_idle_probe(); > if (retval) > return retval; > > - pseries_cpuidle_driver_init(); > - retval = cpuidle_register_driver(&pseries_idle_driver); > + powerpc_cpuidle_driver_init(); > + retval = cpuidle_register_driver(&powerpc_idle_driver); > if (retval) { > - printk(KERN_DEBUG "Registration of pseries driver failed.\n"); > + printk(KERN_DEBUG "Registration of powerpc driver failed.\n"); > return retval; > } > > update_smt_snooze_delay(-1, per_cpu(smt_snooze_delay, 0)); > > - retval = pseries_idle_devices_init(); > + retval = powerpc_idle_devices_init(); Should be remove all *device*, using cpuidle_register. - dongsheng
[PATCH V2 5/6] cpuidle/powerpc: Backend-powerpc idle driver for powernv and pseries.
The following patch extends the current pseries backend idle driver to powernv platform. Signed-off-by: Deepthi Dharwar --- arch/powerpc/include/asm/processor.h |2 - arch/powerpc/sysdev/Kconfig |8 +- arch/powerpc/sysdev/Makefile |2 - arch/powerpc/sysdev/processor_idle.c | 132 ++ 4 files changed, 92 insertions(+), 52 deletions(-) diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h index 47a35b0..e64b817 100644 --- a/arch/powerpc/include/asm/processor.h +++ b/arch/powerpc/include/asm/processor.h @@ -426,7 +426,7 @@ enum idle_boot_override {IDLE_NO_OVERRIDE = 0, IDLE_POWERSAVE_OFF}; extern int powersave_nap; /* set if nap mode can be used in idle loop */ extern void power7_nap(void); -#ifdef CONFIG_PSERIES_IDLE +#ifdef CONFIG_POWERPC_IDLE extern void update_smt_snooze_delay(int cpu, int residency); #else static inline void update_smt_snooze_delay(int cpu, int residency) {} diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig index 8564a3f..f61d794 100644 --- a/arch/powerpc/sysdev/Kconfig +++ b/arch/powerpc/sysdev/Kconfig @@ -35,11 +35,11 @@ config GE_FPGA bool default n -config PSERIES_IDLE - bool "Cpuidle driver for pSeries platforms" +config POWERPC_IDLE + bool "Cpuidle driver for POWERPC platforms" depends on CPU_IDLE - depends on PPC_PSERIES + depends on PPC_PSERIES || PPC_POWERNV default y help Select this option to enable processor idle state management - for pSeries through cpuidle subsystem. + for POWER and POWERNV through cpuidle subsystem. diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile index 93d2cdd..ec290e2 100644 --- a/arch/powerpc/sysdev/Makefile +++ b/arch/powerpc/sysdev/Makefile @@ -49,7 +49,7 @@ endif obj-$(CONFIG_PPC4xx_MSI) += ppc4xx_msi.o obj-$(CONFIG_PPC4xx_CPM) += ppc4xx_cpm.o obj-$(CONFIG_PPC4xx_GPIO) += ppc4xx_gpio.o -obj-$(CONFIG_PSERIES_IDLE) += processor_idle.o +obj-$(CONFIG_POWERPC_IDLE) += processor_idle.o obj-$(CONFIG_CPM) += cpm_common.o obj-$(CONFIG_CPM2) += cpm2.o cpm2_pic.o diff --git a/arch/powerpc/sysdev/processor_idle.c b/arch/powerpc/sysdev/processor_idle.c index 0d75a54..d152f540d 100644 --- a/arch/powerpc/sysdev/processor_idle.c +++ b/arch/powerpc/sysdev/processor_idle.c @@ -20,18 +20,18 @@ #include #include -/* Snooze Delay, pseries_idle */ +/* Snooze Delay, powerpc_idle */ DECLARE_PER_CPU(long, smt_snooze_delay); -struct cpuidle_driver pseries_idle_driver = { - .name = "pseries_idle", +struct cpuidle_driver powerpc_idle_driver = { + .name = "powerpc_idle", .owner= THIS_MODULE, }; #define MAX_IDLE_STATE_COUNT 2 static int max_idle_state = MAX_IDLE_STATE_COUNT - 1; -static struct cpuidle_device __percpu *pseries_cpuidle_devices; +static struct cpuidle_device __percpu *powerpc_cpuidle_devices; static struct cpuidle_state *cpuidle_state_table; static inline void idle_loop_prolog(unsigned long *in_purr) @@ -55,13 +55,14 @@ static int snooze_loop(struct cpuidle_device *dev, int index) { unsigned long in_purr; - int cpu = dev->cpu; +#ifndef PPC_POWERNV idle_loop_prolog(&in_purr); +#endif local_irq_enable(); set_thread_flag(TIF_POLLING_NRFLAG); - while ((!need_resched()) && cpu_online(cpu)) { + while (!need_resched()) { ppc64_runlatch_off(); HMT_low(); HMT_very_low(); @@ -71,7 +72,9 @@ static int snooze_loop(struct cpuidle_device *dev, clear_thread_flag(TIF_POLLING_NRFLAG); smp_mb(); +#ifndef PPC_POWERNV idle_loop_epilog(in_purr); +#endif return index; } @@ -135,10 +138,21 @@ static int shared_cede_loop(struct cpuidle_device *dev, return index; } +#ifdef PPC_POWERNV +static int nap_loop(struct cpuidle_device *dev, + struct cpuidle_driver *drv, + int index) +{ + ppc64_runlatch_off(); + power7_idle(); + return index; +} +#endif + /* * States for dedicated partition case. */ -static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = { +static struct cpuidle_state pseries_dedicated_states[MAX_IDLE_STATE_COUNT] = { { /* Snooze */ .name = "snooze", .desc = "snooze", @@ -158,7 +172,7 @@ static struct cpuidle_state dedicated_states[MAX_IDLE_STATE_COUNT] = { /* * States for shared partition case. */ -static struct cpuidle_state shared_states[MAX_IDLE_STATE_COUNT] = { +static struct cpuidle_state pseries_shared_states[MAX_IDLE_STATE_COUNT] = { { /* Shared Cede */ .name = "Shared Cede", .desc = "Shared Cede", @@ -168,13 +182,34 @@ static struct cpuidle_state shared_sta