[PATCHv2 1/1] omap3: cpuidle: Update statistics for correct state

[PATCHv2 1/1] omap3: cpuidle: Update statistics for correct state

[Sanjeev Premi]

When 'enable_off_mode' is 0, the target power state for MPU
and CORE was locally changed to PWRDM_POWER_RET but, the
statistics are updated for idle state originally selected
by the governor.

This patch 'invalidates' the idle states that lead either of
MPU or Core to PWRDM_POWER_OFF state when 'enable_off_mode'
is '0'. The states are valid once 'enable_off_mode' is set
to '1'.

Added function next_valid_state() to check if current state
is valid; else get the next valid state. It is called from
omap3_enter_idle_bm().

Signed-off-by: Sanjeev Premi <premi@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   98 ++++++++++++++++++++++++++++++++++---
 arch/arm/mach-omap2/pm.h          |    1 +
 arch/arm/mach-omap2/pm34xx.c      |    4 ++
 3 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index fdfa1d5..308865c 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -45,6 +45,8 @@
 #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
+
 struct omap3_processor_cx {
 	u8 valid;
 	u8 type;
@@ -128,13 +130,6 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	local_irq_disable();
 	local_fiq_disable();
 
-	if (!enable_off_mode) {
-		if (mpu_state < PWRDM_POWER_RET)
-			mpu_state = PWRDM_POWER_RET;
-		if (core_state < PWRDM_POWER_RET)
-			core_state = PWRDM_POWER_RET;
-	}
-
 	pwrdm_set_next_pwrst(mpu_pd, mpu_state);
 	pwrdm_set_next_pwrst(core_pd, core_state);
 
@@ -164,6 +159,65 @@ return_sleep_time:
 	return (u32)timespec_to_ns(&ts_idle)/1000;
 }
 
