Re: [PATCH v2] fix i2c_pca_pf_waitforcompletion() return value

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

 



Hello Yegor,

On Tue, Sep 21, 2010 at 11:15:47AM +0200, Yegor Yefremov wrote:
> ret is still -1, if during the polling read_byte() returns at once
> with I2C_PCA_CON_SI set. So ret > 0 would lead 
> i2c_pca_pf_waitforcompletion() to return 0, in spite of
> the proper behavior. 
> 
> Solution: get rid of ret and make immediate return where possible.
> 
> Signed-off-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
> 
> Index: b/drivers/i2c/busses/i2c-pca-platform.c
> ===================================================================
> --- a/drivers/i2c/busses/i2c-pca-platform.c	2010-09-21 10:53:46.000000000 +0200
> +++ b/drivers/i2c/busses/i2c-pca-platform.c	2010-09-21 10:59:18.000000000 +0200
> @@ -80,23 +80,26 @@
>  static int i2c_pca_pf_waitforcompletion(void *pd)
>  {
>  	struct i2c_pca_pf_data *i2c = pd;
> -	long ret = ~0;
>  	unsigned long timeout;
>  
>  	if (i2c->irq) {
> -		ret = wait_event_timeout(i2c->wait,
> +		return wait_event_timeout(i2c->wait,
>  			i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
>  			& I2C_PCA_CON_SI, i2c->adap.timeout);
>  	} else {
>  		/* Do polling */
>  		timeout = jiffies + i2c->adap.timeout;
> -		while (((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> -				& I2C_PCA_CON_SI) == 0)
> -				&& (ret = time_before(jiffies, timeout)))
> -			udelay(100);
> +		while (!(i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
> +				& I2C_PCA_CON_SI))
> +		{
> +			if(time_before(jiffies, timeout))
> +				udelay(100);
> +			else
> +				return 0;
> +		}
>  	}
>  
> -	return ret > 0;
> +	return ~0;
>  }
>  
>  static void i2c_pca_pf_dummyreset(void *pd)

Hmm, I think I'd prefer a centralized exit point. What about this? I think it
might be easier to read. Only compile tested, though. The final patch needs to
be applied to the isa version, too. BTW, good debugging there, fixing this old
issue you reported. Thanks a lot!

Regards,

   Wolfram

diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c
index ef5c784..a13aecb 100644
--- a/drivers/i2c/busses/i2c-pca-platform.c
+++ b/drivers/i2c/busses/i2c-pca-platform.c
@@ -80,8 +80,8 @@ static void i2c_pca_pf_writebyte32(void *pd, int reg, int val)
 static int i2c_pca_pf_waitforcompletion(void *pd)
 {
 	struct i2c_pca_pf_data *i2c = pd;
-	long ret = ~0;
 	unsigned long timeout;
+	long ret;
 
 	if (i2c->irq) {
 		ret = wait_event_timeout(i2c->wait,
@@ -90,10 +90,13 @@ static int i2c_pca_pf_waitforcompletion(void *pd)
 	} else {
 		/* Do polling */
 		timeout = jiffies + i2c->adap.timeout;
-		while (((i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
-				& I2C_PCA_CON_SI) == 0)
-				&& (ret = time_before(jiffies, timeout)))
+		do {
+			ret = time_before(jiffies, timeout);
+			if (i2c->algo_data.read_byte(i2c, I2C_PCA_CON)
+				& I2C_PCA_CON_SI)
+				break;
 			udelay(100);
+		} while (ret);
 	}
 
 	return ret > 0;


-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux