[PATCH] fix PCA waitforcompletion() return value

[PATCH] fix PCA waitforcompletion() return value

[Yegor Yefremov]

ret is still -1, if during the polling read_byte() returns at once
with I2C_PCA_CON_SI set. So ret > 0 would lead *_waitforcompletion()
to return 0, in spite of the proper behavior.

The routine was rewritten, so that ret has always a proper value,
before returning.

Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Index: b/drivers/i2c/busses/i2c-pca-platform.c
===================================================================
--- a/drivers/i2c/busses/i2c-pca-platform.c	2010-09-22 09:31:12.000000000 +0200
+++ b/drivers/i2c/busses/i2c-pca-platform.c	2010-09-22 11:10:58.000000000 +0200
@@ -80,8 +80,8 @@
 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 @@
 	} 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;
Index: b/drivers/i2c/busses/i2c-pca-isa.c
===================================================================
--- a/drivers/i2c/busses/i2c-pca-isa.c	2010-08-27 01:47:12.000000000 +0200
+++ b/drivers/i2c/busses/i2c-pca-isa.c	2010-09-22 11:10:38.000000000 +0200
@@ -71,8 +71,8 @@
 
 static int pca_isa_waitforcompletion(void *pd)
 {
-	long ret = ~0;
 	unsigned long timeout;
+	long ret;
 
 	if (irq > -1) {
 		ret = wait_event_timeout(pca_wait,
@@ -81,11 +81,15 @@
 	} else {
 		/* Do polling */
 		timeout = jiffies + pca_isa_ops.timeout;
-		while (((pca_isa_readbyte(pd, I2C_PCA_CON)
-				& I2C_PCA_CON_SI) == 0)
-				&& (ret = time_before(jiffies, timeout)))
+		do {
+			ret = time_before(jiffies, timeout);
+			if (pca_isa_readbyte(pd, I2C_PCA_CON)
+					& I2C_PCA_CON_SI)
+				break;
 			udelay(100);
+		} while (ret);
 	}
+
 	return ret > 0;
 }
 

Re: [PATCH] fix PCA waitforcompletion() return value

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

On Wed, Sep 22, 2010 at 11:20:48AM +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 *_waitforcompletion()
> to return 0, in spite of the proper behavior.
> 
> The routine was rewritten, so that ret has always a proper value,
> before returning.
> 
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Great! Jean, I think this is .36-relevant. Do you plan another
pull-request (hmm, there is still my clientdata-patch ;))? Should
probably go to stable, too.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] fix PCA waitforcompletion() return value

[Jean Delvare]

Hi Wolfram,

On Wed, 22 Sep 2010 11:31:23 +0200, Wolfram Sang wrote:
> On Wed, Sep 22, 2010 at 11:20:48AM +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 *_waitforcompletion()
> > to return 0, in spite of the proper behavior.
> > 
> > The routine was rewritten, so that ret has always a proper value,
> > before returning.
> > 
> > Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Great! Jean, I think this is .36-relevant. Do you plan another
> pull-request (hmm, there is still my clientdata-patch ;))? Should
> probably go to stable, too.

Yes, I have a few i2c patches scheduled for 2.6.36 and plan to send
Linus a pull request soon. I'm a little busy this week at work, and also
due to a complete workstastion update so I must setup a number of
things again and this temporarily hinders my efficiency.

-- 
Jean Delvare

Re: [PATCH] fix PCA waitforcompletion() return value

[Ben Dooks]

On Wed, Sep 22, 2010 at 12:08:48PM +0200, Jean Delvare wrote:
> Hi Wolfram,
> 
> On Wed, 22 Sep 2010 11:31:23 +0200, Wolfram Sang wrote:
> > On Wed, Sep 22, 2010 at 11:20:48AM +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 *_waitforcompletion()
> > > to return 0, in spite of the proper behavior.
> > > 
> > > The routine was rewritten, so that ret has always a proper value,
> > > before returning.
> > > 
> > > Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > > Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > 
> > Great! Jean, I think this is .36-relevant. Do you plan another
> > pull-request (hmm, there is still my clientdata-patch ;))? Should
> > probably go to stable, too.
> 
> Yes, I have a few i2c patches scheduled for 2.6.36 and plan to send
> Linus a pull request soon. I'm a little busy this week at work, and also
> due to a complete workstastion update so I must setup a number of
> things again and this temporarily hinders my efficiency.

Want me to push it, i've a few fixes too.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH] fix PCA waitforcompletion() return value

[Jean Delvare]

On Wed, 29 Sep 2010 00:08:45 +0100, Ben Dooks wrote:
> On Wed, Sep 22, 2010 at 12:08:48PM +0200, Jean Delvare wrote:
> > Hi Wolfram,
> > 
> > On Wed, 22 Sep 2010 11:31:23 +0200, Wolfram Sang wrote:
> > > On Wed, Sep 22, 2010 at 11:20:48AM +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 *_waitforcompletion()
> > > > to return 0, in spite of the proper behavior.
> > > > 
> > > > The routine was rewritten, so that ret has always a proper value,
> > > > before returning.
> > > > 
> > > > Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> > > > Reviewed-by: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > 
> > > Great! Jean, I think this is .36-relevant. Do you plan another
> > > pull-request (hmm, there is still my clientdata-patch ;))? Should
> > > probably go to stable, too.
> > 
> > Yes, I have a few i2c patches scheduled for 2.6.36 and plan to send
> > Linus a pull request soon. I'm a little busy this week at work, and also
> > due to a complete workstastion update so I must setup a number of
> > things again and this temporarily hinders my efficiency.
> 
> Want me to push it, i've a few fixes too.

No, thanks. It's already in my tree and ready to go to Linus. I know
I'm lagging behind a bit these days but this should get better by the
end of the week.

-- 
Jean Delvare