Subject: Re: [PATCH] reorg RTAS delay code

From: Nathan Lynch <[email-masked]>
Date: Fri, 2 Jun 2006 16:33:08 -0500

> +/* For an RTAS busy status code, perform the hinted delay. */
> +unsigned int rtas_busy_delay(int status)
> +{
> +	unsigned int ms;
>  
> -	return ms; 
> +	ms = rtas_busy_delay_time(status);
> +	if (ms)
> +		msleep(ms);
> +
> +	return ms;
>  }

Can you put a might_sleep() at the beginning of this function so that
we can reliably catch unsafe uses of it?  Otherwise we'll get warnings
only when a delay is actually executed.

> @@ -438,22 +449,14 @@ int rtas_get_power_level(int powerdomain
>  int rtas_set_power_level(int powerdomain, int level, int *setlevel)
>  {
>  	int token = rtas_token("set-power-level");
> -	unsigned int wait_time;
>  	int rc;
>  
>  	if (token == RTAS_UNKNOWN_SERVICE)
>  		return -ENOENT;
>  
> -	while (1) {
> +	do
>  		rc = rtas_call(token, 2, 2, setlevel, powerdomain, level);
> -		if (rc == RTAS_BUSY)
> -			udelay(1);
> -		else if (rtas_is_extended_busy(rc)) {
> -			wait_time = rtas_extended_busy_delay_time(rc);
> -			udelay(wait_time * 1000);
> -		} else
> -			break;
> -	}
> +	while (rtas_busy_delay(rc));

Coding style nit -- am I alone in thinking that do/while without the
brackets looks weird?  Given the single-statement body of the loop, I
guess the brackets aren't necessary, but omitting the brackets doesn't
save lines in this case.