[PATCH v8 0/8] Consolidate cpuidle functionality

[PATCH v8 0/8] Consolidate cpuidle functionality

[Robert Lee]

This patch series moves various functionality duplicated in platform 
cpuidle drivers to the core cpuidle driver. Also, the platform irq
disabling was removed as it appears that all calls into 
cpuidle_call_idle will have already called local_irq_disable().

These changes have been pulled into linux-next.

Len, Andrew, can a request be made for Linus to pull these changes?

Acked-by: Jean Pihet<j-pihet@ti.com>  (v6)
Tested-by: Jean Pihet<j-pihet@ti.com>  (v6, omap3)
Tested-by: Amit Daniel<amit.kachhap@linaro.org>  (v6, Exynos4)
Tested-by: Robert Lee<rob.lee@linaro.org>  (imx51, imx6q)
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Deepthi Dharwar<deepthi@linux.vnet.ibm.com> (core cpuidle only)

v8 changes:
* v7 testing in linux-next revealed a bug reported by Hugh Dickins on x86
platforms.  Investigated and fixed by Daniel Lezcano.
* Made a slight change to the defaut ARM WFI state macro as Daniel Lezcano
suggested the previous interface was prone to mis-use by platforms that 
set power_specified to 1.
* updated to kernel v3.3

These v8 fixes have been applied to the pull tree being used by linux-next:
git://git.linaro.org/people/rob_lee/linux.git cpuidle_consol_pull

v7 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg162290.html

v7 changes:
* Made some  struct whitespace alignment changes.
* Fixed a coding style violation (thanks Jean Pihet)
* Fixed a bug in davinci cpuidle (thanks Jean Pihet)
* Corrected the common ARM cpuidle WFI state description to be ARM platform
 agnostic (thanks Kevin Hilman)
* Fixed the problem causing x86 and PPC builds to fail (thanks Deepthi)
* Re-added a line of code that was mistakenly removed (thanks Deepthi)

v6 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg162018.html

v6 changes:
* Fixed mindless bug in CONFIG_ARCH_HAS_RELAX code in drivers/cpuidle/cpuidle.c
* Removed inline from wrapper function (thanks Mike Turquette and Rob Herring)
* Add zeroing out of last_residency if error value is returned (thanks Mike)
* Made drivers/cpuidle/cpuidle.c more intelligently handle en_core_tk_irqen
flag allowing removal of the if (en_core_tk_irqen) in cpuidle_idle_call (thanks
Daniel Lezcano)
* Moved CPUIDLE_ARM_WFI_STATE macro to arch/arm/include/asm/cpuidle.h (thanks
Jean Pihet)
* Cleaned up some comments and a stray change (thanks Jean)

v5 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg161596.html

v5 changes:
* Added common cpu_do_idle function to core cpuidle
* Added time keep irq en wrapper to core cpuidle
* Removed pre/post enter
* Re-added platforms that can use new common code.

v4 submission can be found here:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/082742.html

v4 changes:
* Removed drivers/cpuidle/common.c
** Removed the initialization helper functions
** Removed the wrapper used to consolidate time keeping and irq enable/disable
* Add time keeping and local_irq_disable handling in cpuidle_call_idle().
* Made necessary modifications to a few platforms that required the most changes
** Note on omap3: changed structure of omap3_idle_drvdata and added
 per_next_state and per_saved_state vars to accomodate new framework.

v3 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg156751.html

v3 changes:
* Made various code organization and style changes as suggested in v2 review.
 (thanks Mark Brown, Rob Herring, Mike Turquette, Kevin Hillman, Daniel Lezcano)
* Removed at91 use of common code.  A separate effort is underway to clean
at91 code and the author has offered to convert to common interface as part
of those changes (if this common interface is accepted in time).
* Made platform cpuidle_driver objects __initdata and dynamically added one
persistent instance of this object in common code.
* Removed imx5 pm usage of gpc_dvfs clock as it is no longer needed after
being enabled during clock initialization.
* Re-organized patches.

v2 submission can be found here:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/144199

v2 changes:
* Common interface moved to drivers/cpuidle and made non arch-specific.
* Made various fixes and suggested additions to the common cpuidle
code from v1 review. (Thanks Rob Herring and Mark Brown)
* Added callback for filling in driver_data field as needed.
* Modified the various platforms with these changes.

v1 submission can be found here:
http://comments.gmane.org/gmane.linux.ports.arm.kernel/142791

Robert Lee (8):
  cpuidle: Add common time keeping and irq enabling
  ARM: at91: Consolidate time keeping and irq enable
  ARM: kirkwood: Consolidate time keeping and irq enable
  ARM: davinci: Consolidate time keeping and irq enable
  ARM: omap: Consolidate OMAP3 time keeping and irq enable
  ARM: omap: Consolidate OMAP4 time keeping and irq enable
  ARM: shmobile: Consolidate time keeping and irq enable
  SH: shmobile: Consolidate time keeping and irq enable

 arch/arm/include/asm/cpuidle.h        |   30 ++++++++++++
 arch/arm/kernel/Makefile              |    2 +-
 arch/arm/kernel/cpuidle.c             |   21 +++++++++
 arch/arm/mach-at91/cpuidle.c          |   67 ++++++++++-----------------
 arch/arm/mach-davinci/cpuidle.c       |   82 +++++++++++++--------------------
 arch/arm/mach-kirkwood/cpuidle.c      |   72 +++++++++--------------------
 arch/arm/mach-omap2/cpuidle34xx.c     |   42 +++++++----------
 arch/arm/mach-omap2/cpuidle44xx.c     |   21 ++-------
 arch/arm/mach-shmobile/cpuidle.c      |   31 +++----------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 ++--
 drivers/cpuidle/cpuidle.c             |   66 +++++++++++++++++++++++---
 include/linux/cpuidle.h               |   13 +++++-
 12 files changed, 231 insertions(+), 226 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h
 create mode 100644 arch/arm/kernel/cpuidle.c

-- 
1.7.9.4

[PATCH v8 1/8] cpuidle: Add common time keeping and irq enabling

[Robert Lee]

Make necessary changes to implement time keeping and irq enabling
in the core cpuidle code.  This will allow the removal of these
functionalities from various platform cpuidle implementations whose
timekeeping and irq enabling follows the form in this common code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Tested-by: Jean Pihet<j-pihet@ti.com>
Tested-by: Amit Daniel<amit.kachhap@linaro.org>
Tested-by: Robert Lee<rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Reviewed-by: Deepthi Dharwar<deepthi@linux.vnet.ibm.com>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/include/asm/cpuidle.h |   30 ++++++++++++++++++
 arch/arm/kernel/Makefile       |    2 +-
 arch/arm/kernel/cpuidle.c      |   21 +++++++++++++
 drivers/cpuidle/cpuidle.c      |   66 +++++++++++++++++++++++++++++++++++-----
 include/linux/cpuidle.h        |   13 +++++++-
 5 files changed, 123 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h
 create mode 100644 arch/arm/kernel/cpuidle.c

diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 0000000..9322f444
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,30 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+#ifdef CONFIG_CPU_IDLE
+extern int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index);
+#else
+static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index) { return -ENODEV; }
+#endif
+
+/* Common ARM WFI state */
+#define ARM_CPUIDLE_WFI_STATE_PWR(p) {\
+	.enter                  = arm_cpuidle_simple_enter,\
+	.exit_latency           = 1,\
+	.target_residency       = 1,\
+	.power_usage		= p,\
+	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
+	.name                   = "WFI",\
+	.desc                   = "ARM WFI",\
+}
+
+/*
+ * in case power_specified == 1, give a default WFI power value needed
+ * by some governors
+ */
+#define ARM_CPUIDLE_WFI_STATE ARM_CPUIDLE_WFI_STATE_PWR(UINT_MAX)
+
+#endif
+
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 43b740d..940c27f 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -21,7 +21,7 @@ obj-$(CONFIG_DEPRECATED_PARAM_STRUCT) += compat.o
 
 obj-$(CONFIG_LEDS)		+= leds.o
 obj-$(CONFIG_OC_ETM)		+= etm.o
-
+obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 obj-$(CONFIG_ISA_DMA_API)	+= dma.o
 obj-$(CONFIG_ARCH_ACORN)	+= ecard.o 
 obj-$(CONFIG_FIQ)		+= fiq.o fiqasm.o
diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
new file mode 100644
index 0000000..89545f6
--- /dev/null
+++ b/arch/arm/kernel/cpuidle.c
@@ -0,0 +1,21 @@
+/*
+ * Copyright 2012 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/cpuidle.h>
+#include <asm/proc-fns.h>
+
+int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	cpu_do_idle();
+
+	return index;
+}
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..4869b55 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -53,6 +53,24 @@ static void cpuidle_kick_cpus(void) {}
 
 static int __cpuidle_register_device(struct cpuidle_device *dev);
 
+static inline int cpuidle_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index)
+{
+	struct cpuidle_state *target_state = &drv->states[index];
+	return target_state->enter(dev, drv, index);
+}
+
+static inline int cpuidle_enter_tk(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv, int index)
+{
+	return cpuidle_wrap_enter(dev, drv, index, cpuidle_enter);
+}
+
+typedef int (*cpuidle_enter_t)(struct cpuidle_device *dev,
+			       struct cpuidle_driver *drv, int index);
+
+static cpuidle_enter_t cpuidle_enter_ops;
+
 /**
  * cpuidle_idle_call - the main idle loop
  *
@@ -63,7 +81,6 @@ int cpuidle_idle_call(void)
 {
 	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
 	struct cpuidle_driver *drv = cpuidle_get_driver();
-	struct cpuidle_state *target_state;
 	int next_state, entered_state;
 
 	if (off)
@@ -92,12 +109,10 @@ int cpuidle_idle_call(void)
 		return 0;
 	}
 
-	target_state = &drv->states[next_state];
-
 	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
 	trace_cpu_idle(next_state, dev->cpu);
 
-	entered_state = target_state->enter(dev, drv, next_state);
+	entered_state = cpuidle_enter_ops(dev, drv, next_state);
 
 	trace_power_end(dev->cpu);
 	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
@@ -110,6 +125,8 @@ int cpuidle_idle_call(void)
 		dev->states_usage[entered_state].time +=
 				(unsigned long long)dev->last_residency;
 		dev->states_usage[entered_state].usage++;
+	} else {
+		dev->last_residency = 0;
 	}
 
 	/* give the governor an opportunity to reflect on the outcome */
@@ -164,6 +181,37 @@ void cpuidle_resume_and_unlock(void)
 
 EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
 
+/**
+ * cpuidle_wrap_enter - performs timekeeping and irqen around enter function
+ * @dev: pointer to a valid cpuidle_device object
+ * @drv: pointer to a valid cpuidle_driver object
+ * @index: index of the target cpuidle state.
+ */
+int cpuidle_wrap_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index,
+				int (*enter)(struct cpuidle_device *dev,
+					struct cpuidle_driver *drv, int index))
+{
+	ktime_t time_start, time_end;
+	s64 diff;
+
+	time_start = ktime_get();
+
+	index = enter(dev, drv, index);
+
+	time_end = ktime_get();
+
+	local_irq_enable();
+
+	diff = ktime_to_us(ktime_sub(time_end, time_start));
+	if (diff > INT_MAX)
+		diff = INT_MAX;
+
+	dev->last_residency = (int) diff;
+
+	return index;
+}
+
 #ifdef CONFIG_ARCH_HAS_CPU_RELAX
 static int poll_idle(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
@@ -212,10 +260,11 @@ static void poll_idle_init(struct cpuidle_driver *drv) {}
 int cpuidle_enable_device(struct cpuidle_device *dev)
 {
 	int ret, i;
+	struct cpuidle_driver *drv = cpuidle_get_driver();
 
 	if (dev->enabled)
 		return 0;
-	if (!cpuidle_get_driver() || !cpuidle_curr_governor)
+	if (!drv || !cpuidle_curr_governor)
 		return -EIO;
 	if (!dev->state_count)
 		return -EINVAL;
@@ -226,13 +275,16 @@ int cpuidle_enable_device(struct cpuidle_device *dev)
 			return ret;
 	}
 
-	poll_idle_init(cpuidle_get_driver());
+	cpuidle_enter_ops = drv->en_core_tk_irqen ?
+		cpuidle_enter_tk : cpuidle_enter;
+
+	poll_idle_init(drv);
 
 	if ((ret = cpuidle_add_state_sysfs(dev)))
 		return ret;
 
 	if (cpuidle_curr_governor->enable &&
-	    (ret = cpuidle_curr_governor->enable(cpuidle_get_driver(), dev)))
+	    (ret = cpuidle_curr_governor->enable(drv, dev)))
 		goto fail_sysfs;
 
 	for (i = 0; i < dev->state_count; i++) {
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 712abcc..927db28 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -15,6 +15,7 @@
 #include <linux/list.h>
 #include <linux/kobject.h>
 #include <linux/completion.h>
+#include <linux/hrtimer.h>
 
 #define CPUIDLE_STATE_MAX	8
 #define CPUIDLE_NAME_LEN	16
@@ -122,6 +123,8 @@ struct cpuidle_driver {
 	struct module 		*owner;
 
 	unsigned int		power_specified:1;
+	/* set to 1 to use the core cpuidle time keeping (for all states). */
+	unsigned int		en_core_tk_irqen:1;
 	struct cpuidle_state	states[CPUIDLE_STATE_MAX];
 	int			state_count;
 	int			safe_state_index;
@@ -140,7 +143,10 @@ extern void cpuidle_pause_and_lock(void);
 extern void cpuidle_resume_and_unlock(void);
 extern int cpuidle_enable_device(struct cpuidle_device *dev);
 extern void cpuidle_disable_device(struct cpuidle_device *dev);
-
+extern int cpuidle_wrap_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index,
+				int (*enter)(struct cpuidle_device *dev,
+					struct cpuidle_driver *drv, int index));
 #else
 static inline void disable_cpuidle(void) { }
 static inline int cpuidle_idle_call(void) { return -ENODEV; }
@@ -157,6 +163,11 @@ static inline void cpuidle_resume_and_unlock(void) { }
 static inline int cpuidle_enable_device(struct cpuidle_device *dev)
 {return -ENODEV; }
 static inline void cpuidle_disable_device(struct cpuidle_device *dev) { }
+static inline int cpuidle_wrap_enter(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv, int index,
+				int (*enter)(struct cpuidle_device *dev,
+					struct cpuidle_driver *drv, int index))
+{ return -ENODEV; }
 
 #endif
 
-- 
1.7.9.4

[PATCH v8 2/8] ARM: at91: Consolidate time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-at91/cpuidle.c |   67 +++++++++++++++---------------------------
 1 file changed, 24 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..d40b3f3 100644
--- a/arch/arm/mach-at91/cpuidle.c
+++ b/arch/arm/mach-at91/cpuidle.c
@@ -17,9 +17,10 @@
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/cpuidle.h>
-#include <asm/proc-fns.h>
 #include <linux/io.h>
 #include <linux/export.h>
+#include <asm/proc-fns.h>
+#include <asm/cpuidle.h>
 
 #include "pm.h"
 
@@ -27,66 +28,46 @@
 
 static DEFINE_PER_CPU(struct cpuidle_device, at91_cpuidle_device);
 
-static struct cpuidle_driver at91_idle_driver = {
-	.name =         "at91_idle",
-	.owner =        THIS_MODULE,
-};
-
 /* Actual code that puts the SoC in different idle states */
 static int at91_enter_idle(struct cpuidle_device *dev,
 			struct cpuidle_driver *drv,
 			       int index)
 {
-	struct timeval before, after;
-	int idle_time;
 	u32 saved_lpr;
 
-	local_irq_disable();
-	do_gettimeofday(&before);
-	if (index == 0)
-		/* Wait for interrupt state */
-		cpu_do_idle();
-	else if (index == 1) {
-		asm("b 1f; .align 5; 1:");
-		asm("mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
-		saved_lpr = sdram_selfrefresh_enable();
-		cpu_do_idle();
-		sdram_selfrefresh_disable(saved_lpr);
-	}
-	do_gettimeofday(&after);
-	local_irq_enable();
-	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
-			(after.tv_usec - before.tv_usec);
+	__asm__("b 1f; .align 5; 1:\n"
+	"	mcr p15, 0, r0, c7, c10, 4");	/* drain write buffer */
+
+	saved_lpr = sdram_selfrefresh_enable();
+	cpu_do_idle();
+	sdram_selfrefresh_disable(saved_lpr);
 
-	dev->last_residency = idle_time;
 	return index;
 }
 
+static struct cpuidle_driver at91_idle_driver = {
+	.name			= "at91_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.states[1]		= {
+		.enter			= at91_enter_idle,
+		.exit_latency		= 10,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "RAM_SR",
+		.desc			= "WFI and DDR Self Refresh",
+	},
+	.state_count = AT91_MAX_STATES,
+};
+
 /* Initialize CPU idle by registering the idle states */
 static int at91_init_cpuidle(void)
 {
 	struct cpuidle_device *device;
-	struct cpuidle_driver *driver = &at91_idle_driver;
 
 	device = &per_cpu(at91_cpuidle_device, smp_processor_id());
 	device->state_count = AT91_MAX_STATES;
-	driver->state_count = AT91_MAX_STATES;
-
-	/* Wait for interrupt state */
-	driver->states[0].enter = at91_enter_idle;
-	driver->states[0].exit_latency = 1;
-	driver->states[0].target_residency = 10000;
-	driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[0].name, "WFI");
-	strcpy(driver->states[0].desc, "Wait for interrupt");
-
-	/* Wait for interrupt and RAM self refresh state */
-	driver->states[1].enter = at91_enter_idle;
-	driver->states[1].exit_latency = 10;
-	driver->states[1].target_residency = 10000;
-	driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[1].name, "RAM_SR");
-	strcpy(driver->states[1].desc, "WFI and RAM Self Refresh");
 
 	cpuidle_register_driver(&at91_idle_driver);
 
