[PATCH 0/5] OMAP: cpuidle code clean-up

[PATCH 0/5] OMAP: cpuidle code clean-up

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

Rework the cpuidle code:
- optimize the cpuidle C-states data registration and storage,
- change the interaction with the debugfs 'enable_off_mode' knob
 and the use of the C-states 'valid' internal field,
- remove dead code,
- improve code readability.

Tested on Beagleboard B5 with cpuidle in RET and OFF modes.

Another 161 lines of OMAP code gone ;p

Notes:
1) the debugfs 'enable_off_mode' knob will be deprecated by the use
 of the devices constraints framework to restrict the power domains
 power modes.
2) the MPU and CORE power domains low power modes are controlled
 by cpuidle, based on the allowed overall sleep+wake-up latencies
 and the wake-up latency constraints on the MPU. This is incorrect.
 The devices constraints framework shall be used instead to control
 all power domains.

ToDo:
- integrate cpuidle with the devices constraints framework, when merged in,
- refine the latency figures and express them in term of available data
 from other frameworks (OMAP PM, constaints framework, omap_devices,
 new VC/VP voltage and DVFS code ...),

Jean Pihet (5):
  OMAP3 cpuidle: remove useless SDP specific timings
  OMAP3: clean-up mach specific cpuidle data structures
  OMAP3: cpuidle: re-organize the C-states data
  OMAP3: cpuidle: code rework for improved readability
  OMAP3: cpuidle: change the power domains modes determination logic

 arch/arm/mach-omap2/board-3430sdp.c |   19 --
 arch/arm/mach-omap2/board-rx51.c    |   15 +-
 arch/arm/mach-omap2/cpuidle34xx.c   |  424 ++++++++++++-----------------------
 arch/arm/mach-omap2/pm.h            |   17 +-
 arch/arm/mach-omap2/pm34xx.c        |   12 -
 5 files changed, 163 insertions(+), 324 deletions(-)

[PATCH 1/5] OMAP3 cpuidle: remove useless SDP specific timings

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

The cpuidle states settings can be overriden by some board-
specific settings, by calling omap3_pm_init_cpuidle.
Remove the 3430SDP specific states settings registration
since the figures are identical to the default ones (in cpuidle34xx.c).

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/board-3430sdp.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index 9afd087..7ffad7b 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -59,24 +59,6 @@
 
 #define TWL4030_MSECURE_GPIO 22
 
-/* FIXME: These values need to be updated based on more profiling on 3430sdp*/
-static struct cpuidle_params omap3_cpuidle_params_table[] = {
-	/* C1 */
-	{1, 2, 2, 5},
-	/* C2 */
-	{1, 10, 10, 30},
-	/* C3 */
-	{1, 50, 50, 300},
-	/* C4 */
-	{1, 1500, 1800, 4000},
-	/* C5 */
-	{1, 2500, 7500, 12000},
-	/* C6 */
-	{1, 3000, 8500, 15000},
-	/* C7 */
-	{1, 10000, 30000, 300000},
-};
-
 static uint32_t board_keymap[] = {
 	KEY(0, 0, KEY_LEFT),
 	KEY(0, 1, KEY_RIGHT),
@@ -883,7 +865,6 @@ static void __init omap_3430sdp_init(void)
 	omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
 	omap_board_config = sdp3430_config;
 	omap_board_config_size = ARRAY_SIZE(sdp3430_config);
-	omap3_pm_init_cpuidle(omap3_cpuidle_params_table);
 	omap3430_i2c_init();
 	omap_display_init(&sdp3430_dss_data);
 	if (omap_rev() > OMAP3430_REV_ES1_0)
-- 
1.7.1

[PATCH 1/5] OMAP3 cpuidle: remove useless SDP specific timings

[Santosh Shilimkar]

On 4/29/2011 2:56 PM, jean.pihet at newoldbits.com wrote:
> From: Jean Pihet<j-pihet@ti.com>
>
> The cpuidle states settings can be overriden by some board-
> specific settings, by calling omap3_pm_init_cpuidle.
> Remove the 3430SDP specific states settings registration
> since the figures are identical to the default ones (in cpuidle34xx.c).
>
> Signed-off-by: Jean Pihet<j-pihet@ti.com>
> ---
>   arch/arm/mach-omap2/board-3430sdp.c |   19 -------------------
>   1 files changed, 0 insertions(+), 19 deletions(-)
>
Looks like the example table to show the API usage never got
updated with board specific timings :)

-19 lines more :)

Regards
Santosh

[PATCH 1/5] OMAP3 cpuidle: remove useless SDP specific timings

[Kevin Hilman]

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> The cpuidle states settings can be overriden by some board-
> specific settings, by calling omap3_pm_init_cpuidle.
> Remove the 3430SDP specific states settings registration
> since the figures are identical to the default ones (in cpuidle34xx.c).
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

Thanks,  queuing for 2.6.40 (branch: for_2.6.40/pm-cleanup)

Kevin

> ---
>  arch/arm/mach-omap2/board-3430sdp.c |   19 -------------------
>  1 files changed, 0 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
> index 9afd087..7ffad7b 100644
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -59,24 +59,6 @@
>  
>  #define TWL4030_MSECURE_GPIO 22
>  
> -/* FIXME: These values need to be updated based on more profiling on 3430sdp*/
> -static struct cpuidle_params omap3_cpuidle_params_table[] = {
> -	/* C1 */
> -	{1, 2, 2, 5},
> -	/* C2 */
> -	{1, 10, 10, 30},
> -	/* C3 */
> -	{1, 50, 50, 300},
> -	/* C4 */
> -	{1, 1500, 1800, 4000},
> -	/* C5 */
> -	{1, 2500, 7500, 12000},
> -	/* C6 */
> -	{1, 3000, 8500, 15000},
> -	/* C7 */
> -	{1, 10000, 30000, 300000},
> -};
> -
>  static uint32_t board_keymap[] = {
>  	KEY(0, 0, KEY_LEFT),
>  	KEY(0, 1, KEY_RIGHT),
> @@ -883,7 +865,6 @@ static void __init omap_3430sdp_init(void)
>  	omap3_mux_init(board_mux, OMAP_PACKAGE_CBB);
>  	omap_board_config = sdp3430_config;
>  	omap_board_config_size = ARRAY_SIZE(sdp3430_config);
> -	omap3_pm_init_cpuidle(omap3_cpuidle_params_table);
>  	omap3430_i2c_init();
>  	omap_display_init(&sdp3430_dss_data);
>  	if (omap_rev() > OMAP3430_REV_ES1_0)

[PATCH 2/5] OMAP3: clean-up mach specific cpuidle data structures

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

- sleep_latency and wake_latency are not used, replace them by
  exit_latency which is used by cpuidle. exit_latency simply is
  the sum of sleep_latency and wake_latency,
- replace threshold by target_residency,
- changed the OMAP3 specific cpuidle code accordingly,
- changed the OMAP3 board code accordingly.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/board-rx51.c  |   15 +++---
 arch/arm/mach-omap2/cpuidle34xx.c |  103 +++++++++++++++---------------------
 arch/arm/mach-omap2/pm.h          |   13 +++--
 3 files changed, 60 insertions(+), 71 deletions(-)

diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index f8ba20a..44656e8 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -58,21 +58,22 @@ static struct platform_device leds_gpio = {
 	},
 };
 
+/* cpuidle C-states definition override from the default values */
 static struct cpuidle_params rx51_cpuidle_params[] = {
 	/* C1 */
-	{1, 110, 162, 5},
+	{110 + 162, 5 , 1},
 	/* C2 */
-	{1, 106, 180, 309},
+	{106 + 180, 309, 1},
 	/* C3 */
-	{0, 107, 410, 46057},
+	{107 + 410, 46057, 0},
 	/* C4 */
-	{0, 121, 3374, 46057},
+	{121 + 3374, 46057, 0},
 	/* C5 */
-	{1, 855, 1146, 46057},
+	{855 + 1146, 46057, 1},
 	/* C6 */
-	{0, 7580, 4134, 484329},
+	{7580 + 4134, 484329, 0},
 	/* C7 */
-	{1, 7505, 15274, 484329},
+	{7505 + 15274, 484329, 1},
 };
 
 static struct omap_lcd_config rx51_lcd_config = {
diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1c240ef..d7bc31a 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -52,11 +52,10 @@
 struct omap3_processor_cx {
 	u8 valid;
 	u8 type;
-	u32 sleep_latency;
-	u32 wakeup_latency;
+	u32 exit_latency;
 	u32 mpu_state;
 	u32 core_state;
-	u32 threshold;
+	u32 target_residency;
 	u32 flags;
 	const char *desc;
 };
@@ -75,19 +74,19 @@ struct powerdomain *cam_pd;
  */
 static struct cpuidle_params cpuidle_params_table[] = {
 	/* C1 */
-	{1, 2, 2, 5},
+	{2 + 2, 5, 1},
 	/* C2 */
-	{1, 10, 10, 30},
+	{10 + 10, 30, 1},
 	/* C3 */
-	{1, 50, 50, 300},
+	{50 + 50, 300, 1},
 	/* C4 */
-	{1, 1500, 1800, 4000},
+	{1500 + 1800, 4000, 1},
 	/* C5 */
-	{1, 2500, 7500, 12000},
+	{2500 + 7500, 12000, 1},
 	/* C6 */
-	{1, 3000, 8500, 15000},
+	{3000 + 8500, 15000, 1},
 	/* C7 */
-	{1, 10000, 30000, 300000},
+	{10000 + 30000, 300000, 1},
 };
 
 static int omap3_idle_bm_check(void)
@@ -330,12 +329,10 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
 		cpuidle_params_table[i].valid =
 			cpuidle_board_params[i].valid;
-		cpuidle_params_table[i].sleep_latency =
-			cpuidle_board_params[i].sleep_latency;
-		cpuidle_params_table[i].wake_latency =
-			cpuidle_board_params[i].wake_latency;
-		cpuidle_params_table[i].threshold =
-			cpuidle_board_params[i].threshold;
+		cpuidle_params_table[i].exit_latency =
+			cpuidle_board_params[i].exit_latency;
+		cpuidle_params_table[i].target_residency =
+			cpuidle_board_params[i].target_residency;
 	}
 	return;
 }
@@ -357,12 +354,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C1].valid =
 			cpuidle_params_table[OMAP3_STATE_C1].valid;
 	omap3_power_states[OMAP3_STATE_C1].type = OMAP3_STATE_C1;
-	omap3_power_states[OMAP3_STATE_C1].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C1].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C1].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C1].wake_latency;
-	omap3_power_states[OMAP3_STATE_C1].threshold =
-			cpuidle_params_table[OMAP3_STATE_C1].threshold;
+	omap3_power_states[OMAP3_STATE_C1].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C1].exit_latency;
+	omap3_power_states[OMAP3_STATE_C1].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C1].target_residency;
 	omap3_power_states[OMAP3_STATE_C1].mpu_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C1].core_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C1].flags = CPUIDLE_FLAG_TIME_VALID;
@@ -372,12 +367,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C2].valid =
 			cpuidle_params_table[OMAP3_STATE_C2].valid;
 	omap3_power_states[OMAP3_STATE_C2].type = OMAP3_STATE_C2;
-	omap3_power_states[OMAP3_STATE_C2].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C2].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C2].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C2].wake_latency;
-	omap3_power_states[OMAP3_STATE_C2].threshold =
-			cpuidle_params_table[OMAP3_STATE_C2].threshold;
+	omap3_power_states[OMAP3_STATE_C2].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C2].exit_latency;
+	omap3_power_states[OMAP3_STATE_C2].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C2].target_residency;
 	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -388,12 +381,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C3].valid =
 			cpuidle_params_table[OMAP3_STATE_C3].valid;
 	omap3_power_states[OMAP3_STATE_C3].type = OMAP3_STATE_C3;
-	omap3_power_states[OMAP3_STATE_C3].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C3].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C3].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C3].wake_latency;
-	omap3_power_states[OMAP3_STATE_C3].threshold =
-			cpuidle_params_table[OMAP3_STATE_C3].threshold;
+	omap3_power_states[OMAP3_STATE_C3].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C3].exit_latency;
+	omap3_power_states[OMAP3_STATE_C3].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C3].target_residency;
 	omap3_power_states[OMAP3_STATE_C3].mpu_state = PWRDM_POWER_RET;
 	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C3].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -404,12 +395,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C4].valid =
 			cpuidle_params_table[OMAP3_STATE_C4].valid;
 	omap3_power_states[OMAP3_STATE_C4].type = OMAP3_STATE_C4;
-	omap3_power_states[OMAP3_STATE_C4].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C4].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C4].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C4].wake_latency;
-	omap3_power_states[OMAP3_STATE_C4].threshold =
-			cpuidle_params_table[OMAP3_STATE_C4].threshold;
+	omap3_power_states[OMAP3_STATE_C4].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C4].exit_latency;
+	omap3_power_states[OMAP3_STATE_C4].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C4].target_residency;
 	omap3_power_states[OMAP3_STATE_C4].mpu_state = PWRDM_POWER_OFF;
 	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_ON;
 	omap3_power_states[OMAP3_STATE_C4].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -420,12 +409,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C5].valid =
 			cpuidle_params_table[OMAP3_STATE_C5].valid;
 	omap3_power_states[OMAP3_STATE_C5].type = OMAP3_STATE_C5;
-	omap3_power_states[OMAP3_STATE_C5].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C5].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C5].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C5].wake_latency;
-	omap3_power_states[OMAP3_STATE_C5].threshold =
-			cpuidle_params_table[OMAP3_STATE_C5].threshold;
+	omap3_power_states[OMAP3_STATE_C5].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C5].exit_latency;
+	omap3_power_states[OMAP3_STATE_C5].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C5].target_residency;
 	omap3_power_states[OMAP3_STATE_C5].mpu_state = PWRDM_POWER_RET;
 	omap3_power_states[OMAP3_STATE_C5].core_state = PWRDM_POWER_RET;
 	omap3_power_states[OMAP3_STATE_C5].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -436,12 +423,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C6].valid =
 			cpuidle_params_table[OMAP3_STATE_C6].valid;
 	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
-	omap3_power_states[OMAP3_STATE_C6].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C6].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C6].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C6].wake_latency;
-	omap3_power_states[OMAP3_STATE_C6].threshold =
-			cpuidle_params_table[OMAP3_STATE_C6].threshold;
+	omap3_power_states[OMAP3_STATE_C6].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C6].exit_latency;
+	omap3_power_states[OMAP3_STATE_C6].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C6].target_residency;
 	omap3_power_states[OMAP3_STATE_C6].mpu_state = PWRDM_POWER_OFF;
 	omap3_power_states[OMAP3_STATE_C6].core_state = PWRDM_POWER_RET;
 	omap3_power_states[OMAP3_STATE_C6].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -452,12 +437,10 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C7].valid =
 			cpuidle_params_table[OMAP3_STATE_C7].valid;
 	omap3_power_states[OMAP3_STATE_C7].type = OMAP3_STATE_C7;
-	omap3_power_states[OMAP3_STATE_C7].sleep_latency =
-			cpuidle_params_table[OMAP3_STATE_C7].sleep_latency;
-	omap3_power_states[OMAP3_STATE_C7].wakeup_latency =
-			cpuidle_params_table[OMAP3_STATE_C7].wake_latency;
-	omap3_power_states[OMAP3_STATE_C7].threshold =
-			cpuidle_params_table[OMAP3_STATE_C7].threshold;
+	omap3_power_states[OMAP3_STATE_C7].exit_latency =
+			cpuidle_params_table[OMAP3_STATE_C7].exit_latency;
+	omap3_power_states[OMAP3_STATE_C7].target_residency =
+			cpuidle_params_table[OMAP3_STATE_C7].target_residency;
 	omap3_power_states[OMAP3_STATE_C7].mpu_state = PWRDM_POWER_OFF;
 	omap3_power_states[OMAP3_STATE_C7].core_state = PWRDM_POWER_OFF;
 	omap3_power_states[OMAP3_STATE_C7].flags = CPUIDLE_FLAG_TIME_VALID |
@@ -512,8 +495,8 @@ int __init omap3_idle_init(void)
 		if (!cx->valid)
 			continue;
 		cpuidle_set_statedata(state, cx);
-		state->exit_latency = cx->sleep_latency + cx->wakeup_latency;
-		state->target_residency = cx->threshold;
+		state->exit_latency = cx->exit_latency;
+		state->target_residency = cx->target_residency;
 		state->flags = cx->flags;
 		state->enter = (state->flags & CPUIDLE_FLAG_CHECK_BM) ?
 			omap3_enter_idle_bm : omap3_enter_idle;
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 797bfd1..32dbc13 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -36,11 +36,16 @@ static inline int omap4_opp_init(void)
 }
 #endif
 
+/*
+ * cpuidle mach specific parameters
+ *
+ * The board code can override the default C-states definition using
+ * omap3_pm_init_cpuidle
+ */
 struct cpuidle_params {
-	u8  valid;
-	u32 sleep_latency;
-	u32 wake_latency;
-	u32 threshold;
+	u32 exit_latency;	/* exit_latency = sleep + wake-up latencies */
+	u32 target_residency;
+	u8 valid;		/* validates the C-state */
 };
 
 #if defined(CONFIG_PM) && defined(CONFIG_CPU_IDLE)
-- 
1.7.1

[PATCH 2/5] OMAP3: clean-up mach specific cpuidle data structures

[Kevin Hilman]

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> - sleep_latency and wake_latency are not used, replace them by
>   exit_latency which is used by cpuidle. exit_latency simply is
>   the sum of sleep_latency and wake_latency,
> - replace threshold by target_residency,
> - changed the OMAP3 specific cpuidle code accordingly,
> - changed the OMAP3 board code accordingly.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

One minor comment...

> ---
>  arch/arm/mach-omap2/board-rx51.c  |   15 +++---
>  arch/arm/mach-omap2/cpuidle34xx.c |  103 +++++++++++++++---------------------
>  arch/arm/mach-omap2/pm.h          |   13 +++--
>  3 files changed, 60 insertions(+), 71 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
> index f8ba20a..44656e8 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -58,21 +58,22 @@ static struct platform_device leds_gpio = {
>  	},
>  };
>  
> +/* cpuidle C-states definition override from the default values */