+
+/* next_valid_state - Find next valid c-state
+ * @dev: cpuidle device
+ * @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 (as defined in omap3_power_states[]).
+ */
+static struct cpuidle_state* next_valid_state(struct cpuidle_device *dev,
+						struct cpuidle_state *curr)
+{
+	struct cpuidle_state *next = NULL;
+	struct omap3_processor_cx *cx;
+
+	cx = (struct omap3_processor_cx *)cpuidle_get_statedata(curr);
+
+	/* Check if current state is valid */
+	if (cx->valid) {
+		next = curr;
+	}
+	else {
+		u8 idx = OMAP3_STATE_MAX;
+
+		/* Reach the current state starting at highest C-state */
+		for (; idx >= OMAP3_STATE_C1; idx--) {
+			if (&dev->states[idx] == curr) {
+				next = &dev->states[idx];
+				break;
+			}
+		}
+
+		/*
+		 * Should never hit this condition.
+		 */
+		BUG_ON(next == NULL);
+
+		/* Drop to next valid state.
+		 * Start search from the next (lower) state.
+		 */
+		idx--;
+		for (; idx >= OMAP3_STATE_C1; idx--) {
+			if (((struct omap3_processor_cx *)
+				cpuidle_get_statedata(
+					&dev->states[idx]))->valid) {
+				next = &dev->states[idx];
+				break;
+			}
+		}
+		/*
+		 * C1 and C2 are always valid.
+		 * So, no need to check for 'next==NULL' outside this loop.
+		 */
+	}
+
+	return next;
+}
+
+
 /**
  * omap3_enter_idle_bm - Checks for any bus activity
  * @dev: cpuidle device
@@ -176,7 +230,7 @@ return_sleep_time:
 static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
 {
-	struct cpuidle_state *new_state = state;
+	struct cpuidle_state *new_state= next_valid_state(dev, state);
 
 	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
 		BUG_ON(!dev->safe_state);
@@ -207,6 +261,32 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
 	return;
 }
 
+/**
+ * omap3_cpuidle_update_states - Update the cpuidle states.
+ *
+ * Currently, this function toggles the validity of idle states based upon
+ * the flag 'enable_off_mode'. When the flag is set all states are valid.
+ * Else, states leading to OFF state set to be invalid.
+ */
+void omap3_cpuidle_update_states(void)
+{
+	int i;
+
+	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
+		if (enable_off_mode) {
+			omap3_power_states[i].valid = 1;
+		}
+		else {
+			if ((omap3_power_states[i].mpu_state
+					== PWRDM_POWER_OFF)
+				|| (omap3_power_states[i].core_state
+					== PWRDM_POWER_OFF))
+			omap3_power_states[i].valid = 0;
+		}
+	}
+}
+
+
 /* omap3_init_power_states - Initialises the OMAP3 specific C states.
  *
  * Below is the desciption of each C state.
@@ -365,6 +445,8 @@ int __init omap3_idle_init(void)
 		return -EINVAL;
 	dev->state_count = count;
 
+	omap3_cpuidle_update_states();
+
 	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 b9421e8..9351490 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -51,6 +51,7 @@ struct cpuidle_params {
 extern void omap3_pm_init_vc(struct prm_setup_vc *setup_vc);
 #ifdef CONFIG_CPU_IDLE
 extern void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params);
+extern void omap3_cpuidle_update_states(void);
 #else
 static inline void omap3_pm_init_cpuidle(
 			struct cpuidle_params *cpuidle_board_params)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 90c4949..d9f0626 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -981,6 +981,10 @@ void omap3_pm_off_mode_enable(int enable)
 	else
 		state = PWRDM_POWER_RET;
 
+#ifdef CONFIG_CPU_IDLE
+	omap3_cpuidle_update_states();
+#endif
+
 #ifdef CONFIG_OMAP_PM_SRF
 	resource_lock_opp(VDD1_OPP);
 	resource_lock_opp(VDD2_OPP);
-- 
1.6.2.2

Re: [PATCHv2 1/1] omap3: cpuidle: Update statistics for correct state

[Kevin Hilman]

Sanjeev Premi <premi@ti.com> writes:

> When 'enable_off_mode' is 0, the target power state for MPU
> and CORE was locally changed to PWRDM_POWER_RET but, the
> statistics are updated for idle state originally selected
> by the governor.
>
> This patch 'invalidates' the idle states that lead either of
> MPU or Core to PWRDM_POWER_OFF state when 'enable_off_mode'
> is '0'. The states are valid once 'enable_off_mode' is set
> to '1'.
>
> Added function next_valid_state() to check if current state
> is valid; else get the next valid state. It is called from
> omap3_enter_idle_bm().
>
> Signed-off-by: Sanjeev Premi <premi@ti.com>

Hi Sanjeev,

I quite like this approach, but have a few minor comments.  First,
please fix the checkpatch errors and warnings, as well as a few minor
things below...

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   98 ++++++++++++++++++++++++++++++++++---
>  arch/arm/mach-omap2/pm.h          |    1 +
>  arch/arm/mach-omap2/pm34xx.c      |    4 ++
>  3 files changed, 95 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index fdfa1d5..308865c 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -45,6 +45,8 @@
>  #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
> +
>  struct omap3_processor_cx {
>  	u8 valid;
>  	u8 type;
> @@ -128,13 +130,6 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
>  	local_irq_disable();
>  	local_fiq_disable();
>  
> -	if (!enable_off_mode) {
> -		if (mpu_state < PWRDM_POWER_RET)
> -			mpu_state = PWRDM_POWER_RET;
> -		if (core_state < PWRDM_POWER_RET)
> -			core_state = PWRDM_POWER_RET;
> -	}
> -
>  	pwrdm_set_next_pwrst(mpu_pd, mpu_state);
>  	pwrdm_set_next_pwrst(core_pd, core_state);
>  
> @@ -164,6 +159,65 @@ return_sleep_time:
>  	return (u32)timespec_to_ns(&ts_idle)/1000;
>  }
>  
> +
> +/* next_valid_state - Find next valid c-state
> + * @dev: cpuidle device
> + * @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 (as defined in omap3_power_states[]).
> + */

Please fix kerneldoc style.   Opening comment should be '/**'

> +static struct cpuidle_state* next_valid_state(struct cpuidle_device *dev,
> +						struct cpuidle_state *curr)
> +{
> +	struct cpuidle_state *next = NULL;
> +	struct omap3_processor_cx *cx;
> +
> +	cx = (struct omap3_processor_cx *)cpuidle_get_statedata(curr);
> +
> +	/* Check if current state is valid */
> +	if (cx->valid) {
> +		next = curr;

could just return curr here

> +	}
> +	else {
> +		u8 idx = OMAP3_STATE_MAX;
> +
> +		/* Reach the current state starting at highest C-state */
> +		for (; idx >= OMAP3_STATE_C1; idx--) {
> +			if (&dev->states[idx] == curr) {
> +				next = &dev->states[idx];
> +				break;
> +			}
> +		}
> +
> +		/*
> +		 * Should never hit this condition.
> +		 */
> +		BUG_ON(next == NULL);

This shouldn't trigger a panic.  Please fail more cleanly using WARN_ON()

> +		/* Drop to next valid state.
> +		 * Start search from the next (lower) state.
> +		 */

fix multiline comment.  Should start with '/*' on its own line.
(c.f. Documentation/CodingStyle, search for 'multi-line')

> +		idx--;
> +		for (; idx >= OMAP3_STATE_C1; idx--) {
> +			if (((struct omap3_processor_cx *)
> +				cpuidle_get_statedata(
> +					&dev->states[idx]))->valid) {
> +				next = &dev->states[idx];
> +				break;
> +			}
> +		}

Due to line-wrapping etc., this has readabiliyt problems.  Consider this
instead:

		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];
				break;
			}
		}

> +		/*
> +		 * C1 and C2 are always valid.
> +		 * So, no need to check for 'next==NULL' outside this loop.
> +		 */
> +	}
> +
> +	return next;
> +}
> +
> +
>  /**
>   * omap3_enter_idle_bm - Checks for any bus activity
>   * @dev: cpuidle device
> @@ -176,7 +230,7 @@ return_sleep_time:
>  static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  			       struct cpuidle_state *state)
>  {
> -	struct cpuidle_state *new_state = state;
> +	struct cpuidle_state *new_state= next_valid_state(dev, state);
                                       ^

missing space

>  
>  	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
>  		BUG_ON(!dev->safe_state);
> @@ -207,6 +261,32 @@ void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params)
>  	return;
>  }
>  
> +/**
> + * omap3_cpuidle_update_states - Update the cpuidle states.
> + *
> + * Currently, this function toggles the validity of idle states based upon
> + * the flag 'enable_off_mode'. When the flag is set all states are valid.
> + * Else, states leading to OFF state set to be invalid.
> + */
> +void omap3_cpuidle_update_states(void)
> +{
> +	int i;
> +
> +	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
> +		if (enable_off_mode) {
> +			omap3_power_states[i].valid = 1;
> +		}
> +		else {
> +			if ((omap3_power_states[i].mpu_state
> +					== PWRDM_POWER_OFF)
> +				|| (omap3_power_states[i].core_state
> +					== PWRDM_POWER_OFF))
> +			omap3_power_states[i].valid = 0;
> +		}
> +	}
> +}

more readability problems due to wrapping.  consider:

void omap3_cpuidle_update_states(void)
{
	int i;

	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
		struct omap3_processor_cx *cx = &omap3_power_states[i]

		if (enable_off_mode)
			cx->valid = 1;
		else
			if ((cx->mpu_state == PWRDM_POWER_OFF) ||
			    (cx->core_state == PWRDM_POWER_OFF))
				cx->valid = 0;
	}
}

Kevin


