All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/9] Consolidate cpuidle functionality
@ 2012-02-29  3:11 ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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().

NOTE to Maintainers: Platform code changes are not required due to patch 1/9
but please review and push these platform changes as possible to allow
this consolidation to occur.

Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
http://www.spinics.net/lists/linux-samsung-soc/msg09467.html

v5 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg161596.html
Changes since v5:
* 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)


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

Changes since v4:
* 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.

v3 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg156751.html
Changes since v3:
* 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.

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

Changes since v2:
* Made various code organization and style changes as suggested in v1 review.
* 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.

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

Changes since v1:
* 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.
* Added callback for filling in driver_data field as needed.
* Modified the various platforms with these changes.

Robert Lee (9):
  cpuidle: Add common time keeping and irq enabling
  ARM: at91: Consolidate time keeping and irq enable
  ARM: exynos: 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        |   14 +++++
 arch/arm/mach-at91/cpuidle.c          |   67 +++++++++----------------
 arch/arm/mach-davinci/cpuidle.c       |   78 +++++++++++-----------------
 arch/arm/mach-exynos/cpuidle.c        |   53 ++-----------------
 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      |   23 +-------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +---
 drivers/cpuidle/cpuidle.c             |   90 ++++++++++++++++++++++++++-------
 include/linux/cpuidle.h               |   13 +++++
 11 files changed, 207 insertions(+), 276 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 0/9] Consolidate cpuidle functionality
@ 2012-02-29  3:11 ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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().

NOTE to Maintainers: Platform code changes are not required due to patch 1/9
but please review and push these platform changes as possible to allow
this consolidation to occur.

Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
http://www.spinics.net/lists/linux-samsung-soc/msg09467.html

v5 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg161596.html
Changes since v5:
* 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)


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

Changes since v4:
* 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.

v3 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg156751.html
Changes since v3:
* 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.

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

Changes since v2:
* Made various code organization and style changes as suggested in v1 review.
* 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.

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

Changes since v1:
* 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.
* Added callback for filling in driver_data field as needed.
* Modified the various platforms with these changes.

Robert Lee (9):
  cpuidle: Add common time keeping and irq enabling
  ARM: at91: Consolidate time keeping and irq enable
  ARM: exynos: 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        |   14 +++++
 arch/arm/mach-at91/cpuidle.c          |   67 +++++++++----------------
 arch/arm/mach-davinci/cpuidle.c       |   78 +++++++++++-----------------
 arch/arm/mach-exynos/cpuidle.c        |   53 ++-----------------
 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      |   23 +-------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +---
 drivers/cpuidle/cpuidle.c             |   90 ++++++++++++++++++++++++++-------
 include/linux/cpuidle.h               |   13 +++++
 11 files changed, 207 insertions(+), 276 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 0/9] Consolidate cpuidle functionality
@ 2012-02-29  3:11 ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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().

NOTE to Maintainers: Platform code changes are not required due to patch 1/9
but please review and push these platform changes as possible to allow
this consolidation to occur.

Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
http://www.spinics.net/lists/linux-samsung-soc/msg09467.html

v5 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg161596.html
Changes since v5:
* 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)


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

Changes since v4:
* 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.

v3 submission can be found here:
http://www.spinics.net/lists/arm-kernel/msg156751.html
Changes since v3:
* 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.

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

Changes since v2:
* Made various code organization and style changes as suggested in v1 review.
* 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.

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

Changes since v1:
* 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.
* Added callback for filling in driver_data field as needed.
* Modified the various platforms with these changes.

Robert Lee (9):
  cpuidle: Add common time keeping and irq enabling
  ARM: at91: Consolidate time keeping and irq enable
  ARM: exynos: 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        |   14 +++++
 arch/arm/mach-at91/cpuidle.c          |   67 +++++++++----------------
 arch/arm/mach-davinci/cpuidle.c       |   78 +++++++++++-----------------
 arch/arm/mach-exynos/cpuidle.c        |   53 ++-----------------
 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      |   23 +-------
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +---
 drivers/cpuidle/cpuidle.c             |   90 ++++++++++++++++++++++++++-------
 include/linux/cpuidle.h               |   13 +++++
 11 files changed, 207 insertions(+), 276 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 arch/arm/include/asm/cpuidle.h |   14 ++++++
 drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
 include/linux/cpuidle.h        |   13 ++++++
 3 files changed, 99 insertions(+), 18 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h

diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 0000000..1d2075b
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+/* Common ARM WFI state */
+#define CPUIDLE_ARM_WFI_STATE {\
+	.enter                  = cpuidle_simple_enter,\
+	.exit_latency           = 1,\
+	.target_residency       = 1,\
+	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
+	.name                   = "WFI",\
+	.desc                   = "ARM core clock gating (WFI)",\
+}
+
+#endif
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..00b02f5 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -18,6 +18,7 @@
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
 #include <linux/module.h>
+#include <asm/proc-fns.h>
 #include <trace/events/power.h>
 
 #include "cpuidle.h"
@@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
 	if (cpuidle_curr_governor->reflect)
@@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
 
 EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
 
-#ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int poll_idle(struct cpuidle_device *dev,
-		struct cpuidle_driver *drv, int index)
+/**
+ * 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	t1, t2;
+	ktime_t time_start, time_end;
 	s64 diff;
 
-	t1 = ktime_get();
+	time_start = ktime_get();
+
+	index = enter(dev, drv, index);
+
+	time_end = ktime_get();
+
 	local_irq_enable();
-	while (!need_resched())
-		cpu_relax();
 
-	t2 = ktime_get();
-	diff = ktime_to_us(ktime_sub(t2, t1));
+	diff = ktime_to_us(ktime_sub(time_end, time_start));
 	if (diff > INT_MAX)
 		diff = INT_MAX;
 
@@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
 	return index;
 }
 
+int cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	cpu_do_idle();
+
+	return index;
+}
+
+#ifdef CONFIG_ARCH_HAS_CPU_RELAX
+static inline int __poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	while (!need_resched())
+		cpu_relax();
+
+	return index;
+}
+
+static int poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	return cpuidle_wrap_enter(dev,	drv, index,
+				__poll_idle);
+}
+
 static void poll_idle_init(struct cpuidle_driver *drv)
 {
 	struct cpuidle_state *state = &drv->states[0];
@@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
 	state->exit_latency = 0;
 	state->target_residency = 0;
 	state->power_usage = -1;
-	state->flags = 0;
 	state->enter = poll_idle;
 }
 #else
@@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index);
+
+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) { }
@@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{return -ENODEV; }
 
 #endif
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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>
---
 arch/arm/include/asm/cpuidle.h |   14 ++++++
 drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
 include/linux/cpuidle.h        |   13 ++++++
 3 files changed, 99 insertions(+), 18 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h

diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 0000000..1d2075b
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+/* Common ARM WFI state */
+#define CPUIDLE_ARM_WFI_STATE {\
+	.enter                  = cpuidle_simple_enter,\
+	.exit_latency           = 1,\
+	.target_residency       = 1,\
+	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
+	.name                   = "WFI",\
+	.desc                   = "ARM core clock gating (WFI)",\
+}
+
+#endif
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..00b02f5 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -18,6 +18,7 @@
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
 #include <linux/module.h>
+#include <asm/proc-fns.h>
 #include <trace/events/power.h>
 
 #include "cpuidle.h"
@@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
 	if (cpuidle_curr_governor->reflect)
@@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
 
 EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
 
-#ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int poll_idle(struct cpuidle_device *dev,
-		struct cpuidle_driver *drv, int index)
+/**
+ * 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	t1, t2;
+	ktime_t time_start, time_end;
 	s64 diff;
 
-	t1 = ktime_get();
+	time_start = ktime_get();
+
+	index = enter(dev, drv, index);
+
+	time_end = ktime_get();
+
 	local_irq_enable();
-	while (!need_resched())
-		cpu_relax();
 
-	t2 = ktime_get();
-	diff = ktime_to_us(ktime_sub(t2, t1));
+	diff = ktime_to_us(ktime_sub(time_end, time_start));
 	if (diff > INT_MAX)
 		diff = INT_MAX;
 
@@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
 	return index;
 }
 
+int cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	cpu_do_idle();
+
+	return index;
+}
+
+#ifdef CONFIG_ARCH_HAS_CPU_RELAX
+static inline int __poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	while (!need_resched())
+		cpu_relax();
+
+	return index;
+}
+
+static int poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	return cpuidle_wrap_enter(dev,	drv, index,
+				__poll_idle);
+}
+
 static void poll_idle_init(struct cpuidle_driver *drv)
 {
 	struct cpuidle_state *state = &drv->states[0];
@@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
 	state->exit_latency = 0;
 	state->target_residency = 0;
 	state->power_usage = -1;
-	state->flags = 0;
 	state->enter = poll_idle;
 }
 #else
@@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index);
+
+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) { }
@@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{return -ENODEV; }
 
 #endif
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 arch/arm/include/asm/cpuidle.h |   14 ++++++
 drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
 include/linux/cpuidle.h        |   13 ++++++
 3 files changed, 99 insertions(+), 18 deletions(-)
 create mode 100644 arch/arm/include/asm/cpuidle.h

diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
new file mode 100644
index 0000000..1d2075b
--- /dev/null
+++ b/arch/arm/include/asm/cpuidle.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_ARM_CPUIDLE_H
+#define __ASM_ARM_CPUIDLE_H
+
+/* Common ARM WFI state */
+#define CPUIDLE_ARM_WFI_STATE {\
+	.enter                  = cpuidle_simple_enter,\
+	.exit_latency           = 1,\
+	.target_residency       = 1,\
+	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
+	.name                   = "WFI",\
+	.desc                   = "ARM core clock gating (WFI)",\
+}
+
+#endif
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 59f4261..00b02f5 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -18,6 +18,7 @@
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
 #include <linux/module.h>
