Re: [patch 1/8] I2C: S3C2410: Check ACK on byte transmission

[Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>]

Hi Ben,

On Thu, 29 May 2008 14:22:45 +0100, Ben Dooks wrote:
> We should check for the reception of an ACK after transmissint each

Spelling: transmitting.

> data byte. The address send has been correctly checking this, but the
> data write byte state should have also been checking for these failures.
> 
> As part of the same fix, we remove the ACK checking from the receive
> path where it should not have been checking for an ACK which our hardware
> was sending.
> 
> Signed-off-by: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
> 
> Index: linux-2.6.26-rc4-quilt1/drivers/i2c/busses/i2c-s3c2410.c
> ===================================================================
> --- linux-2.6.26-rc4-quilt1.orig/drivers/i2c/busses/i2c-s3c2410.c	2008-05-28 11:49:18.000000000 +0100
> +++ linux-2.6.26-rc4-quilt1/drivers/i2c/busses/i2c-s3c2410.c	2008-05-28 11:56:14.000000000 +0100
> @@ -324,6 +324,15 @@ static int i2s_s3c_irq_nextbyte(struct s
>  		 */
>  
>  	retry_write:
> +		if (!(i2c->msg->flags & I2C_M_IGNORE_NAK)) {
> +			if (iicstat & S3C2410_IICSTAT_LASTBIT) {
> +				dev_dbg(i2c->dev, "WRITE: No Ack\n");
> +
> +				s3c24xx_i2c_stop(i2c, -ECONNREFUSED);
> +				goto out_ack;
> +			}
> +		}
> +

Do you really want to check this again when you jump to retry_write? I
doubt it. My guess is that you want to add the code before the
retry_write label, rather than after it.

>  		if (!is_msgend(i2c)) {
>  			byte = i2c->msg->buf[i2c->msg_ptr++];
>  			writeb(byte, i2c->regs + S3C2410_IICDS);
> @@ -377,17 +386,6 @@ static int i2s_s3c_irq_nextbyte(struct s
>  		 * going to do any more read/write
>  		 */
>  
> -		if (!(i2c->msg->flags & I2C_M_IGNORE_NAK) &&
> -		    !(is_msglast(i2c) && is_lastmsg(i2c))) {
> -
> -			if (iicstat & S3C2410_IICSTAT_LASTBIT) {
> -				dev_dbg(i2c->dev, "READ: No Ack\n");
> -
> -				s3c24xx_i2c_stop(i2c, -ECONNREFUSED);
> -				goto out_ack;
> -			}
> -		}
> -
>  		byte = readb(i2c->regs + S3C2410_IICDS);
>  		i2c->msg->buf[i2c->msg_ptr++] = byte;
>  
> 


-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c