> +
>  /* omap3_init_power_states - Initialises the OMAP3 specific C states.
>   *
>   * Below is the desciption of each C state.
> @@ -365,6 +445,8 @@ int __init omap3_idle_init(void)
>  		return -EINVAL;
>  	dev->state_count = count;
>  
> +	omap3_cpuidle_update_states();
> +
>  	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 b9421e8..9351490 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -51,6 +51,7 @@ struct cpuidle_params {
>  extern void omap3_pm_init_vc(struct prm_setup_vc *setup_vc);
>  #ifdef CONFIG_CPU_IDLE
>  extern void omap3_pm_init_cpuidle(struct cpuidle_params *cpuidle_board_params);
> +extern void omap3_cpuidle_update_states(void);
>  #else
>  static inline void omap3_pm_init_cpuidle(
>  			struct cpuidle_params *cpuidle_board_params)
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 90c4949..d9f0626 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -981,6 +981,10 @@ void omap3_pm_off_mode_enable(int enable)
>  	else
>  		state = PWRDM_POWER_RET;
>  
> +#ifdef CONFIG_CPU_IDLE
> +	omap3_cpuidle_update_states();
> +#endif
> +
>  #ifdef CONFIG_OMAP_PM_SRF
>  	resource_lock_opp(VDD1_OPP);
>  	resource_lock_opp(VDD2_OPP);
> -- 
> 1.6.2.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv2 1/1] omap3: cpuidle: Update statistics for correct state

[Premi, Sanjeev]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@deeprootsystems.com] 
> Sent: Tuesday, January 26, 2010 1:25 AM
> To: Premi, Sanjeev
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCHv2 1/1] omap3: cpuidle: Update statistics 
> for correct state
> 
> Sanjeev Premi <premi@ti.com> writes:
> 
> > When 'enable_off_mode' is 0, the target power state for MPU
> > and CORE was locally changed to PWRDM_POWER_RET but, the
> > statistics are updated for idle state originally selected
> > by the governor.
> >
> > This patch 'invalidates' the idle states that lead either of
> > MPU or Core to PWRDM_POWER_OFF state when 'enable_off_mode'
> > is '0'. The states are valid once 'enable_off_mode' is set
> > to '1'.
> >
> > Added function next_valid_state() to check if current state
> > is valid; else get the next valid state. It is called from
> > omap3_enter_idle_bm().
> >
> > Signed-off-by: Sanjeev Premi <premi@ti.com>
> 
> Hi Sanjeev,
> 
> I quite like this approach, but have a few minor comments.  First,
> please fix the checkpatch errors and warnings, as well as a few minor
> things below...
> 
> > ---
> >  arch/arm/mach-omap2/cpuidle34xx.c |   98 
> ++++++++++++++++++++++++++++++++++---
> >  arch/arm/mach-omap2/pm.h          |    1 +
> >  arch/arm/mach-omap2/pm34xx.c      |    4 ++
> >  3 files changed, 95 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
> b/arch/arm/mach-omap2/cpuidle34xx.c
> > index fdfa1d5..308865c 100644
> > --- a/arch/arm/mach-omap2/cpuidle34xx.c
> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> > @@ -45,6 +45,8 @@
> >  #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
> > +
> >  struct omap3_processor_cx {
> >  	u8 valid;
> >  	u8 type;
> > @@ -128,13 +130,6 @@ static int omap3_enter_idle(struct 
> cpuidle_device *dev,
> >  	local_irq_disable();
> >  	local_fiq_disable();
> >  
> > -	if (!enable_off_mode) {
> > -		if (mpu_state < PWRDM_POWER_RET)
> > -			mpu_state = PWRDM_POWER_RET;
> > -		if (core_state < PWRDM_POWER_RET)
> > -			core_state = PWRDM_POWER_RET;
> > -	}
> > -
> >  	pwrdm_set_next_pwrst(mpu_pd, mpu_state);
> >  	pwrdm_set_next_pwrst(core_pd, core_state);
> >  
> > @@ -164,6 +159,65 @@ return_sleep_time:
> >  	return (u32)timespec_to_ns(&ts_idle)/1000;
> >  }
> >  
> > +
> > +/* next_valid_state - Find next valid c-state
> > + * @dev: cpuidle device
> > + * @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 (as defined in omap3_power_states[]).
> > + */
> 
> Please fix kerneldoc style.   Opening comment should be '/**'
> 
> > +static struct cpuidle_state* next_valid_state(struct 
> cpuidle_device *dev,
> > +						struct 
> cpuidle_state *curr)
> > +{
> > +	struct cpuidle_state *next = NULL;
> > +	struct omap3_processor_cx *cx;
> > +
> > +	cx = (struct omap3_processor_cx *)cpuidle_get_statedata(curr);
> > +
> > +	/* Check if current state is valid */
> > +	if (cx->valid) {
> > +		next = curr;
> 
> could just return curr here
> 
> > +	}
> > +	else {
> > +		u8 idx = OMAP3_STATE_MAX;
> > +
> > +		/* Reach the current state starting at highest 
> C-state */
> > +		for (; idx >= OMAP3_STATE_C1; idx--) {
> > +			if (&dev->states[idx] == curr) {
> > +				next = &dev->states[idx];
> > +				break;
> > +			}
> > +		}
> > +
> > +		/*
> > +		 * Should never hit this condition.
> > +		 */
> > +		BUG_ON(next == NULL);
> 
> This shouldn't trigger a panic.  Please fail more cleanly 
> using WARN_ON()
> 
> > +		/* Drop to next valid state.
> > +		 * Start search from the next (lower) state.
> > +		 */
> 
> fix multiline comment.  Should start with '/*' on its own line.
> (c.f. Documentation/CodingStyle, search for 'multi-line')
> 
> > +		idx--;
> > +		for (; idx >= OMAP3_STATE_C1; idx--) {
> > +			if (((struct omap3_processor_cx *)
> > +				cpuidle_get_statedata(
> > +					&dev->states[idx]))->valid) {
> > +				next = &dev->states[idx];
> > +				break;
> > +			}
> > +		}
> 
> Due to line-wrapping etc., this has readabiliyt problems.  
> Consider this
> instead:
> 
> 		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];
> 				break;
> 			}
> 		}
> 
> > +		/*
> > +		 * C1 and C2 are always valid.
> > +		 * So, no need to check for 'next==NULL' 
> outside this loop.
> > +		 */
> > +	}
> > +
> > +	return next;
> > +}
> > +
> > +
> >  /**
> >   * omap3_enter_idle_bm - Checks for any bus activity
> >   * @dev: cpuidle device
> > @@ -176,7 +230,7 @@ return_sleep_time:
> >  static int omap3_enter_idle_bm(struct cpuidle_device *dev,
> >  			       struct cpuidle_state *state)
> >  {
> > -	struct cpuidle_state *new_state = state;
> > +	struct cpuidle_state *new_state= next_valid_state(dev, state);
>                                        ^
> 
> missing space
> 
> >  
> >  	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && 
> omap3_idle_bm_check()) {
> >  		BUG_ON(!dev->safe_state);
> > @@ -207,6 +261,32 @@ void omap3_pm_init_cpuidle(struct 
> cpuidle_params *cpuidle_board_params)
> >  	return;
> >  }
> >  
> > +/**
> > + * omap3_cpuidle_update_states - Update the cpuidle states.
> > + *
> > + * Currently, this function toggles the validity of idle 
> states based upon
> > + * the flag 'enable_off_mode'. When the flag is set all 
> states are valid.
> > + * Else, states leading to OFF state set to be invalid.
> > + */
> > +void omap3_cpuidle_update_states(void)
> > +{
> > +	int i;
> > +
> > +	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
> > +		if (enable_off_mode) {
> > +			omap3_power_states[i].valid = 1;
> > +		}
> > +		else {
> > +			if ((omap3_power_states[i].mpu_state
> > +					== PWRDM_POWER_OFF)
> > +				|| (omap3_power_states[i].core_state
> > +					== PWRDM_POWER_OFF))
> > +			omap3_power_states[i].valid = 0;
> > +		}
> > +	}
> > +}
> 
> more readability problems due to wrapping.  consider:
> 
> void omap3_cpuidle_update_states(void)
> {
> 	int i;
> 
> 	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
> 		struct omap3_processor_cx *cx = &omap3_power_states[i]
> 
> 		if (enable_off_mode)
> 			cx->valid = 1;
> 		else
> 			if ((cx->mpu_state == PWRDM_POWER_OFF) ||
> 			    (cx->core_state == PWRDM_POWER_OFF))
> 				cx->valid = 0;
> 	}
> }
> 
> Kevin

Thanks for comments Kevin.

Updated just started to test the updated patch. Will be posting soon.

Best regards,
Sanjeev

> 
> 
> > +
> >  /* omap3_init_power_states - Initialises the OMAP3 
> specific C states.
> >   *
> >   * Below is the desciption of each C state.
> > @@ -365,6 +445,8 @@ int __init omap3_idle_init(void)
> >  		return -EINVAL;
> >  	dev->state_count = count;
> >  
> > +	omap3_cpuidle_update_states();
> > +
> >  	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 b9421e8..9351490 100644
> > --- a/arch/arm/mach-omap2/pm.h
> > +++ b/arch/arm/mach-omap2/pm.h
> > @@ -51,6 +51,7 @@ struct cpuidle_params {
> >  extern void omap3_pm_init_vc(struct prm_setup_vc *setup_vc);
> >  #ifdef CONFIG_CPU_IDLE
> >  extern void omap3_pm_init_cpuidle(struct cpuidle_params 
> *cpuidle_board_params);
> > +extern void omap3_cpuidle_update_states(void);
> >  #else
> >  static inline void omap3_pm_init_cpuidle(
> >  			struct cpuidle_params *cpuidle_board_params)
> > diff --git a/arch/arm/mach-omap2/pm34xx.c 
> b/arch/arm/mach-omap2/pm34xx.c
> > index 90c4949..d9f0626 100644
> > --- a/arch/arm/mach-omap2/pm34xx.c
> > +++ b/arch/arm/mach-omap2/pm34xx.c
> > @@ -981,6 +981,10 @@ void omap3_pm_off_mode_enable(int enable)
> >  	else
> >  		state = PWRDM_POWER_RET;
> >  
> > +#ifdef CONFIG_CPU_IDLE
> > +	omap3_cpuidle_update_states();
> > +#endif
> > +
> >  #ifdef CONFIG_OMAP_PM_SRF
> >  	resource_lock_opp(VDD1_OPP);
> >  	resource_lock_opp(VDD2_OPP);
> > -- 
> > 1.6.2.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>