+#include <asm/proc-fns.h>
 #include <trace/events/power.h>
 
 #include "cpuidle.h"
@@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
 	if (cpuidle_curr_governor->reflect)
@@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
 
 EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
 
-#ifdef CONFIG_ARCH_HAS_CPU_RELAX
-static int poll_idle(struct cpuidle_device *dev,
-		struct cpuidle_driver *drv, int index)
+/**
+ * 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	t1, t2;
+	ktime_t time_start, time_end;
 	s64 diff;
 
-	t1 = ktime_get();
+	time_start = ktime_get();
+
+	index = enter(dev, drv, index);
+
+	time_end = ktime_get();
+
 	local_irq_enable();
-	while (!need_resched())
-		cpu_relax();
 
-	t2 = ktime_get();
-	diff = ktime_to_us(ktime_sub(t2, t1));
+	diff = ktime_to_us(ktime_sub(time_end, time_start));
 	if (diff > INT_MAX)
 		diff = INT_MAX;
 
@@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
 	return index;
 }
 
+int cpuidle_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	cpu_do_idle();
+
+	return index;
+}
+
+#ifdef CONFIG_ARCH_HAS_CPU_RELAX
+static inline int __poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	while (!need_resched())
+		cpu_relax();
+
+	return index;
+}
+
+static int poll_idle(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{
+	return cpuidle_wrap_enter(dev,	drv, index,
+				__poll_idle);
+}
+
 static void poll_idle_init(struct cpuidle_driver *drv)
 {
 	struct cpuidle_state *state = &drv->states[0];
@@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
 	state->exit_latency = 0;
 	state->target_residency = 0;
 	state->power_usage = -1;
-	state->flags = 0;
 	state->enter = poll_idle;
 }
 #else
@@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index);
+
+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) { }
@@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int index)
+{return -ENODEV; }
 
 #endif
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 2/9] ARM: at91: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-at91/cpuidle.c |   67 +++++++++++++++---------------------------
 1 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..324f802 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] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 2/9] ARM: at91: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-at91/cpuidle.c |   67 +++++++++++++++---------------------------
 1 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..324f802 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] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 2/9] ARM: at91: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-at91/cpuidle.c |   67 +++++++++++++++---------------------------
 1 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/arch/arm/mach-at91/cpuidle.c b/arch/arm/mach-at91/cpuidle.c
index a851e6c..324f802 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] = CPUIDLE_ARM_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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |   53 ++++-----------------------------------
 1 files changed, 6 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 9bf6743..75bb88b 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -20,6 +20,7 @@
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 #include <asm/unified.h>
+#include <asm/cpuidle.h>
 #include <mach/regs-pmu.h>
 #include <mach/pmu.h>
 
@@ -34,22 +35,12 @@
 
 #define S5P_CHECK_AFTR		0xFCBA0D10
 
-static int exynos4_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-			      int index);
 static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index);
 
 static struct cpuidle_state exynos4_cpuidle_set[] = {
-	[0] = {
-		.enter			= exynos4_enter_idle,
-		.exit_latency		= 1,
-		.target_residency	= 100000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "C0",
-		.desc			= "ARM clock gating(WFI)",
-	},
+	[0] = CPUIDLE_ARM_WFI_STATE,
 	[1] = {
 		.enter			= exynos4_enter_lowpower,
 		.exit_latency		= 300,
@@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
 static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
 
 static struct cpuidle_driver exynos4_idle_driver = {
-	.name		= "exynos4_idle",
-	.owner		= THIS_MODULE,
+	.name			= "exynos4_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
 };
 
 /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
@@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index)
 {
-	struct timeval before, after;
-	int idle_time;
 	unsigned long tmp;
 
-	local_irq_disable();
-	do_gettimeofday(&before);
-
 	exynos4_set_wakeupmask();
 
 	/* Set value of power down register for aftr mode */
@@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 	/* Clear wakeup state register */
 	__raw_writel(0x0, S5P_WAKEUP_STAT);
 
-	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 exynos4_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);
-
-	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);
-
-	dev->last_residency = idle_time;
 	return index;
 }
 
@@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 		new_index = drv->safe_state_index;
 
 	if (new_index = 0)
-		return exynos4_enter_idle(dev, drv, new_index);
+		return cpuidle_simple_enter(dev, drv, new_index);
 	else
 		return exynos4_enter_core0_aftr(dev, drv, new_index);
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |   53 ++++-----------------------------------
 1 files changed, 6 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 9bf6743..75bb88b 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -20,6 +20,7 @@
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 #include <asm/unified.h>
+#include <asm/cpuidle.h>
 #include <mach/regs-pmu.h>
 #include <mach/pmu.h>
 
@@ -34,22 +35,12 @@
 
 #define S5P_CHECK_AFTR		0xFCBA0D10
 
-static int exynos4_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-			      int index);
 static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index);
 
 static struct cpuidle_state exynos4_cpuidle_set[] = {
-	[0] = {
-		.enter			= exynos4_enter_idle,
-		.exit_latency		= 1,
-		.target_residency	= 100000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "C0",
-		.desc			= "ARM clock gating(WFI)",
-	},
+	[0] = CPUIDLE_ARM_WFI_STATE,
 	[1] = {
 		.enter			= exynos4_enter_lowpower,
 		.exit_latency		= 300,
@@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
 static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
 
 static struct cpuidle_driver exynos4_idle_driver = {
-	.name		= "exynos4_idle",
-	.owner		= THIS_MODULE,
+	.name			= "exynos4_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
 };
 
 /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
@@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index)
 {
-	struct timeval before, after;
-	int idle_time;
 	unsigned long tmp;
 
-	local_irq_disable();
-	do_gettimeofday(&before);
-
 	exynos4_set_wakeupmask();
 
 	/* Set value of power down register for aftr mode */
@@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 	/* Clear wakeup state register */
 	__raw_writel(0x0, S5P_WAKEUP_STAT);
 
-	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 exynos4_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);
-
-	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);
-
-	dev->last_residency = idle_time;
 	return index;
 }
 
@@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 		new_index = drv->safe_state_index;
 
 	if (new_index == 0)
-		return exynos4_enter_idle(dev, drv, new_index);
+		return cpuidle_simple_enter(dev, drv, new_index);
 	else
 		return exynos4_enter_core0_aftr(dev, drv, new_index);
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-exynos/cpuidle.c |   53 ++++-----------------------------------
 1 files changed, 6 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
index 9bf6743..75bb88b 100644
--- a/arch/arm/mach-exynos/cpuidle.c
+++ b/arch/arm/mach-exynos/cpuidle.c
@@ -20,6 +20,7 @@
 #include <asm/smp_scu.h>
 #include <asm/suspend.h>
 #include <asm/unified.h>
+#include <asm/cpuidle.h>
 #include <mach/regs-pmu.h>
 #include <mach/pmu.h>
 
@@ -34,22 +35,12 @@
 
 #define S5P_CHECK_AFTR		0xFCBA0D10
 
-static int exynos4_enter_idle(struct cpuidle_device *dev,
-			struct cpuidle_driver *drv,
-			      int index);
 static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index);
 
 static struct cpuidle_state exynos4_cpuidle_set[] = {
-	[0] = {
-		.enter			= exynos4_enter_idle,
-		.exit_latency		= 1,
-		.target_residency	= 100000,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
-		.name			= "C0",
-		.desc			= "ARM clock gating(WFI)",
-	},
+	[0] = CPUIDLE_ARM_WFI_STATE,
 	[1] = {
 		.enter			= exynos4_enter_lowpower,
 		.exit_latency		= 300,
@@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
 static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
 
 static struct cpuidle_driver exynos4_idle_driver = {
-	.name		= "exynos4_idle",
-	.owner		= THIS_MODULE,
+	.name			= "exynos4_idle",
+	.owner			= THIS_MODULE,
+	.en_core_tk_irqen	= 1,
 };
 
 /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
@@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 				struct cpuidle_driver *drv,
 				int index)
 {
-	struct timeval before, after;
-	int idle_time;
 	unsigned long tmp;
 
-	local_irq_disable();
-	do_gettimeofday(&before);
-
 	exynos4_set_wakeupmask();
 
 	/* Set value of power down register for aftr mode */
@@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
 	/* Clear wakeup state register */
 	__raw_writel(0x0, S5P_WAKEUP_STAT);
 
-	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 exynos4_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);
-
-	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);
-
-	dev->last_residency = idle_time;
 	return index;
 }
 
@@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
 		new_index = drv->safe_state_index;
 
 	if (new_index == 0)
-		return exynos4_enter_idle(dev, drv, new_index);
+		return cpuidle_simple_enter(dev, drv, new_index);
 	else
 		return exynos4_enter_core0_aftr(dev, drv, new_index);
 }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 4/9] ARM: kirkwood: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-kirkwood/cpuidle.c |   72 +++++++++++---------------------------
 1 files changed, 21 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index 7088180..7fd1f88 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] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 4/9] ARM: kirkwood: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-kirkwood/cpuidle.c |   72 +++++++++++---------------------------
 1 files changed, 21 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index 7088180..7fd1f88 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] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 4/9] ARM: kirkwood: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-kirkwood/cpuidle.c |   72 +++++++++++---------------------------
 1 files changed, 21 insertions(+), 51 deletions(-)

