[U-Boot] [PATCH] Exynos5: i2c: Fix read NACK handling and remove some redundancy

[Minkyu Kang]

On 25/03/13 20:46, Akshay Saraswat wrote:
> From: Gabe Black <gabeblack at google.com>
> 
> The exynos s3c24x0 i2c driver wouldn't do the right thing when a NACK was
> received on a read because it didn't attempt a read before waiting for the
> read to finish. Putting a call to ReadWriteByte in the NACK path fixed a
> problem where getting a NACK reading from a device would jam up the bus and
> prevent future accesses like probing from working.
> 
> Because other than the ReadWriteByte call the NACK and normal paths were
> almost the same thing, and to avoid future instances of the NACK path not
> working because it's not exercised normally, this change also consolidates
> those two paths.
> 
> Signed-off-by: Gabe Black <gabeblack at google.com>
> Signed-off-by: Akshay Saraswat <akshay.s at samsung.com>
> ---
>  drivers/i2c/s3c24x0_i2c.c | 53 ++++++++++++++++-------------------------------
>  1 file changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c
> index d2b4eb0..91298a7 100644
> --- a/drivers/i2c/s3c24x0_i2c.c
> +++ b/drivers/i2c/s3c24x0_i2c.c
> @@ -366,21 +366,25 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
>  		break;
>  
>  	case I2C_READ:
> -		if (result == I2C_OK) {
> -			writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> -			writel(chip, &i2c->iicds);
> -			/* send START */
> -			writel(readl(&i2c->iicstat) | I2C_START_STOP,
> -			       &i2c->iicstat);
> -			ReadWriteByte(i2c);
> -			result = WaitForXfer(i2c);
> +	{
> +		int was_ok = (result == I2C_OK);

do you really need the was_ok?
If not, please remove it.

> +
> +		writel(chip, &i2c->iicds);
> +		/* resend START */
> +		writel(I2C_MODE_MR | I2C_TXRX_ENA |
> +					I2C_START_STOP, &i2c->iicstat);
> +		ReadWriteByte(i2c);
> +		result = WaitForXfer(i2c);
> +
> +		if (was_ok || IsACK(i2c)) {
>  			i = 0;
>  			while ((i < data_len) && (result == I2C_OK)) {
>  				/* disable ACK for final READ */
> -				if (i == data_len - 1)
> -					writel(readl(&i2c->iiccon)
> -							& ~I2CCON_ACKGEN,
> -							&i2c->iiccon);
> +				if (i == data_len - 1) {
> +					writel(readl(&i2c->iiccon) &
> +					      ~I2CCON_ACKGEN,
> +					      &i2c->iiccon);
> +				}
>  				ReadWriteByte(i2c);
>  				result = WaitForXfer(i2c);
>  				data[i] = readl(&i2c->iicds);
> @@ -388,35 +392,14 @@ static int i2c_transfer(struct s3c24x0_i2c *i2c,
>  			}
>  
>  		} else {
> -			writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
> -			writel(chip, &i2c->iicds);
> -			/* send START */
> -			writel(readl(&i2c->iicstat) | I2C_START_STOP,
> -			       &i2c->iicstat);
> -			result = WaitForXfer(i2c);
> -
> -			if (IsACK(i2c)) {
> -				i = 0;
> -				while ((i < data_len) && (result == I2C_OK)) {
> -					/* disable ACK for final READ */
> -					if (i == data_len - 1)
> -						writel(readl(&i2c->iiccon) &
> -							~I2CCON_ACKGEN,
> -							&i2c->iiccon);
> -					ReadWriteByte(i2c);
> -					result = WaitForXfer(i2c);
> -					data[i] = readl(&i2c->iicds);
> -					i++;
> -				}
> -			} else {
> -				result = I2C_NACK;
> -			}
> +			result = I2C_NACK;
>  		}
>  
>  		/* send STOP */
>  		writel(I2C_MODE_MR | I2C_TXRX_ENA, &i2c->iicstat);
>  		ReadWriteByte(i2c);
>  		break;
> +	}
>  
>  	default:
>  		debug("i2c_transfer: bad call\n");
> 

Thanks,
Minkyu Kang.