Re: [PATCH V4 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.
Hi Bartlomiej,
Thanks for the review.
On 08/22/2013 04:26 PM, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Thursday, August 22, 2013 11:00:29 AM Deepthi Dharwar wrote:
This patch involves moving the current pseries_idle backend driver code
from pseries/processor_idle.c to drivers/cpuidle/cpuidle-powerpc.c,
and making the backend code generic enough to be able to extend this
driver code for both powernv and pseries.
It enables the support for pseries platform, such that going forward the
same code
with minimal efforts can be re-used for a common driver on powernv
and can be further extended to support cpuidle idle state mgmt for the rest
of the powerpc platforms in the future. This removes a lot of code duplicacy,
making the code elegant.
This patch mixes the code movement with the actual code changes which is
not a good practice as it makes review more difficult and is generally bad
from the long term maintainance POV.
Please split this patch on code movement and code changes parts.
Sure. I shall do so.
V4 of this patch now also seems to contain changes which I posted on
Tuesday as a part of dev-state_count removal patchset:
http://permalink.gmane.org/gmane.linux.power-management.general/37392
http://permalink.gmane.org/gmane.linux.power-management.general/37393
so some work probably got duplicated. :(
Sorry about that. I have been re-writing this driver over the last few
weeks and this cleanup was on my to-do list since V1 as pointed out by
Daniel. I have missed seeing your cleanup.
Thanks for patch !
Regards,
Deepthi
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics
Signed-off-by: Deepthi Dharwar deep...@linux.vnet.ibm.com
---
arch/powerpc/include/asm/paca.h | 23 +
arch/powerpc/include/asm/processor.h|2
arch/powerpc/platforms/pseries/Kconfig |9 -
arch/powerpc/platforms/pseries/Makefile |1
arch/powerpc/platforms/pseries/processor_idle.c | 360
---
drivers/cpuidle/Kconfig |7
drivers/cpuidle/Makefile|2
drivers/cpuidle/cpuidle-powerpc.c | 304 +++
8 files changed, 337 insertions(+), 371 deletions(-)
delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
create mode 100644 drivers/cpuidle/cpuidle-powerpc.c
diff --git a/arch/powerpc/include/asm/paca.h
b/arch/powerpc/include/asm/paca.h
index 77c91e7..7bd83ff 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -175,6 +175,29 @@ extern void setup_paca(struct paca_struct *new_paca);
extern void allocate_pacas(void);
extern void free_unused_pacas(void);
+#ifdef CONFIG_PPC_BOOK3S
+#define get_lppaca_is_shared_proc() get_paca()-lppaca_ptr-shared_proc
+static inline void set_lppaca_idle(u8 idle)
+{
+get_paca()-lppaca_ptr-idle = idle;
+}
+
+static inline void add_lppaca_wait_state(u64 cycles)
+{
+get_paca()-lppaca_ptr-wait_state_cycles += cycles;
+}
+
+static inline void set_lppaca_donate_dedicated_cpu(u8 value)
+{
+get_paca()-lppaca_ptr-donate_dedicated_cpu = value;
+}
+#else
+#define get_lppaca_is_shared_proc() -1
+static inline void set_lppaca_idle(u8 idle) { }
+static inline void add_lppaca_wait_state(u64 cycles) { }
+static inline void set_lppaca_donate_dedicated_cpu(u8 value) { }
+#endif
+
#else /* CONFIG_PPC64 */
static inline void allocate_pacas(void) { };
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index e378ccc..5f57c56 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -430,7 +430,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_CPU_IDLE_POWERPC
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/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 62b4f80..bb59bb0 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -119,12 +119,3 @@ config DTL
which are accessible through a debugfs file.
Say N if you are unsure.
-
-config PSERIES_IDLE
-bool Cpuidle driver for pSeries platforms
-depends on CPU_IDLE
-depends on PPC_PSERIES
-default y
-help
- Select this option to enable processor idle state management
- through cpuidle subsystem.
diff --git a/arch/powerpc/platforms/pseries/Makefile
b/arch/powerpc/platforms/pseries/Makefile
index 8ae0103..4b22379 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@
Re: [PATCH V4 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.
Hi,
On Thursday, August 22, 2013 11:00:29 AM Deepthi Dharwar wrote:
This patch involves moving the current pseries_idle backend driver code
from pseries/processor_idle.c to drivers/cpuidle/cpuidle-powerpc.c,
and making the backend code generic enough to be able to extend this
driver code for both powernv and pseries.
It enables the support for pseries platform, such that going forward the same
code
with minimal efforts can be re-used for a common driver on powernv
and can be further extended to support cpuidle idle state mgmt for the rest
of the powerpc platforms in the future. This removes a lot of code duplicacy,
making the code elegant.
This patch mixes the code movement with the actual code changes which is
not a good practice as it makes review more difficult and is generally bad
from the long term maintainance POV.
Please split this patch on code movement and code changes parts.
V4 of this patch now also seems to contain changes which I posted on
Tuesday as a part of dev-state_count removal patchset:
http://permalink.gmane.org/gmane.linux.power-management.general/37392
http://permalink.gmane.org/gmane.linux.power-management.general/37393
so some work probably got duplicated. :(
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics
Signed-off-by: Deepthi Dharwar deep...@linux.vnet.ibm.com
---
arch/powerpc/include/asm/paca.h | 23 +
arch/powerpc/include/asm/processor.h|2
arch/powerpc/platforms/pseries/Kconfig |9 -
arch/powerpc/platforms/pseries/Makefile |1
arch/powerpc/platforms/pseries/processor_idle.c | 360
---
drivers/cpuidle/Kconfig |7
drivers/cpuidle/Makefile|2
drivers/cpuidle/cpuidle-powerpc.c | 304 +++
8 files changed, 337 insertions(+), 371 deletions(-)
delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
create mode 100644 drivers/cpuidle/cpuidle-powerpc.c
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 77c91e7..7bd83ff 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -175,6 +175,29 @@ extern void setup_paca(struct paca_struct *new_paca);
extern void allocate_pacas(void);
extern void free_unused_pacas(void);
+#ifdef CONFIG_PPC_BOOK3S
+#define get_lppaca_is_shared_proc() get_paca()-lppaca_ptr-shared_proc
+static inline void set_lppaca_idle(u8 idle)
+{
+ get_paca()-lppaca_ptr-idle = idle;
+}
+
+static inline void add_lppaca_wait_state(u64 cycles)
+{
+ get_paca()-lppaca_ptr-wait_state_cycles += cycles;
+}
+
+static inline void set_lppaca_donate_dedicated_cpu(u8 value)
+{
+ get_paca()-lppaca_ptr-donate_dedicated_cpu = value;
+}
+#else
+#define get_lppaca_is_shared_proc() -1
+static inline void set_lppaca_idle(u8 idle) { }
+static inline void add_lppaca_wait_state(u64 cycles) { }
+static inline void set_lppaca_donate_dedicated_cpu(u8 value) { }
+#endif
+
#else /* CONFIG_PPC64 */
static inline void allocate_pacas(void) { };
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index e378ccc..5f57c56 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -430,7 +430,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_CPU_IDLE_POWERPC
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/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 62b4f80..bb59bb0 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -119,12 +119,3 @@ config DTL
which are accessible through a debugfs file.
Say N if you are unsure.
-
-config PSERIES_IDLE
- bool Cpuidle driver for pSeries platforms
- depends on CPU_IDLE
- depends on PPC_PSERIES
- default y
- help
- Select this option to enable processor idle state management
- through cpuidle subsystem.
diff --git a/arch/powerpc/platforms/pseries/Makefile
b/arch/powerpc/platforms/pseries/Makefile
index 8ae0103..4b22379 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -21,7 +21,6 @@ obj-$(CONFIG_HCALL_STATS) += hvCall_inst.o
obj-$(CONFIG_CMM)+= cmm.o
obj-$(CONFIG_DTL)+= dtl.o
obj-$(CONFIG_IO_EVENT_IRQ) += io_event_irq.o
-obj-$(CONFIG_PSERIES_IDLE) += processor_idle.o
ifeq ($(CONFIG_PPC_PSERIES),y)
obj-$(CONFIG_SUSPEND)+= suspend.o
diff --git
[PATCH V4 3/5] powerpc/cpuidle: Generic powerpc backend cpuidle driver.
This patch involves moving the current pseries_idle backend driver code
from pseries/processor_idle.c to drivers/cpuidle/cpuidle-powerpc.c,
and making the backend code generic enough to be able to extend this
driver code for both powernv and pseries.
It enables the support for pseries platform, such that going forward the same
code
with minimal efforts can be re-used for a common driver on powernv
and can be further extended to support cpuidle idle state mgmt for the rest
of the powerpc platforms in the future. This removes a lot of code duplicacy,
making the code elegant.
Signed-off-by: Deepthi Dharwar deep...@linux.vnet.ibm.com
---
arch/powerpc/include/asm/paca.h | 23 +
arch/powerpc/include/asm/processor.h|2
arch/powerpc/platforms/pseries/Kconfig |9 -
arch/powerpc/platforms/pseries/Makefile |1
arch/powerpc/platforms/pseries/processor_idle.c | 360 ---
drivers/cpuidle/Kconfig |7
drivers/cpuidle/Makefile|2
drivers/cpuidle/cpuidle-powerpc.c | 304 +++
8 files changed, 337 insertions(+), 371 deletions(-)
delete mode 100644 arch/powerpc/platforms/pseries/processor_idle.c
create mode 100644 drivers/cpuidle/cpuidle-powerpc.c
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 77c91e7..7bd83ff 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -175,6 +175,29 @@ extern void setup_paca(struct paca_struct *new_paca);
extern void allocate_pacas(void);
extern void free_unused_pacas(void);
+#ifdef CONFIG_PPC_BOOK3S
+#define get_lppaca_is_shared_proc() get_paca()-lppaca_ptr-shared_proc
+static inline void set_lppaca_idle(u8 idle)
+{
+ get_paca()-lppaca_ptr-idle = idle;
+}
+
+static inline void add_lppaca_wait_state(u64 cycles)
+{
+ get_paca()-lppaca_ptr-wait_state_cycles += cycles;
+}
+
+static inline void set_lppaca_donate_dedicated_cpu(u8 value)
+{
+ get_paca()-lppaca_ptr-donate_dedicated_cpu = value;
+}
+#else
+#define get_lppaca_is_shared_proc()-1
+static inline void set_lppaca_idle(u8 idle) { }
+static inline void add_lppaca_wait_state(u64 cycles) { }
+static inline void set_lppaca_donate_dedicated_cpu(u8 value) { }
+#endif
+
#else /* CONFIG_PPC64 */
static inline void allocate_pacas(void) { };
diff --git a/arch/powerpc/include/asm/processor.h
b/arch/powerpc/include/asm/processor.h
index e378ccc..5f57c56 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -430,7 +430,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_CPU_IDLE_POWERPC
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/platforms/pseries/Kconfig
b/arch/powerpc/platforms/pseries/Kconfig
index 62b4f80..bb59bb0 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -119,12 +119,3 @@ config DTL
which are accessible through a debugfs file.
Say N if you are unsure.
-
-config PSERIES_IDLE
- bool Cpuidle driver for pSeries platforms
- depends on CPU_IDLE
- depends on PPC_PSERIES
- default y
- help
- Select this option to enable processor idle state management
- through cpuidle subsystem.
diff --git a/arch/powerpc/platforms/pseries/Makefile
b/arch/powerpc/platforms/pseries/Makefile
index 8ae0103..4b22379 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -21,7 +21,6 @@ obj-$(CONFIG_HCALL_STATS) += hvCall_inst.o
obj-$(CONFIG_CMM) += cmm.o
obj-$(CONFIG_DTL) += dtl.o
obj-$(CONFIG_IO_EVENT_IRQ) += io_event_irq.o
-obj-$(CONFIG_PSERIES_IDLE) += processor_idle.o
ifeq ($(CONFIG_PPC_PSERIES),y)
obj-$(CONFIG_SUSPEND) += suspend.o
diff --git a/arch/powerpc/platforms/pseries/processor_idle.c
b/arch/powerpc/platforms/pseries/processor_idle.c
deleted file mode 100644
index c905b99..000
--- a/arch/powerpc/platforms/pseries/processor_idle.c
+++ /dev/null
@@ -1,360 +0,0 @@
-/*
- * processor_idle - idle state cpuidle driver.
- * Adapted from drivers/idle/intel_idle.c and
- * drivers/acpi/processor_idle.c
- *
- */
-
-#include linux/kernel.h
-#include linux/module.h
-#include linux/init.h
-#include linux/moduleparam.h
-#include linux/cpuidle.h
-#include linux/cpu.h
-#include linux/notifier.h
-
-#include asm/paca.h
-#include asm/reg.h
-#include asm/machdep.h
-#include asm/firmware.h
-#include asm/runlatch.h
-#include asm/plpar_wrappers.h
-
-struct cpuidle_driver pseries_idle_driver = {
- .name = pseries_idle,
- .owner