-- 
1.7.9.4

[PATCH v8 3/8] ARM: kirkwood: Consolidate time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-kirkwood/cpuidle.c |   72 +++++++++++---------------------------
 1 file changed, 21 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index 7088180..0f17109 100644
--- a/arch/arm/mach-kirkwood/cpuidle.c
+++ b/arch/arm/mach-kirkwood/cpuidle.c
@@ -20,77 +20,47 @@
 #include <linux/io.h>
 #include <linux/export.h>
 #include <asm/proc-fns.h>
+#include <asm/cpuidle.h>
 #include <mach/kirkwood.h>
 
 #define KIRKWOOD_MAX_STATES	2
 
-static struct cpuidle_driver kirkwood_idle_driver = {
-	.name =         "kirkwood_idle",
-	.owner =        THIS_MODULE,
-};
-
-static DEFINE_PER_CPU(struct cpuidle_device, kirkwood_cpuidle_device);
-
 /* Actual code that puts the SoC in different idle states */
 static int kirkwood_enter_idle(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 			       int index)
 {
-	struct timeval before, after;
-	int idle_time;
-
-	local_irq_disable();
-	do_gettimeofday(&before);
-	if (index == 0)
-		/* Wait for interrupt state */
-		cpu_do_idle();
-	else if (index == 1) {
-		/*
-		 * Following write will put DDR in self refresh.
-		 * Note that we have 256 cycles before DDR puts it
-		 * self in self-refresh, so the wait-for-interrupt
-		 * call afterwards won't get the DDR from self refresh
-		 * mode.
-		 */
-		writel(0x7, DDR_OPERATION_BASE);
-		cpu_do_idle();
-	}
-	do_gettimeofday(&after);
-	local_irq_enable();
-	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
-			(after.tv_usec - before.tv_usec);
-
-	/* Update last residency */
-	dev->last_residency = idle_time;
+	writel(0x7, DDR_OPERATION_BASE);
+	cpu_do_idle();
 
 	return index;
 }
 
+static struct cpuidle_driver kirkwood_idle_driver = {
+	.name			= "kirkwood_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.states[1]		= {
+		.enter			= kirkwood_enter_idle,
+		.exit_latency		= 10,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "DDR SR",
+		.desc			= "WFI and DDR Self Refresh",
+	},
+	.state_count = KIRKWOOD_MAX_STATES,
+};
+
+static DEFINE_PER_CPU(struct cpuidle_device, kirkwood_cpuidle_device);
+
 /* Initialize CPU idle by registering the idle states */
 static int kirkwood_init_cpuidle(void)
 {
 	struct cpuidle_device *device;
-	struct cpuidle_driver *driver = &kirkwood_idle_driver;
 
 	device = &per_cpu(kirkwood_cpuidle_device, smp_processor_id());
 	device->state_count = KIRKWOOD_MAX_STATES;
-	driver->state_count = KIRKWOOD_MAX_STATES;
-
-	/* Wait for interrupt state */
-	driver->states[0].enter = kirkwood_enter_idle;
-	driver->states[0].exit_latency = 1;
-	driver->states[0].target_residency = 10000;
-	driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[0].name, "WFI");
-	strcpy(driver->states[0].desc, "Wait for interrupt");
-
-	/* Wait for interrupt and DDR self refresh state */
-	driver->states[1].enter = kirkwood_enter_idle;
-	driver->states[1].exit_latency = 10;
-	driver->states[1].target_residency = 10000;
-	driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[1].name, "DDR SR");
-	strcpy(driver->states[1].desc, "WFI and DDR Self Refresh");
 
 	cpuidle_register_driver(&kirkwood_idle_driver);
 	if (cpuidle_register_device(device)) {
-- 
1.7.9.4

[PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-davinci/cpuidle.c |   82 ++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 49 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..93ae096 100644
--- a/arch/arm/mach-davinci/cpuidle.c
+++ b/arch/arm/mach-davinci/cpuidle.c
@@ -18,6 +18,7 @@
 #include <linux/io.h>
 #include <linux/export.h>
 #include <asm/proc-fns.h>
+#include <asm/cpuidle.h>
 
 #include <mach/cpuidle.h>
 #include <mach/ddr2.h>
@@ -30,12 +31,42 @@ struct davinci_ops {
 	u32 flags;
 };
 
+/* Actual code that puts the SoC in different idle states */
+static int davinci_enter_idle(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+						int index)
+{
+	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
+	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
+
+	if (ops && ops->enter)
+		ops->enter(ops->flags);
+
+	index = cpuidle_wrap_enter(dev,	drv, index,
+				arm_cpuidle_simple_enter);
+
+	if (ops && ops->exit)
+		ops->exit(ops->flags);
+
+	return index;
+}
+
 /* fields in davinci_ops.flags */
 #define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN	BIT(0)
 
 static struct cpuidle_driver davinci_idle_driver = {
-	.name	= "cpuidle-davinci",
-	.owner	= THIS_MODULE,
+	.name		= "cpuidle-davinci",
+	.owner		= THIS_MODULE,
+	.states[0]	= ARM_CPUIDLE_WFI_STATE,
+	.states[1]	= {
+		.enter			= davinci_enter_idle,
+		.exit_latency		= 10,
+		.target_residency	= 100000,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "DDR SR",
+		.desc			= "WFI and DDR Self Refresh",
+	},
+	.state_count = DAVINCI_CPUIDLE_MAX_STATES,
 };
 
 static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
@@ -77,41 +108,10 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
 	},
 };
 
-/* Actual code that puts the SoC in different idle states */
-static int davinci_enter_idle(struct cpuidle_device *dev,
-				struct cpuidle_driver *drv,
-						int index)
-{
-	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
-	struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
-	struct timeval before, after;
-	int idle_time;
-
-	local_irq_disable();
-	do_gettimeofday(&before);
-
-	if (ops && ops->enter)
-		ops->enter(ops->flags);
-	/* Wait for interrupt state */
-	cpu_do_idle();
-	if (ops && ops->exit)
-		ops->exit(ops->flags);
-
-	do_gettimeofday(&after);
-	local_irq_enable();
-	idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
-			(after.tv_usec - before.tv_usec);
-
-	dev->last_residency = idle_time;
-
-	return index;
-}
-
 static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 {
 	int ret;
 	struct cpuidle_device *device;
-	struct cpuidle_driver *driver = &davinci_idle_driver;
 	struct davinci_cpuidle_config *pdata = pdev->dev.platform_data;
 
 	device = &per_cpu(davinci_cpuidle_device, smp_processor_id());
@@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
 
 	ddr2_reg_base = pdata->ddr2_ctlr_base;
 
-	/* Wait for interrupt state */
-	driver->states[0].enter = davinci_enter_idle;
-	driver->states[0].exit_latency = 1;
-	driver->states[0].target_residency = 10000;
-	driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[0].name, "WFI");
-	strcpy(driver->states[0].desc, "Wait for interrupt");
-
-	/* Wait for interrupt and DDR self refresh state */
-	driver->states[1].enter = davinci_enter_idle;
-	driver->states[1].exit_latency = 10;
-	driver->states[1].target_residency = 10000;
-	driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
-	strcpy(driver->states[1].name, "DDR SR");
-	strcpy(driver->states[1].desc, "WFI and DDR Self Refresh");
 	if (pdata->ddr2_pdown)
 		davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
 	cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
 
 	device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
