OMAP3 CPUidle driver

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Signed-off-by Rajendra Nayak <[email protected]>

---
 arch/arm/mach-omap2/Makefile      |    2 
 arch/arm/mach-omap2/cpuidle34xx.c |  293 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/cpuidle34xx.h |   51 ++++++
 arch/arm/mach-omap2/pm34xx.c      |    5 
 drivers/cpuidle/cpuidle.c         |   10 +
 5 files changed, 359 insertions(+), 2 deletions(-)

Index: linux-omap-2.6/arch/arm/mach-omap2/Makefile
===================================================================

arch/arm/mach-omap2/Makefile +1 -1

@@ -20,7 +20,7 @@ obj-y					+= pm.o
 obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
 obj-$(CONFIG_ARCH_OMAP2420)		+= sleep242x.o
 obj-$(CONFIG_ARCH_OMAP2430)		+= sleep243x.o
-obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o
+obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o cpuidle34xx.o
 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
 endif
 

Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
===================================================================

arch/arm/mach-omap2/cpuidle34xx.c new file +293 -0

@@ -0,0 +1,293 @@
+/*
+ * linux/arch/arm/mach-omap2/cpuidle34xx.c
+ *
+ * OMAP3 CPU IDLE Routines
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Rajendra Nayak <[email protected]>
+ *
+ * Copyright (C) 2007 Texas Instruments, Inc.
+ * Karthik Dasu <[email protected]>
+ *
+ * Copyright (C) 2006 Nokia Corporation
+ * Tony Lindgren <[email protected]>
+ *
+ * Copyright (C) 2005 Texas Instruments, Inc.
+ * Richard Woodruff <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/cpuidle.h>
+#include <asm/arch/pm.h>
+#include <asm/arch/prcm.h>
+#include <asm/arch/powerdomain.h>
+#include <asm/arch/clockdomain.h>
+#include <asm/arch/irqs.h>
+#include "cpuidle34xx.h"
+
+#ifdef CONFIG_CPU_IDLE
+
+struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
+struct omap3_processor_cx current_cx_state;
+
+static int omap3_idle_bm_check(void)
+{
+	/* Check for omap3_fclks_active() here once available */
+	return 0;
+}
+
+/* omap3_enter_idle - Programs OMAP3 to enter the specified state.
+ * returns the total time during which the system was idle.
+ */
+static int omap3_enter_idle(struct cpuidle_device *dev,
+			struct cpuidle_state *state)
+{
+	struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
+	struct timespec ts_preidle, ts_postidle, ts_idle;
+	struct powerdomain *mpu_pd, *core_pd, *per_pd, *neon_pd;
+	int neon_pwrst;
+
+	current_cx_state = *cx;
+
+	if (cx->type == OMAP3_STATE_C0) {
+		/* Do nothing for C0, not even a wfi */
+		return 0;
+	}
+
+	/* Used to keep track of the total time in idle */
+	getnstimeofday(&ts_preidle);
+
+	mpu_pd = pwrdm_lookup("mpu_pwrdm");
+	core_pd = pwrdm_lookup("core_pwrdm");
+	per_pd = pwrdm_lookup("per_pwrdm");
+	neon_pd = pwrdm_lookup("neon_pwrdm");
+
+	/* Reset previous power state registers */
+	pwrdm_clear_all_prev_pwrst(mpu_pd);
+	pwrdm_clear_all_prev_pwrst(neon_pd);
+	pwrdm_clear_all_prev_pwrst(core_pd);
+	pwrdm_clear_all_prev_pwrst(per_pd);
+
+	if (omap_irq_pending())
+		return 0;
+
+	neon_pwrst = pwrdm_read_pwrst(neon_pd);
+
+	/* Program MPU/NEON to target state */
+	if (cx->mpu_state < PWRDM_POWER_ON) {
+		if (neon_pwrst == PWRDM_POWER_ON) {
+                        if (cx->mpu_state == PWRDM_POWER_RET)
+                                pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_RET);
+                        else if (cx->mpu_state == PWRDM_POWER_OFF)
+                                pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_OFF);
+		}
+		pwrdm_set_next_pwrst(mpu_pd, cx->mpu_state);
+	}
+
+	/* Program CORE to target state */
+	if (cx->core_state < PWRDM_POWER_ON)
+		pwrdm_set_next_pwrst(core_pd, cx->core_state);
+
+	/* Execute ARM wfi */
+	omap_sram_idle();
+
+	/* Program MPU/NEON to ON */
+	if (cx->mpu_state < PWRDM_POWER_ON) {
+		if (neon_pwrst == PWRDM_POWER_ON)
+			pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_ON);
+		pwrdm_set_next_pwrst(mpu_pd, PWRDM_POWER_ON);
+	}
+
+	if (cx->core_state < PWRDM_POWER_ON)
+		pwrdm_set_next_pwrst(core_pd, PWRDM_POWER_ON);
+
+	getnstimeofday(&ts_postidle);
+	ts_idle = timespec_sub(ts_postidle, ts_preidle);
+	return timespec_to_ns(&ts_idle);
+}
+
+/*
+ * omap3_enter_idle_bm - enter function for states with CPUIDLE_FLAG_CHECK_BM
+ *
+ * This function checks for all the pre-requisites needed for OMAP3 to enter
+ * CORE RET/OFF state. It then calls omap3_enter_idle to program the desired
+ * C state.
+ */
+static int omap3_enter_idle_bm(struct cpuidle_device *dev,
+			       struct cpuidle_state *state)
+{
+	struct cpuidle_state *new_state = NULL;
+	int i, j;
+
+	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
+
+		/* Find current state in list */
+		for (i = 0; i < OMAP3_MAX_STATES; i++)
+			if (state == &dev->states[i])
+				break;
+		BUG_ON(i == OMAP3_MAX_STATES);
+
+		/* Back up to non 'CHECK_BM' state */
+		for (j = i - 1;  j > 0; j--) {
+			struct cpuidle_state *s = &dev->states[j];
+
+			if (!(s->flags & CPUIDLE_FLAG_CHECK_BM)) {
+				new_state = s;
+				break;
+			}
+		}
+
+		pr_debug("%s: Bus activity: Entering %s (instead of %s)\n",
+			__FUNCTION__, new_state->name, state->name);
+	}
+
+	return omap3_enter_idle(dev, new_state ? : state);
+}
+
+DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
+
+/* omap3_init_power_states - Initialises the OMAP3 specific C states.
+ * Below is the desciption of each C state.
+ *
+	C0 . System executing code
+	C1 . MPU WFI + Core active
+	C2 . MPU CSWR + Core active
+	C3 . MPU OFF + Core active
+	C4 . MPU CSWR + Core CSWR
+	C5 . MPU OFF + Core CSWR
+	C6 . MPU OFF + Core OFF
+ */
+void omap_init_power_states(void)
+{
+	/* C0 . System executing code */
+	omap3_power_states[0].valid = 1;
+	omap3_power_states[0].type = OMAP3_STATE_C0;
+	omap3_power_states[0].sleep_latency = 0;
+	omap3_power_states[0].wakeup_latency = 0;
+	omap3_power_states[0].threshold = 0;
+	omap3_power_states[0].mpu_state = PWRDM_POWER_ON;
+	omap3_power_states[0].core_state = PWRDM_POWER_ON;
+	omap3_power_states[0].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_SHALLOW;
+
+	/* C1 . MPU WFI + Core active */
+	omap3_power_states[1].valid = 1;
+	omap3_power_states[1].type = OMAP3_STATE_C1;
+	omap3_power_states[1].sleep_latency = 10;
+	omap3_power_states[1].wakeup_latency = 10;
+	omap3_power_states[1].threshold = 30;
+	omap3_power_states[1].mpu_state = PWRDM_POWER_ON;
+	omap3_power_states[1].core_state = PWRDM_POWER_ON;
+	omap3_power_states[1].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_SHALLOW;
+
+	/* C2 . MPU CSWR + Core active */
+	omap3_power_states[2].valid = 1;
+	omap3_power_states[2].type = OMAP3_STATE_C2;
+	omap3_power_states[2].sleep_latency = 50;
+	omap3_power_states[2].wakeup_latency = 50;
+	omap3_power_states[2].threshold = 300;
+	omap3_power_states[2].mpu_state = PWRDM_POWER_RET;
+	omap3_power_states[2].core_state = PWRDM_POWER_ON;
+	omap3_power_states[2].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_BALANCED;
+
+	/* C3 . MPU OFF + Core active */
+	omap3_power_states[3].valid = 0;
+	omap3_power_states[3].type = OMAP3_STATE_C3;
+	omap3_power_states[3].sleep_latency = 1500;
+	omap3_power_states[3].wakeup_latency = 1800;
+	omap3_power_states[3].threshold = 4000;
+	omap3_power_states[3].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[3].core_state = PWRDM_POWER_RET;
+	omap3_power_states[3].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_BALANCED;
+
+	/* C4 . MPU CSWR + Core CSWR*/
+	omap3_power_states[4].valid = 1;
+	omap3_power_states[4].type = OMAP3_STATE_C4;
+	omap3_power_states[4].sleep_latency = 2500;
+	omap3_power_states[4].wakeup_latency = 7500;
+	omap3_power_states[4].threshold = 12000;
+	omap3_power_states[4].mpu_state = PWRDM_POWER_RET;
+	omap3_power_states[4].core_state = PWRDM_POWER_RET;
+	omap3_power_states[4].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
+
+	/* C5 . MPU OFF + Core CSWR */
+	omap3_power_states[5].valid = 0;
+	omap3_power_states[5].type = OMAP3_STATE_C5;
+	omap3_power_states[5].sleep_latency = 3000;
+	omap3_power_states[5].wakeup_latency = 8500;
+	omap3_power_states[5].threshold = 15000;
+	omap3_power_states[5].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[5].core_state = PWRDM_POWER_RET;
+	omap3_power_states[5].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
+
+	/* C6 . MPU OFF + Core OFF */
+	omap3_power_states[6].valid = 0;
+	omap3_power_states[6].type = OMAP3_STATE_C6;
+	omap3_power_states[6].sleep_latency = 10000;
+	omap3_power_states[6].wakeup_latency = 30000;
+	omap3_power_states[6].threshold = 300000;
+	omap3_power_states[6].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[6].core_state = PWRDM_POWER_OFF;
+	omap3_power_states[6].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_DEEP | CPUIDLE_FLAG_CHECK_BM;
+}
+
+struct cpuidle_driver omap3_idle_driver = {
+	.name = 	"omap3_idle",
+	.owner = 	THIS_MODULE,
+};
+/*
+ * omap3_idle_init - Init routine for OMAP3 idle.
+ * Registers the OMAP3 specific cpuidle driver with the cpuidle f/w
+ * with the valid set of states.
+ */
+int omap3_idle_init(void)
+{
+	int i, count = 0;
+	struct omap3_processor_cx *cx;
+	struct cpuidle_state *state;
+	struct cpuidle_device *dev;
+
+	omap_init_power_states();
+	cpuidle_register_driver(&omap3_idle_driver);
+
+	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
+
+	for (i = 0; i < OMAP3_MAX_STATES; i++) {
+		cx = &omap3_power_states[i];
+		state = &dev->states[count];
+
+		if (!cx->valid)
+			continue;
+		cpuidle_set_statedata(state, cx);
+		state->exit_latency = cx->sleep_latency + cx->wakeup_latency;
+		state->target_residency = cx->threshold;
+		state->flags = cx->flags;
+		state->enter = (state->flags & CPUIDLE_FLAG_CHECK_BM) ?
+			omap3_enter_idle_bm : omap3_enter_idle;
+		sprintf(state->name, "C%d", count+1);
+		count++;
+	}
+
+	if (!count)
+		return -EINVAL;
+	dev->state_count = count;
+
+	if (cpuidle_register_device(dev)) {
+		printk(KERN_ERR "%s: CPUidle register device failed\n",
+		       __FUNCTION__);
+		return -EIO;
+	}
+
+	return 0;
+}
+__initcall(omap3_idle_init);
+#endif /* CONFIG_CPU_IDLE */

Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.h
===================================================================