diff --git a/arch/arm/mach-kirkwood/cpuidle.c b/arch/arm/mach-kirkwood/cpuidle.c
index 7088180..7fd1f88 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] = CPUIDLE_ARM_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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..6f457f1 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);
+
+	return cpuidle_wrap_enter(dev,	drv, index,
+				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,
+	.states[0] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..6f457f1 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);
+
+	return cpuidle_wrap_enter(dev,	drv, index,
+				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,
+	.states[0] = CPUIDLE_ARM_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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
 1 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
index a30c7c5..6f457f1 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);
+
+	return cpuidle_wrap_enter(dev,	drv, index,
+				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,
+	.states[0] = CPUIDLE_ARM_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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 6/9] ARM: omap: Consolidate OMAP3 time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   42 +++++++++++++++----------------------
 1 files 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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 6/9] ARM: omap: Consolidate OMAP3 time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   42 +++++++++++++++----------------------
 1 files 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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 6/9] ARM: omap: Consolidate OMAP3 time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   42 +++++++++++++++----------------------
 1 files 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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 7/9] ARM: omap: Consolidate OMAP4 time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle44xx.c |   21 +++------------------
 1 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index cfdbb86..9729d2e 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -62,16 +62,10 @@ 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 new_state_idx;
 	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();
 
 	/*
@@ -129,26 +123,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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 7/9] ARM: omap: Consolidate OMAP4 time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle44xx.c |   21 +++------------------
 1 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index cfdbb86..9729d2e 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -62,16 +62,10 @@ 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 new_state_idx;
 	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();
 
 	/*
@@ -129,26 +123,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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 7/9] ARM: omap: Consolidate OMAP4 time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-omap2/cpuidle44xx.c |   21 +++------------------
 1 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle44xx.c b/arch/arm/mach-omap2/cpuidle44xx.c
index cfdbb86..9729d2e 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -62,16 +62,10 @@ 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 new_state_idx;
 	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();
 
 	/*
@@ -129,26 +123,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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 8/9] ARM: shmobile: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-shmobile/cpuidle.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 1b23342..47b56a9 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,21 +30,8 @@ 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;
 }
 
@@ -51,13 +39,8 @@ 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,
-	},
+	.en_core_tk_irqen = 1,
+	.states[0] = CPUIDLE_ARM_WFI_STATE,
 	.safe_state_index = 0, /* C1 */
 	.state_count = 1,
 };
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 8/9] ARM: shmobile: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-shmobile/cpuidle.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 1b23342..47b56a9 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,21 +30,8 @@ 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;
 }
 
@@ -51,13 +39,8 @@ 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,
-	},
+	.en_core_tk_irqen = 1,
+	.states[0] = CPUIDLE_ARM_WFI_STATE,
 	.safe_state_index = 0, /* C1 */
 	.state_count = 1,
 };
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 8/9] ARM: shmobile: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/arm/mach-shmobile/cpuidle.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-shmobile/cpuidle.c b/arch/arm/mach-shmobile/cpuidle.c
index 1b23342..47b56a9 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,21 +30,8 @@ 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;
 }
 
@@ -51,13 +39,8 @@ 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,
-	},
+	.en_core_tk_irqen = 1,
+	.states[0] = CPUIDLE_ARM_WFI_STATE,
 	.safe_state_index = 0, /* C1 */
 	.state_count = 1,
 };
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 9/9] SH: shmobile: Consolidate time keeping and irq enable
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  3:11   ` Robert Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +++-------
 1 files 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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 9/9] SH: shmobile: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: len.brown, khilman
  Cc: robherring2, Baohua.Song, amit.kucheria, nicolas.ferre, linux,
	kgene.kim, amit.kachhap, magnus.damm, nsekhar, daniel.lezcano,
	mturquette, vincent.guittot, arnd.bergmann, linux-arm-kernel,
	linaro-dev, patches, deepthi, broonie, nicolas.pitre, linux,
	jean.pihet, venki, ccross, g.trinabh, kernel, lethal, jon-hunter,
	tony, linux-omap, linux-sh, linux-pm

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +++-------
 1 files 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.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH v6 9/9] SH: shmobile: Consolidate time keeping and irq enable
@ 2012-02-29  3:11   ` Robert Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Robert Lee @ 2012-02-29  3:11 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Robert Lee <rob.lee@linaro.org>
---
 arch/sh/kernel/cpu/shmobile/cpuidle.c |   10 +++-------
 1 files 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.1

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
  2012-02-29  3:11   ` Robert Lee
  (?)
@ 2012-02-29  6:41     ` Amit Kachhap
  -1 siblings, 0 replies; 60+ messages in thread
From: Amit Kachhap @ 2012-02-29  6:41 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, magnus.damm, nsekhar,
	daniel.lezcano, mturquette, vincent.guittot, arnd.bergmann,
	linux-arm-kernel, linaro-dev, patches, deepthi, broonie,
	nicolas.pitre, linux, jean.pihet, venki, ccross, g.trinabh,
	kernel, lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Hi,

I verified this patch on exynos4 based origen board.

Tested-by: Amit Daniel <amit.kachhap@linaro.org>

Thanks,
Amit D

On 29 February 2012 08:41, 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>
> ---
>  arch/arm/mach-exynos/cpuidle.c |   53 ++++-----------------------------------
>  1 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index 9bf6743..75bb88b 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -20,6 +20,7 @@
>  #include <asm/smp_scu.h>
>  #include <asm/suspend.h>
>  #include <asm/unified.h>
> +#include <asm/cpuidle.h>
>  #include <mach/regs-pmu.h>
>  #include <mach/pmu.h>
>
> @@ -34,22 +35,12 @@
>
>  #define S5P_CHECK_AFTR         0xFCBA0D10
>
> -static int exynos4_enter_idle(struct cpuidle_device *dev,
> -                       struct cpuidle_driver *drv,
> -                             int index);
>  static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>                                struct cpuidle_driver *drv,
>                                int index);
>
>  static struct cpuidle_state exynos4_cpuidle_set[] = {
> -       [0] = {
> -               .enter                  = exynos4_enter_idle,
> -               .exit_latency           = 1,
> -               .target_residency       = 100000,
> -               .flags                  = CPUIDLE_FLAG_TIME_VALID,
> -               .name                   = "C0",
> -               .desc                   = "ARM clock gating(WFI)",
> -       },
> +       [0] = CPUIDLE_ARM_WFI_STATE,
>        [1] = {
>                .enter                  = exynos4_enter_lowpower,
>                .exit_latency           = 300,
> @@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
>  static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
>
>  static struct cpuidle_driver exynos4_idle_driver = {
> -       .name           = "exynos4_idle",
> -       .owner          = THIS_MODULE,
> +       .name                   = "exynos4_idle",
> +       .owner                  = THIS_MODULE,
> +       .en_core_tk_irqen       = 1,
>  };
>
>  /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> @@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
>                                struct cpuidle_driver *drv,
>                                int index)
>  {
> -       struct timeval before, after;
> -       int idle_time;
>        unsigned long tmp;
>
> -       local_irq_disable();
> -       do_gettimeofday(&before);
> -
>        exynos4_set_wakeupmask();
>
>        /* Set value of power down register for aftr mode */
> @@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
>        /* Clear wakeup state register */
>        __raw_writel(0x0, S5P_WAKEUP_STAT);
>
> -       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 exynos4_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);
> -
> -       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);
> -
> -       dev->last_residency = idle_time;
>        return index;
>  }
>
> @@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>                new_index = drv->safe_state_index;
>
>        if (new_index == 0)
> -               return exynos4_enter_idle(dev, drv, new_index);
> +               return cpuidle_simple_enter(dev, drv, new_index);
>        else
>                return exynos4_enter_core0_aftr(dev, drv, new_index);
>  }
> --
> 1.7.1
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
@ 2012-02-29  6:41     ` Amit Kachhap
  0 siblings, 0 replies; 60+ messages in thread
From: Amit Kachhap @ 2012-02-29  6:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I verified this patch on exynos4 based origen board.

Tested-by: Amit Daniel <amit.kachhap@linaro.org>

Thanks,
Amit D

On 29 February 2012 08:41, 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>
> ---
> ?arch/arm/mach-exynos/cpuidle.c | ? 53 ++++-----------------------------------
> ?1 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index 9bf6743..75bb88b 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -20,6 +20,7 @@
> ?#include <asm/smp_scu.h>
> ?#include <asm/suspend.h>
> ?#include <asm/unified.h>
> +#include <asm/cpuidle.h>
> ?#include <mach/regs-pmu.h>
> ?#include <mach/pmu.h>
>
> @@ -34,22 +35,12 @@
>
> ?#define S5P_CHECK_AFTR ? ? ? ? 0xFCBA0D10
>
> -static int exynos4_enter_idle(struct cpuidle_device *dev,
> - ? ? ? ? ? ? ? ? ? ? ? struct cpuidle_driver *drv,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? int index);
> ?static int exynos4_enter_lowpower(struct cpuidle_device *dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int index);
>
> ?static struct cpuidle_state exynos4_cpuidle_set[] = {
> - ? ? ? [0] = {
> - ? ? ? ? ? ? ? .enter ? ? ? ? ? ? ? ? ?= exynos4_enter_idle,
> - ? ? ? ? ? ? ? .exit_latency ? ? ? ? ? = 1,
> - ? ? ? ? ? ? ? .target_residency ? ? ? = 100000,
> - ? ? ? ? ? ? ? .flags ? ? ? ? ? ? ? ? ?= CPUIDLE_FLAG_TIME_VALID,
> - ? ? ? ? ? ? ? .name ? ? ? ? ? ? ? ? ? = "C0",
> - ? ? ? ? ? ? ? .desc ? ? ? ? ? ? ? ? ? = "ARM clock gating(WFI)",
> - ? ? ? },
> + ? ? ? [0] = CPUIDLE_ARM_WFI_STATE,
> ? ? ? ?[1] = {
> ? ? ? ? ? ? ? ?.enter ? ? ? ? ? ? ? ? ?= exynos4_enter_lowpower,
> ? ? ? ? ? ? ? ?.exit_latency ? ? ? ? ? = 300,
> @@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
> ?static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
>
> ?static struct cpuidle_driver exynos4_idle_driver = {
> - ? ? ? .name ? ? ? ? ? = "exynos4_idle",
> - ? ? ? .owner ? ? ? ? ?= THIS_MODULE,
> + ? ? ? .name ? ? ? ? ? ? ? ? ? = "exynos4_idle",
> + ? ? ? .owner ? ? ? ? ? ? ? ? ?= THIS_MODULE,
> + ? ? ? .en_core_tk_irqen ? ? ? = 1,
> ?};
>
> ?/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> @@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpuidle_driver *drv,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int index)
> ?{
> - ? ? ? struct timeval before, after;
> - ? ? ? int idle_time;
> ? ? ? ?unsigned long tmp;
>
> - ? ? ? local_irq_disable();
> - ? ? ? do_gettimeofday(&before);
> -
> ? ? ? ?exynos4_set_wakeupmask();
>
> ? ? ? ?/* Set value of power down register for aftr mode */
> @@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
> ? ? ? ?/* Clear wakeup state register */
> ? ? ? ?__raw_writel(0x0, S5P_WAKEUP_STAT);
>
> - ? ? ? 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 exynos4_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);
> -
> - ? ? ? 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);
> -
> - ? ? ? dev->last_residency = idle_time;
> ? ? ? ?return index;
> ?}
>
> @@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
> ? ? ? ? ? ? ? ?new_index = drv->safe_state_index;
>
> ? ? ? ?if (new_index == 0)
> - ? ? ? ? ? ? ? return exynos4_enter_idle(dev, drv, new_index);
> + ? ? ? ? ? ? ? return cpuidle_simple_enter(dev, drv, new_index);
> ? ? ? ?else
> ? ? ? ? ? ? ? ?return exynos4_enter_core0_aftr(dev, drv, new_index);
> ?}
> --
> 1.7.1
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 3/9] ARM: exynos: Consolidate time keeping and irq enable
@ 2012-02-29  6:41     ` Amit Kachhap
  0 siblings, 0 replies; 60+ messages in thread