-	driver->state_count = DAVINCI_CPUIDLE_MAX_STATES;
 
 	ret = cpuidle_register_driver(&davinci_idle_driver);
 	if (ret) {
-- 
1.7.9.4

[PATCH v8 5/8] ARM: omap: Consolidate OMAP3 time keeping and irq enable

[Robert Lee]

Use core cpuidle timekeeping and irqen wrapper and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Jean Pihet<j-pihet@ti.com>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   42 +++++++++++++++----------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 464cffd..5358664 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -87,29 +87,14 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
 	return 0;
 }
 
-/**
- * omap3_enter_idle - Programs OMAP3 to enter the specified state
- * @dev: cpuidle device
- * @drv: cpuidle driver
- * @index: the index of state to be entered
- *
- * Called from the CPUidle framework to program the device to the
- * specified target state selected by the governor.
- */
-static int omap3_enter_idle(struct cpuidle_device *dev,
+static int __omap3_enter_idle(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index)
 {
 	struct omap3_idle_statedata *cx =
 			cpuidle_get_statedata(&dev->states_usage[index]);
-	struct timespec ts_preidle, ts_postidle, ts_idle;
 	u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
-	int idle_time;
-
-	/* Used to keep track of the total time in idle */
-	getnstimeofday(&ts_preidle);
 
-	local_irq_disable();
 	local_fiq_disable();
 
 	pwrdm_set_next_pwrst(mpu_pd, mpu_state);
@@ -148,22 +133,29 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	}
 
 return_sleep_time:
-	getnstimeofday(&ts_postidle);
-	ts_idle = timespec_sub(ts_postidle, ts_preidle);
 
-	local_irq_enable();
 	local_fiq_enable();
 
-	idle_time = ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * \
-								USEC_PER_SEC;
-
-	/* Update cpuidle counters */
-	dev->last_residency = idle_time;
-
 	return index;
 }
 
 /**
+ * omap3_enter_idle - Programs OMAP3 to enter the specified state
+ * @dev: cpuidle device
+ * @drv: cpuidle driver
+ * @index: the index of state to be entered
+ *
+ * Called from the CPUidle framework to program the device to the
+ * specified target state selected by the governor.
+ */
+static inline int omap3_enter_idle(struct cpuidle_device *dev,
+				struct cpuidle_driver *drv,
+				int index)
+{
+	return cpuidle_wrap_enter(dev, drv, index, __omap3_enter_idle);
+}
+
+/**
  * next_valid_state - Find next valid C-state
  * @dev: cpuidle device
  * @drv: cpuidle driver
-- 
1.7.9.4

[PATCH v8 6/8] ARM: omap: Consolidate OMAP4 time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle44xx.c |   21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index 72e018b..f386cbe 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -62,15 +62,9 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
 {
 	struct omap4_idle_statedata *cx =
 			cpuidle_get_statedata(&dev->states_usage[index]);
-	struct timespec ts_preidle, ts_postidle, ts_idle;
 	u32 cpu1_state;
-	int idle_time;
 	int cpu_id = smp_processor_id();
 
-	/* Used to keep track of the total time in idle */
-	getnstimeofday(&ts_preidle);
-
-	local_irq_disable();
 	local_fiq_disable();
 
 	/*
@@ -128,26 +122,17 @@ static int omap4_enter_idle(struct cpuidle_device *dev,
 	if (index > 0)
 		clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
 
-	getnstimeofday(&ts_postidle);
-	ts_idle = timespec_sub(ts_postidle, ts_preidle);
-
-	local_irq_enable();
 	local_fiq_enable();
 
-	idle_time = ts_idle.tv_nsec / NSEC_PER_USEC + ts_idle.tv_sec * \
-								USEC_PER_SEC;
-
-	/* Update cpuidle counters */
-	dev->last_residency = idle_time;
-
 	return index;
 }
 
 DEFINE_PER_CPU(struct cpuidle_device, omap4_idle_dev);
 
 struct cpuidle_driver omap4_idle_driver = {
-	.name =		"omap4_idle",
-	.owner =	THIS_MODULE,
+	.name				= "omap4_idle",
+	.owner				= THIS_MODULE,
+	.en_core_tk_irqen		= 1,
 };
 
 static inline void _fill_cstate(struct cpuidle_driver *drv,
-- 
1.7.9.4

[PATCH v8 7/8] ARM: shmobile: Consolidate time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/arm/mach-shmobile/cpuidle.c |   31 +++++++------------------------
 1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 1b23342..ca23b20 100644
--- a/arch/arm/mach-shmobile/cpuidle.c
+++ b/arch/arm/mach-shmobile/cpuidle.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/err.h>
 #include <asm/system.h>
+#include <asm/cpuidle.h>
 #include <asm/io.h>
 
 static void shmobile_enter_wfi(void)
@@ -29,37 +30,19 @@ static int shmobile_cpuidle_enter(struct cpuidle_device *dev,
 				  struct cpuidle_driver *drv,
 				  int index)
 {
-	ktime_t before, after;
-
-	before = ktime_get();
-
-	local_irq_disable();
-	local_fiq_disable();
-
 	shmobile_cpuidle_modes[index]();
 
-	local_irq_enable();
-	local_fiq_enable();
-
-	after = ktime_get();
-	dev->last_residency = ktime_to_ns(ktime_sub(after, before)) >> 10;
-
 	return index;
 }
 
 static struct cpuidle_device shmobile_cpuidle_dev;
 static struct cpuidle_driver shmobile_cpuidle_driver = {
-	.name =		"shmobile_cpuidle",
-	.owner =	THIS_MODULE,
-	.states[0] = {
-		.name = "C1",
-		.desc = "WFI",
-		.exit_latency = 1,
-		.target_residency = 1 * 2,
-		.flags = CPUIDLE_FLAG_TIME_VALID,
-	},
-	.safe_state_index = 0, /* C1 */
-	.state_count = 1,
+	.name			= "shmobile_cpuidle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
+	.states[0]		= ARM_CPUIDLE_WFI_STATE,
+	.safe_state_index	= 0, /* C1 */
+	.state_count		= 1,
 };
 
 void (*shmobile_cpuidle_setup)(struct cpuidle_driver *drv);
-- 
1.7.9.4

[PATCH v8 8/8] SH: shmobile: Consolidate time keeping and irq enable

[Robert Lee]

Enable core cpuidle timekeeping and irq enabling and remove that
handling from this code.

Signed-off-by: Robert Lee <rob.lee@linaro.org>
Reviewed-by: Kevin Hilman <khilman@ti.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Jean Pihet<j-pihet@ti.com>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/sh/kernel/cpu/shmobile/cpuidle.c b/arch/sh/kernel/cpu/shmobile/cpuidle.c
index 6d62eb4..1ddc876 100644
--- a/arch/sh/kernel/cpu/shmobile/cpuidle.c
+++ b/arch/sh/kernel/cpu/shmobile/cpuidle.c
@@ -29,7 +29,6 @@ static int cpuidle_sleep_enter(struct cpuidle_device *dev,
 				int index)
 {
 	unsigned long allowed_mode = SUSP_SH_SLEEP;
-	ktime_t before, after;
 	int requested_state = index;
 	int allowed_state;
 	int k;
@@ -47,19 +46,16 @@ static int cpuidle_sleep_enter(struct cpuidle_device *dev,
 	 */
 	k = min_t(int, allowed_state, requested_state);
 
-	before = ktime_get();
 	sh_mobile_call_standby(cpuidle_mode[k]);
-	after = ktime_get();
-
-	dev->last_residency = (int)ktime_to_ns(ktime_sub(after, before)) >> 10;
 
 	return k;
 }
 
 static struct cpuidle_device cpuidle_dev;
 static struct cpuidle_driver cpuidle_driver = {
-	.name =		"sh_idle",
-	.owner =	THIS_MODULE,
+	.name			= "sh_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
 };
 
 void sh_mobile_setup_cpuidle(void)
-- 
1.7.9.4

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Arnd Bergmann]

On Tuesday 20 March 2012, Robert Lee wrote:
> This patch series moves various functionality duplicated in platform 
> cpuidle drivers to the core cpuidle driver. Also, the platform irq
> disabling was removed as it appears that all calls into 
> cpuidle_call_idle will have already called local_irq_disable().
> 
> These changes have been pulled into linux-next.
> 
> Len, Andrew, can a request be made for Linus to pull these changes?

FWIW, Len seems to be rather inactive on the kernel mailing list right
now and generally not very interested in anything outside of x86 and
acpi. If he doesn't reply in the next few days and Andrew also isn't
interested in handling these patches, I'd suggest you just send the pull
request to Linus, with Len on Cc and explain that you tried to send
them through him but gave up in the end.

	Arnd

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Kevin Hilman]