arch/arm/mach-omap2/cpuidle34xx.h new file +51 -0

@@ -0,0 +1,51 @@
+/*
+ * linux/arch/arm/mach-omap2/cpuidle34xx.h
+ *
+ * OMAP3 cpuidle structure definitions
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Written by Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * History:
+ *
+ */
+
+#ifndef ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX
+#define ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX
+
+#define OMAP3_MAX_STATES 7
+#define OMAP3_STATE_C0 0 /* C0 - System executing code */
+#define OMAP3_STATE_C1 1 /* C1 - MPU WFI + Core active */
+#define OMAP3_STATE_C2 2 /* C2 - MPU CSWR + Core active */
+#define OMAP3_STATE_C3 3 /* C3 - MPU OFF + Core active */
+#define OMAP3_STATE_C4 4 /* C4 - MPU RET + Core RET */
+#define OMAP3_STATE_C5 5 /* C5 - MPU OFF + Core RET */
+#define OMAP3_STATE_C6 6 /* C6 - MPU OFF + Core OFF */
+
+extern void omap_sram_idle(void);
+extern int omap3_irq_pending(void);
+
+struct omap3_processor_cx {
+	u8 valid;
+	u8 type;
+	u32 sleep_latency;
+	u32 wakeup_latency;
+	u32 mpu_state;
+	u32 core_state;
+	u32 threshold;
+	u32 flags;
+};
+
+void omap_init_power_states(void);
+int omap3_idle_init(void);
+
+#endif /* ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX */
+