From: Amit Kachhap @ 2012-02-29  6:53 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I verified this patch on exynos4 based origen board.

Tested-by: Amit Daniel <amit.kachhap@linaro.org>

Thanks,
Amit D

On 29 February 2012 08:41, 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>
> ---
>  arch/arm/mach-exynos/cpuidle.c |   53 ++++-----------------------------------
>  1 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/cpuidle.c b/arch/arm/mach-exynos/cpuidle.c
> index 9bf6743..75bb88b 100644
> --- a/arch/arm/mach-exynos/cpuidle.c
> +++ b/arch/arm/mach-exynos/cpuidle.c
> @@ -20,6 +20,7 @@
>  #include <asm/smp_scu.h>
>  #include <asm/suspend.h>
>  #include <asm/unified.h>
> +#include <asm/cpuidle.h>
>  #include <mach/regs-pmu.h>
>  #include <mach/pmu.h>
>
> @@ -34,22 +35,12 @@
>
>  #define S5P_CHECK_AFTR         0xFCBA0D10
>
> -static int exynos4_enter_idle(struct cpuidle_device *dev,
> -                       struct cpuidle_driver *drv,
> -                             int index);
>  static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>                                struct cpuidle_driver *drv,
>                                int index);
>
>  static struct cpuidle_state exynos4_cpuidle_set[] = {
> -       [0] = {
> -               .enter                  = exynos4_enter_idle,
> -               .exit_latency           = 1,
> -               .target_residency       = 100000,
> -               .flags                  = CPUIDLE_FLAG_TIME_VALID,
> -               .name                   = "C0",
> -               .desc                   = "ARM clock gating(WFI)",
> -       },
> +       [0] = CPUIDLE_ARM_WFI_STATE,
>        [1] = {
>                .enter                  = exynos4_enter_lowpower,
>                .exit_latency           = 300,
> @@ -63,8 +54,9 @@ static struct cpuidle_state exynos4_cpuidle_set[] = {
>  static DEFINE_PER_CPU(struct cpuidle_device, exynos4_cpuidle_device);
>
>  static struct cpuidle_driver exynos4_idle_driver = {
> -       .name           = "exynos4_idle",
> -       .owner          = THIS_MODULE,
> +       .name                   = "exynos4_idle",
> +       .owner                  = THIS_MODULE,
> +       .en_core_tk_irqen       = 1,
>  };
>
>  /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> @@ -103,13 +95,8 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
>                                struct cpuidle_driver *drv,
>                                int index)
>  {
> -       struct timeval before, after;
> -       int idle_time;
>        unsigned long tmp;
>
> -       local_irq_disable();
> -       do_gettimeofday(&before);
> -
>        exynos4_set_wakeupmask();
>
>        /* Set value of power down register for aftr mode */
> @@ -148,34 +135,6 @@ static int exynos4_enter_core0_aftr(struct cpuidle_device *dev,
>        /* Clear wakeup state register */
>        __raw_writel(0x0, S5P_WAKEUP_STAT);
>
> -       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 exynos4_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);
> -
> -       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);
> -
> -       dev->last_residency = idle_time;
>        return index;
>  }
>
> @@ -190,7 +149,7 @@ static int exynos4_enter_lowpower(struct cpuidle_device *dev,
>                new_index = drv->safe_state_index;
>
>        if (new_index = 0)
> -               return exynos4_enter_idle(dev, drv, new_index);
> +               return cpuidle_simple_enter(dev, drv, new_index);
>        else
>                return exynos4_enter_core0_aftr(dev, drv, new_index);
>  }
> --
> 1.7.1
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29  3:11   ` Robert Lee
  (?)
@ 2012-02-29  8:30     ` Jean Pihet
  -1 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
>
...

> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
...

> @@ -110,7 +126,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;
Braces are required here, according to the coding style doc.

...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29  8:30     ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:30 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, venki, ccross, g.trinabh, kernel,
	lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Hi Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
>
...

> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
...

> @@ -110,7 +126,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;
Braces are required here, according to the coding style doc.

...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29  8:30     ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
> 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>
> ---
> ?arch/arm/include/asm/cpuidle.h | ? 14 ++++++
> ?drivers/cpuidle/cpuidle.c ? ? ?| ? 90 ++++++++++++++++++++++++++++++++--------
> ?include/linux/cpuidle.h ? ? ? ?| ? 13 ++++++
> ?3 files changed, 99 insertions(+), 18 deletions(-)
> ?create mode 100644 arch/arm/include/asm/cpuidle.h
>
...

> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
...

> @@ -110,7 +126,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;
Braces are required here, according to the coding style doc.

...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
  2012-02-29  3:11   ` Robert Lee
  (?)
@ 2012-02-29  8:36     ` Jean Pihet
  -1 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, 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>
> ---
>  arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
>  1 files changed, 31 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index a30c7c5..6f457f1 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
...

> @@ -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);
> +
> +       return cpuidle_wrap_enter(dev,  drv, index,
> +                               cpuidle_simple_enter);
This does not look right since ops->exit will never be called.

> +
> +       if (ops && ops->exit)
> +               ops->exit(ops->flags);
> +
> +       return index;
> +}
> +
...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29  8:36     ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:36 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, venki, ccross, g.trinabh, kernel,
	lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, 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>
> ---
>  arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
>  1 files changed, 31 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index a30c7c5..6f457f1 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
...

> @@ -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);
> +
> +       return cpuidle_wrap_enter(dev,  drv, index,
> +                               cpuidle_simple_enter);
This does not look right since ops->exit will never be called.

> +
> +       if (ops && ops->exit)
> +               ops->exit(ops->flags);
> +
> +       return index;
> +}
> +
...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29  8:36     ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  8:36 UTC (permalink / raw)
  To: linux-arm-kernel

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, 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>
> ---
> ?arch/arm/mach-davinci/cpuidle.c | ? 78 +++++++++++++++-----------------------
> ?1 files changed, 31 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index a30c7c5..6f457f1 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
...

> @@ -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);
> +
> + ? ? ? return cpuidle_wrap_enter(dev, ?drv, index,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpuidle_simple_enter);
This does not look right since ops->exit will never be called.

> +
> + ? ? ? if (ops && ops->exit)
> + ? ? ? ? ? ? ? ops->exit(ops->flags);
> +
> + ? ? ? return index;
> +}
> +
...

Regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 0/9] Consolidate cpuidle functionality
  2012-02-29  3:11 ` Robert Lee
  (?)
@ 2012-02-29  9:40   ` Jean Pihet
  -1 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> 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().
>
> NOTE to Maintainers: Platform code changes are not required due to patch 1/9
> but please review and push these platform changes as possible to allow
> this consolidation to occur.
>
> Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
> http://www.spinics.net/lists/linux-samsung-soc/msg09467.html
>
> v5 submission can be found here:
> http://www.spinics.net/lists/arm-kernel/msg161596.html
> Changes since v5:
> * 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)