Please add a line here describing the sum in the table entries
(e.g. it's sleep + wakeup latency.)

>  static struct cpuidle_params rx51_cpuidle_params[] = {
>  	/* C1 */
> -	{1, 110, 162, 5},
> +	{110 + 162, 5 , 1},
>  	/* C2 */
> -	{1, 106, 180, 309},
> +	{106 + 180, 309, 1},
>  	/* C3 */
> -	{0, 107, 410, 46057},
> +	{107 + 410, 46057, 0},
>  	/* C4 */
> -	{0, 121, 3374, 46057},
> +	{121 + 3374, 46057, 0},
>  	/* C5 */
> -	{1, 855, 1146, 46057},
> +	{855 + 1146, 46057, 1},
>  	/* C6 */
> -	{0, 7580, 4134, 484329},
> +	{7580 + 4134, 484329, 0},
>  	/* C7 */
> -	{1, 7505, 15274, 484329},
> +	{7505 + 15274, 484329, 1},
>  };

Kevin

[PATCH 3/5] OMAP3: cpuidle: re-organize the C-states data

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

The current implementation defines an internal structure and a
C-states array. Using those structures is redundant to the
structs used by the cpuidle framework.

This patch provides a clean-up of the internal struct, removes the
internal C-states array, stores the data using the existing cpuidle
per C-state struct and registers the mach specific data to cpuidle
C-state driver_data (accessed using cpuidle_[gs]et_statedata).
Also removes unused macros, fields and code and compacts the repeating
code using common macros.

The result is more compact and more readable code as well as
reduced data RAM usage.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |  286 +++++++++++++------------------------
 1 files changed, 97 insertions(+), 189 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index d7bc31a..f84315c 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -36,34 +36,25 @@
 
 #ifdef CONFIG_CPU_IDLE
 
-#define OMAP3_MAX_STATES 7
-#define OMAP3_STATE_C1 0 /* C1 - MPU WFI + Core active */
-#define OMAP3_STATE_C2 1 /* C2 - MPU WFI + Core inactive */
-#define OMAP3_STATE_C3 2 /* C3 - MPU CSWR + Core inactive */
-#define OMAP3_STATE_C4 3 /* C4 - MPU OFF + Core iactive */
-#define OMAP3_STATE_C5 4 /* C5 - MPU RET + Core RET */
-#define OMAP3_STATE_C6 5 /* C6 - MPU OFF + Core RET */
-#define OMAP3_STATE_C7 6 /* C7 - MPU OFF + Core OFF */
-
-#define OMAP3_STATE_MAX OMAP3_STATE_C7
-
-#define CPUIDLE_FLAG_CHECK_BM	0x10000	/* use omap3_enter_idle_bm() */
-
-struct omap3_processor_cx {
-	u8 valid;
-	u8 type;
-	u32 exit_latency;
+#define OMAP3_STATE_C1		0 /* C1 - MPU WFI + Core active */
+#define OMAP3_STATE_C2		1 /* C2 - MPU WFI + Core inactive */
+#define OMAP3_STATE_C3		2 /* C3 - MPU CSWR + Core inactive */
+#define OMAP3_STATE_C4		3 /* C4 - MPU OFF + Core inactive */
+#define OMAP3_STATE_C5		4 /* C5 - MPU RET + Core RET */
+#define OMAP3_STATE_C6		5 /* C6 - MPU OFF + Core RET */
+#define OMAP3_STATE_C7		6 /* C7 - MPU OFF + Core OFF */
+#define OMAP3_STATE_MAX		OMAP3_STATE_C7
+#define OMAP3_MAX_STATES	7
+
+/* Mach specific information to be recorded in the C-state driver_data */
+struct omap3_idle_statedata {
 	u32 mpu_state;
 	u32 core_state;
-	u32 target_residency;
-	u32 flags;
-	const char *desc;
+	u8 valid;
 };
+struct omap3_idle_statedata omap3_idle_data[OMAP3_MAX_STATES];
 
-struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
-struct omap3_processor_cx current_cx_state;
-struct powerdomain *mpu_pd, *core_pd, *per_pd;
-struct powerdomain *cam_pd;
+struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
 
 /*
  * The latencies/thresholds for various C states have
@@ -72,7 +63,7 @@ struct powerdomain *cam_pd;
  * the best power savings) used on boards which do not
  * pass these details from the board file.
  */
-static struct cpuidle_params cpuidle_params_table[] = {
+static struct cpuidle_params cpuidle_params_table[OMAP3_MAX_STATES] = {
 	/* C1 */
 	{2 + 2, 5, 1},
 	/* C2 */
@@ -121,12 +112,10 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
 static int omap3_enter_idle(struct cpuidle_device *dev,
 			struct cpuidle_state *state)
 {
-	struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
+	struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
 	struct timespec ts_preidle, ts_postidle, ts_idle;
 	u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
 
-	current_cx_state = *cx;
-
 	/* Used to keep track of the total time in idle */
 	getnstimeofday(&ts_preidle);
 
@@ -139,7 +128,8 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	if (omap_irq_pending() || need_resched())
 		goto return_sleep_time;
 
-	if (cx->type == OMAP3_STATE_C1) {
+	/* Deny idle for C1 */
+	if (state == &dev->states[0]) {
 		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
 		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
 	}
@@ -147,7 +137,8 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	/* Execute ARM wfi */
 	omap_sram_idle();
 
-	if (cx->type == OMAP3_STATE_C1) {
+	/* Re-allow idle for C1 */
+	if (state == &dev->states[0]) {
 		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
 		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
 	}
@@ -169,15 +160,15 @@ return_sleep_time:
  *
  * If the current state is valid, it is returned back to the caller.
  * Else, this function searches for a lower c-state which is still
- * valid (as defined in omap3_power_states[]).
+ * valid.
  */
 static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
-						struct cpuidle_state *curr)
+					      struct cpuidle_state *curr)
 {
 	struct cpuidle_state *next = NULL;
-	struct omap3_processor_cx *cx;
+	struct omap3_idle_statedata *cx;
 
-	cx = (struct omap3_processor_cx *)cpuidle_get_statedata(curr);
+	cx = cpuidle_get_statedata(curr);
 
 	/* Check if current state is valid */
 	if (cx->valid) {
@@ -206,8 +197,6 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 		 */
 		idx--;
 		for (; idx >= OMAP3_STATE_C1; idx--) {
-			struct omap3_processor_cx *cx;
-
 			cx = cpuidle_get_statedata(&dev->states[idx]);
 			if (cx->valid) {
 				next = &dev->states[idx];
@@ -228,9 +217,8 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
  * @dev: cpuidle device
  * @state: The target state to be programmed
  *
- * Used for C states with CPUIDLE_FLAG_CHECK_BM flag set. This
- * function checks for any pending activity and then programs the
- * device to the specified or a safer state.
+ * This function checks for any pending activity and then programs
+ * the device to the specified or a safer state.
  */
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
@@ -238,10 +226,10 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	struct cpuidle_state *new_state = next_valid_state(dev, state);
 	u32 core_next_state, per_next_state = 0, per_saved_state = 0;
 	u32 cam_state;
-	struct omap3_processor_cx *cx;
+	struct omap3_idle_statedata *cx;
 	int ret;
 
-	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
+	if (omap3_idle_bm_check()) {
 		BUG_ON(!dev->safe_state);
 		new_state = dev->safe_state;
 		goto select_state;
@@ -308,7 +296,7 @@ void omap3_cpuidle_update_states(u32 mpu_deepest_state, u32 core_deepest_state)
 	int i;
 
 	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
-		struct omap3_processor_cx *cx = &omap3_power_states[i];
+		struct omap3_idle_statedata *cx = &omap3_idle_data[i];
 
 		if ((cx->mpu_state >= mpu_deepest_state) &&
 		    (cx->core_state >= core_deepest_state)) {
@@ -327,8 +315,7 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 		return;
 
 	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
-		cpuidle_params_table[i].valid =
-			cpuidle_board_params[i].valid;
+		cpuidle_params_table[i].valid =	cpuidle_board_params[i].valid;
 		cpuidle_params_table[i].exit_latency =
 			cpuidle_board_params[i].exit_latency;
 		cpuidle_params_table[i].target_residency =
@@ -337,134 +324,28 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 	return;
 }
 
-/* omap3_init_power_states - Initialises the OMAP3 specific C states.
- *
- * Below is the desciption of each C state.
- * 	C1 . MPU WFI + Core active
- *	C2 . MPU WFI + Core inactive
- *	C3 . MPU CSWR + Core inactive
- *	C4 . MPU OFF + Core inactive
- *	C5 . MPU CSWR + Core CSWR
- *	C6 . MPU OFF + Core CSWR
- *	C7 . MPU OFF + Core OFF
- */
-void omap_init_power_states(void)
-{
-	/* C1 . MPU WFI + Core active */
-	omap3_power_states[OMAP3_STATE_C1].valid =
-			cpuidle_params_table[OMAP3_STATE_C1].valid;
-	omap3_power_states[OMAP3_STATE_C1].type = OMAP3_STATE_C1;
-	omap3_power_states[OMAP3_STATE_C1].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C1].exit_latency;
-	omap3_power_states[OMAP3_STATE_C1].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C1].target_residency;
-	omap3_power_states[OMAP3_STATE_C1].mpu_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C1].core_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C1].flags = CPUIDLE_FLAG_TIME_VALID;
-	omap3_power_states[OMAP3_STATE_C1].desc = "MPU ON + CORE ON";
-
-	/* C2 . MPU WFI + Core inactive */
-	omap3_power_states[OMAP3_STATE_C2].valid =
-			cpuidle_params_table[OMAP3_STATE_C2].valid;
-	omap3_power_states[OMAP3_STATE_C2].type = OMAP3_STATE_C2;
-	omap3_power_states[OMAP3_STATE_C2].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C2].exit_latency;
-	omap3_power_states[OMAP3_STATE_C2].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C2].target_residency;
-	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C2].desc = "MPU ON + CORE ON";
-
-	/* C3 . MPU CSWR + Core inactive */
-	omap3_power_states[OMAP3_STATE_C3].valid =
-			cpuidle_params_table[OMAP3_STATE_C3].valid;
-	omap3_power_states[OMAP3_STATE_C3].type = OMAP3_STATE_C3;
-	omap3_power_states[OMAP3_STATE_C3].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C3].exit_latency;
-	omap3_power_states[OMAP3_STATE_C3].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C3].target_residency;
-	omap3_power_states[OMAP3_STATE_C3].mpu_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C3].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C3].desc = "MPU RET + CORE ON";
-
-	/* C4 . MPU OFF + Core inactive */
-	omap3_power_states[OMAP3_STATE_C4].valid =
-			cpuidle_params_table[OMAP3_STATE_C4].valid;
-	omap3_power_states[OMAP3_STATE_C4].type = OMAP3_STATE_C4;
-	omap3_power_states[OMAP3_STATE_C4].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C4].exit_latency;
-	omap3_power_states[OMAP3_STATE_C4].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C4].target_residency;
-	omap3_power_states[OMAP3_STATE_C4].mpu_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C4].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C4].desc = "MPU OFF + CORE ON";
-
-	/* C5 . MPU CSWR + Core CSWR*/
-	omap3_power_states[OMAP3_STATE_C5].valid =
-			cpuidle_params_table[OMAP3_STATE_C5].valid;
-	omap3_power_states[OMAP3_STATE_C5].type = OMAP3_STATE_C5;
-	omap3_power_states[OMAP3_STATE_C5].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C5].exit_latency;
-	omap3_power_states[OMAP3_STATE_C5].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C5].target_residency;
-	omap3_power_states[OMAP3_STATE_C5].mpu_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C5].core_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C5].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C5].desc = "MPU RET + CORE RET";
-
-	/* C6 . MPU OFF + Core CSWR */
-	omap3_power_states[OMAP3_STATE_C6].valid =
-			cpuidle_params_table[OMAP3_STATE_C6].valid;
-	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
-	omap3_power_states[OMAP3_STATE_C6].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C6].exit_latency;
-	omap3_power_states[OMAP3_STATE_C6].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C6].target_residency;
-	omap3_power_states[OMAP3_STATE_C6].mpu_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C6].core_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C6].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C6].desc = "MPU OFF + CORE RET";
-
-	/* C7 . MPU OFF + Core OFF */
-	omap3_power_states[OMAP3_STATE_C7].valid =
-			cpuidle_params_table[OMAP3_STATE_C7].valid;
-	omap3_power_states[OMAP3_STATE_C7].type = OMAP3_STATE_C7;
-	omap3_power_states[OMAP3_STATE_C7].exit_latency =
-			cpuidle_params_table[OMAP3_STATE_C7].exit_latency;
-	omap3_power_states[OMAP3_STATE_C7].target_residency =
-			cpuidle_params_table[OMAP3_STATE_C7].target_residency;
-	omap3_power_states[OMAP3_STATE_C7].mpu_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C7].core_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C7].flags = CPUIDLE_FLAG_TIME_VALID |
-				CPUIDLE_FLAG_CHECK_BM;
-	omap3_power_states[OMAP3_STATE_C7].desc = "MPU OFF + CORE OFF";
-
-	/*
-	 * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
-	 * enable OFF mode in a stable form for previous revisions.
-	 * we disable C7 state as a result.
-	 */
-	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
-		omap3_power_states[OMAP3_STATE_C7].valid = 0;
-		cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
-		pr_warn("%s: core off state C7 disabled due to i583\n",
-				__func__);
-	}
-}
-
 struct cpuidle_driver omap3_idle_driver = {
 	.name = 	"omap3_idle",
 	.owner = 	THIS_MODULE,
 };
 
+/* Fill in the state data from the mach tables and register the driver_data */
+#define FILL_IN_STATE(idx, descr)					\
+do {									\
+	state				= &dev->states[count];		\
+	params				= &cpuidle_params_table[idx];	\
+	data				= &omap3_idle_data[idx];	\
+	state->exit_latency		= params->exit_latency;		\
+	state->target_residency		= params->target_residency;	\
+	state->flags			= CPUIDLE_FLAG_TIME_VALID;	\
+	state->enter			= omap3_enter_idle_bm;		\
+	sprintf(state->name, "C%d", count + 1);				\
+	strncpy(state->desc, descr, CPUIDLE_DESC_LEN);			\
+	data->valid			= params->valid;		\
+	cpuidle_set_statedata(state, data);				\
+	count++;							\
+} while (0);
+
 /**
  * omap3_idle_init - Init routine for OMAP3 idle
  *
@@ -473,42 +354,69 @@ struct cpuidle_driver omap3_idle_driver = {
  */
 int __init omap3_idle_init(void)
 {
-	int i, count = 0;
-	struct omap3_processor_cx *cx;
+	int count = 0;
 	struct cpuidle_state *state;
 	struct cpuidle_device *dev;
+	struct cpuidle_params *params;
+	struct omap3_idle_statedata *data;
 
 	mpu_pd = pwrdm_lookup("mpu_pwrdm");
 	core_pd = pwrdm_lookup("core_pwrdm");
 	per_pd = pwrdm_lookup("per_pwrdm");
 	cam_pd = pwrdm_lookup("cam_pwrdm");
 
-	omap_init_power_states();
 	cpuidle_register_driver(&omap3_idle_driver);
 
 	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
 
-	for (i = OMAP3_STATE_C1; 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->exit_latency;
-		state->target_residency = cx->target_residency;
-		state->flags = cx->flags;
-		state->enter = (state->flags & CPUIDLE_FLAG_CHECK_BM) ?
-			omap3_enter_idle_bm : omap3_enter_idle;
-		if (cx->type == OMAP3_STATE_C1)
-			dev->safe_state = state;
-		sprintf(state->name, "C%d", count+1);
-		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
-		count++;
+	/* C1 . MPU WFI + Core active */
+	FILL_IN_STATE(OMAP3_STATE_C1, "MPU ON + CORE ON")
+	state->enter = omap3_enter_idle;
+	dev->safe_state = state;
+	data->valid = 1;	/* C1 is always valid */
+	data->mpu_state = PWRDM_POWER_ON;
+	data->core_state = PWRDM_POWER_ON;
+
+	/* C2 . MPU WFI + Core inactive */
+	FILL_IN_STATE(OMAP3_STATE_C2, "MPU ON + CORE ON")
+	data->mpu_state = PWRDM_POWER_ON;
+	data->core_state = PWRDM_POWER_ON;
+
+	/* C3 . MPU CSWR + Core inactive */
+	FILL_IN_STATE(OMAP3_STATE_C3, "MPU RET + CORE ON")
+	data->mpu_state = PWRDM_POWER_RET;
+	data->core_state = PWRDM_POWER_ON;
+
+	/* C4 . MPU OFF + Core inactive */
+	FILL_IN_STATE(OMAP3_STATE_C4, "MPU OFF + CORE ON")
+	data->mpu_state = PWRDM_POWER_OFF;
+	data->core_state = PWRDM_POWER_ON;
+
+	/* C5 . MPU RET + Core RET */
+	FILL_IN_STATE(OMAP3_STATE_C5, "MPU RET + CORE RET")
+	data->mpu_state = PWRDM_POWER_RET;
+	data->core_state = PWRDM_POWER_RET;
+
+	/* C6 . MPU OFF + Core RET */
+	FILL_IN_STATE(OMAP3_STATE_C6, "MPU OFF + CORE RET")
+	data->mpu_state = PWRDM_POWER_OFF;
+	data->core_state = PWRDM_POWER_RET;
+
+	/* C7 . MPU OFF + Core OFF */
+	FILL_IN_STATE(OMAP3_STATE_C7, "MPU OFF + CORE OFF")
+	/*
+	 * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
+	 * enable OFF mode in a stable form for previous revisions.
+	 * We disable C7 state as a result.
+	 */
+	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
+		data->valid = 0;
+		pr_warn("%s: core off state C7 disabled due to i583\n",
+			__func__);
 	}
+	data->mpu_state = PWRDM_POWER_OFF;
+	data->core_state = PWRDM_POWER_OFF;
 
-	if (!count)
-		return -EINVAL;
 	dev->state_count = count;
 
 	if (enable_off_mode)
-- 
1.7.1

[PATCH 3/5] OMAP3: cpuidle: re-organize the C-states data

[Santosh Shilimkar]

Jean,

On 4/29/2011 2:56 PM, jean.pihet at newoldbits.com wrote:
> From: Jean Pihet<j-pihet@ti.com>
>
> The current implementation defines an internal structure and a
> C-states array. Using those structures is redundant to the
> structs used by the cpuidle framework.
>
> This patch provides a clean-up of the internal struct, removes the
> internal C-states array, stores the data using the existing cpuidle
> per C-state struct and registers the mach specific data to cpuidle
> C-state driver_data (accessed using cpuidle_[gs]et_statedata).
> Also removes unused macros, fields and code and compacts the repeating
> code using common macros.
>
> The result is more compact and more readable code as well as
> reduced data RAM usage.
>
> Signed-off-by: Jean Pihet<j-pihet@ti.com>
> ---
>   arch/arm/mach-omap2/cpuidle34xx.c |  286 +++++++++++++------------------------
>   1 files changed, 97 insertions(+), 189 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index d7bc31a..f84315c 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -36,34 +36,25 @@
>
>   #ifdef CONFIG_CPU_IDLE
>
> -#define OMAP3_MAX_STATES 7
> -#define OMAP3_STATE_C1 0 /* C1 - MPU WFI + Core active */
> -#define OMAP3_STATE_C2 1 /* C2 - MPU WFI + Core inactive */
> -#define OMAP3_STATE_C3 2 /* C3 - MPU CSWR + Core inactive */
> -#define OMAP3_STATE_C4 3 /* C4 - MPU OFF + Core iactive */
> -#define OMAP3_STATE_C5 4 /* C5 - MPU RET + Core RET */
> -#define OMAP3_STATE_C6 5 /* C6 - MPU OFF + Core RET */
> -#define OMAP3_STATE_C7 6 /* C7 - MPU OFF + Core OFF */
> -
> -#define OMAP3_STATE_MAX OMAP3_STATE_C7
> -
> -#define CPUIDLE_FLAG_CHECK_BM	0x10000	/* use omap3_enter_idle_bm() */
> -
> -struct omap3_processor_cx {
> -	u8 valid;
> -	u8 type;
> -	u32 exit_latency;
> +#define OMAP3_STATE_C1		0 /* C1 - MPU WFI + Core active */
> +#define OMAP3_STATE_C2		1 /* C2 - MPU WFI + Core inactive */
> +#define OMAP3_STATE_C3		2 /* C3 - MPU CSWR + Core inactive */
> +#define OMAP3_STATE_C4		3 /* C4 - MPU OFF + Core inactive */
> +#define OMAP3_STATE_C5		4 /* C5 - MPU RET + Core RET */
> +#define OMAP3_STATE_C6		5 /* C6 - MPU OFF + Core RET */
> +#define OMAP3_STATE_C7		6 /* C7 - MPU OFF + Core OFF */
> +#define OMAP3_STATE_MAX		OMAP3_STATE_C7
> +#define OMAP3_MAX_STATES	7
> +
> +/* Mach specific information to be recorded in the C-state driver_data */
> +struct omap3_idle_statedata {
>   	u32 mpu_state;
>   	u32 core_state;
> -	u32 target_residency;
> -	u32 flags;
> -	const char *desc;
> +	u8 valid;
>   };
> +struct omap3_idle_statedata omap3_idle_data[OMAP3_MAX_STATES];
>
> -struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
> -struct omap3_processor_cx current_cx_state;
> -struct powerdomain *mpu_pd, *core_pd, *per_pd;
> -struct powerdomain *cam_pd;
> +struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
>
>   /*
>    * The latencies/thresholds for various C states have
> @@ -72,7 +63,7 @@ struct powerdomain *cam_pd;
>    * the best power savings) used on boards which do not
>    * pass these details from the board file.
>    */
> -static struct cpuidle_params cpuidle_params_table[] = {
> +static struct cpuidle_params cpuidle_params_table[OMAP3_MAX_STATES] = {

Nice cleanup. Using idle per device C-state struct is much cleaner
than the those static arrays.

[...]

>
> +/* Fill in the state data from the mach tables and register the driver_data */
> +#define FILL_IN_STATE(idx, descr)					\
> +do {									\
> +	state				=&dev->states[count];		\
> +	params				=&cpuidle_params_table[idx];	\
> +	data				=&omap3_idle_data[idx];	\
> +	state->exit_latency		= params->exit_latency;		\
> +	state->target_residency		= params->target_residency;	\
> +	state->flags			= CPUIDLE_FLAG_TIME_VALID;	\
> +	state->enter			= omap3_enter_idle_bm;		\
> +	sprintf(state->name, "C%d", count + 1);				\
> +	strncpy(state->desc, descr, CPUIDLE_DESC_LEN);			\
> +	data->valid			= params->valid;		\
> +	cpuidle_set_statedata(state, data);				\
> +	count++;							\
> +} while (0);
> +
I like this macro as well. It avoids un-necessary lines on the C-state
initialization code.

Good work.

Regards
Santosh

[PATCH 3/5] OMAP3: cpuidle: re-organize the C-states data

[Kevin Hilman]

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>
>
> The current implementation defines an internal structure and a
> C-states array. Using those structures is redundant to the
> structs used by the cpuidle framework.
>
> This patch provides a clean-up of the internal struct, removes the
> internal C-states array, stores the data using the existing cpuidle
> per C-state struct and registers the mach specific data to cpuidle
> C-state driver_data (accessed using cpuidle_[gs]et_statedata).
> Also removes unused macros, fields and code and compacts the repeating
> code using common macros.
>
> The result is more compact and more readable code as well as
> reduced data RAM usage.
>
> Signed-off-by: Jean Pihet <j-pihet@ti.com>

Looks like a nice cleanup, a couple minor comment below...

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |  286 +++++++++++++------------------------
>  1 files changed, 97 insertions(+), 189 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index d7bc31a..f84315c 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -36,34 +36,25 @@
>  
>  #ifdef CONFIG_CPU_IDLE
>  
> -#define OMAP3_MAX_STATES 7
> -#define OMAP3_STATE_C1 0 /* C1 - MPU WFI + Core active */
> -#define OMAP3_STATE_C2 1 /* C2 - MPU WFI + Core inactive */
> -#define OMAP3_STATE_C3 2 /* C3 - MPU CSWR + Core inactive */
> -#define OMAP3_STATE_C4 3 /* C4 - MPU OFF + Core iactive */
> -#define OMAP3_STATE_C5 4 /* C5 - MPU RET + Core RET */
> -#define OMAP3_STATE_C6 5 /* C6 - MPU OFF + Core RET */
> -#define OMAP3_STATE_C7 6 /* C7 - MPU OFF + Core OFF */
> -
> -#define OMAP3_STATE_MAX OMAP3_STATE_C7
> -
> -#define CPUIDLE_FLAG_CHECK_BM	0x10000	/* use omap3_enter_idle_bm() */
> -
> -struct omap3_processor_cx {
> -	u8 valid;
> -	u8 type;
> -	u32 exit_latency;
> +#define OMAP3_STATE_C1		0 /* C1 - MPU WFI + Core active */
> +#define OMAP3_STATE_C2		1 /* C2 - MPU WFI + Core inactive */
> +#define OMAP3_STATE_C3		2 /* C3 - MPU CSWR + Core inactive */
> +#define OMAP3_STATE_C4		3 /* C4 - MPU OFF + Core inactive */
> +#define OMAP3_STATE_C5		4 /* C5 - MPU RET + Core RET */
> +#define OMAP3_STATE_C6		5 /* C6 - MPU OFF + Core RET */
> +#define OMAP3_STATE_C7		6 /* C7 - MPU OFF + Core OFF */
> +#define OMAP3_STATE_MAX		OMAP3_STATE_C7
> +#define OMAP3_MAX_STATES	7

While at it, we should get rid of these #defines all together.    It's
just a mapping from Cn --> n, so why not just use 'n' in the code.

You did that in a few cases below already, but might as well continue
that.

> +/* Mach specific information to be recorded in the C-state driver_data */
> +struct omap3_idle_statedata {
>  	u32 mpu_state;
>  	u32 core_state;
> -	u32 target_residency;
> -	u32 flags;
> -	const char *desc;
> +	u8 valid;
>  };
> +struct omap3_idle_statedata omap3_idle_data[OMAP3_MAX_STATES];
>  
> -struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
> -struct omap3_processor_cx current_cx_state;
> -struct powerdomain *mpu_pd, *core_pd, *per_pd;
> -struct powerdomain *cam_pd;
> +struct powerdomain *mpu_pd, *core_pd, *per_pd, *cam_pd;
>  
>  /*
>   * The latencies/thresholds for various C states have
> @@ -72,7 +63,7 @@ struct powerdomain *cam_pd;
>   * the best power savings) used on boards which do not
>   * pass these details from the board file.
>   */
> -static struct cpuidle_params cpuidle_params_table[] = {
> +static struct cpuidle_params cpuidle_params_table[OMAP3_MAX_STATES] = {
>  	/* C1 */
>  	{2 + 2, 5, 1},
>  	/* C2 */
> @@ -121,12 +112,10 @@ static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
>  static int omap3_enter_idle(struct cpuidle_device *dev,
>  			struct cpuidle_state *state)
>  {
> -	struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
> +	struct omap3_idle_statedata *cx = cpuidle_get_statedata(state);
>  	struct timespec ts_preidle, ts_postidle, ts_idle;
>  	u32 mpu_state = cx->mpu_state, core_state = cx->core_state;
>  
> -	current_cx_state = *cx;
> -
>  	/* Used to keep track of the total time in idle */
>  	getnstimeofday(&ts_preidle);
>  
> @@ -139,7 +128,8 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>  	if (omap_irq_pending() || need_resched())
>  		goto return_sleep_time;
>  
> -	if (cx->type == OMAP3_STATE_C1) {
> +	/* Deny idle for C1 */
> +	if (state == &dev->states[0]) {
>  		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
>  		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
>  	}
> @@ -147,7 +137,8 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>  	/* Execute ARM wfi */
>  	omap_sram_idle();
>  
> -	if (cx->type == OMAP3_STATE_C1) {
> +	/* Re-allow idle for C1 */
> +	if (state == &dev->states[0]) {
>  		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
>  		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
>  	}
> @@ -169,15 +160,15 @@ return_sleep_time:
>   *
>   * If the current state is valid, it is returned back to the caller.
>   * Else, this function searches for a lower c-state which is still
> - * valid (as defined in omap3_power_states[]).
> + * valid.
>   */
>  static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
> -						struct cpuidle_state *curr)
> +					      struct cpuidle_state *curr)
>  {
>  	struct cpuidle_state *next = NULL;
> -	struct omap3_processor_cx *cx;
> +	struct omap3_idle_statedata *cx;
>  
> -	cx = (struct omap3_processor_cx *)cpuidle_get_statedata(curr);
> +	cx = cpuidle_get_statedata(curr);
>  
>  	/* Check if current state is valid */
>  	if (cx->valid) {
> @@ -206,8 +197,6 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>  		 */
>  		idx--;
>  		for (; idx >= OMAP3_STATE_C1; idx--) {
> -			struct omap3_processor_cx *cx;
> -
>  			cx = cpuidle_get_statedata(&dev->states[idx]);
>  			if (cx->valid) {
>  				next = &dev->states[idx];
> @@ -228,9 +217,8 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>   * @dev: cpuidle device
>   * @state: The target state to be programmed
>   *
> - * Used for C states with CPUIDLE_FLAG_CHECK_BM flag set. This
> - * function checks for any pending activity and then programs the
> - * device to the specified or a safer state.
> + * This function checks for any pending activity and then programs
> + * the device to the specified or a safer state.
>   */
>  static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  			       struct cpuidle_state *state)
> @@ -238,10 +226,10 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  	struct cpuidle_state *new_state = next_valid_state(dev, state);
>  	u32 core_next_state, per_next_state = 0, per_saved_state = 0;
>  	u32 cam_state;
> -	struct omap3_processor_cx *cx;
> +	struct omap3_idle_statedata *cx;
>  	int ret;
>  
> -	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
> +	if (omap3_idle_bm_check()) {
>  		BUG_ON(!dev->safe_state);
>  		new_state = dev->safe_state;
>  		goto select_state;
> @@ -308,7 +296,7 @@ void omap3_cpuidle_update_states(u32 mpu_deepest_state, u32 core_deepest_state)
>  	int i;
>  
>  	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
> -		struct omap3_processor_cx *cx = &omap3_power_states[i];
> +		struct omap3_idle_statedata *cx = &omap3_idle_data[i];
>  
>  		if ((cx->mpu_state >= mpu_deepest_state) &&
>  		    (cx->core_state >= core_deepest_state)) {
> @@ -327,8 +315,7 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
>  		return;
>  
>  	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
> -		cpuidle_params_table[i].valid =
> -			cpuidle_board_params[i].valid;
> +		cpuidle_params_table[i].valid =	cpuidle_board_params[i].valid;
>  		cpuidle_params_table[i].exit_latency =
>  			cpuidle_board_params[i].exit_latency;
>  		cpuidle_params_table[i].target_residency =
> @@ -337,134 +324,28 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
>  	return;
>  }
>  
> -/* omap3_init_power_states - Initialises the OMAP3 specific C states.
> - *
> - * Below is the desciption of each C state.
> - * 	C1 . MPU WFI + Core active
> - *	C2 . MPU WFI + Core inactive
> - *	C3 . MPU CSWR + Core inactive
> - *	C4 . MPU OFF + Core inactive
> - *	C5 . MPU CSWR + Core CSWR
> - *	C6 . MPU OFF + Core CSWR
> - *	C7 . MPU OFF + Core OFF
> - */
> -void omap_init_power_states(void)
> -{
> -	/* C1 . MPU WFI + Core active */
> -	omap3_power_states[OMAP3_STATE_C1].valid =
> -			cpuidle_params_table[OMAP3_STATE_C1].valid;
> -	omap3_power_states[OMAP3_STATE_C1].type = OMAP3_STATE_C1;
> -	omap3_power_states[OMAP3_STATE_C1].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C1].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C1].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C1].target_residency;
> -	omap3_power_states[OMAP3_STATE_C1].mpu_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C1].core_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C1].flags = CPUIDLE_FLAG_TIME_VALID;
> -	omap3_power_states[OMAP3_STATE_C1].desc = "MPU ON + CORE ON";
> -
> -	/* C2 . MPU WFI + Core inactive */
> -	omap3_power_states[OMAP3_STATE_C2].valid =
> -			cpuidle_params_table[OMAP3_STATE_C2].valid;
> -	omap3_power_states[OMAP3_STATE_C2].type = OMAP3_STATE_C2;
> -	omap3_power_states[OMAP3_STATE_C2].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C2].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C2].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C2].target_residency;
> -	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C2].desc = "MPU ON + CORE ON";
> -
> -	/* C3 . MPU CSWR + Core inactive */
> -	omap3_power_states[OMAP3_STATE_C3].valid =
> -			cpuidle_params_table[OMAP3_STATE_C3].valid;
> -	omap3_power_states[OMAP3_STATE_C3].type = OMAP3_STATE_C3;
> -	omap3_power_states[OMAP3_STATE_C3].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C3].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C3].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C3].target_residency;
> -	omap3_power_states[OMAP3_STATE_C3].mpu_state = PWRDM_POWER_RET;
> -	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C3].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C3].desc = "MPU RET + CORE ON";
> -
> -	/* C4 . MPU OFF + Core inactive */
> -	omap3_power_states[OMAP3_STATE_C4].valid =
> -			cpuidle_params_table[OMAP3_STATE_C4].valid;
> -	omap3_power_states[OMAP3_STATE_C4].type = OMAP3_STATE_C4;
> -	omap3_power_states[OMAP3_STATE_C4].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C4].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C4].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C4].target_residency;
> -	omap3_power_states[OMAP3_STATE_C4].mpu_state = PWRDM_POWER_OFF;
> -	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_ON;
> -	omap3_power_states[OMAP3_STATE_C4].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C4].desc = "MPU OFF + CORE ON";
> -
> -	/* C5 . MPU CSWR + Core CSWR*/
> -	omap3_power_states[OMAP3_STATE_C5].valid =
> -			cpuidle_params_table[OMAP3_STATE_C5].valid;
> -	omap3_power_states[OMAP3_STATE_C5].type = OMAP3_STATE_C5;
> -	omap3_power_states[OMAP3_STATE_C5].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C5].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C5].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C5].target_residency;
> -	omap3_power_states[OMAP3_STATE_C5].mpu_state = PWRDM_POWER_RET;
> -	omap3_power_states[OMAP3_STATE_C5].core_state = PWRDM_POWER_RET;
> -	omap3_power_states[OMAP3_STATE_C5].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C5].desc = "MPU RET + CORE RET";
> -
> -	/* C6 . MPU OFF + Core CSWR */
> -	omap3_power_states[OMAP3_STATE_C6].valid =
> -			cpuidle_params_table[OMAP3_STATE_C6].valid;
> -	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
> -	omap3_power_states[OMAP3_STATE_C6].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C6].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C6].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C6].target_residency;
> -	omap3_power_states[OMAP3_STATE_C6].mpu_state = PWRDM_POWER_OFF;
> -	omap3_power_states[OMAP3_STATE_C6].core_state = PWRDM_POWER_RET;
> -	omap3_power_states[OMAP3_STATE_C6].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C6].desc = "MPU OFF + CORE RET";
> -
> -	/* C7 . MPU OFF + Core OFF */
> -	omap3_power_states[OMAP3_STATE_C7].valid =
> -			cpuidle_params_table[OMAP3_STATE_C7].valid;
> -	omap3_power_states[OMAP3_STATE_C7].type = OMAP3_STATE_C7;
> -	omap3_power_states[OMAP3_STATE_C7].exit_latency =
> -			cpuidle_params_table[OMAP3_STATE_C7].exit_latency;
> -	omap3_power_states[OMAP3_STATE_C7].target_residency =
> -			cpuidle_params_table[OMAP3_STATE_C7].target_residency;
> -	omap3_power_states[OMAP3_STATE_C7].mpu_state = PWRDM_POWER_OFF;
> -	omap3_power_states[OMAP3_STATE_C7].core_state = PWRDM_POWER_OFF;
> -	omap3_power_states[OMAP3_STATE_C7].flags = CPUIDLE_FLAG_TIME_VALID |
> -				CPUIDLE_FLAG_CHECK_BM;
> -	omap3_power_states[OMAP3_STATE_C7].desc = "MPU OFF + CORE OFF";
> -
> -	/*
> -	 * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
> -	 * enable OFF mode in a stable form for previous revisions.
> -	 * we disable C7 state as a result.
> -	 */
> -	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
> -		omap3_power_states[OMAP3_STATE_C7].valid = 0;
> -		cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
> -		pr_warn("%s: core off state C7 disabled due to i583\n",
> -				__func__);
> -	}
> -}
> -
>  struct cpuidle_driver omap3_idle_driver = {
>  	.name = 	"omap3_idle",
>  	.owner = 	THIS_MODULE,
>  };
>  
> +/* Fill in the state data from the mach tables and register the driver_data */
> +#define FILL_IN_STATE(idx, descr)					\
> +do {									\
> +	state				= &dev->states[count];		\
> +	params				= &cpuidle_params_table[idx];	\
> +	data				= &omap3_idle_data[idx];	\
> +	state->exit_latency		= params->exit_latency;		\
> +	state->target_residency		= params->target_residency;	\
> +	state->flags			= CPUIDLE_FLAG_TIME_VALID;	\
> +	state->enter			= omap3_enter_idle_bm;		\
> +	sprintf(state->name, "C%d", count + 1);				\
> +	strncpy(state->desc, descr, CPUIDLE_DESC_LEN);			\
> +	data->valid			= params->valid;		\
> +	cpuidle_set_statedata(state, data);				\
> +	count++;							\
> +} while (0);