Index: linux-omap-2.6/arch/arm/mach-omap2/pm34xx.c
===================================================================

arch/arm/mach-omap2/pm34xx.c +4 -1

@@ -141,7 +141,7 @@ static irqreturn_t prcm_interrupt_handle
 	return IRQ_HANDLED;
 }
 
-static void omap_sram_idle(void)
+void omap_sram_idle(void)
 {
 	/* Variable to tell what needs to be saved and restored
 	 * in omap_sram_idle*/

@@ -156,6 +156,7 @@ static void omap_sram_idle(void)
 
 	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
 	switch (mpu_next_state) {
+	case PWRDM_POWER_ON:
 	case PWRDM_POWER_RET:
 		/* No need to save context */
 		save_state = 0;

@@ -386,7 +387,9 @@ int __init omap3_pm_init(void)
 
 	prcm_setup_regs();
 
+#ifndef CONFIG_CPU_IDLE
 	pm_idle = omap3_pm_idle;
+#endif
 
 err1:
 	return ret;

Index: linux-omap-2.6/drivers/cpuidle/cpuidle.c
===================================================================

drivers/cpuidle/cpuidle.c +10 -1

@@ -58,6 +58,11 @@ static void cpuidle_idle_call(void)
 		return;
 	}
 
+#ifdef CONFIG_ARCH_OMAP3
+	local_irq_disable();
+	local_fiq_disable();
+#endif
+
 	/* ask the governor for the next state */
 	next_state = cpuidle_curr_governor->select(dev);
 	if (need_resched())

@@ -70,6 +75,11 @@ static void cpuidle_idle_call(void)
 	target_state->time += (unsigned long long)dev->last_residency;
 	target_state->usage++;
 
+#ifdef CONFIG_ARCH_OMAP3
+	local_irq_enable();
+	local_fiq_enable();
+#endif
+
 	/* give the governor an opportunity to reflect on the outcome */
 	if (cpuidle_curr_governor->reflect)
 		cpuidle_curr_governor->reflect(dev);
 
--

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

-----Original Message-----
From: Kevin Hilman [mailto:[email protected]]
Sent: Tuesday, June 17, 2008 4:51 AM
To: Rajendra Nayak
Cc: [email protected]
Subject: Re: [PATCH 01/02] OMAP3 CPUidle driver

"Rajendra Nayak" <[email protected]> writes:

Quoted 3 lines

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

Kevin

Quoted 5 lines

Signed-off-by Rajendra Nayak <[email protected]>

---
 arch/arm/mach-omap2/Makefile      |    2 
 arch/arm/mach-omap2/cpuidle34xx.c |  293 

++++++++++++++++++++++++++++++++++++++

Quoted 8 lines

 arch/arm/mach-omap2/cpuidle34xx.h |   51 ++++++
 arch/arm/mach-omap2/pm34xx.c      |    5 
 drivers/cpuidle/cpuidle.c         |   10 +
 5 files changed, 359 insertions(+), 2 deletions(-)

Index: linux-omap-2.6/arch/arm/mach-omap2/Makefile
===================================================================
--- linux-omap-2.6.orig/arch/arm/mach-omap2/Makefile

2008-06-09 20:15:33.855303920 +0530

Quoted 1 line

+++ linux-omap-2.6/arch/arm/mach-omap2/Makefile

2008-06-09 20:15:39.569121361 +0530

arch/arm/mach-omap2/Makefile obj-y += pm.o · line 23

Quoted 6 lines

arch/arm/mach-omap2/Makefile +1 -1

@@ -20,7 +20,7 @@ obj-y					+= pm.o
 obj-$(CONFIG_ARCH_OMAP2)		+= pm24xx.o
 obj-$(CONFIG_ARCH_OMAP2420)		+= sleep242x.o
 obj-$(CONFIG_ARCH_OMAP2430)		+= sleep243x.o
-obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o
+obj-$(CONFIG_ARCH_OMAP3)		+= pm34xx.o sleep34xx.o 

cpuidle34xx.o

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 7 lines

arch/arm/mach-omap2/Makefile +0 -0

 obj-$(CONFIG_PM_DEBUG)			+= pm-debug.o
 endif
 

Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
===================================================================

arch/arm/mach-omap2/cpuidle34xx.c new file +0 -0

2008-06-10 11:41:27.644820323 +0530

Quoted 9 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +8 -0

@@ -0,0 +1,293 @@
+/*
+ * linux/arch/arm/mach-omap2/cpuidle34xx.c
+ *
+ * OMAP3 CPU IDLE Routines
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Rajendra Nayak <[email protected]>
+ *

arch/arm/mach-omap2/cpuidle34xx.c line 18

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +10 -0

+ * Copyright (C) 2007 Texas Instruments, Inc.
+ * Karthik Dasu <[email protected]>
+ *
+ * Copyright (C) 2006 Nokia Corporation
+ * Tony Lindgren <[email protected]>
+ *
+ * Copyright (C) 2005 Texas Instruments, Inc.
+ * Richard Woodruff <[email protected]>
+ *
+ * This program is free software; you can redistribute it 

and/or modify

arch/arm/mach-omap2/cpuidle34xx.c line 19

Quoted 1 line

arch/arm/mach-omap2/cpuidle34xx.c new file +1 -0

+ * it under the terms of the GNU General Public License 

version 2 as

Quoted 54 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +54 -0

+ * published by the Free Software Foundation.
+ */
+
+#include <linux/cpuidle.h>
+#include <asm/arch/pm.h>
+#include <asm/arch/prcm.h>
+#include <asm/arch/powerdomain.h>
+#include <asm/arch/clockdomain.h>
+#include <asm/arch/irqs.h>
+#include "cpuidle34xx.h"
+
+#ifdef CONFIG_CPU_IDLE
+
+struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
+struct omap3_processor_cx current_cx_state;
+
+static int omap3_idle_bm_check(void)
+{
+	/* Check for omap3_fclks_active() here once available */
+	return 0;
+}
+
+/* omap3_enter_idle - Programs OMAP3 to enter the specified state.
+ * returns the total time during which the system was idle.
+ */
+static int omap3_enter_idle(struct cpuidle_device *dev,
+			struct cpuidle_state *state)
+{
+	struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
+	struct timespec ts_preidle, ts_postidle, ts_idle;
+	struct powerdomain *mpu_pd, *core_pd, *per_pd, *neon_pd;
+	int neon_pwrst;
+
+	current_cx_state = *cx;
+
+	if (cx->type == OMAP3_STATE_C0) {
+		/* Do nothing for C0, not even a wfi */
+		return 0;
+	}
+
+	/* Used to keep track of the total time in idle */
+	getnstimeofday(&ts_preidle);
+
+	mpu_pd = pwrdm_lookup("mpu_pwrdm");
+	core_pd = pwrdm_lookup("core_pwrdm");
+	per_pd = pwrdm_lookup("per_pwrdm");
+	neon_pd = pwrdm_lookup("neon_pwrdm");
+
+	/* Reset previous power state registers */
+	pwrdm_clear_all_prev_pwrst(mpu_pd);
+	pwrdm_clear_all_prev_pwrst(neon_pd);
+	pwrdm_clear_all_prev_pwrst(core_pd);
+	pwrdm_clear_all_prev_pwrst(per_pd);
+

arch/arm/mach-omap2/cpuidle34xx.c line 89

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +10 -0

+	if (omap_irq_pending())
+		return 0;
+
+	neon_pwrst = pwrdm_read_pwrst(neon_pd);
+
+	/* Program MPU/NEON to target state */
+	if (cx->mpu_state < PWRDM_POWER_ON) {
+		if (neon_pwrst == PWRDM_POWER_ON) {
+                        if (cx->mpu_state == PWRDM_POWER_RET)
+                                

pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_RET);

arch/arm/mach-omap2/cpuidle34xx.c line 85

Quoted 2 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +2 -0

+                        else if (cx->mpu_state == PWRDM_POWER_OFF)
+                                

pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_OFF);

Quoted 18 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +18 -0

+		}
+		pwrdm_set_next_pwrst(mpu_pd, cx->mpu_state);
+	}
+
+	/* Program CORE to target state */
+	if (cx->core_state < PWRDM_POWER_ON)
+		pwrdm_set_next_pwrst(core_pd, cx->core_state);
+
+	/* Execute ARM wfi */
+	omap_sram_idle();
+
+	/* Program MPU/NEON to ON */
+	if (cx->mpu_state < PWRDM_POWER_ON) {
+		if (neon_pwrst == PWRDM_POWER_ON)
+			pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_ON);
+		pwrdm_set_next_pwrst(mpu_pd, PWRDM_POWER_ON);
+	}
+

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +10 -0

+	if (cx->core_state < PWRDM_POWER_ON)
+		pwrdm_set_next_pwrst(core_pd, PWRDM_POWER_ON);
+
+	getnstimeofday(&ts_postidle);
+	ts_idle = timespec_sub(ts_postidle, ts_preidle);
+	return timespec_to_ns(&ts_idle);
+}
+
+/*
+ * omap3_enter_idle_bm - enter function for states with 

CPUIDLE_FLAG_CHECK_BM

arch/arm/mach-omap2/cpuidle34xx.c line 115

Quoted 2 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +2 -0

+ *
+ * This function checks for all the pre-requisites needed 

for OMAP3 to enter

arch/arm/mach-omap2/cpuidle34xx.c line 116

Quoted 1 line

arch/arm/mach-omap2/cpuidle34xx.c new file +1 -0

+ * CORE RET/OFF state. It then calls omap3_enter_idle to 

program the desired

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 9 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +9 -0

+ * C state.
+ */
+static int omap3_enter_idle_bm(struct cpuidle_device *dev,
+			       struct cpuidle_state *state)
+{
+	struct cpuidle_state *new_state = NULL;
+	int i, j;
+
+	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && 

omap3_idle_bm_check()) {

Quoted 8 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +8 -0

+
+		/* Find current state in list */
+		for (i = 0; i < OMAP3_MAX_STATES; i++)
+			if (state == &dev->states[i])
+				break;
+		BUG_ON(i == OMAP3_MAX_STATES);
+
+		/* Back up to non 'CHECK_BM' state */

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +10 -0

+		for (j = i - 1;  j > 0; j--) {
+			struct cpuidle_state *s = &dev->states[j];
+
+			if (!(s->flags & CPUIDLE_FLAG_CHECK_BM)) {
+				new_state = s;
+				break;
+			}
+		}
+
+		pr_debug("%s: Bus activity: Entering %s 

(instead of %s)\n",

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 9 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +9 -0

+			__FUNCTION__, new_state->name, state->name);
+	}
+
+	return omap3_enter_idle(dev, new_state ? : state);
+}
+
+DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
+
+/* omap3_init_power_states - Initialises the OMAP3 

specific C states.

Quoted 109 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +109 -0

+ * Below is the desciption of each C state.
+ *
+	C0 . System executing code
+	C1 . MPU WFI + Core active
+	C2 . MPU CSWR + Core active
+	C3 . MPU OFF + Core active
+	C4 . MPU CSWR + Core CSWR
+	C5 . MPU OFF + Core CSWR
+	C6 . MPU OFF + Core OFF
+ */
+void omap_init_power_states(void)
+{
+	/* C0 . System executing code */
+	omap3_power_states[0].valid = 1;
+	omap3_power_states[0].type = OMAP3_STATE_C0;
+	omap3_power_states[0].sleep_latency = 0;
+	omap3_power_states[0].wakeup_latency = 0;
+	omap3_power_states[0].threshold = 0;
+	omap3_power_states[0].mpu_state = PWRDM_POWER_ON;
+	omap3_power_states[0].core_state = PWRDM_POWER_ON;
+	omap3_power_states[0].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_SHALLOW;
+
+	/* C1 . MPU WFI + Core active */
+	omap3_power_states[1].valid = 1;
+	omap3_power_states[1].type = OMAP3_STATE_C1;
+	omap3_power_states[1].sleep_latency = 10;
+	omap3_power_states[1].wakeup_latency = 10;
+	omap3_power_states[1].threshold = 30;
+	omap3_power_states[1].mpu_state = PWRDM_POWER_ON;
+	omap3_power_states[1].core_state = PWRDM_POWER_ON;
+	omap3_power_states[1].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_SHALLOW;
+
+	/* C2 . MPU CSWR + Core active */
+	omap3_power_states[2].valid = 1;
+	omap3_power_states[2].type = OMAP3_STATE_C2;
+	omap3_power_states[2].sleep_latency = 50;
+	omap3_power_states[2].wakeup_latency = 50;
+	omap3_power_states[2].threshold = 300;
+	omap3_power_states[2].mpu_state = PWRDM_POWER_RET;
+	omap3_power_states[2].core_state = PWRDM_POWER_ON;
+	omap3_power_states[2].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_BALANCED;
+
+	/* C3 . MPU OFF + Core active */
+	omap3_power_states[3].valid = 0;
+	omap3_power_states[3].type = OMAP3_STATE_C3;
+	omap3_power_states[3].sleep_latency = 1500;
+	omap3_power_states[3].wakeup_latency = 1800;
+	omap3_power_states[3].threshold = 4000;
+	omap3_power_states[3].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[3].core_state = PWRDM_POWER_RET;
+	omap3_power_states[3].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_BALANCED;
+
+	/* C4 . MPU CSWR + Core CSWR*/
+	omap3_power_states[4].valid = 1;
+	omap3_power_states[4].type = OMAP3_STATE_C4;
+	omap3_power_states[4].sleep_latency = 2500;
+	omap3_power_states[4].wakeup_latency = 7500;
+	omap3_power_states[4].threshold = 12000;
+	omap3_power_states[4].mpu_state = PWRDM_POWER_RET;
+	omap3_power_states[4].core_state = PWRDM_POWER_RET;
+	omap3_power_states[4].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
+
+	/* C5 . MPU OFF + Core CSWR */
+	omap3_power_states[5].valid = 0;
+	omap3_power_states[5].type = OMAP3_STATE_C5;
+	omap3_power_states[5].sleep_latency = 3000;
+	omap3_power_states[5].wakeup_latency = 8500;
+	omap3_power_states[5].threshold = 15000;
+	omap3_power_states[5].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[5].core_state = PWRDM_POWER_RET;
+	omap3_power_states[5].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
+
+	/* C6 . MPU OFF + Core OFF */
+	omap3_power_states[6].valid = 0;
+	omap3_power_states[6].type = OMAP3_STATE_C6;
+	omap3_power_states[6].sleep_latency = 10000;
+	omap3_power_states[6].wakeup_latency = 30000;
+	omap3_power_states[6].threshold = 300000;
+	omap3_power_states[6].mpu_state = PWRDM_POWER_OFF;
+	omap3_power_states[6].core_state = PWRDM_POWER_OFF;
+	omap3_power_states[6].flags = CPUIDLE_FLAG_TIME_VALID |
+			CPUIDLE_FLAG_DEEP | CPUIDLE_FLAG_CHECK_BM;
+}
+
+struct cpuidle_driver omap3_idle_driver = {
+	.name = 	"omap3_idle",
+	.owner = 	THIS_MODULE,
+};
+/*
+ * omap3_idle_init - Init routine for OMAP3 idle.
+ * Registers the OMAP3 specific cpuidle driver with the cpuidle f/w
+ * with the valid set of states.
+ */
+int omap3_idle_init(void)
+{
+	int i, count = 0;
+	struct omap3_processor_cx *cx;
+	struct cpuidle_state *state;
+	struct cpuidle_device *dev;
+
+	omap_init_power_states();
+	cpuidle_register_driver(&omap3_idle_driver);
+

arch/arm/mach-omap2/cpuidle34xx.c

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +10 -0

+	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
+
+	for (i = 0; i < OMAP3_MAX_STATES; i++) {
+		cx = &omap3_power_states[i];
+		state = &dev->states[count];
+
+		if (!cx->valid)
+			continue;
+		cpuidle_set_statedata(state, cx);
+		state->exit_latency = cx->sleep_latency + 

cx->wakeup_latency;

arch/arm/mach-omap2/cpuidle34xx.h

Quoted 26 lines

arch/arm/mach-omap2/cpuidle34xx.c new file +22 -0

+		state->target_residency = cx->threshold;
+		state->flags = cx->flags;
+		state->enter = (state->flags & CPUIDLE_FLAG_CHECK_BM) ?
+			omap3_enter_idle_bm : omap3_enter_idle;
+		sprintf(state->name, "C%d", count+1);
+		count++;
+	}
+
+	if (!count)
+		return -EINVAL;
+	dev->state_count = count;
+
+	if (cpuidle_register_device(dev)) {
+		printk(KERN_ERR "%s: CPUidle register device failed\n",
+		       __FUNCTION__);
+		return -EIO;
+	}
+
+	return 0;
+}
+__initcall(omap3_idle_init);
+#endif /* CONFIG_CPU_IDLE */

Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.h
===================================================================

arch/arm/mach-omap2/cpuidle34xx.h new file +0 -0

2008-06-09 20:15:39.569121361 +0530

arch/arm/mach-omap2/cpuidle34xx.h line 9

Quoted 10 lines

arch/arm/mach-omap2/cpuidle34xx.h new file +9 -0

@@ -0,0 +1,51 @@
+/*
+ * linux/arch/arm/mach-omap2/cpuidle34xx.h
+ *
+ * OMAP3 cpuidle structure definitions
+ *
+ * Copyright (C) 2007-2008 Texas Instruments, Inc.
+ * Written by Rajendra Nayak <[email protected]>
+ *
+ * This program is free software; you can redistribute it 

and/or modify

arch/arm/mach-omap2/cpuidle34xx.h line 10

Quoted 1 line

arch/arm/mach-omap2/cpuidle34xx.h new file +1 -0

+ * it under the terms of the GNU General Public License 

version 2 as

arch/arm/mach-omap2/cpuidle34xx.h

Quoted 5 lines

arch/arm/mach-omap2/cpuidle34xx.h new file +5 -0

+ * published by the Free Software Foundation.
+ *
+ * THIS PACKAGE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED
+ * WARRANTIES OF MERCHANTIBILITY AND FITNESS FOR A 

PARTICULAR PURPOSE.

Quoted 36 lines

arch/arm/mach-omap2/cpuidle34xx.h new file +36 -0

+ *
+ * History:
+ *
+ */
+
+#ifndef ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX
+#define ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX
+
+#define OMAP3_MAX_STATES 7
+#define OMAP3_STATE_C0 0 /* C0 - System executing code */
+#define OMAP3_STATE_C1 1 /* C1 - MPU WFI + Core active */
+#define OMAP3_STATE_C2 2 /* C2 - MPU CSWR + Core active */
+#define OMAP3_STATE_C3 3 /* C3 - MPU OFF + Core active */
+#define OMAP3_STATE_C4 4 /* C4 - MPU RET + Core RET */
+#define OMAP3_STATE_C5 5 /* C5 - MPU OFF + Core RET */
+#define OMAP3_STATE_C6 6 /* C6 - MPU OFF + Core OFF */
+
+extern void omap_sram_idle(void);
+extern int omap3_irq_pending(void);
+
+struct omap3_processor_cx {
+	u8 valid;
+	u8 type;
+	u32 sleep_latency;
+	u32 wakeup_latency;
+	u32 mpu_state;
+	u32 core_state;
+	u32 threshold;
+	u32 flags;
+};
+
+void omap_init_power_states(void);
+int omap3_idle_init(void);
+
+#endif /* ARCH_ARM_MACH_OMAP2_CPUIDLE_34XX */
+