Except the comments I sent to the list, this version looks good:
Acked-by: Jean Pihet <j-pihet@ti.com>

Tested OK using cpuidle in RETention and OFF modes on Beagleboard
(OMAP3530 ES2.1):
Tested-by: Jean Pihet <j-pihet@ti.com>

Thanks & regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 0/9] Consolidate cpuidle functionality
@ 2012-02-29  9:40   ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  9:40 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, venki, ccross, g.trinabh, kernel,
	lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> 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().
>
> NOTE to Maintainers: Platform code changes are not required due to patch 1/9
> but please review and push these platform changes as possible to allow
> this consolidation to occur.
>
> Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
> http://www.spinics.net/lists/linux-samsung-soc/msg09467.html
>
> v5 submission can be found here:
> http://www.spinics.net/lists/arm-kernel/msg161596.html
> Changes since v5:
> * 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)

Except the comments I sent to the list, this version looks good:
Acked-by: Jean Pihet <j-pihet@ti.com>

Tested OK using cpuidle in RETention and OFF modes on Beagleboard
(OMAP3530 ES2.1):
Tested-by: Jean Pihet <j-pihet@ti.com>

Thanks & regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 0/9] Consolidate cpuidle functionality
@ 2012-02-29  9:40   ` Jean Pihet
  0 siblings, 0 replies; 60+ messages in thread
From: Jean Pihet @ 2012-02-29  9:40 UTC (permalink / raw)
  To: linux-arm-kernel

Rob,

On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> 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().
>
> NOTE to Maintainers: Platform code changes are not required due to patch 1/9
> but please review and push these platform changes as possible to allow
> this consolidation to occur.
>
> Based on 3.3-rc5 plus recent exynos cpuidle patch (affect exynos cpuidle only):
> http://www.spinics.net/lists/linux-samsung-soc/msg09467.html
>
> v5 submission can be found here:
> http://www.spinics.net/lists/arm-kernel/msg161596.html
> Changes since v5:
> * 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)

Except the comments I sent to the list, this version looks good:
Acked-by: Jean Pihet <j-pihet@ti.com>

Tested OK using cpuidle in RETention and OFF modes on Beagleboard
(OMAP3530 ES2.1):
Tested-by: Jean Pihet <j-pihet@ti.com>

Thanks & regards,
Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29  3:11   ` Robert Lee
  (?)
@ 2012-02-29 11:13     ` Deepthi Dharwar
  -1 siblings, 0 replies; 60+ messages in thread
From: Deepthi Dharwar @ 2012-02-29 11:13 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, broonie,
	nicolas.pitre, linux, jean.pihet, venki, ccross, g.trinabh,
	kernel, lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Hi Rob,


On 02/29/2012 08:41 AM, Robert Lee wrote:

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
> 
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}
> +
> +#endif
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -18,6 +18,7 @@
>  #include <linux/ktime.h>
>  #include <linux/hrtimer.h>
>  #include <linux/module.h>
> +#include <asm/proc-fns.h>
>  #include <trace/events/power.h>
> 
>  #include "cpuidle.h"
> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>  	if (cpuidle_curr_governor->reflect)
> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
> 
>  EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
> 
> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> -static int poll_idle(struct cpuidle_device *dev,
> -		struct cpuidle_driver *drv, int index)
> +/**
> + * 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	t1, t2;
> +	ktime_t time_start, time_end;
>  	s64 diff;
> 
> -	t1 = ktime_get();
> +	time_start = ktime_get();
> +
> +	index = enter(dev, drv, index);
> +
> +	time_end = ktime_get();
> +
>  	local_irq_enable();
> -	while (!need_resched())
> -		cpu_relax();
> 
> -	t2 = ktime_get();
> -	diff = ktime_to_us(ktime_sub(t2, t1));
> +	diff = ktime_to_us(ktime_sub(time_end, time_start));
>  	if (diff > INT_MAX)
>  		diff = INT_MAX;
> 
> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>  	return index;
>  }
> 
> +int cpuidle_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	cpu_do_idle();
> +
> +	return index;
> +}


The enter routines are always part of  the specific driver code rather
than in the generic cpuidle framework code.
The above function is arm driver specific enter routine, and should be
in arch specific file.
Right now, this causes a build break on Powerpc and Intel boxes.

drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
directory
drivers/cpuidle/cpuidle.c: In function ‘cpuidle_simple_enter’:
drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
‘cpu_do_idle’
make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers/cpuidle] Error 2
make[1]: *** Waiting for unfinished jobs....


> +
> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> +static inline int __poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	while (!need_resched())
> +		cpu_relax();
> +
> +	return index;
> +}
> +
> +static int poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	return cpuidle_wrap_enter(dev,	drv, index,
> +				__poll_idle);
> +}
> +
>  static void poll_idle_init(struct cpuidle_driver *drv)
>  {
>  	struct cpuidle_state *state = &drv->states[0];
> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>  	state->exit_latency = 0;
>  	state->target_residency = 0;
>  	state->power_usage = -1;
> -	state->flags = 0;


Also, why is that these flags have been eliminated ?

>  	state->enter = poll_idle;
>  }
>  #else
> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index);
> +
> +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) { }
> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{return -ENODEV; }
> 
>  #endif
> 


I am for generic time keeping framework, this would remove
loads of redundant code for the same in all the backend drivers.

Cheers,
Deepthi

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 11:13     ` Deepthi Dharwar
  0 siblings, 0 replies; 60+ messages in thread
From: Deepthi Dharwar @ 2012-02-29 11:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,


