[RFC] I2C-MPC: Fix up error handling

[Jean Delvare]

Hi all,

Could anyone please comment on this i2c-mpc patch, and give it some
testing on different systems? I'm fine with the error propagation
fixes, but less fine with the random resets being added, and rather
unhappy with the "retry once more just in case" thing, which I think,
if really needed, should instead be implemented using the standard
i2c_adapter.retries mechanism.

Thanks.

From: Kumar Gala <galak at kernel.crashing.org>

* If we have an Unfinished (MCF) or Arbitration Lost (MAL) error and
  the bus is still busy reset the controller.  This prevents the
  controller from getting in a hung state for transactions for other
  devices.

* Fixed up propogating the errors from i2c_wait.

Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
Cc: Jean Delvare <khali at linux-fr.org>
Cc: Greg KH <greg at kroah.com>
Signed-off-by: Andrew Morton <akpm at osdl.org>
---

 drivers/i2c/busses/i2c-mpc.c |   43 +++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff -puN drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling drivers/i2c/busses/i2c-mpc.c
--- devel/drivers/i2c/busses/i2c-mpc.c~i2c-mpc-fix-up-error-handling	2006-06-01 20:22:55.000000000 -0700
+++ devel-akpm/drivers/i2c/busses/i2c-mpc.c	2006-06-01 20:22:55.000000000 -0700
@@ -115,11 +115,20 @@ static int i2c_wait(struct mpc_i2c *i2c,
 
 	if (!(x & CSR_MCF)) {
 		pr_debug("I2C: unfinished\n");
+
+		/* reset the controller if the bus is still busy */
+		if (x & CSR_MBB)
+			writeccr(i2c, 0);
+
 		return -EIO;
 	}
 
 	if (x & CSR_MAL) {
 		pr_debug("I2C: MAL\n");
+
+		/* reset the controller if the bus is still busy */
+		if (x & CSR_MBB)
+			writeccr(i2c, 0);
 		return -EIO;
 	}
 
@@ -160,7 +169,7 @@ static void mpc_i2c_stop(struct mpc_i2c 
 static int mpc_write(struct mpc_i2c *i2c, int target,
 		     const u8 * data, int length, int restart)
 {
-	int i;
+	int i, ret;
 	unsigned timeout = i2c->adap.timeout;
 	u32 flags = restart ? CCR_RSTA : 0;
 
@@ -172,15 +181,17 @@ static int mpc_write(struct mpc_i2c *i2c
 	/* Write target byte */
 	writeb((target << 1), i2c->base + MPC_I2C_DR);
 
-	if (i2c_wait(i2c, timeout, 1) < 0)
-		return -1;
+	ret = i2c_wait(i2c, timeout, 1);
+	if (ret < 0)
+		return ret;
 
 	for (i = 0; i < length; i++) {
 		/* Write data byte */
 		writeb(data[i], i2c->base + MPC_I2C_DR);
 
-		if (i2c_wait(i2c, timeout, 1) < 0)
-			return -1;
+		ret = i2c_wait(i2c, timeout, 1);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -190,7 +201,7 @@ static int mpc_read(struct mpc_i2c *i2c,
 		    u8 * data, int length, int restart)
 {
 	unsigned timeout = i2c->adap.timeout;
-	int i;
+	int i, ret;
 	u32 flags = restart ? CCR_RSTA : 0;
 
 	/* Start with MEN */
@@ -201,8 +212,9 @@ static int mpc_read(struct mpc_i2c *i2c,
 	/* Write target address byte - this time with the read flag set */
 	writeb((target << 1) | 1, i2c->base + MPC_I2C_DR);
 
-	if (i2c_wait(i2c, timeout, 1) < 0)
-		return -1;
+	ret = i2c_wait(i2c, timeout, 1);
+	if (ret < 0)
+		return ret;
 
 	if (length) {
 		if (length == 1)
@@ -214,8 +226,9 @@ static int mpc_read(struct mpc_i2c *i2c,
 	}
 
 	for (i = 0; i < length; i++) {
-		if (i2c_wait(i2c, timeout, 0) < 0)
-			return -1;
+		ret = i2c_wait(i2c, timeout, 0);
+		if (ret < 0)
+			return ret;
 
 		/* Generate txack on next to last byte */
 		if (i == length - 2)
@@ -246,8 +259,13 @@ static int mpc_xfer(struct i2c_adapter *
 			return -EINTR;
 		}
 		if (time_after(jiffies, orig_jiffies + HZ)) {
-			pr_debug("I2C: timeout\n");
-			return -EIO;
+			writeccr(i2c, 0);
+
+			/* try one more time before we error */
+			if (readb(i2c->base + MPC_I2C_SR) & CSR_MBB) {
+				pr_debug("I2C: timeout\n");
+				return -EIO;
+			}
 		}
 		schedule();
 	}
@@ -325,6 +343,7 @@ static int fsl_i2c_probe(struct platform
 			goto fail_irq;
 		}
 
+	writeccr(i2c, 0);
 	mpc_i2c_setclock(i2c);
 	platform_set_drvdata(pdev, i2c);
 
_


-- 
Jean Delvare