Re: [PATCH 2/2 v2] i2c,algo: handle timeout correctly

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

On Fri, 06 Feb 2009 14:11:46 +0100, Roel Kluin wrote:
> Let's first adress the other issues at hand.
> This patch should be applied on top of my previous cleanup patch

Agreed.

> 
> --------------------------->8---------------------8<---------------------------
> With a postfix decrement these timeouts reach -1 rather than 0, but after the
> loop it is tested whether they have become 0.
> 
> As pointed out by Jean Delvare, the msg_num should be tested before the timeout.
> With the current order, you could exit with a timeout error while all the
> messages were successfully transferred.
> 
> Signed-off-by: Roel Kluin <roel.kluin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
> index ce916d7..2862f11 100644
> --- a/drivers/i2c/algos/i2c-algo-pcf.c
> +++ b/drivers/i2c/algos/i2c-algo-pcf.c
> @@ -115,15 +115,17 @@ static int wait_for_bb(struct i2c_algo_pcf_data *adap)
>  
>  	status = get_pcf(adap, 1);
>  
> -	while (timeout-- && !(status & I2C_PCF_BB)) {
> +	while (!(status & I2C_PCF_BB) && --timeout) {
>  		udelay(100); /* wait for 100 us */
>  		status = get_pcf(adap, 1);
>  	}
>  
> -	if (timeout <= 0)
> +	if (timeout == 0) {
>  		printk(KERN_ERR "Timeout waiting for Bus Busy\n");
> +		return -ETIMEDOUT;
> +	}
>  
> -	return timeout <= 0;
> +	return 0;
>  }
>  
>  static int wait_for_pin(struct i2c_algo_pcf_data *adap, int *status)
> @@ -133,7 +135,7 @@ static int wait_for_pin(struct i2c_algo_pcf_data *adap, int *status)
>  
>  	*status = get_pcf(adap, 1);
>  
> -	while (timeout-- && (*status & I2C_PCF_PIN)) {
> +	while ((*status & I2C_PCF_PIN) && --timeout) {
>  		adap->waitforpin(adap->data);
>  		*status = get_pcf(adap, 1);
>  	}
> @@ -142,10 +144,10 @@ static int wait_for_pin(struct i2c_algo_pcf_data *adap, int *status)
>  		return -EINTR;
>  	}
>  
> -	if (timeout <= 0)
> -		return -1;
> -	else
> -		return 0;
> +	if (timeout == 0)
> +		return -ETIMEDOUT;
> +
> +	return 0;
>  }
>  
>  /*

Applied, thanks. It would be great if Eric (or anyone with a
PCF8584-based adapter) could test the two patches, just to make sure we
didn't accidentally break anything:

ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-algo-pcf-01-style-cleanups.patch
ftp://ftp.kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/i2c-algo-pcf-02-handle-timeout-correctly.patch

-- 
Jean Delvare