Arnd Bergmann <arnd.bergmann@linaro.org> writes:

> On Tuesday 20 March 2012, Robert Lee wrote:
>> This patch series moves various functionality duplicated in platform 
>> cpuidle drivers to the core cpuidle driver. Also, the platform irq
>> disabling was removed as it appears that all calls into 
>> cpuidle_call_idle will have already called local_irq_disable().
>> 
>> These changes have been pulled into linux-next.
>> 
>> Len, Andrew, can a request be made for Linus to pull these changes?
>
> FWIW, Len seems to be rather inactive on the kernel mailing list right
> now and generally not very interested in anything outside of x86 and
> acpi. If he doesn't reply in the next few days and Andrew also isn't
> interested in handling these patches, I'd suggest you just send the pull
> request to Linus, with Len on Cc and explain that you tried to send
> them through him but gave up in the end.

FWIW, I have not had good luck getting response for proposed core
CPUidle changes either:

        http://lkml.org/lkml/2011/9/19/374

Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
discussions of scheduler changes that affect load estimation, I suspect
we're all going to have a bit of CPUidle work to do in the
not-so-distant future.
 
Kevin

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Arnd Bergmann]

On Tuesday 20 March 2012, Kevin Hilman wrote:
> Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
> discussions of scheduler changes that affect load estimation, I suspect
> we're all going to have a bit of CPUidle work to do in the
> not-so-distant future.

Hmm, according to the script, you are the maintainer ;-)

$ scripts/get_maintainer.pl -f drivers/cpuidle/cpuidle.c
Kevin Hilman <khilman@ti.com> (commit_signer:8/10=80%)
Len Brown <len.brown@intel.com> (commit_signer:7/10=70%)
Trinabh Gupta <g.trinabh@gmail.com> (commit_signer:4/10=40%)
Arjan van de Ven <arjan@linux.intel.com> (commit_signer:4/10=40%)
Deepthi Dharwar <deepthi@linux.vnet.ibm.com> (commit_signer:4/10=40%)

While I realize that the get_maintainers.pl is not the final word,
you could be the acting cpuidle maintainer for this merge window
and send the pull request.

	Arnd

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Amit Kucheria]

On Wed, Mar 21, 2012 at 12:48 AM, Kevin Hilman <khilman@ti.com> wrote:
> Arnd Bergmann <arnd.bergmann@linaro.org> writes:
>
>> On Tuesday 20 March 2012, Robert Lee wrote:
>>> This patch series moves various functionality duplicated in platform
>>> cpuidle drivers to the core cpuidle driver. Also, the platform irq
>>> disabling was removed as it appears that all calls into
>>> cpuidle_call_idle will have already called local_irq_disable().
>>>
>>> These changes have been pulled into linux-next.
>>>
>>> Len, Andrew, can a request be made for Linus to pull these changes?
>>
>> FWIW, Len seems to be rather inactive on the kernel mailing list right
>> now and generally not very interested in anything outside of x86 and
>> acpi. If he doesn't reply in the next few days and Andrew also isn't
>> interested in handling these patches, I'd suggest you just send the pull
>> request to Linus, with Len on Cc and explain that you tried to send
>> them through him but gave up in the end.
>
> FWIW, I have not had good luck getting response for proposed core
> CPUidle changes either:
>
>        http://lkml.org/lkml/2011/9/19/374
>
> Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
> discussions of scheduler changes that affect load estimation, I suspect
> we're all going to have a bit of CPUidle work to do in the
> not-so-distant future.
>

I don't mean to be piling on to Len here, but Daniel Lezcano too has a
bunch of clean ups that didn't get any maintainer review for over two
months. He has now refreshed them for 3.3 and is getting ready to send
them out again. We (Linaro) expect to be spending a lot of time on
cpuidle in the future and would be glad to help review, test and
collect patches into a tree for Linus/Andrew to pull while we wait for
Len to respond or another maintainer to emerge.

/Amit

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Kevin Hilman]

Arnd Bergmann <arnd.bergmann@linaro.org> writes:

> On Tuesday 20 March 2012, Kevin Hilman wrote:
>> Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
>> discussions of scheduler changes that affect load estimation, I suspect
>> we're all going to have a bit of CPUidle work to do in the
>> not-so-distant future.
>
> Hmm, according to the script, you are the maintainer ;-)

doh, I knew I should've kept quiet :/

> $ scripts/get_maintainer.pl -f drivers/cpuidle/cpuidle.c
> Kevin Hilman <khilman@ti.com> (commit_signer:8/10=80%)
> Len Brown <len.brown@intel.com> (commit_signer:7/10=70%)
> Trinabh Gupta <g.trinabh@gmail.com> (commit_signer:4/10=40%)
> Arjan van de Ven <arjan@linux.intel.com> (commit_signer:4/10=40%)
> Deepthi Dharwar <deepthi@linux.vnet.ibm.com> (commit_signer:4/10=40%)
>
> While I realize that the get_maintainers.pl is not the final word,
> you could be the acting cpuidle maintainer for this merge window
> and send the pull request.

Not exactly the outcome I was hoping for (I have enough stuff to look
after at the moment.) 

However, if you think it would help and won't be seen as a take over
attempt (I don't want to maintain CPUidle), I can possibly do it for
this merge window.  

Before doing that though, I'll need give this series a final once over
before I'm willing to send a potentially flame-attracting pull request.

Kevin

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Rafael J. Wysocki]

On Wednesday, March 21, 2012, Amit Kucheria wrote:
> On Wed, Mar 21, 2012 at 12:48 AM, Kevin Hilman <khilman@ti.com> wrote:
> > Arnd Bergmann <arnd.bergmann@linaro.org> writes:
> >
> >> On Tuesday 20 March 2012, Robert Lee wrote:
> >>> This patch series moves various functionality duplicated in platform
> >>> cpuidle drivers to the core cpuidle driver. Also, the platform irq
> >>> disabling was removed as it appears that all calls into
> >>> cpuidle_call_idle will have already called local_irq_disable().
> >>>
> >>> These changes have been pulled into linux-next.
> >>>
> >>> Len, Andrew, can a request be made for Linus to pull these changes?
> >>
> >> FWIW, Len seems to be rather inactive on the kernel mailing list right
> >> now and generally not very interested in anything outside of x86 and
> >> acpi. If he doesn't reply in the next few days and Andrew also isn't
> >> interested in handling these patches, I'd suggest you just send the pull
> >> request to Linus, with Len on Cc and explain that you tried to send
> >> them through him but gave up in the end.
> >
> > FWIW, I have not had good luck getting response for proposed core
> > CPUidle changes either:
> >
> >        http://lkml.org/lkml/2011/9/19/374
> >
> > Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
> > discussions of scheduler changes that affect load estimation, I suspect
> > we're all going to have a bit of CPUidle work to do in the
> > not-so-distant future.
> >
> 
> I don't mean to be piling on to Len here, but Daniel Lezcano too has a
> bunch of clean ups that didn't get any maintainer review for over two
> months. He has now refreshed them for 3.3 and is getting ready to send
> them out again. We (Linaro) expect to be spending a lot of time on
> cpuidle in the future and would be glad to help review, test and
> collect patches into a tree for Linus/Andrew to pull while we wait for
> Len to respond or another maintainer to emerge.

Well, I discussed that before with Arjan and he said he would maintain
CPUidle if Len didn't have the time, but it seems he didn't expect that
there would be a lot of work on it in the near future.

So, I suggest that if neither Len nor Arjan reappear shortly, people can
send CPUidle patches to me.

Thanks,
Rafael

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Kevin Hilman]

Hi Rob,

Robert Lee <rob.lee@linaro.org> writes:

> This patch series moves various functionality duplicated in platform 
> cpuidle drivers to the core cpuidle driver. Also, the platform irq
> disabling was removed as it appears that all calls into 
> cpuidle_call_idle will have already called local_irq_disable().
>
> These changes have been pulled into linux-next.
>
> Len, Andrew, can a request be made for Linus to pull these changes?
>
> Acked-by: Jean Pihet<j-pihet@ti.com>  (v6)
> Tested-by: Jean Pihet<j-pihet@ti.com>  (v6, omap3)
> Tested-by: Amit Daniel<amit.kachhap@linaro.org>  (v6, Exynos4)
> Tested-by: Robert Lee<rob.lee@linaro.org>  (imx51, imx6q)

Note that there's a space missing between the name and email in these
tags (and for Deepthi's below also.)  That seems to exist in all the
patches.

> Reviewed-by: Kevin Hilman <khilman@ti.com>

For my Reviewed-by, it only applies to the core code and the OMAP
changes.  I haven't reviewed the other platform-specific drivers.  I
believe the same applies to Jean Pihet who works with me on OMAP.

> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Reviewed-by: Deepthi Dharwar<deepthi@linux.vnet.ibm.com> (core cpuidle only)

Looks like you never heard from anyone actively working on at91,
shmobile, kirwood or davinci.

I'm not sure we should merge those platform-specific changes without an
ack from those platform maintainers.

For 3.4, maybe we should just merge the core code and the platforms that
have been reviewed/ack'd, and for 3.5, spent some time nagging the other
platform maintainers to review and test.

Kevin

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Nicolas Pitre]

On Tue, 20 Mar 2012, Kevin Hilman wrote:

> Looks like you never heard from anyone actively working on at91,
> shmobile, kirwood or davinci.
> 
> I'm not sure we should merge those platform-specific changes without an
> ack from those platform maintainers.

Depends.  There is a limit to how long you may be willing to wait after 
people.  Sometimes the only way to make progress is to merge the thing 
and hope that someone who cares will test the stuff during the ~2 months 
-rc period and provide any fix then.  And if that doesn't happen during 
that time then either the code is just right and no one noticed the 
change, or no one cares anymore.

IMHO, Rob posted those patches many times now for quite a while (many 
months) already.  So I'd suggest simply pushing that stuff into mainline 
as is.


Nicolas

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1091 bytes --]

On Wed, 21 Mar 2012, Amit Kucheria wrote:

> On Wed, Mar 21, 2012 at 12:48 AM, Kevin Hilman <khilman@ti.com> wrote:
> > Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
> > discussions of scheduler changes that affect load estimation, I suspect
> > we're all going to have a bit of CPUidle work to do in the
> > not-so-distant future.
> >
> 
> I don't mean to be piling on to Len here, but Daniel Lezcano too has a
> bunch of clean ups that didn't get any maintainer review for over two
> months. He has now refreshed them for 3.3 and is getting ready to send
> them out again. We (Linaro) expect to be spending a lot of time on
> cpuidle in the future and would be glad to help review, test and
> collect patches into a tree for Linus/Andrew to pull while we wait for
> Len to respond or another maintainer to emerge.

As I advised recently, it is usually a good idea for the person/group 
who is working the most on a subsystem to simply take the lead and 
become the new maintainer.  Waiting for another maintainer to emerge 
could be compared to cowardice.


Nicolas

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Deepthi Dharwar]

On 03/21/2012 05:31 AM, Rafael J. Wysocki wrote:

> On Wednesday, March 21, 2012, Amit Kucheria wrote:
>> On Wed, Mar 21, 2012 at 12:48 AM, Kevin Hilman <khilman@ti.com> wrote:
>>> Arnd Bergmann <arnd.bergmann@linaro.org> writes:
>>>
>>>> On Tuesday 20 March 2012, Robert Lee wrote:
>>>>> This patch series moves various functionality duplicated in platform
>>>>> cpuidle drivers to the core cpuidle driver. Also, the platform irq
>>>>> disabling was removed as it appears that all calls into
>>>>> cpuidle_call_idle will have already called local_irq_disable().
>>>>>
>>>>> These changes have been pulled into linux-next.
>>>>>
>>>>> Len, Andrew, can a request be made for Linus to pull these changes?
>>>>
>>>> FWIW, Len seems to be rather inactive on the kernel mailing list right
>>>> now and generally not very interested in anything outside of x86 and
>>>> acpi. If he doesn't reply in the next few days and Andrew also isn't
>>>> interested in handling these patches, I'd suggest you just send the pull
>>>> request to Linus, with Len on Cc and explain that you tried to send
>>>> them through him but gave up in the end.
>>>
>>> FWIW, I have not had good luck getting response for proposed core
>>> CPUidle changes either:
>>>
>>>        http://lkml.org/lkml/2011/9/19/374
>>>
>>> Maybe it's time that drivers/cpuidle gets a maintainer.  With lots of
>>> discussions of scheduler changes that affect load estimation, I suspect
>>> we're all going to have a bit of CPUidle work to do in the
>>> not-so-distant future.
>>>
>>
>> I don't mean to be piling on to Len here, but Daniel Lezcano too has a
>> bunch of clean ups that didn't get any maintainer review for over two
>> months. He has now refreshed them for 3.3 and is getting ready to send
>> them out again. We (Linaro) expect to be spending a lot of time on
>> cpuidle in the future and would be glad to help review, test and
>> collect patches into a tree for Linus/Andrew to pull while we wait for
>> Len to respond or another maintainer to emerge.
> 
> Well, I discussed that before with Arjan and he said he would maintain
> CPUidle if Len didn't have the time, but it seems he didn't expect that
> there would be a lot of work on it in the near future.
> 
> So, I suggest that if neither Len nor Arjan reappear shortly, people can
> send CPUidle patches to me.
> 
> Thanks,
> Rafael
> 


I'll be glad to assist you in this.

Cheers,
Deepthi

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Rob Lee]