Quoted 3 lines

Index: linux-omap-2.6/arch/arm/mach-omap2/pm34xx.c
===================================================================
--- linux-omap-2.6.orig/arch/arm/mach-omap2/pm34xx.c

2008-06-09 20:15:33.855303920 +0530

Quoted 1 line

+++ linux-omap-2.6/arch/arm/mach-omap2/pm34xx.c

2008-06-09 20:16:20.976798343 +0530

Quoted 27 lines

arch/arm/mach-omap2/cpuidle34xx.h new file +4 -1

@@ -141,7 +141,7 @@ static irqreturn_t prcm_interrupt_handle
 	return IRQ_HANDLED;
 }
 
-static void omap_sram_idle(void)
+void omap_sram_idle(void)
 {
 	/* Variable to tell what needs to be saved and restored
 	 * in omap_sram_idle*/

@@ -156,6 +156,7 @@ static void omap_sram_idle(void)
 
 	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
 	switch (mpu_next_state) {
+	case PWRDM_POWER_ON:
 	case PWRDM_POWER_RET:
 		/* No need to save context */
 		save_state = 0;

@@ -386,7 +387,9 @@ int __init omap3_pm_init(void)
 
 	prcm_setup_regs();
 
+#ifndef CONFIG_CPU_IDLE
 	pm_idle = omap3_pm_idle;
+#endif
 
 err1:
 	return ret;

Quoted 3 lines

Index: linux-omap-2.6/drivers/cpuidle/cpuidle.c
===================================================================
--- linux-omap-2.6.orig/drivers/cpuidle/cpuidle.c

2008-06-09 20:15:33.856303888 +0530

Quoted 1 line

+++ linux-omap-2.6/drivers/cpuidle/cpuidle.c

2008-06-09 20:15:39.570121329 +0530

Quoted 26 lines

arch/arm/mach-omap2/cpuidle34xx.h new file +10 -1

@@ -58,6 +58,11 @@ static void cpuidle_idle_call(void)
 		return;
 	}
 
+#ifdef CONFIG_ARCH_OMAP3
+	local_irq_disable();
+	local_fiq_disable();
+#endif
+
 	/* ask the governor for the next state */
 	next_state = cpuidle_curr_governor->select(dev);
 	if (need_resched())

@@ -70,6 +75,11 @@ static void cpuidle_idle_call(void)
 	target_state->time += (unsigned long long)dev->last_residency;
 	target_state->usage++;
 
+#ifdef CONFIG_ARCH_OMAP3
+	local_irq_enable();
+	local_fiq_enable();
+#endif
+
 	/* give the governor an opportunity to reflect on the outcome */
 	if (cpuidle_curr_governor->reflect)
 		cpuidle_curr_governor->reflect(dev);
 
--

Quoted 1 line

To unsubscribe from this list: send the line "unsubscribe

linux-omap" in

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

An interrupt in between could end up with a previously selected state to be programmed.

Any suggestions on a better way to handle this?

Quoted 6 lines · Rajendra Nayak

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 6 lines

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

I understand that, but I still don't understand exactly what you're trying to prevent. Did you have a specific bug that this prevented?

Quoted 2 lines

An interrupt in between could end up with a previously selected state to be programmed.

Remember that this function _is_ the idle loop, meaning when this runs nothing else is happening. After the select, if other system activity has happened (e.g. and interrupt, or thread wakeup etc.), it will run before the target_state->enter() because of the check for need_resched().

Quoted 2 lines

Any suggestions on a better way to handle this?

Just drop the IRQ/FIQ disables altogether.

Quoted 18 lines · ext Kevin Hilman

Quoted 6 lines · Rajendra Nayak

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 6 lines

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

I understand that, but I still don't understand exactly what you're trying to prevent. Did you have a specific bug that this prevented?

Quoted 8 lines

Quoted 2 lines

An interrupt in between could end up with a previously selected state to be programmed.

Remember that this function _is_ the idle loop, meaning when this runs nothing else is happening. After the select, if other system activity has happened (e.g. and interrupt, or thread wakeup etc.), it will run before the target_state->enter() because of the check for need_resched().

What happens if this interrupt, or thread wakeup causes change on latency requirements? Then we are entering sleep state which was selected using wrong latency requirement data.

Quoted 4 lines

Quoted 2 lines

Any suggestions on a better way to handle this?

Just drop the IRQ/FIQ disables altogether.

At least these are needed at some point in idle loop. Otherwise we might stepout from idle and sram in a point where it is not acceptable.

Quoted 29 lines · [email protected] (Högander Jouni) writes:

Quoted 26 lines · ext Kevin Hilman

Quoted 6 lines · Rajendra Nayak

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 6 lines

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

I understand that, but I still don't understand exactly what you're trying to prevent. Did you have a specific bug that this prevented?

Quoted 2 lines

An interrupt in between could end up with a previously selected state to be programmed.

Remember that this function _is_ the idle loop, meaning when this runs nothing else is happening. After the select, if other system activity has happened (e.g. and interrupt, or thread wakeup etc.), it will run before the target_state->enter() because of the check for need_resched().

Quoted 3 lines

What happens if this interrupt, or thread wakeup causes change on latency requirements? Then we are entering sleep state which was selected using wrong latency requirement data.

If a thread is awoken by an interrupt, then need_resched() will be true, and the idle loop will exit without trying to enter the idle state.

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Any suggestions on a better way to handle this?

Just drop the IRQ/FIQ disables altogether.

At least these are needed at some point in idle loop. Otherwise we might stepout from idle and sram in a point where it is not acceptable.

Agreed, interrupt enable/disable is needed in the idle loop, but not in the CPUidle code.

The current OMAP idle loop code (omap2_pm_idle) already does this, but the CPUidle "enter state" hook does not.

The omap3_enter_idle() function should be the one who enables/disables interrupts. At a minimulm, the omap_sram_idle should be called with interrupts disabled.

Quoted 31 lines · ext Kevin Hilman

Quoted 29 lines · [email protected] (Högander Jouni) writes:

Quoted 26 lines · ext Kevin Hilman

Quoted 6 lines · Rajendra Nayak

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 6 lines

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

I understand that, but I still don't understand exactly what you're trying to prevent. Did you have a specific bug that this prevented?

Quoted 2 lines

An interrupt in between could end up with a previously selected state to be programmed.

Remember that this function _is_ the idle loop, meaning when this runs nothing else is happening. After the select, if other system activity has happened (e.g. and interrupt, or thread wakeup etc.), it will run before the target_state->enter() because of the check for need_resched().

Quoted 7 lines

Quoted 3 lines

What happens if this interrupt, or thread wakeup causes change on latency requirements? Then we are entering sleep state which was selected using wrong latency requirement data.

If a thread is awoken by an interrupt, then need_resched() will be true, and the idle loop will exit without trying to enter the idle state.

Even in that case there is still period in idle loop, were it is possible that latency requirement changes and cpuidle doesn't know this on frist enter state. This is after need_resched.

I don't know this area too good, but isn't it also possible that interrupt which is handled, has caused latency requirement change and there is no NEED_RESCHED flag set?

Quoted 15 lines

Quoted 8 lines

Quoted 5 lines

Quoted 3 lines

Any suggestions on a better way to handle this?

Just drop the IRQ/FIQ disables altogether.

At least these are needed at some point in idle loop. Otherwise we might stepout from idle and sram in a point where it is not acceptable.

Agreed, interrupt enable/disable is needed in the idle loop, but not in the CPUidle code.

The current OMAP idle loop code (omap2_pm_idle) already does this, but the CPUidle "enter state" hook does not.

Quoted 3 lines

The omap3_enter_idle() function should be the one who enables/disables interrupts. At a minimulm, the omap_sram_idle should be called with interrupts disabled.

If done this way, it should be checked that target state is still valid before entering omap_sram_idle. How could CPUidle "enter state" know wether it is still valid?

Quoted 45 lines · [email protected] (Högander Jouni) writes:

Quoted 31 lines · ext Kevin Hilman

Quoted 29 lines · [email protected] (Högander Jouni) writes:

Quoted 26 lines · ext Kevin Hilman

Quoted 6 lines · Rajendra Nayak

Quoted 6 lines

Quoted 3 lines · Rajendra Nayak

This patch adds the OMAP3 cpuidle driver. Irq enable/disable is done in the core cpuidle driver before it queries the governor for the next state.

Quoted 6 lines

Quoted 3 lines

Can you explain why you need the IRQ/FIQ disable added to cpuidle_idle_call()

This was done to prevent any interrupts firing in between a cpuidle_curr_governor->select() and target_state->enter().

I understand that, but I still don't understand exactly what you're trying to prevent. Did you have a specific bug that this prevented?

Quoted 2 lines

An interrupt in between could end up with a previously selected state to be programmed.

Remember that this function _is_ the idle loop, meaning when this runs nothing else is happening. After the select, if other system activity has happened (e.g. and interrupt, or thread wakeup etc.), it will run before the target_state->enter() because of the check for need_resched().

Quoted 7 lines

Quoted 3 lines

What happens if this interrupt, or thread wakeup causes change on latency requirements? Then we are entering sleep state which was selected using wrong latency requirement data.

If a thread is awoken by an interrupt, then need_resched() will be true, and the idle loop will exit without trying to enter the idle state.

Even in that case there is still period in idle loop, were it is possible that latency requirement changes and cpuidle doesn't know this on frist enter state. This is after need_resched.

Quoted 3 lines

I don't know this area too good, but isn't it also possible that interrupt which is handled, has caused latency requirement change and there is no NEED_RESCHED flag set?

IMHO, these things should be handled on a per-driver/subsystem basis, not by a global interrupt lock.

Remember that this patchset is still missing a very important piece which is the omap3_idle_bm_check(). This is an of how these special cases are handled.

For example, if an interrupt fires which triggers driver activity, this function is supposed to look for clock activity and potentially adjust the depth of sleep that can be entered.

Quoted 17 lines

Quoted 17 lines

Quoted 10 lines

Quoted 7 lines

Quoted 5 lines

Any suggestions on a better way to handle this?

Just drop the IRQ/FIQ disables altogether.

At least these are needed at some point in idle loop. Otherwise we might stepout from idle and sram in a point where it is not acceptable.

Agreed, interrupt enable/disable is needed in the idle loop, but not in the CPUidle code.

The current OMAP idle loop code (omap2_pm_idle) already does this, but the CPUidle "enter state" hook does not.

Quoted 8 lines

Quoted 3 lines

The omap3_enter_idle() function should be the one who enables/disables interrupts. At a minimulm, the omap_sram_idle should be called with interrupts disabled.

If done this way, it should be checked that target state is still valid before entering omap_sram_idle. How could CPUidle "enter state" know wether it is still valid?

Again, this is the job of the OMAP specific idle loop, not the global CPUidle code.

For example, the current idle loop simply does:

	if (omap_irq_pending())
		goto out;

so it doesn't enter idle if there's an interrupt pending.

So, I'm not arguing that your concerns aren't valid, because indeed they are valid corner cases. I'm just arguing that these special cases should be handled in the OMAP code, not the global CPUidle code.

Kevin