RE: [PATCHv2] PM: Start C-state definitions from base 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] 
> Sent: Friday, March 13, 2009 4:49 AM
> To: Premi, Sanjeev
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCHv2] PM: Start C-state definitions from base 0
> 
> Sanjeev Premi <premi@xxxxxx> writes:
> 
> > The current definition of C-states starts from base 1.
> > Whereas, the cpuidle driver uses base 0. This patch eliminates need 
> > for explicit mapping due to different base values.
> >
> > Signed-off-by: Sanjeev Premi <premi@xxxxxx>
> > ---
> >  arch/arm/mach-omap2/cpuidle34xx.c |   23 ++++++++++-------------
> >  1 files changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
> > b/arch/arm/mach-omap2/cpuidle34xx.c
> > index 62fbb2e..50bf412 100644
> > --- a/arch/arm/mach-omap2/cpuidle34xx.c
> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> > @@ -33,13 +33,13 @@
> >  
> >  #ifdef CONFIG_CPU_IDLE
> >  
> > -#define OMAP3_MAX_STATES 7
> > -#define OMAP3_STATE_C1 1 /* C1 - MPU WFI + Core active */ -#define 
> > OMAP3_STATE_C2 2 /* C2 - MPU CSWR + Core active */ -#define 
> > OMAP3_STATE_C3 3 /* C3 - MPU OFF + Core active */ -#define 
> > OMAP3_STATE_C4 4 /* C4 - MPU RET + Core RET */ -#define 
> OMAP3_STATE_C5 
> > 5 /* C5 - MPU OFF + Core RET */ -#define OMAP3_STATE_C6 6 
> /* C6 - MPU 
> > OFF + Core OFF */
> > +#define OMAP3_MAX_STATES 6
> > +#define OMAP3_STATE_C1 0 /* C1 - MPU WFI + Core active */ #define 
> > +OMAP3_STATE_C2 1 /* C2 - MPU CSWR + Core active */ #define 
> > +OMAP3_STATE_C3 2 /* C3 - MPU OFF + Core active */ #define 
> > +OMAP3_STATE_C4 3 /* C4 - MPU RET + Core RET */ #define 
> OMAP3_STATE_C5 
> > +4 /* C5 - MPU OFF + Core RET */ #define OMAP3_STATE_C6 5 
> /* C6 - MPU 
> > +OFF + Core OFF */
> >  
> >  struct omap3_processor_cx {
> >  	u8 valid;
> > @@ -228,7 +228,7 @@ struct cpuidle_driver omap3_idle_driver = {
> >   */
> >  int omap3_idle_init(void)
> >  {
> > -	int i, count = 0;
> > +	int count;
> >  	struct omap3_processor_cx *cx;
> >  	struct cpuidle_state *state;
> >  	struct cpuidle_device *dev;
> > @@ -244,8 +244,8 @@ int omap3_idle_init(void)
> >  
> >  	dev = &per_cpu(omap3_idle_dev, smp_processor_id());
> >  
> > -	for (i = 1; i < OMAP3_MAX_STATES; i++) {
> > -		cx = &omap3_power_states[i];
> > +	for (count = 0; count < OMAP3_MAX_STATES; count++) {
> > +		cx = &omap3_power_states[count];
> >  		state = &dev->states[count];
> >  
> >  		if (!cx->valid)
> 
> Still not quite right.
> 
> The reason for a different loop iterator and 'count' was so 
> that states that are not 'valid' do not get regsitered with 
> CPUidle.  With this patch, some of the dev->states[] pointers 
> that get registered with CPUidle will have bogus values if 
> there are any !valid states at boot time.
> 
> Since you are not fixing the valid state problem in this 
> patch, I suggest you keep the separate iterator and count, 
> and just start i at zero instead of 1.
> 
> Then, when you fix the valid state problem, you can drop the 
> checking for valid sates at init time all together, and then 
> use a single iterator for this loop.

Since the ".valid" for each of the states was 1 in the init
for all states; I did the change above. Will re-submit v3
later today.

~sanjeev

> 
> Kevin
> 
> > @@ -259,11 +259,8 @@ int omap3_idle_init(void)
> >  		if (cx->type == OMAP3_STATE_C1)
> >  			dev->safe_state = state;
> >  		sprintf(state->name, "C%d", count+1);
> > -		count++;
> >  	}
> >  
> > -	if (!count)
> > -		return -EINVAL;
> >  	dev->state_count = count;
> >  
> >  	if (cpuidle_register_device(dev)) {
> > --
> > 1.5.6
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" 
> > in the body of a message to majordomo@xxxxxxxxxxxxxxx More 
> majordomo 
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux