> -----Original Message-----
> From: Kevin Hilman [mailto:[email protected]]
> Sent: Friday, March 13, 2009 4:49 AM
> To: Premi, Sanjeev
> Cc: [email protected]
> Subject: Re: [PATCHv2] PM: Start C-state definitions from base 0
>
> Sanjeev Premi <[email protected]> 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 <[email protected]>
> > ---
> > 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 [email protected] 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