A static inline function here would improve readabiliy.

e.g. below, readabilty suffers as all the assignments use 'data', which
appears to be the same pointer.  It's not obvious from the macro use
that that the data ptr is changed by the macro.

Kevin

>  /**
>   * omap3_idle_init - Init routine for OMAP3 idle
>   *
> @@ -473,42 +354,69 @@ struct cpuidle_driver omap3_idle_driver = {
>   */
>  int __init omap3_idle_init(void)
>  {
> -	int i, count = 0;
> -	struct omap3_processor_cx *cx;
> +	int count = 0;
>  	struct cpuidle_state *state;
>  	struct cpuidle_device *dev;
> +	struct cpuidle_params *params;
> +	struct omap3_idle_statedata *data;
>  
>  	mpu_pd = pwrdm_lookup("mpu_pwrdm");
>  	core_pd = pwrdm_lookup("core_pwrdm");
>  	per_pd = pwrdm_lookup("per_pwrdm");
>  	cam_pd = pwrdm_lookup("cam_pwrdm");
>  
> -	omap_init_power_states();
>  	cpuidle_register_driver(&omap3_idle_driver);
>  
>  	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
>  
> -	for (i = OMAP3_STATE_C1; 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->exit_latency;
> -		state->target_residency = cx->target_residency;
> -		state->flags = cx->flags;
> -		state->enter = (state->flags & CPUIDLE_FLAG_CHECK_BM) ?
> -			omap3_enter_idle_bm : omap3_enter_idle;
> -		if (cx->type == OMAP3_STATE_C1)
> -			dev->safe_state = state;
> -		sprintf(state->name, "C%d", count+1);
> -		strncpy(state->desc, cx->desc, CPUIDLE_DESC_LEN);
> -		count++;
> +	/* C1 . MPU WFI + Core active */
> +	FILL_IN_STATE(OMAP3_STATE_C1, "MPU ON + CORE ON")
> +	state->enter = omap3_enter_idle;
> +	dev->safe_state = state;
> +	data->valid = 1;	/* C1 is always valid */
> +	data->mpu_state = PWRDM_POWER_ON;
> +	data->core_state = PWRDM_POWER_ON;
> +
> +	/* C2 . MPU WFI + Core inactive */
> +	FILL_IN_STATE(OMAP3_STATE_C2, "MPU ON + CORE ON")
> +	data->mpu_state = PWRDM_POWER_ON;
> +	data->core_state = PWRDM_POWER_ON;
> +
> +	/* C3 . MPU CSWR + Core inactive */
> +	FILL_IN_STATE(OMAP3_STATE_C3, "MPU RET + CORE ON")
> +	data->mpu_state = PWRDM_POWER_RET;
> +	data->core_state = PWRDM_POWER_ON;
> +
> +	/* C4 . MPU OFF + Core inactive */
> +	FILL_IN_STATE(OMAP3_STATE_C4, "MPU OFF + CORE ON")
> +	data->mpu_state = PWRDM_POWER_OFF;
> +	data->core_state = PWRDM_POWER_ON;
> +
> +	/* C5 . MPU RET + Core RET */
> +	FILL_IN_STATE(OMAP3_STATE_C5, "MPU RET + CORE RET")
> +	data->mpu_state = PWRDM_POWER_RET;
> +	data->core_state = PWRDM_POWER_RET;
> +
> +	/* C6 . MPU OFF + Core RET */
> +	FILL_IN_STATE(OMAP3_STATE_C6, "MPU OFF + CORE RET")
> +	data->mpu_state = PWRDM_POWER_OFF;
> +	data->core_state = PWRDM_POWER_RET;
> +
> +	/* C7 . MPU OFF + Core OFF */
> +	FILL_IN_STATE(OMAP3_STATE_C7, "MPU OFF + CORE OFF")
> +	/*
> +	 * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
> +	 * enable OFF mode in a stable form for previous revisions.
> +	 * We disable C7 state as a result.
> +	 */
> +	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
> +		data->valid = 0;
> +		pr_warn("%s: core off state C7 disabled due to i583\n",
> +			__func__);
>  	}
> +	data->mpu_state = PWRDM_POWER_OFF;
> +	data->core_state = PWRDM_POWER_OFF;
>  
> -	if (!count)
> -		return -EINVAL;
>  	dev->state_count = count;
>  
>  	if (enable_off_mode)

[PATCH 4/5] OMAP3: cpuidle: code rework for improved readability

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   52 +++++++++++++-----------------------
 1 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index f84315c..4673cc6 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -80,13 +80,6 @@ static struct cpuidle_params cpuidle_params_table[OMAP3_MAX_STATES] = {
 	{10000 + 30000, 300000, 1},
 };
 
-static int omap3_idle_bm_check(void)
-{
-	if (!omap3_can_sleep())
-		return 1;
-	return 0;
-}
-
 static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
 				struct clockdomain *clkdm)
 {
@@ -166,9 +159,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 					      struct cpuidle_state *curr)
 {
 	struct cpuidle_state *next = NULL;
-	struct omap3_idle_statedata *cx;
-
-	cx = cpuidle_get_statedata(curr);
+	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
 
 	/* Check if current state is valid */
 	if (cx->valid) {
@@ -176,9 +167,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 	} else {
 		u8 idx = OMAP3_STATE_MAX;
 
-		/*
-		 * Reach the current state starting at highest C-state
-		 */
+		/* Reach the current state starting at highest C-state */
 		for (; idx >= OMAP3_STATE_C1; idx--) {
 			if (&dev->states[idx] == curr) {
 				next = &dev->states[idx];
@@ -186,9 +175,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 			}
 		}
 
-		/*
-		 * Should never hit this condition.
-		 */
+		/* Should never hit this condition */
 		WARN_ON(next == NULL);
 
 		/*
@@ -223,29 +210,16 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
 {
-	struct cpuidle_state *new_state = next_valid_state(dev, state);
-	u32 core_next_state, per_next_state = 0, per_saved_state = 0;
-	u32 cam_state;
+	struct cpuidle_state *new_state;
+	u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
 	struct omap3_idle_statedata *cx;
 	int ret;
 
-	if (omap3_idle_bm_check()) {
-		BUG_ON(!dev->safe_state);
+	if (!omap3_can_sleep()) {
 		new_state = dev->safe_state;
 		goto select_state;
 	}
 
-	cx = cpuidle_get_statedata(state);
-	core_next_state = cx->core_state;
-
-	/*
-	 * FIXME: we currently manage device-specific idle states
-	 *        for PER and CORE in combination with CPU-specific
-	 *        idle states.  This is wrong, and device-specific
-	 *        idle management needs to be separated out into 
-	 *        its own code.
-	 */
-
 	/*
 	 * Prevent idle completely if CAM is active.
 	 * CAM does not have wakeup capability in OMAP3.
@@ -257,9 +231,19 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	}
 
 	/*
+	 * FIXME: we currently manage device-specific idle states
+	 *        for PER and CORE in combination with CPU-specific
+	 *        idle states.  This is wrong, and device-specific
+	 *        idle management needs to be separated out into
+	 *        its own code.
+	 */
+
+	/*
 	 * Prevent PER off if CORE is not in retention or off as this
 	 * would disable PER wakeups completely.
 	 */
+	cx = cpuidle_get_statedata(state);
+	core_next_state = cx->core_state;
 	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
 	if ((per_next_state == PWRDM_POWER_OFF) &&
 	    (core_next_state > PWRDM_POWER_RET))
@@ -269,6 +253,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_next_state);
 
+	new_state = next_valid_state(dev, state);
+
 select_state:
 	dev->last_state = new_state;
 	ret = omap3_enter_idle(dev, new_state);
@@ -329,7 +315,7 @@ struct cpuidle_driver omap3_idle_driver = {
 	.owner = 	THIS_MODULE,
 };
 
-/* Fill in the state data from the mach tables and register the driver_data */
+/* Helper to fill the C-state common data and register the driver_data */
 #define FILL_IN_STATE(idx, descr)					\
 do {									\
 	state				= &dev->states[count];		\
-- 
1.7.1

[PATCH 4/5] OMAP3: cpuidle: code rework for improved readability

[Kevin Hilman]

jean.pihet at newoldbits.com writes:

> From: Jean Pihet <j-pihet@ti.com>

Please summarize changes here.

Kevin

> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   52 +++++++++++++-----------------------
>  1 files changed, 19 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index f84315c..4673cc6 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -80,13 +80,6 @@ static struct cpuidle_params cpuidle_params_table[OMAP3_MAX_STATES] = {
>  	{10000 + 30000, 300000, 1},
>  };
>  
> -static int omap3_idle_bm_check(void)
> -{
> -	if (!omap3_can_sleep())
> -		return 1;
> -	return 0;
> -}
> -
>  static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
>  				struct clockdomain *clkdm)
>  {
> @@ -166,9 +159,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>  					      struct cpuidle_state *curr)
>  {
>  	struct cpuidle_state *next = NULL;
> -	struct omap3_idle_statedata *cx;
> -
> -	cx = cpuidle_get_statedata(curr);
> +	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
>  
>  	/* Check if current state is valid */
>  	if (cx->valid) {
> @@ -176,9 +167,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>  	} else {
>  		u8 idx = OMAP3_STATE_MAX;
>  
> -		/*
> -		 * Reach the current state starting at highest C-state
> -		 */
> +		/* Reach the current state starting at highest C-state */
>  		for (; idx >= OMAP3_STATE_C1; idx--) {
>  			if (&dev->states[idx] == curr) {
>  				next = &dev->states[idx];
> @@ -186,9 +175,7 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>  			}
>  		}
>  
> -		/*
> -		 * Should never hit this condition.
> -		 */
> +		/* Should never hit this condition */
>  		WARN_ON(next == NULL);
>  
>  		/*
> @@ -223,29 +210,16 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>  static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  			       struct cpuidle_state *state)
>  {
> -	struct cpuidle_state *new_state = next_valid_state(dev, state);
> -	u32 core_next_state, per_next_state = 0, per_saved_state = 0;
> -	u32 cam_state;
> +	struct cpuidle_state *new_state;
> +	u32 core_next_state, per_next_state = 0, per_saved_state = 0, cam_state;
>  	struct omap3_idle_statedata *cx;
>  	int ret;
>  
> -	if (omap3_idle_bm_check()) {
> -		BUG_ON(!dev->safe_state);
> +	if (!omap3_can_sleep()) {
>  		new_state = dev->safe_state;
>  		goto select_state;
>  	}
>  
> -	cx = cpuidle_get_statedata(state);
> -	core_next_state = cx->core_state;
> -
> -	/*
> -	 * FIXME: we currently manage device-specific idle states
> -	 *        for PER and CORE in combination with CPU-specific
> -	 *        idle states.  This is wrong, and device-specific
> -	 *        idle management needs to be separated out into 
> -	 *        its own code.
> -	 */
> -
>  	/*
>  	 * Prevent idle completely if CAM is active.
>  	 * CAM does not have wakeup capability in OMAP3.
> @@ -257,9 +231,19 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  	}
>  
>  	/*
> +	 * FIXME: we currently manage device-specific idle states
> +	 *        for PER and CORE in combination with CPU-specific
> +	 *        idle states.  This is wrong, and device-specific
> +	 *        idle management needs to be separated out into
> +	 *        its own code.
> +	 */
> +
> +	/*
>  	 * Prevent PER off if CORE is not in retention or off as this
>  	 * would disable PER wakeups completely.
>  	 */
> +	cx = cpuidle_get_statedata(state);
> +	core_next_state = cx->core_state;
>  	per_next_state = per_saved_state = pwrdm_read_next_pwrst(per_pd);
>  	if ((per_next_state == PWRDM_POWER_OFF) &&
>  	    (core_next_state > PWRDM_POWER_RET))
> @@ -269,6 +253,8 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  	if (per_next_state != per_saved_state)
>  		pwrdm_set_next_pwrst(per_pd, per_next_state);
>  
> +	new_state = next_valid_state(dev, state);
> +
>  select_state:
>  	dev->last_state = new_state;
>  	ret = omap3_enter_idle(dev, new_state);
> @@ -329,7 +315,7 @@ struct cpuidle_driver omap3_idle_driver = {
>  	.owner = 	THIS_MODULE,
>  };
>  
> -/* Fill in the state data from the mach tables and register the driver_data */
> +/* Helper to fill the C-state common data and register the driver_data */
>  #define FILL_IN_STATE(idx, descr)					\
>  do {									\
>  	state				= &dev->states[count];		\

[PATCH 5/5] OMAP3: cpuidle: change the power domains modes determination logic

[jean.pihet at newoldbits.com]

From: Jean Pihet <j-pihet@ti.com>

The achievable power modes of the power domains in cpuidle
depends on the system wide 'enable_off_mode' knob in debugfs.
Upon changing enable_off_mode, do not change the C-states
'valid' field but instead dynamically restrict the power modes
when entering idle.

The C-states 'valid' field is just used to enable/disable some
C-states at init and shall not be changed later on.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   59 +++++++++++++++---------------------
 arch/arm/mach-omap2/pm.h          |    4 --
 arch/arm/mach-omap2/pm34xx.c      |   12 -------
 3 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 4673cc6..45ccfa7 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -147,22 +147,40 @@ return_sleep_time:
 }
 
 /**
- * next_valid_state - Find next valid c-state
+ * next_valid_state - Find next valid C-state
  * @dev: cpuidle device
- * @state: Currently selected c-state
+ * @state: Currently selected C-state
  *
  * If the current state is valid, it is returned back to the caller.
  * Else, this function searches for a lower c-state which is still
  * valid.
+ *
+ * A state is valid if the 'valid' field is enabled and
+ * if it satisfies the enable_off_mode condition.
  */
 static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 					      struct cpuidle_state *curr)
 {
 	struct cpuidle_state *next = NULL;
 	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
+	u32 mpu_deepest_state = PWRDM_POWER_RET;
+	u32 core_deepest_state = PWRDM_POWER_RET;
+
+	if (enable_off_mode) {
+		mpu_deepest_state = PWRDM_POWER_OFF;
+		/*
+		 * Erratum i583: valable for ES rev < Es1.2 on 3630.
+		 * CORE OFF mode is not supported in a stable form, restrict
+		 * instead the CORE state to RET.
+		 */
+		if (!IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583))
+			core_deepest_state = PWRDM_POWER_OFF;
+	}
 
 	/* Check if current state is valid */
-	if (cx->valid) {
+	if ((cx->valid) &&
+	    (cx->mpu_state >= mpu_deepest_state) &&
+	    (cx->core_state >= core_deepest_state)) {
 		return curr;
 	} else {
 		u8 idx = OMAP3_STATE_MAX;
@@ -185,7 +203,9 @@ static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
 		idx--;
 		for (; idx >= OMAP3_STATE_C1; idx--) {
 			cx = cpuidle_get_statedata(&dev->states[idx]);
-			if (cx->valid) {
+			if ((cx->valid) &&
+			    (cx->mpu_state >= mpu_deepest_state) &&
+			    (cx->core_state >= core_deepest_state)) {
 				next = &dev->states[idx];
 				break;
 			}
@@ -268,31 +288,6 @@ select_state:
 
 DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
 
-/**
- * omap3_cpuidle_update_states() - Update the cpuidle states
- * @mpu_deepest_state:	Enable states up to and including this for mpu domain
- * @core_deepest_state:	Enable states up to and including this for core domain
- *
- * This goes through the list of states available and enables and disables the
- * validity of C states based on deepest state that can be achieved for the
- * variable domain
- */
-void omap3_cpuidle_update_states(u32 mpu_deepest_state, u32 core_deepest_state)
-{
-	int i;
-
-	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
-		struct omap3_idle_statedata *cx = &omap3_idle_data[i];
-
-		if ((cx->mpu_state >= mpu_deepest_state) &&
-		    (cx->core_state >= core_deepest_state)) {
-			cx->valid = 1;
-		} else {
-			cx->valid = 0;
-		}
-	}
-}
-
 void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 {
 	int i;
@@ -365,6 +360,7 @@ int __init omap3_idle_init(void)
 
 	/* C2 . MPU WFI + Core inactive */
 	FILL_IN_STATE(OMAP3_STATE_C2, "MPU ON + CORE ON")
+	data->valid = 1;	/* C2 is always valid */
 	data->mpu_state = PWRDM_POWER_ON;
 	data->core_state = PWRDM_POWER_ON;
 
@@ -405,11 +401,6 @@ int __init omap3_idle_init(void)
 
 	dev->state_count = count;
 
-	if (enable_off_mode)
-		omap3_cpuidle_update_states(PWRDM_POWER_OFF, PWRDM_POWER_OFF);
-	else
-		omap3_cpuidle_update_states(PWRDM_POWER_RET, PWRDM_POWER_RET);
-
 	if (cpuidle_register_device(dev)) {
 		printk(KERN_ERR "%s: CPUidle register device failed\n",
 		       __func__);
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 32dbc13..45bcfce 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -78,10 +78,6 @@ extern u32 sleep_while_idle;
 #define sleep_while_idle 0
 #endif
 
-#if defined(CONFIG_CPU_IDLE)
-extern void omap3_cpuidle_update_states(u32, u32);
-#endif
-
 #if defined(CONFIG_PM_DEBUG) && defined(CONFIG_DEBUG_FS)
 extern void pm_dbg_update_time(struct powerdomain *pwrdm, int prev);
 extern int pm_dbg_regset_save(int reg_set);
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 0c5e3a4..caf9f6c 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -779,18 +779,6 @@ void omap3_pm_off_mode_enable(int enable)
 	else
 		state = PWRDM_POWER_RET;
 
-#ifdef CONFIG_CPU_IDLE
-	/*
-	 * Erratum i583: implementation for ES rev < Es1.2 on 3630. We cannot
-	 * enable OFF mode in a stable form for previous revisions, restrict
-	 * instead to RET
-	 */
-	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583))
-		omap3_cpuidle_update_states(state, PWRDM_POWER_RET);
-	else
-		omap3_cpuidle_update_states(state, state);
-#endif
-
 	list_for_each_entry(pwrst, &pwrst_list, node) {
 		if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583) &&
 				pwrst->pwrdm == core_pwrdm &&
-- 
1.7.1

[PATCH 5/5] OMAP3: cpuidle: change the power domains modes determination logic

[Santosh Shilimkar]

On 4/29/2011 2:56 PM, jean.pihet at newoldbits.com wrote:
> From: Jean Pihet<j-pihet@ti.com>
>
> The achievable power modes of the power domains in cpuidle
> depends on the system wide 'enable_off_mode' knob in debugfs.
> Upon changing enable_off_mode, do not change the C-states
> 'valid' field but instead dynamically restrict the power modes
> when entering idle.
>
> The C-states 'valid' field is just used to enable/disable some
> C-states at init and shall not be changed later on.
>
> Signed-off-by: Jean Pihet<j-pihet@ti.com>
> ---
>   arch/arm/mach-omap2/cpuidle34xx.c |   59 +++++++++++++++---------------------
>   arch/arm/mach-omap2/pm.h          |    4 --
>   arch/arm/mach-omap2/pm34xx.c      |   12 -------
>   3 files changed, 25 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 4673cc6..45ccfa7 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -147,22 +147,40 @@ return_sleep_time:
>   }
>
>   /**
> - * next_valid_state - Find next valid c-state
> + * next_valid_state - Find next valid C-state
>    * @dev: cpuidle device
> - * @state: Currently selected c-state
> + * @state: Currently selected C-state
>    *
>    * If the current state is valid, it is returned back to the caller.
>    * Else, this function searches for a lower c-state which is still
>    * valid.
> + *
> + * A state is valid if the 'valid' field is enabled and
> + * if it satisfies the enable_off_mode condition.
>    */
>   static struct cpuidle_state *next_valid_state(struct cpuidle_device *dev,
>   					      struct cpuidle_state *curr)
>   {
>   	struct cpuidle_state *next = NULL;
>   	struct omap3_idle_statedata *cx = cpuidle_get_statedata(curr);
> +	u32 mpu_deepest_state = PWRDM_POWER_RET;
> +	u32 core_deepest_state = PWRDM_POWER_RET;
> +
> +	if (enable_off_mode) {
> +		mpu_deepest_state = PWRDM_POWER_OFF;
> +		/*
> +		 * Erratum i583: valable for ES rev<  Es1.2 on 3630.
> +		 * CORE OFF mode is not supported in a stable form, restrict
> +		 * instead the CORE state to RET.
> +		 */
> +		if (!IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583))
> +			core_deepest_state = PWRDM_POWER_OFF;
> +	}
>
Since you have started clean-up, we should get rid of the
"enable_off_mode" some how. I got that done for OMAP4 using
prepare() hook and IGNORE flag as per Kevin's suggestion
but unfortunately its seems to getting removed from
core cpuidle code.

Regards
Santosh

[PATCH 5/5] OMAP3: cpuidle: change the power domains modes determination logic

[Jean Pihet]

Hi Santosh,

On Fri, Apr 29, 2011 at 1:29 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On 4/29/2011 2:56 PM, jean.pihet at newoldbits.com wrote:
>>
>> From: Jean Pihet<j-pihet@ti.com>
>>
>> The achievable power modes of the power domains in cpuidle
>> depends on the system wide 'enable_off_mode' knob in debugfs.
>> Upon changing enable_off_mode, do not change the C-states
>> 'valid' field but instead dynamically restrict the power modes
>> when entering idle.
>>
>> The C-states 'valid' field is just used to enable/disable some
>> C-states at init and shall not be changed later on.
>>
>> Signed-off-by: Jean Pihet<j-pihet@ti.com>
...

> Since you have started clean-up, we should get rid of the
> "enable_off_mode" some how. I got that done for OMAP4 using
> prepare() hook and IGNORE flag as per Kevin's suggestion
> but unfortunately its seems to getting removed from
> core cpuidle code.

The idea is to remove the enable_off_mode knob from debugfs and
instead have the devices constraints API control the power domains
behavior.
Cf. PATCH 0/5 of this series for some remarks about further clean-up work.

Thanks!
Jean

>
> Regards
> Santosh
>