[patch 07/12] i2c-algo-bit: Read block data bugfix

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: <linux-kernel@...>, <stable@...>
Cc: Justin Forbes <jmforbes@...>, Zwane Mwaikambo <zwane@...>, Theodore Ts'o <tytso@...>, Randy Dunlap <rdunlap@...>, Dave Jones <davej@...>, Chuck Wolber <chuckw@...>, Chris Wedgwood <reviews@...>, Michael Krufky <mkrufky@...>, Chuck Ebbert <cebbert@...>, Domenico Andreoli <cavokz@...>, <torvalds@...>, <akpm@...>, <alan@...>, David Brownell <david-b@...>, David Brownell <dbrownell@...>, Jean Delvare <khali@...>
Date: Monday, October 8, 2007 - 2:06 pm

From: David Brownell <david-b@pacbell.net>

In Linus tree already:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=939bc4943d...

This fixes a bug in the way i2c-algo-bit handles I2C_M_RECV_LEN,
used to implement i2c_smbus_read_block_data().  Previously, in the
absence of PEC (rarely used!) it would NAK the "length" byte:

	S addr Rd [A] [length] NA

That prevents the subsequent data bytes from being read:

	S addr Rd [A] [length] { A [data] }* NA

The primary fix just reorders two code blocks, so the length used
in the "should I NAK now?" check incorporates the data which it
just read from the slave device.

However, that move also highlighted other fault handling glitches.
This fixes those by abstracting the RX path ack/nak logic, so it
can be used in more than one location.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Jean Delvare <khali@linux-fr.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/i2c/algos/i2c-algo-bit.c |   52 ++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 20 deletions(-)

--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -357,13 +357,29 @@ static int sendbytes(struct i2c_adapter 
 	return wrcount;
 }
 
+static int acknak(struct i2c_adapter *i2c_adap, int is_ack)
+{
+	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
+
+	/* assert: sda is high */
+	if (is_ack)		/* send ack */
+		setsda(adap, 0);
+	udelay((adap->udelay + 1) / 2);
+	if (sclhi(adap) < 0) {	/* timeout */
+		dev_err(&i2c_adap->dev, "readbytes: ack/nak timeout\n");
+		return -ETIMEDOUT;
+	}
+	scllo(adap);
+	return 0;
+}
+
 static int readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
 {
 	int inval;
 	int rdcount=0;   	/* counts bytes read */
-	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
 	unsigned char *temp = msg->buf;
 	int count = msg->len;
+	const unsigned flags = msg->flags;
 
 	while (count > 0) {
 		inval = i2c_inb(i2c_adap);
@@ -377,28 +393,12 @@ static int readbytes(struct i2c_adapter 
 		temp++;
 		count--;
 
-		if (msg->flags & I2C_M_NO_RD_ACK) {
-			bit_dbg(2, &i2c_adap->dev, "i2c_inb: 0x%02x\n",
-				inval);
-			continue;
-		}
-
-		/* assert: sda is high */
-		if (count)		/* send ack */
-			setsda(adap, 0);
-		udelay((adap->udelay + 1) / 2);
-		bit_dbg(2, &i2c_adap->dev, "i2c_inb: 0x%02x %s\n", inval,
-			count ? "A" : "NA");
-		if (sclhi(adap)<0) {	/* timeout */
-			dev_err(&i2c_adap->dev, "readbytes: timeout at ack\n");
-			return -ETIMEDOUT;
-		};
-		scllo(adap);
-
 		/* Some SMBus transactions require that we receive the
 		   transaction length as the first read byte. */
-		if (rdcount == 1 && (msg->flags & I2C_M_RECV_LEN)) {
+		if (rdcount == 1 && (flags & I2C_M_RECV_LEN)) {
 			if (inval <= 0 || inval > I2C_SMBUS_BLOCK_MAX) {
+				if (!(flags & I2C_M_NO_RD_ACK))
+					acknak(i2c_adap, 0);
 				dev_err(&i2c_adap->dev, "readbytes: invalid "
 					"block length (%d)\n", inval);
 				return -EREMOTEIO;
@@ -409,6 +409,18 @@ static int readbytes(struct i2c_adapter 
 			count += inval;
 			msg->len += inval;
 		}
+
+		bit_dbg(2, &i2c_adap->dev, "readbytes: 0x%02x %s\n",
+			inval,
+			(flags & I2C_M_NO_RD_ACK)
+				? "(no ack/nak)"
+				: (count ? "A" : "NA"));
+
+		if (!(flags & I2C_M_NO_RD_ACK)) {
+			inval = acknak(i2c_adap, count);
+			if (inval < 0)
+				return inval;
+		}
 	}
 	return rdcount;
 }

-- 
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[patch 00/12] 2.6.22.10 -stable review, Greg KH, (Mon Oct 8, 2:05 pm)
Re: [patch 00/12] 2.6.22.10 -stable review, Greg KH, (Mon Oct 8, 2:09 pm)
[patch 09/12] Fix SMP poweroff hangs, Greg KH, (Mon Oct 8, 2:06 pm)
Re: [patch 09/12] Fix SMP poweroff hangs, Olof Johansson, (Tue Oct 9, 11:17 am)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Thomas Gleixner, (Tue Oct 9, 7:21 pm)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Linus Torvalds, (Tue Oct 9, 7:27 pm)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Paul Mackerras, (Wed Oct 10, 8:24 pm)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Olof Johansson, (Tue Oct 9, 8:03 pm)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Thomas Gleixner, (Tue Oct 9, 7:35 pm)
Re: [stable] [patch 09/12] Fix SMP poweroff hangs, Linus Torvalds, (Wed Oct 10, 1:29 am)
[patch 07/12] i2c-algo-bit: Read block data bugfix, Greg KH, (Mon Oct 8, 2:06 pm)
[patch 06/12] Fix ppp_mppe kernel stack usage., Greg KH, (Mon Oct 8, 2:06 pm)
[patch 05/12] libata: update drive blacklists, Greg KH, (Mon Oct 8, 2:06 pm)