On 02/29/2012 08:41 AM, Robert Lee wrote:

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
> 
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}
> +
> +#endif
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -18,6 +18,7 @@
>  #include <linux/ktime.h>
>  #include <linux/hrtimer.h>
>  #include <linux/module.h>
> +#include <asm/proc-fns.h>
>  #include <trace/events/power.h>
> 
>  #include "cpuidle.h"
> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>  	if (cpuidle_curr_governor->reflect)
> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
> 
>  EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
> 
> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> -static int poll_idle(struct cpuidle_device *dev,
> -		struct cpuidle_driver *drv, int index)
> +/**
> + * 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	t1, t2;
> +	ktime_t time_start, time_end;
>  	s64 diff;
> 
> -	t1 = ktime_get();
> +	time_start = ktime_get();
> +
> +	index = enter(dev, drv, index);
> +
> +	time_end = ktime_get();
> +
>  	local_irq_enable();
> -	while (!need_resched())
> -		cpu_relax();
> 
> -	t2 = ktime_get();
> -	diff = ktime_to_us(ktime_sub(t2, t1));
> +	diff = ktime_to_us(ktime_sub(time_end, time_start));
>  	if (diff > INT_MAX)
>  		diff = INT_MAX;
> 
> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>  	return index;
>  }
> 
> +int cpuidle_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	cpu_do_idle();
> +
> +	return index;
> +}


The enter routines are always part of  the specific driver code rather
than in the generic cpuidle framework code.
The above function is arm driver specific enter routine, and should be
in arch specific file.
Right now, this causes a build break on Powerpc and Intel boxes.

drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
directory
drivers/cpuidle/cpuidle.c: In function ?cpuidle_simple_enter?:
drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
?cpu_do_idle?
make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers/cpuidle] Error 2
make[1]: *** Waiting for unfinished jobs....


> +
> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> +static inline int __poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	while (!need_resched())
> +		cpu_relax();
> +
> +	return index;
> +}
> +
> +static int poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	return cpuidle_wrap_enter(dev,	drv, index,
> +				__poll_idle);
> +}
> +
>  static void poll_idle_init(struct cpuidle_driver *drv)
>  {
>  	struct cpuidle_state *state = &drv->states[0];
> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>  	state->exit_latency = 0;
>  	state->target_residency = 0;
>  	state->power_usage = -1;
> -	state->flags = 0;


Also, why is that these flags have been eliminated ?

>  	state->enter = poll_idle;
>  }
>  #else
> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index);
> +
> +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) { }
> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{return -ENODEV; }
> 
>  #endif
> 


I am for generic time keeping framework, this would remove
loads of redundant code for the same in all the backend drivers.

Cheers,
Deepthi

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 11:13     ` Deepthi Dharwar
  0 siblings, 0 replies; 60+ messages in thread
From: Deepthi Dharwar @ 2012-02-29 11:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rob,


On 02/29/2012 08:41 AM, Robert Lee wrote:

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
> 
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}
> +
> +#endif
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 59f4261..00b02f5 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -18,6 +18,7 @@
>  #include <linux/ktime.h>
>  #include <linux/hrtimer.h>
>  #include <linux/module.h>
> +#include <asm/proc-fns.h>
>  #include <trace/events/power.h>
> 
>  #include "cpuidle.h"
> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>  	if (cpuidle_curr_governor->reflect)
> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
> 
>  EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
> 
> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> -static int poll_idle(struct cpuidle_device *dev,
> -		struct cpuidle_driver *drv, int index)
> +/**
> + * 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	t1, t2;
> +	ktime_t time_start, time_end;
>  	s64 diff;
> 
> -	t1 = ktime_get();
> +	time_start = ktime_get();
> +
> +	index = enter(dev, drv, index);
> +
> +	time_end = ktime_get();
> +
>  	local_irq_enable();
> -	while (!need_resched())
> -		cpu_relax();
> 
> -	t2 = ktime_get();
> -	diff = ktime_to_us(ktime_sub(t2, t1));
> +	diff = ktime_to_us(ktime_sub(time_end, time_start));
>  	if (diff > INT_MAX)
>  		diff = INT_MAX;
> 
> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>  	return index;
>  }
> 
> +int cpuidle_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	cpu_do_idle();
> +
> +	return index;
> +}


The enter routines are always part of  the specific driver code rather
than in the generic cpuidle framework code.
The above function is arm driver specific enter routine, and should be
in arch specific file.
Right now, this causes a build break on Powerpc and Intel boxes.

drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
directory
drivers/cpuidle/cpuidle.c: In function ‘cpuidle_simple_enter’:
drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
‘cpu_do_idle’
make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [drivers/cpuidle] Error 2
make[1]: *** Waiting for unfinished jobs....


> +
> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
> +static inline int __poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	while (!need_resched())
> +		cpu_relax();
> +
> +	return index;
> +}
> +
> +static int poll_idle(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{
> +	return cpuidle_wrap_enter(dev,	drv, index,
> +				__poll_idle);
> +}
> +
>  static void poll_idle_init(struct cpuidle_driver *drv)
>  {
>  	struct cpuidle_state *state = &drv->states[0];
> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>  	state->exit_latency = 0;
>  	state->target_residency = 0;
>  	state->power_usage = -1;
> -	state->flags = 0;


Also, why is that these flags have been eliminated ?

>  	state->enter = poll_idle;
>  }
>  #else
> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index);
> +
> +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) { }
> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
> +		struct cpuidle_driver *drv, int index)
> +{return -ENODEV; }
> 
>  #endif
> 


I am for generic time keeping framework, this would remove
loads of redundant code for the same in all the backend drivers.

Cheers,
Deepthi


^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29  8:30     ` Jean Pihet
  (?)
@ 2012-02-29 13:30       ` Rob Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 2:30 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Hi Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
> ...
>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
> ...
>
>> @@ -110,7 +126,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;
> Braces are required here, according to the coding style doc.

Thanks.

>
> ...
>
> Regards,
> Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 13:30       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:30 UTC (permalink / raw)
  To: Jean Pihet
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, venki, ccross, g.trinabh, kernel,
	lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

On Wed, Feb 29, 2012 at 2:30 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Hi Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
> ...
>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
> ...
>
>> @@ -110,7 +126,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;
> Braces are required here, according to the coding style doc.

Thanks.

>
> ...
>
> Regards,
> Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 13:30       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 2:30 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Hi Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, Robert Lee <rob.lee@linaro.org> wrote:
>> 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>
>> ---
>> ?arch/arm/include/asm/cpuidle.h | ? 14 ++++++
>> ?drivers/cpuidle/cpuidle.c ? ? ?| ? 90 ++++++++++++++++++++++++++++++++--------
>> ?include/linux/cpuidle.h ? ? ? ?| ? 13 ++++++
>> ?3 files changed, 99 insertions(+), 18 deletions(-)
>> ?create mode 100644 arch/arm/include/asm/cpuidle.h
>>
> ...
>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
> ...
>
>> @@ -110,7 +126,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;
> Braces are required here, according to the coding style doc.

Thanks.

>
> ...
>
> Regards,
> Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29 11:13     ` Deepthi Dharwar
  (?)
@ 2012-02-29 13:33       ` Rob Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Deepthi,

On Wed, Feb 29, 2012 at 5:13 AM, Deepthi Dharwar
<deepthi@linux.vnet.ibm.com> wrote:
> Hi Rob,
>
>
> On 02/29/2012 08:41 AM, Robert Lee wrote:
>
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> +     .enter                  = cpuidle_simple_enter,\
>> +     .exit_latency           = 1,\
>> +     .target_residency       = 1,\
>> +     .flags                  = CPUIDLE_FLAG_TIME_VALID,\
>> +     .name                   = "WFI",\
>> +     .desc                   = "ARM core clock gating (WFI)",\
>> +}
>> +
>> +#endif
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/ktime.h>
>>  #include <linux/hrtimer.h>
>>  #include <linux/module.h>
>> +#include <asm/proc-fns.h>
>>  #include <trace/events/power.h>
>>
>>  #include "cpuidle.h"
>> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>>       if (cpuidle_curr_governor->reflect)
>> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
>>
>>  EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
>>
>> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> -static int poll_idle(struct cpuidle_device *dev,
>> -             struct cpuidle_driver *drv, int index)
>> +/**
>> + * 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 t1, t2;
>> +     ktime_t time_start, time_end;
>>       s64 diff;
>>
>> -     t1 = ktime_get();
>> +     time_start = ktime_get();
>> +
>> +     index = enter(dev, drv, index);
>> +
>> +     time_end = ktime_get();
>> +
>>       local_irq_enable();
>> -     while (!need_resched())
>> -             cpu_relax();
>>
>> -     t2 = ktime_get();
>> -     diff = ktime_to_us(ktime_sub(t2, t1));
>> +     diff = ktime_to_us(ktime_sub(time_end, time_start));
>>       if (diff > INT_MAX)
>>               diff = INT_MAX;
>>
>> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>>       return index;
>>  }
>>
>> +int cpuidle_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     cpu_do_idle();
>> +
>> +     return index;
>> +}
>
>
> The enter routines are always part of  the specific driver code rather
> than in the generic cpuidle framework code.
> The above function is arm driver specific enter routine, and should be
> in arch specific file.
> Right now, this causes a build break on Powerpc and Intel boxes.
>

Ok, I can move this out on next version.

> drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
> directory
> drivers/cpuidle/cpuidle.c: In function ‘cpuidle_simple_enter’:
> drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
> ‘cpu_do_idle’
> make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [drivers/cpuidle] Error 2
> make[1]: *** Waiting for unfinished jobs....
>
>
>> +
>> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> +static inline int __poll_idle(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     while (!need_resched())
>> +             cpu_relax();
>> +
>> +     return index;
>> +}
>> +
>> +static int poll_idle(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     return cpuidle_wrap_enter(dev,  drv, index,
>> +                             __poll_idle);
>> +}
>> +
>>  static void poll_idle_init(struct cpuidle_driver *drv)
>>  {
>>       struct cpuidle_state *state = &drv->states[0];
>> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>>       state->exit_latency = 0;
>>       state->target_residency = 0;
>>       state->power_usage = -1;
>> -     state->flags = 0;
>
>
> Also, why is that these flags have been eliminated ?
>

This shouldn't be there.  Will fix this in next version.

>>       state->enter = poll_idle;
>>  }
>>  #else
>> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index);
>> +
>> +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) { }
>> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{return -ENODEV; }
>>
>>  #endif
>>
>
>
> I am for generic time keeping framework, this would remove
> loads of redundant code for the same in all the backend drivers.
>
> Cheers,
> Deepthi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 13:33       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:33 UTC (permalink / raw)
  To: Deepthi Dharwar
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, broonie,
	nicolas.pitre, linux, jean.pihet, venki, ccross, g.trinabh,
	kernel, lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

Hello Deepthi,

On Wed, Feb 29, 2012 at 5:13 AM, Deepthi Dharwar
<deepthi@linux.vnet.ibm.com> wrote:
> Hi Rob,
>
>
> On 02/29/2012 08:41 AM, Robert Lee wrote:
>
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> +     .enter                  = cpuidle_simple_enter,\
>> +     .exit_latency           = 1,\
>> +     .target_residency       = 1,\
>> +     .flags                  = CPUIDLE_FLAG_TIME_VALID,\
>> +     .name                   = "WFI",\
>> +     .desc                   = "ARM core clock gating (WFI)",\
>> +}
>> +
>> +#endif
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/ktime.h>
>>  #include <linux/hrtimer.h>
>>  #include <linux/module.h>
>> +#include <asm/proc-fns.h>
>>  #include <trace/events/power.h>
>>
>>  #include "cpuidle.h"
>> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>>       if (cpuidle_curr_governor->reflect)
>> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
>>
>>  EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
>>
>> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> -static int poll_idle(struct cpuidle_device *dev,
>> -             struct cpuidle_driver *drv, int index)
>> +/**
>> + * 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 t1, t2;
>> +     ktime_t time_start, time_end;
>>       s64 diff;
>>
>> -     t1 = ktime_get();
>> +     time_start = ktime_get();
>> +
>> +     index = enter(dev, drv, index);
>> +
>> +     time_end = ktime_get();
>> +
>>       local_irq_enable();
>> -     while (!need_resched())
>> -             cpu_relax();
>>
>> -     t2 = ktime_get();
>> -     diff = ktime_to_us(ktime_sub(t2, t1));
>> +     diff = ktime_to_us(ktime_sub(time_end, time_start));
>>       if (diff > INT_MAX)
>>               diff = INT_MAX;
>>
>> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>>       return index;
>>  }
>>
>> +int cpuidle_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     cpu_do_idle();
>> +
>> +     return index;
>> +}
>
>
> The enter routines are always part of  the specific driver code rather
> than in the generic cpuidle framework code.
> The above function is arm driver specific enter routine, and should be
> in arch specific file.
> Right now, this causes a build break on Powerpc and Intel boxes.
>

Ok, I can move this out on next version.

> drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
> directory
> drivers/cpuidle/cpuidle.c: In function ‘cpuidle_simple_enter’:
> drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
> ‘cpu_do_idle’
> make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [drivers/cpuidle] Error 2
> make[1]: *** Waiting for unfinished jobs....
>
>
>> +
>> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> +static inline int __poll_idle(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     while (!need_resched())
>> +             cpu_relax();
>> +
>> +     return index;
>> +}
>> +
>> +static int poll_idle(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{
>> +     return cpuidle_wrap_enter(dev,  drv, index,
>> +                             __poll_idle);
>> +}
>> +
>>  static void poll_idle_init(struct cpuidle_driver *drv)
>>  {
>>       struct cpuidle_state *state = &drv->states[0];
>> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>>       state->exit_latency = 0;
>>       state->target_residency = 0;
>>       state->power_usage = -1;
>> -     state->flags = 0;
>
>
> Also, why is that these flags have been eliminated ?
>

This shouldn't be there.  Will fix this in next version.

>>       state->enter = poll_idle;
>>  }
>>  #else
>> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index);
>> +
>> +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) { }
>> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
>> +             struct cpuidle_driver *drv, int index)
>> +{return -ENODEV; }
>>
>>  #endif
>>
>
>
> I am for generic time keeping framework, this would remove
> loads of redundant code for the same in all the backend drivers.
>
> Cheers,
> Deepthi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 13:33       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Deepthi,

On Wed, Feb 29, 2012 at 5:13 AM, Deepthi Dharwar
<deepthi@linux.vnet.ibm.com> wrote:
> Hi Rob,
>
>
> On 02/29/2012 08:41 AM, Robert Lee wrote:
>
>> 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>
>> ---
>> ?arch/arm/include/asm/cpuidle.h | ? 14 ++++++
>> ?drivers/cpuidle/cpuidle.c ? ? ?| ? 90 ++++++++++++++++++++++++++++++++--------
>> ?include/linux/cpuidle.h ? ? ? ?| ? 13 ++++++
>> ?3 files changed, 99 insertions(+), 18 deletions(-)
>> ?create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> + ? ? .enter ? ? ? ? ? ? ? ? ?= cpuidle_simple_enter,\
>> + ? ? .exit_latency ? ? ? ? ? = 1,\
>> + ? ? .target_residency ? ? ? = 1,\
>> + ? ? .flags ? ? ? ? ? ? ? ? ?= CPUIDLE_FLAG_TIME_VALID,\
>> + ? ? .name ? ? ? ? ? ? ? ? ? = "WFI",\
>> + ? ? .desc ? ? ? ? ? ? ? ? ? = "ARM core clock gating (WFI)",\
>> +}
>> +
>> +#endif
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index 59f4261..00b02f5 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -18,6 +18,7 @@
>> ?#include <linux/ktime.h>
>> ?#include <linux/hrtimer.h>
>> ?#include <linux/module.h>
>> +#include <asm/proc-fns.h>
>> ?#include <trace/events/power.h>
>>
>> ?#include "cpuidle.h"
>> @@ -53,6 +54,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 +82,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 +110,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,7 +126,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 */
>> ? ? ? if (cpuidle_curr_governor->reflect)
>> @@ -164,20 +181,29 @@ void cpuidle_resume_and_unlock(void)
>>
>> ?EXPORT_SYMBOL_GPL(cpuidle_resume_and_unlock);
>>
>> -#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> -static int poll_idle(struct cpuidle_device *dev,
>> - ? ? ? ? ? ? struct cpuidle_driver *drv, int index)
>> +/**
>> + * 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 t1, t2;
>> + ? ? ktime_t time_start, time_end;
>> ? ? ? s64 diff;
>>
>> - ? ? t1 = ktime_get();
>> + ? ? time_start = ktime_get();
>> +
>> + ? ? index = enter(dev, drv, index);
>> +
>> + ? ? time_end = ktime_get();
>> +
>> ? ? ? local_irq_enable();
>> - ? ? while (!need_resched())
>> - ? ? ? ? ? ? cpu_relax();
>>
>> - ? ? t2 = ktime_get();
>> - ? ? diff = ktime_to_us(ktime_sub(t2, t1));
>> + ? ? diff = ktime_to_us(ktime_sub(time_end, time_start));
>> ? ? ? if (diff > INT_MAX)
>> ? ? ? ? ? ? ? diff = INT_MAX;
>>
>> @@ -186,6 +212,31 @@ static int poll_idle(struct cpuidle_device *dev,
>> ? ? ? return index;
>> ?}
>>
>> +int cpuidle_simple_enter(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? struct cpuidle_driver *drv, int index)
>> +{
>> + ? ? cpu_do_idle();
>> +
>> + ? ? return index;
>> +}
>
>
> The enter routines are always part of ?the specific driver code rather
> than in the generic cpuidle framework code.
> The above function is arm driver specific enter routine, and should be
> in arch specific file.
> Right now, this causes a build break on Powerpc and Intel boxes.
>

Ok, I can move this out on next version.

> drivers/cpuidle/cpuidle.c:21:26: error: asm/proc-fns.h: No such file or
> directory
> drivers/cpuidle/cpuidle.c: In function ?cpuidle_simple_enter?:
> drivers/cpuidle/cpuidle.c:218: error: implicit declaration of function
> ?cpu_do_idle?
> make[2]: *** [drivers/cpuidle/cpuidle.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [drivers/cpuidle] Error 2
> make[1]: *** Waiting for unfinished jobs....
>
>
>> +
>> +#ifdef CONFIG_ARCH_HAS_CPU_RELAX
>> +static inline int __poll_idle(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? struct cpuidle_driver *drv, int index)
>> +{
>> + ? ? while (!need_resched())
>> + ? ? ? ? ? ? cpu_relax();
>> +
>> + ? ? return index;
>> +}
>> +
>> +static int poll_idle(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? struct cpuidle_driver *drv, int index)
>> +{
>> + ? ? return cpuidle_wrap_enter(dev, ?drv, index,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? __poll_idle);
>> +}
>> +
>> ?static void poll_idle_init(struct cpuidle_driver *drv)
>> ?{
>> ? ? ? struct cpuidle_state *state = &drv->states[0];
>> @@ -195,7 +246,6 @@ static void poll_idle_init(struct cpuidle_driver *drv)
>> ? ? ? state->exit_latency = 0;
>> ? ? ? state->target_residency = 0;
>> ? ? ? state->power_usage = -1;
>> - ? ? state->flags = 0;
>
>
> Also, why is that these flags have been eliminated ?
>

This shouldn't be there.  Will fix this in next version.

>> ? ? ? state->enter = poll_idle;
>> ?}
>> ?#else
>> @@ -212,10 +262,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 +277,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..c91b018 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,6 +143,13 @@ 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_simple_enter(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? struct cpuidle_driver *drv, int index);
>> +
>> +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) { }
>> @@ -157,6 +167,9 @@ 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_simple_enter(struct cpuidle_device *dev,
>> + ? ? ? ? ? ? struct cpuidle_driver *drv, int index)
>> +{return -ENODEV; }
>>
>> ?#endif
>>
>
>
> I am for generic time keeping framework, this would remove
> loads of redundant code for the same in all the backend drivers.
>
> Cheers,
> Deepthi
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
  2012-02-29  8:36     ` Jean Pihet
  (?)
@ 2012-02-29 13:36       ` Rob Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 2:36 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, 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>
>> ---
>>  arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
>>  1 files changed, 31 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
>> index a30c7c5..6f457f1 100644
>> --- a/arch/arm/mach-davinci/cpuidle.c
>> +++ b/arch/arm/mach-davinci/cpuidle.c
> ...
>
>> @@ -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);
>> +
>> +       return cpuidle_wrap_enter(dev,  drv, index,
>> +                               cpuidle_simple_enter);
> This does not look right since ops->exit will never be called.
>

Yes, thanks.  The 'return' should be 'index ='

>> +
>> +       if (ops && ops->exit)
>> +               ops->exit(ops->flags);
>> +
>> +       return index;
>> +}
>> +
> ...
>
> Regards,
> Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29 13:36       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:36 UTC (permalink / raw)
  To: Jean Pihet
  Cc: len.brown, khilman, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, venki, ccross, g.trinabh, kernel,
	lethal, jon-hunter, tony, linux-omap, linux-sh, linux-pm

On Wed, Feb 29, 2012 at 2:36 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, 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>
>> ---
>>  arch/arm/mach-davinci/cpuidle.c |   78 +++++++++++++++-----------------------
>>  1 files changed, 31 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
>> index a30c7c5..6f457f1 100644
>> --- a/arch/arm/mach-davinci/cpuidle.c
>> +++ b/arch/arm/mach-davinci/cpuidle.c
> ...
>
>> @@ -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);
>> +
>> +       return cpuidle_wrap_enter(dev,  drv, index,
>> +                               cpuidle_simple_enter);
> This does not look right since ops->exit will never be called.
>

Yes, thanks.  The 'return' should be 'index ='

>> +
>> +       if (ops && ops->exit)
>> +               ops->exit(ops->flags);
>> +
>> +       return index;
>> +}
>> +
> ...
>
> Regards,
> Jean
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 5/9] ARM: davinci: Consolidate time keeping and irq enable
@ 2012-02-29 13:36       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 13:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 2:36 AM, Jean Pihet <jean.pihet@newoldbits.com> wrote:
> Rob,
>
> On Wed, Feb 29, 2012 at 4:11 AM, 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>
>> ---
>> ?arch/arm/mach-davinci/cpuidle.c | ? 78 +++++++++++++++-----------------------
>> ?1 files changed, 31 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
>> index a30c7c5..6f457f1 100644
>> --- a/arch/arm/mach-davinci/cpuidle.c
>> +++ b/arch/arm/mach-davinci/cpuidle.c
> ...
>
>> @@ -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);
>> +
>> + ? ? ? return cpuidle_wrap_enter(dev, ?drv, index,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpuidle_simple_enter);
> This does not look right since ops->exit will never be called.
>

Yes, thanks.  The 'return' should be 'index ='

>> +
>> + ? ? ? if (ops && ops->exit)
>> + ? ? ? ? ? ? ? ops->exit(ops->flags);
>> +
>> + ? ? ? return index;
>> +}
>> +
> ...
>
> Regards,
> Jean

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29  3:11   ` Robert Lee
  (?)
@ 2012-02-29 18:43     ` Kevin Hilman
  -1 siblings, 0 replies; 60+ messages in thread
From: Kevin Hilman @ 2012-02-29 18:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
>
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}

nit: just name this ARM WFI.  Clock gating is platform specific, and
"core" has platform-specific meanings, so in order to keep this truly
generic, I think hat ARM WFI is the best name.

Kevin



^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 18:43     ` Kevin Hilman
  0 siblings, 0 replies; 60+ messages in thread
From: Kevin Hilman @ 2012-02-29 18:43 UTC (permalink / raw)
  To: Robert Lee
  Cc: len.brown, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, jean.pihet, venki, ccross,
	g.trinabh, kernel, lethal, jon-hunter, tony, linux-omap,
	linux-sh, linux-pm

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

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
>
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}

nit: just name this ARM WFI.  Clock gating is platform specific, and
"core" has platform-specific meanings, so in order to keep this truly
generic, I think hat ARM WFI is the best name.

Kevin



^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 18:43     ` Kevin Hilman
  0 siblings, 0 replies; 60+ messages in thread
From: Kevin Hilman @ 2012-02-29 18:43 UTC (permalink / raw)
  To: linux-arm-kernel

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

> 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>
> ---
>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>  include/linux/cpuidle.h        |   13 ++++++
>  3 files changed, 99 insertions(+), 18 deletions(-)
>  create mode 100644 arch/arm/include/asm/cpuidle.h
>
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> new file mode 100644
> index 0000000..1d2075b
> --- /dev/null
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -0,0 +1,14 @@
> +#ifndef __ASM_ARM_CPUIDLE_H
> +#define __ASM_ARM_CPUIDLE_H
> +
> +/* Common ARM WFI state */
> +#define CPUIDLE_ARM_WFI_STATE {\
> +	.enter                  = cpuidle_simple_enter,\
> +	.exit_latency           = 1,\
> +	.target_residency       = 1,\
> +	.flags                  = CPUIDLE_FLAG_TIME_VALID,\
> +	.name                   = "WFI",\
> +	.desc                   = "ARM core clock gating (WFI)",\
> +}

