From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean Delvare Subject: Re: [patch 1/8] I2C: S3C2410: Check ACK on byte transmission Date: Fri, 30 May 2008 21:45:40 +0200 Message-ID: <20080530214540.638008b7@hyperion.delvare> References: <20080529132244.818543231@fluff.org.uk> <20080529132405.971048345@fluff.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20080529132405.971048345-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org Errors-To: i2c-bounces-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org To: Ben Dooks Cc: i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org List-Id: linux-i2c@vger.kernel.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 > > 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