On Tue, Mar 20, 2012 at 7:10 PM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Rob,
>
> Robert Lee <rob.lee@linaro.org> writes:
>
>> This patch series moves various functionality duplicated in platform
>> cpuidle drivers to the core cpuidle driver. Also, the platform irq
>> disabling was removed as it appears that all calls into
>> cpuidle_call_idle will have already called local_irq_disable().
>>
>> These changes have been pulled into linux-next.
>>
>> Len, Andrew, can a request be made for Linus to pull these changes?
>>
>> Acked-by: Jean Pihet<j-pihet@ti.com>  (v6)
>> Tested-by: Jean Pihet<j-pihet@ti.com>  (v6, omap3)
>> Tested-by: Amit Daniel<amit.kachhap@linaro.org>  (v6, Exynos4)
>> Tested-by: Robert Lee<rob.lee@linaro.org>  (imx51, imx6q)
>
> Note that there's a space missing between the name and email in these
> tags (and for Deepthi's below also.)  That seems to exist in all the
> patches.
>

Thanks.  This is now fixed in my pull tree.

>> Reviewed-by: Kevin Hilman <khilman@ti.com>
>
> For my Reviewed-by, it only applies to the core code and the OMAP
> changes.  I haven't reviewed the other platform-specific drivers.  I
> believe the same applies to Jean Pihet who works with me on OMAP.
>
>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Reviewed-by: Deepthi Dharwar<deepthi@linux.vnet.ibm.com> (core cpuidle only)
>
> Looks like you never heard from anyone actively working on at91,
> shmobile, kirwood or davinci.
>
> I'm not sure we should merge those platform-specific changes without an
> ack from those platform maintainers.
>
> For 3.4, maybe we should just merge the core code and the platforms that
> have been reviewed/ack'd, and for 3.5, spent some time nagging the other
> platform maintainers to review and test.
>
> Kevin
>

Re: [PATCH v8 0/8] Consolidate cpuidle functionality

[Len Brown]

> So, I suggest that if neither Len nor Arjan reappear shortly, people can
> send CPUidle patches to me.


/me reappears

this series is in my tree now, and I'll be poking at it a bit tomorrow.
If everything is happy I'll send it for 3.4.


thanks,
-Len

Re: [PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable

[Rob Lee]

Sekhar tested this patch on Davinci last night and found a problem.  I
looked at the code again and found a mindless omission on my part (see
below).  Fix is trivial.  I've check all other platforms and confirmed
this problem does not exist for those.  Will resend a v9 of the
patchset shortly.

On Tue, Mar 20, 2012 at 3:22 PM, Robert Lee <rob.lee@linaro.org> wrote:
> Enable core cpuidle timekeeping and irq enabling and remove that
> handling from this code.
>
> Signed-off-by: Robert Lee <rob.lee@linaro.org>
> Reviewed-by: Kevin Hilman <khilman@ti.com>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Acked-by: Jean Pihet<j-pihet@ti.com>
> ---
>  arch/arm/mach-davinci/cpuidle.c |   82 ++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 49 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index a30c7c5..93ae096 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
> @@ -18,6 +18,7 @@
>  #include <linux/io.h>
>  #include <linux/export.h>
>  #include <asm/proc-fns.h>
> +#include <asm/cpuidle.h>
>
>  #include <mach/cpuidle.h>
>  #include <mach/ddr2.h>
> @@ -30,12 +31,42 @@ struct davinci_ops {
>        u32 flags;
>  };
>
> +/* Actual code that puts the SoC in different idle states */
> +static int davinci_enter_idle(struct cpuidle_device *dev,
> +                               struct cpuidle_driver *drv,
> +                                               int index)
> +{
> +       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> +       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
> +
> +       if (ops && ops->enter)
> +               ops->enter(ops->flags);
> +
> +       index = cpuidle_wrap_enter(dev, drv, index,
> +                               arm_cpuidle_simple_enter);
> +
> +       if (ops && ops->exit)
> +               ops->exit(ops->flags);
> +
> +       return index;
> +}
> +
>  /* fields in davinci_ops.flags */
>  #define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN        BIT(0)
>
>  static struct cpuidle_driver davinci_idle_driver = {
> -       .name   = "cpuidle-davinci",
> -       .owner  = THIS_MODULE,
> +       .name           = "cpuidle-davinci",
> +       .owner          = THIS_MODULE,

Argh! I am missing the ".en_core_tk_irqen = 1" here.


> +       .states[0]      = ARM_CPUIDLE_WFI_STATE,
> +       .states[1]      = {
> +               .enter                  = davinci_enter_idle,
> +               .exit_latency           = 10,
> +               .target_residency       = 100000,
> +               .flags                  = CPUIDLE_FLAG_TIME_VALID,
> +               .name                   = "DDR SR",
> +               .desc                   = "WFI and DDR Self Refresh",
> +       },
> +       .state_count = DAVINCI_CPUIDLE_MAX_STATES,
>  };
>
>  static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
> @@ -77,41 +108,10 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
>        },
>  };
>
> -/* Actual code that puts the SoC in different idle states */
> -static int davinci_enter_idle(struct cpuidle_device *dev,
> -                               struct cpuidle_driver *drv,
> -                                               int index)
> -{
> -       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> -       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
> -       struct timeval before, after;
> -       int idle_time;
> -
> -       local_irq_disable();
> -       do_gettimeofday(&before);
> -
> -       if (ops && ops->enter)
> -               ops->enter(ops->flags);
> -       /* Wait for interrupt state */
> -       cpu_do_idle();
> -       if (ops && ops->exit)
> -               ops->exit(ops->flags);
> -
> -       do_gettimeofday(&after);
> -       local_irq_enable();
> -       idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
> -                       (after.tv_usec - before.tv_usec);
> -
> -       dev->last_residency = idle_time;
> -
> -       return index;
> -}
> -
>  static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>  {
>        int ret;
>        struct cpuidle_device *device;
> -       struct cpuidle_driver *driver = &davinci_idle_driver;
>        struct davinci_cpuidle_config *pdata = pdev->dev.platform_data;
>
>        device = &per_cpu(davinci_cpuidle_device, smp_processor_id());
> @@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>
>        ddr2_reg_base = pdata->ddr2_ctlr_base;
>
> -       /* Wait for interrupt state */
> -       driver->states[0].enter = davinci_enter_idle;
> -       driver->states[0].exit_latency = 1;
> -       driver->states[0].target_residency = 10000;
> -       driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> -       strcpy(driver->states[0].name, "WFI");
> -       strcpy(driver->states[0].desc, "Wait for interrupt");
> -
> -       /* Wait for interrupt and DDR self refresh state */
> -       driver->states[1].enter = davinci_enter_idle;
> -       driver->states[1].exit_latency = 10;
> -       driver->states[1].target_residency = 10000;
> -       driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> -       strcpy(driver->states[1].name, "DDR SR");
> -       strcpy(driver->states[1].desc, "WFI and DDR Self Refresh");
>        if (pdata->ddr2_pdown)
>                davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
>        cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
>
>        device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
> -       driver->state_count = DAVINCI_CPUIDLE_MAX_STATES;
>
>        ret = cpuidle_register_driver(&davinci_idle_driver);
>        if (ret) {
> --
> 1.7.9.4
>

Re: [PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable

[Rob Lee]

On Wed, Mar 21, 2012 at 9:28 AM, Rob Lee <rob.lee@linaro.org> wrote:
> Sekhar tested this patch on Davinci last night and found a problem.  I
> looked at the code again and found a mindless omission on my part (see
> below).  Fix is trivial.  I've check all other platforms and confirmed
> this problem does not exist for those.  Will resend a v9 of the
> patchset shortly.
>

Kevin just informed me that Len already has v8 patchset queued so I'll
just send this fix out as a separate patch.

> On Tue, Mar 20, 2012 at 3:22 PM, Robert Lee <rob.lee@linaro.org> wrote:
>> Enable core cpuidle timekeeping and irq enabling and remove that
>> handling from this code.
>>
>> Signed-off-by: Robert Lee <rob.lee@linaro.org>
>> Reviewed-by: Kevin Hilman <khilman@ti.com>
>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Acked-by: Jean Pihet<j-pihet@ti.com>
>> ---
>>  arch/arm/mach-davinci/cpuidle.c |   82 ++++++++++++++++-----------------------
>>  1 file changed, 33 insertions(+), 49 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
>> index a30c7c5..93ae096 100644
>> --- a/arch/arm/mach-davinci/cpuidle.c
>> +++ b/arch/arm/mach-davinci/cpuidle.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/io.h>
>>  #include <linux/export.h>
>>  #include <asm/proc-fns.h>
>> +#include <asm/cpuidle.h>
>>
>>  #include <mach/cpuidle.h>
>>  #include <mach/ddr2.h>
>> @@ -30,12 +31,42 @@ struct davinci_ops {
>>        u32 flags;
>>  };
>>
>> +/* Actual code that puts the SoC in different idle states */
>> +static int davinci_enter_idle(struct cpuidle_device *dev,
>> +                               struct cpuidle_driver *drv,
>> +                                               int index)
>> +{
>> +       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
>> +       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
>> +
>> +       if (ops && ops->enter)
>> +               ops->enter(ops->flags);
>> +
>> +       index = cpuidle_wrap_enter(dev, drv, index,
>> +                               arm_cpuidle_simple_enter);
>> +
>> +       if (ops && ops->exit)
>> +               ops->exit(ops->flags);
>> +
>> +       return index;
>> +}
>> +
>>  /* fields in davinci_ops.flags */
>>  #define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN        BIT(0)
>>
>>  static struct cpuidle_driver davinci_idle_driver = {
>> -       .name   = "cpuidle-davinci",
>> -       .owner  = THIS_MODULE,
>> +       .name           = "cpuidle-davinci",
>> +       .owner          = THIS_MODULE,
>
> Argh! I am missing the ".en_core_tk_irqen = 1" here.
>
>
>> +       .states[0]      = ARM_CPUIDLE_WFI_STATE,
>> +       .states[1]      = {
>> +               .enter                  = davinci_enter_idle,
>> +               .exit_latency           = 10,
>> +               .target_residency       = 100000,
>> +               .flags                  = CPUIDLE_FLAG_TIME_VALID,
>> +               .name                   = "DDR SR",
>> +               .desc                   = "WFI and DDR Self Refresh",
>> +       },
>> +       .state_count = DAVINCI_CPUIDLE_MAX_STATES,
>>  };
>>
>>  static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
>> @@ -77,41 +108,10 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
>>        },
>>  };
>>
>> -/* Actual code that puts the SoC in different idle states */
>> -static int davinci_enter_idle(struct cpuidle_device *dev,
>> -                               struct cpuidle_driver *drv,
>> -                                               int index)
>> -{
>> -       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
>> -       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
>> -       struct timeval before, after;
>> -       int idle_time;
>> -
>> -       local_irq_disable();
>> -       do_gettimeofday(&before);
>> -
>> -       if (ops && ops->enter)
>> -               ops->enter(ops->flags);
>> -       /* Wait for interrupt state */
>> -       cpu_do_idle();
>> -       if (ops && ops->exit)
>> -               ops->exit(ops->flags);
>> -
>> -       do_gettimeofday(&after);
>> -       local_irq_enable();
>> -       idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
>> -                       (after.tv_usec - before.tv_usec);
>> -
>> -       dev->last_residency = idle_time;
>> -
>> -       return index;
>> -}
>> -
>>  static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>>  {
>>        int ret;
>>        struct cpuidle_device *device;
>> -       struct cpuidle_driver *driver = &davinci_idle_driver;
>>        struct davinci_cpuidle_config *pdata = pdev->dev.platform_data;
>>
>>        device = &per_cpu(davinci_cpuidle_device, smp_processor_id());
>> @@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>>
>>        ddr2_reg_base = pdata->ddr2_ctlr_base;
>>
>> -       /* Wait for interrupt state */
>> -       driver->states[0].enter = davinci_enter_idle;
>> -       driver->states[0].exit_latency = 1;
>> -       driver->states[0].target_residency = 10000;
>> -       driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
>> -       strcpy(driver->states[0].name, "WFI");
>> -       strcpy(driver->states[0].desc, "Wait for interrupt");
>> -
>> -       /* Wait for interrupt and DDR self refresh state */
>> -       driver->states[1].enter = davinci_enter_idle;
>> -       driver->states[1].exit_latency = 10;
>> -       driver->states[1].target_residency = 10000;
>> -       driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
>> -       strcpy(driver->states[1].name, "DDR SR");
>> -       strcpy(driver->states[1].desc, "WFI and DDR Self Refresh");
>>        if (pdata->ddr2_pdown)
>>                davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
>>        cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
>>
>>        device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
>> -       driver->state_count = DAVINCI_CPUIDLE_MAX_STATES;
>>
>>        ret = cpuidle_register_driver(&davinci_idle_driver);
>>        if (ret) {
>> --
>> 1.7.9.4
>>

Re: [PATCH v8 4/8] ARM: davinci: Consolidate time keeping and irq enable

[Amit Kucheria]

Rob, you should start a new '3.4-fixes' branch containing such
bugfixes that can be pushed to Len after -rc1.

On Wed, Mar 21, 2012 at 4:28 PM, Rob Lee <rob.lee@linaro.org> wrote:
> Sekhar tested this patch on Davinci last night and found a problem.  I
> looked at the code again and found a mindless omission on my part (see
> below).  Fix is trivial.  I've check all other platforms and confirmed
> this problem does not exist for those.  Will resend a v9 of the
> patchset shortly.
>
> On Tue, Mar 20, 2012 at 3:22 PM, Robert Lee <rob.lee@linaro.org> wrote:
>> Enable core cpuidle timekeeping and irq enabling and remove that
>> handling from this code.
>>
>> Signed-off-by: Robert Lee <rob.lee@linaro.org>
>> Reviewed-by: Kevin Hilman <khilman@ti.com>
>> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Acked-by: Jean Pihet<j-pihet@ti.com>
>> ---
>>  arch/arm/mach-davinci/cpuidle.c |   82 ++++++++++++++++-----------------------
>>  1 file changed, 33 insertions(+), 49 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
>> index a30c7c5..93ae096 100644
>> --- a/arch/arm/mach-davinci/cpuidle.c
>> +++ b/arch/arm/mach-davinci/cpuidle.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/io.h>
>>  #include <linux/export.h>
>>  #include <asm/proc-fns.h>
>> +#include <asm/cpuidle.h>
>>
>>  #include <mach/cpuidle.h>
>>  #include <mach/ddr2.h>
>> @@ -30,12 +31,42 @@ struct davinci_ops {
>>        u32 flags;
>>  };
>>
>> +/* Actual code that puts the SoC in different idle states */
>> +static int davinci_enter_idle(struct cpuidle_device *dev,
>> +                               struct cpuidle_driver *drv,
>> +                                               int index)
>> +{
>> +       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
>> +       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
>> +
>> +       if (ops && ops->enter)
>> +               ops->enter(ops->flags);
>> +
>> +       index = cpuidle_wrap_enter(dev, drv, index,
>> +                               arm_cpuidle_simple_enter);
>> +
>> +       if (ops && ops->exit)
>> +               ops->exit(ops->flags);
>> +
>> +       return index;
>> +}
>> +
>>  /* fields in davinci_ops.flags */
>>  #define DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN        BIT(0)
>>
>>  static struct cpuidle_driver davinci_idle_driver = {
>> -       .name   = "cpuidle-davinci",
>> -       .owner  = THIS_MODULE,
>> +       .name           = "cpuidle-davinci",
>> +       .owner          = THIS_MODULE,
>
> Argh! I am missing the ".en_core_tk_irqen = 1" here.
>
>
>> +       .states[0]      = ARM_CPUIDLE_WFI_STATE,
>> +       .states[1]      = {
>> +               .enter                  = davinci_enter_idle,
>> +               .exit_latency           = 10,
>> +               .target_residency       = 100000,
>> +               .flags                  = CPUIDLE_FLAG_TIME_VALID,
>> +               .name                   = "DDR SR",
>> +               .desc                   = "WFI and DDR Self Refresh",
>> +       },
>> +       .state_count = DAVINCI_CPUIDLE_MAX_STATES,
>>  };
>>
>>  static DEFINE_PER_CPU(struct cpuidle_device, davinci_cpuidle_device);
>> @@ -77,41 +108,10 @@ static struct davinci_ops davinci_states[DAVINCI_CPUIDLE_MAX_STATES] = {
>>        },
>>  };
>>
>> -/* Actual code that puts the SoC in different idle states */
>> -static int davinci_enter_idle(struct cpuidle_device *dev,
>> -                               struct cpuidle_driver *drv,
>> -                                               int index)
>> -{
>> -       struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
>> -       struct davinci_ops *ops = cpuidle_get_statedata(state_usage);
>> -       struct timeval before, after;
>> -       int idle_time;
>> -
>> -       local_irq_disable();
>> -       do_gettimeofday(&before);
>> -
>> -       if (ops && ops->enter)
>> -               ops->enter(ops->flags);
>> -       /* Wait for interrupt state */
>> -       cpu_do_idle();
>> -       if (ops && ops->exit)
>> -               ops->exit(ops->flags);
>> -
>> -       do_gettimeofday(&after);
>> -       local_irq_enable();
>> -       idle_time = (after.tv_sec - before.tv_sec) * USEC_PER_SEC +
>> -                       (after.tv_usec - before.tv_usec);
>> -
>> -       dev->last_residency = idle_time;
>> -
>> -       return index;
>> -}
>> -
>>  static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>>  {
>>        int ret;
>>        struct cpuidle_device *device;
>> -       struct cpuidle_driver *driver = &davinci_idle_driver;
>>        struct davinci_cpuidle_config *pdata = pdev->dev.platform_data;
>>
>>        device = &per_cpu(davinci_cpuidle_device, smp_processor_id());
>> @@ -123,27 +123,11 @@ static int __init davinci_cpuidle_probe(struct platform_device *pdev)
>>
>>        ddr2_reg_base = pdata->ddr2_ctlr_base;
>>
>> -       /* Wait for interrupt state */
>> -       driver->states[0].enter = davinci_enter_idle;
>> -       driver->states[0].exit_latency = 1;
>> -       driver->states[0].target_residency = 10000;
>> -       driver->states[0].flags = CPUIDLE_FLAG_TIME_VALID;
>> -       strcpy(driver->states[0].name, "WFI");
>> -       strcpy(driver->states[0].desc, "Wait for interrupt");
>> -
>> -       /* Wait for interrupt and DDR self refresh state */
>> -       driver->states[1].enter = davinci_enter_idle;
>> -       driver->states[1].exit_latency = 10;
>> -       driver->states[1].target_residency = 10000;
>> -       driver->states[1].flags = CPUIDLE_FLAG_TIME_VALID;
>> -       strcpy(driver->states[1].name, "DDR SR");
>> -       strcpy(driver->states[1].desc, "WFI and DDR Self Refresh");
>>        if (pdata->ddr2_pdown)
>>                davinci_states[1].flags |= DAVINCI_CPUIDLE_FLAGS_DDR2_PWDN;
>>        cpuidle_set_statedata(&device->states_usage[1], &davinci_states[1]);
>>
>>        device->state_count = DAVINCI_CPUIDLE_MAX_STATES;
>> -       driver->state_count = DAVINCI_CPUIDLE_MAX_STATES;
>>
>>        ret = cpuidle_register_driver(&davinci_idle_driver);
>>        if (ret) {
>> --
>> 1.7.9.4
>>