[RFC] I2C-MPC: Fix up error handling

[Kumar Gala]

On Jul 30, 2006, at 6:14 AM, Jean Delvare wrote:

> Quoting myself:
>> 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.
>
> Can anyone please comment on this patch? Andrew Morton keeps  
> forwarding
> it to me, but I don't think I can accept it as is (see my complaints
> above.)

If I find some time, let me split out the error propagation so we can  
get that fix up stream.  The other code was to handle a buggy device  
that was connected in one system I was working on.

- kumar

>> 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