Hi Jouni,

one quick review comment:

On Fri, 14 Nov 2008, Jouni Hogander wrote:

> Check that wanted sleep state is supported by powerdomain. If it is
> not supported, then use next lowest supported state.
> 
> Check also on suspend that state of pwrdm was lower or equal.

...

> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index da098d2..babead5 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -239,8 +239,13 @@ static int set_pwrdm_state(struct powerdomain *pwrdm, 
> u32 state)
>       if (pwrdm == NULL || IS_ERR(pwrdm))
>               return -EINVAL;
>  
> -     cur_state = pwrdm_read_next_pwrst(pwrdm);
> +     while (!(pwrdm->pwrsts & (1 << state))) {
> +             if (state < PWRDM_POWER_OFF)
> +                     return ret;

This if-statement will never execute, since 'state' is unsigned, and 
PWRDM_POWER_OFF is 0.  Maybe the easiest way to fix it would be to make 
'state' a signed int?

Other than that, looks good to me.


> +             state--;
> +     }
>  
> +     cur_state = pwrdm_read_next_pwrst(pwrdm);
>       if (cur_state == state)
>               return ret;


- Paul
--
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

Reply via email to