nit: just name this ARM WFI.  Clock gating is platform specific, and
"core" has platform-specific meanings, so in order to keep this truly
generic, I think hat ARM WFI is the best name.

Kevin

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
  2012-02-29 18:43     ` Kevin Hilman
  (?)
@ 2012-02-29 21:17       ` Rob Lee
  -1 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 21:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 12:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> Robert Lee <rob.lee@linaro.org> writes:
>
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> +     .enter                  = cpuidle_simple_enter,\
>> +     .exit_latency           = 1,\
>> +     .target_residency       = 1,\
>> +     .flags                  = CPUIDLE_FLAG_TIME_VALID,\
>> +     .name                   = "WFI",\
>> +     .desc                   = "ARM core clock gating (WFI)",\
>> +}
>
> nit: just name this ARM WFI.  Clock gating is platform specific, and
> "core" has platform-specific meanings, so in order to keep this truly
> generic, I think hat ARM WFI is the best name.
>

Agree.  I'll make that change.

> Kevin
>
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 21:17       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 21:17 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: len.brown, robherring2, Baohua.Song, amit.kucheria,
	nicolas.ferre, linux, kgene.kim, amit.kachhap, magnus.damm,
	nsekhar, daniel.lezcano, mturquette, vincent.guittot,
	arnd.bergmann, linux-arm-kernel, linaro-dev, patches, deepthi,
	broonie, nicolas.pitre, linux, jean.pihet, venki, ccross,
	g.trinabh, kernel, lethal, jon-hunter, tony, linux-omap,
	linux-sh, linux-pm

On Wed, Feb 29, 2012 at 12:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> Robert Lee <rob.lee@linaro.org> writes:
>
>> 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>
>> ---
>>  arch/arm/include/asm/cpuidle.h |   14 ++++++
>>  drivers/cpuidle/cpuidle.c      |   90 ++++++++++++++++++++++++++++++++--------
>>  include/linux/cpuidle.h        |   13 ++++++
>>  3 files changed, 99 insertions(+), 18 deletions(-)
>>  create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> +     .enter                  = cpuidle_simple_enter,\
>> +     .exit_latency           = 1,\
>> +     .target_residency       = 1,\
>> +     .flags                  = CPUIDLE_FLAG_TIME_VALID,\
>> +     .name                   = "WFI",\
>> +     .desc                   = "ARM core clock gating (WFI)",\
>> +}
>
> nit: just name this ARM WFI.  Clock gating is platform specific, and
> "core" has platform-specific meanings, so in order to keep this truly
> generic, I think hat ARM WFI is the best name.
>

Agree.  I'll make that change.

> Kevin
>
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling
@ 2012-02-29 21:17       ` Rob Lee
  0 siblings, 0 replies; 60+ messages in thread
From: Rob Lee @ 2012-02-29 21:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 29, 2012 at 12:43 PM, Kevin Hilman <khilman@ti.com> wrote:
> Robert Lee <rob.lee@linaro.org> writes:
>
>> 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>
>> ---
>> ?arch/arm/include/asm/cpuidle.h | ? 14 ++++++
>> ?drivers/cpuidle/cpuidle.c ? ? ?| ? 90 ++++++++++++++++++++++++++++++++--------
>> ?include/linux/cpuidle.h ? ? ? ?| ? 13 ++++++
>> ?3 files changed, 99 insertions(+), 18 deletions(-)
>> ?create mode 100644 arch/arm/include/asm/cpuidle.h
>>
>> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
>> new file mode 100644
>> index 0000000..1d2075b
>> --- /dev/null
>> +++ b/arch/arm/include/asm/cpuidle.h
>> @@ -0,0 +1,14 @@
>> +#ifndef __ASM_ARM_CPUIDLE_H
>> +#define __ASM_ARM_CPUIDLE_H
>> +
>> +/* Common ARM WFI state */
>> +#define CPUIDLE_ARM_WFI_STATE {\
>> + ? ? .enter ? ? ? ? ? ? ? ? ?= cpuidle_simple_enter,\
>> + ? ? .exit_latency ? ? ? ? ? = 1,\
>> + ? ? .target_residency ? ? ? = 1,\
>> + ? ? .flags ? ? ? ? ? ? ? ? ?= CPUIDLE_FLAG_TIME_VALID,\
>> + ? ? .name ? ? ? ? ? ? ? ? ? = "WFI",\
>> + ? ? .desc ? ? ? ? ? ? ? ? ? = "ARM core clock gating (WFI)",\
>> +}
>
> nit: just name this ARM WFI. ?Clock gating is platform specific, and
> "core" has platform-specific meanings, so in order to keep this truly
> generic, I think hat ARM WFI is the best name.
>

Agree.  I'll make that change.

> Kevin
>
>

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2012-02-29 21:17 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-29  3:11 [PATCH v6 0/9] Consolidate cpuidle functionality Robert Lee
2012-02-29  3:11 ` Robert Lee
2012-02-29  3:11 ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 1/9] cpuidle: Add common time keeping and irq enabling Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  8:30   ` Jean Pihet
2012-02-29  8:30     ` Jean Pihet
2012-02-29  8:30     ` Jean Pihet
2012-02-29 13:30     ` Rob Lee
2012-02-29 13:30       ` Rob Lee
2012-02-29 13:30       ` Rob Lee
2012-02-29 11:13   ` Deepthi Dharwar
2012-02-29 11:25     ` Deepthi Dharwar
2012-02-29 11:13     ` Deepthi Dharwar
2012-02-29 13:33     ` Rob Lee
2012-02-29 13:33       ` Rob Lee
2012-02-29 13:33       ` Rob Lee
2012-02-29 18:43   ` Kevin Hilman
2012-02-29 18:43     ` Kevin Hilman
2012-02-29 18:43     ` Kevin Hilman
2012-02-29 21:17     ` Rob Lee
2012-02-29 21:17       ` Rob Lee
2012-02-29 21:17       ` Rob Lee
2012-02-29  3:11 ` [PATCH v6 2/9] ARM: at91: Consolidate time keeping and irq enable Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 3/9] ARM: exynos: " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  6:41   ` Amit Kachhap
2012-02-29  6:53     ` Amit Kachhap
2012-02-29  6:41     ` Amit Kachhap
2012-02-29  3:11 ` [PATCH v6 4/9] ARM: kirkwood: " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 5/9] ARM: davinci: " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  8:36   ` Jean Pihet
2012-02-29  8:36     ` Jean Pihet
2012-02-29  8:36     ` Jean Pihet
2012-02-29 13:36     ` Rob Lee
2012-02-29 13:36       ` Rob Lee
2012-02-29 13:36       ` Rob Lee
2012-02-29  3:11 ` [PATCH v6 6/9] ARM: omap: Consolidate OMAP3 " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 7/9] ARM: omap: Consolidate OMAP4 " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 8/9] ARM: shmobile: Consolidate " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11 ` [PATCH v6 9/9] SH: " Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  3:11   ` Robert Lee
2012-02-29  9:40 ` [PATCH v6 0/9] Consolidate cpuidle functionality Jean Pihet
2012-02-29  9:40   ` Jean Pihet
2012-02-29  9:40   ` Jean Pihet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.