[i2c] [patch 2.6.23-rc6] i2c-algo-bit whitespace fixes (+ NAK/ARB comments)

David Brownell david-b at pacbell.net
Fri Sep 14 19:58:12 CEST 2007


Update comments related to NAKs and arbitration ... both of which
are handled rather dubiously.  Best to think of arbitration as not
yet supported.

Also fix *LOTS* of whitespace glitches, and a couple of strangely
parenthisized ternary expressions.  This driver now conforms much
more closely to kernel CodingStyle.

Signed-off-by: David Brownell <dbrownell at users.sourceforge.net>
---
 drivers/i2c/algos/i2c-algo-bit.c |  187 +++++++++++++++++++++------------------
 1 files changed, 104 insertions(+), 83 deletions(-)

--- a/drivers/i2c/algos/i2c-algo-bit.c	2007-09-14 10:46:01.000000000 -0700
+++ b/drivers/i2c/algos/i2c-algo-bit.c	2007-09-14 10:50:23.000000000 -0700
@@ -1,7 +1,7 @@
-/* ------------------------------------------------------------------------- */
-/* i2c-algo-bit.c i2c driver algorithms for bit-shift adapters		     */
-/* ------------------------------------------------------------------------- */
-/*   Copyright (C) 1995-2000 Simon G. Vogl
+/* -------------------------------------------------------------------------
+ * i2c-algo-bit.c i2c driver algorithms for bit-shift adapters
+ * -------------------------------------------------------------------------
+ *   Copyright (C) 1995-2000 Simon G. Vogl
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -15,8 +15,8 @@
 
     You should have received a copy of the GNU General Public License
     along with this program; if not, write to the Free Software
-    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.		     */
-/* ------------------------------------------------------------------------- */
+    Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ * ------------------------------------------------------------------------- */
 
 /* With some changes from Frodo Looijaard <frodol at dds.nl>, Ky?sti M?lkki
    <kmalkki at cc.hut.fi> and Jean Delvare <khali at linux-fr.org> */
@@ -97,13 +97,13 @@ static int sclhi(struct i2c_algo_bit_dat
 	if (!adap->getscl)
 		goto done;
 
-	start=jiffies;
-	while (! getscl(adap) ) {	
- 		/* the hw knows how to read the clock line,
- 		 * so we wait until it actually gets high.
- 		 * This is safer as some chips may hold it low
- 		 * while they are processing data internally. 
- 		 */
+	start = jiffies;
+	while (!getscl(adap)) {
+		/* the hw knows how to read the clock line,
+		 * so we wait until it actually gets high.
+		 * This is safer as some chips may hold it low
+		 * while they are processing data internally.
+		 */
 		if (time_after_eq(jiffies, start+adap->timeout)) {
 			return -ETIMEDOUT;
 		}
@@ -118,11 +118,11 @@ static int sclhi(struct i2c_algo_bit_dat
 done:
 	udelay(adap->udelay);
 	return 0;
-} 
+}
 
 
 /* --- other auxiliary functions --------------------------------------	*/
-static void i2c_start(struct i2c_algo_bit_data *adap) 
+static void i2c_start(struct i2c_algo_bit_data *adap)
 {
 	/* assert: scl, sda are high */
 	setsda(adap, 0);
@@ -130,7 +130,7 @@ static void i2c_start(struct i2c_algo_bi
 	scllo(adap);
 }
 
-static void i2c_repstart(struct i2c_algo_bit_data *adap) 
+static void i2c_repstart(struct i2c_algo_bit_data *adap)
 {
 	/* assert: scl is low */
 	sdahi(adap);
@@ -141,18 +141,18 @@ static void i2c_repstart(struct i2c_algo
 }
 
 
-static void i2c_stop(struct i2c_algo_bit_data *adap) 
+static void i2c_stop(struct i2c_algo_bit_data *adap)
 {
 	/* assert: scl is low */
 	sdalo(adap);
-	sclhi(adap); 
+	sclhi(adap);
 	setsda(adap, 1);
 	udelay(adap->udelay);
 }
 
 
 
-/* send a byte without start cond., look for arbitration, 
+/* send a byte without start cond., look for arbitration,
    check ackn. from slave */
 /* returns:
  * 1 if the device acknowledged
@@ -167,7 +167,7 @@ static int i2c_outb(struct i2c_adapter *
 	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
 
 	/* assert: scl is low */
-	for ( i=7 ; i>=0 ; i-- ) {
+	for (i = 7; i >= 0; i--) {
 		sb = (c >> i) & 1;
 		setsda(adap,sb);
 		udelay((adap->udelay + 1) / 2);
@@ -176,8 +176,8 @@ static int i2c_outb(struct i2c_adapter *
 				"timeout at bit #%d\n", (int)c, i);
 			return -ETIMEDOUT;
 		};
-		/* do arbitration here: 
-		 * if ( sb && ! getsda(adap) ) -> ouch! Get out of here.
+		/* do arbitration here:
+		 * if (sb && ! getsda(adap)) -> ouch! Get out of here.
 		 */
 		scllo(adap);
 	}
@@ -187,7 +187,9 @@ static int i2c_outb(struct i2c_adapter *
 			"timeout at ack\n", (int)c);
 		return -ETIMEDOUT;
 	};
-	/* read ack: SDA should be pulled down by slave */
+	/* read ack: SDA should be pulled down by slave, or it may
+	 * NAK (usually to report problems with the data we wrote).
+	 */
 	ack = !getsda(adap);    /* ack: sda is pulled low -> success */
 	bit_dbg(2, &i2c_adap->dev, "i2c_outb: 0x%02x %s\n", (int)c,
 		ack ? "A" : "NA");
@@ -198,24 +200,24 @@ static int i2c_outb(struct i2c_adapter *
 }
 
 
-static int i2c_inb(struct i2c_adapter *i2c_adap) 
+static int i2c_inb(struct i2c_adapter *i2c_adap)
 {
 	/* read byte via i2c port, without start/stop sequence	*/
 	/* acknowledge is sent in i2c_read.			*/
 	int i;
-	unsigned char indata=0;
+	unsigned char indata = 0;
 	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
 
 	/* assert: scl is low */
 	sdahi(adap);
-	for (i=0;i<8;i++) {
+	for (i = 0; i < 8; i++) {
 		if (sclhi(adap)<0) { /* timeout */
 			bit_dbg(1, &i2c_adap->dev, "i2c_inb: timeout at bit "
 				"#%d\n", 7 - i);
 			return -ETIMEDOUT;
 		};
 		indata *= 2;
-		if ( getsda(adap) ) 
+		if (getsda(adap))
 			indata |= 0x01;
 		setscl(adap, 0);
 		udelay(i == 7 ? adap->udelay / 2 : adap->udelay);
@@ -231,65 +233,65 @@ static int i2c_inb(struct i2c_adapter *i
 static int test_bus(struct i2c_algo_bit_data *adap, char* name) {
 	int scl,sda;
 
-	if (adap->getscl==NULL)
+	if (adap->getscl == NULL)
 		pr_info("%s: Testing SDA only, SCL is not readable\n", name);
 
-	sda=getsda(adap);
-	scl=(adap->getscl==NULL?1:getscl(adap));
-	if (!scl || !sda ) {
+	sda = getsda(adap);
+	scl = (adap->getscl == NULL) ? 1 : getscl(adap);
+	if (!scl || !sda) {
 		printk(KERN_WARNING "%s: bus seems to be busy\n", name);
 		goto bailout;
 	}
 
 	sdalo(adap);
-	sda=getsda(adap);
-	scl=(adap->getscl==NULL?1:getscl(adap));
-	if ( 0 != sda ) {
+	sda = getsda(adap);
+	scl = (adap->getscl == NULL) ? 1 : getscl(adap);
+	if (0 != sda) {
 		printk(KERN_WARNING "%s: SDA stuck high!\n", name);
 		goto bailout;
 	}
-	if ( 0 == scl ) {
+	if (0 == scl) {
 		printk(KERN_WARNING "%s: SCL unexpected low "
-		       "while pulling SDA low!\n", name);
+				"while pulling SDA low!\n", name);
 		goto bailout;
-	}		
+	}
 
 	sdahi(adap);
-	sda=getsda(adap);
-	scl=(adap->getscl==NULL?1:getscl(adap));
-	if ( 0 == sda ) {
+	sda = getsda(adap);
+	scl = (adap->getscl == NULL) ? 1 : getscl(adap);
+	if (0 == sda) {
 		printk(KERN_WARNING "%s: SDA stuck low!\n", name);
 		goto bailout;
 	}
-	if ( 0 == scl ) {
+	if (0 == scl) {
 		printk(KERN_WARNING "%s: SCL unexpected low "
-		       "while pulling SDA high!\n", name);
+				"while pulling SDA high!\n", name);
 		goto bailout;
 	}
 
 	scllo(adap);
-	sda=getsda(adap);
-	scl=(adap->getscl==NULL?0:getscl(adap));
-	if ( 0 != scl ) {
+	sda = getsda(adap);
+	scl = (adap->getscl == NULL?0:getscl(adap));
+	if (0 != scl) {
 		printk(KERN_WARNING "%s: SCL stuck high!\n", name);
 		goto bailout;
 	}
-	if ( 0 == sda ) {
+	if (0 == sda) {
 		printk(KERN_WARNING "%s: SDA unexpected low "
-		       "while pulling SCL low!\n", name);
+				"while pulling SCL low!\n", name);
 		goto bailout;
 	}
-	
+
 	sclhi(adap);
-	sda=getsda(adap);
-	scl=(adap->getscl==NULL?1:getscl(adap));
-	if ( 0 == scl ) {
+	sda = getsda(adap);
+	scl = (adap->getscl == NULL?1:getscl(adap));
+	if (0 == scl) {
 		printk(KERN_WARNING "%s: SCL stuck low!\n", name);
 		goto bailout;
 	}
-	if ( 0 == sda ) {
+	if (0 == sda) {
 		printk(KERN_WARNING "%s: SDA unexpected low "
-		       "while pulling SCL high!\n", name);
+				"while pulling SCL high!\n", name);
 		goto bailout;
 	}
 	pr_info("%s: Test OK\n", name);
@@ -311,11 +313,11 @@ bailout:
  * -x transmission error
  */
 static int try_address(struct i2c_adapter *i2c_adap,
-		       unsigned char addr, int retries)
+		unsigned char addr, int retries)
 {
 	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
 	int i,ret = -1;
-	for (i=0;i<=retries;i++) {
+	for (i = 0; i <= retries; i++) {
 		ret = i2c_outb(i2c_adap,addr);
 		if (ret == 1 || i == retries)
 			break;
@@ -338,18 +340,38 @@ static int sendbytes(struct i2c_adapter 
 {
 	const unsigned char *temp = msg->buf;
 	int count = msg->len;
-	unsigned short nak_ok = msg->flags & I2C_M_IGNORE_NAK; 
+	unsigned short nak_ok = msg->flags & I2C_M_IGNORE_NAK;
 	int retval;
-	int wrcount=0;
+	int wrcount = 0;
 
 	while (count > 0) {
 		retval = i2c_outb(i2c_adap, *temp);
-		if ((retval>0) || (nak_ok && (retval==0)))  { /* ok or ignored NAK */
-			count--; 
+		/* ok or ignored NAK */
+		if ((retval > 0) || (nak_ok && (retval == 0))) {
+			count--;
 			temp++;
 			wrcount++;
-		} else { /* arbitration or no acknowledge */
-			dev_err(&i2c_adap->dev, "sendbytes: error - bailout.\n");
+		} else {
+			/* lost arbitration; or byte was NAKed
+			 *
+			 * FIXME lost ARB implies retrying the transaction
+			 * from the first message, after the "winning" master.
+			 * issues its STOP.  As a rule, upper layer code has
+			 * no reason to know or care about this..
+			 *
+			 * On the other hand, a slave NAKing the master means
+			 * the slave didn't like something about the data it
+			 * saw.  (For example, maybe the SMBus PEC was wrong.)
+			 * That might mean the transaction should be aborted
+			 * immediately (no more messages); and the failure
+			 * should certainly reported to the message submitter.
+			 *
+			 * In short, these two faults should NOT be reported
+			 * as being the same... the "saving" grace here being
+			 * that this code doesn't handle arbitration anyway.
+			 */
+			dev_err(&i2c_adap->dev, "sendbytes: NAK, or"
+					"or lost arbitration; bailout.\n");
 			return (retval<0)? retval : -EFAULT;
 			        /* got a better one ?? */
 		}
@@ -376,14 +398,14 @@ static int acknak(struct i2c_adapter *i2
 static int readbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
 {
 	int inval;
-	int rdcount=0;   	/* counts bytes read */
+	int rdcount = 0;	/* counts bytes read */
 	unsigned char *temp = msg->buf;
 	int count = msg->len;
 	const unsigned flags = msg->flags;
 
 	while (count > 0) {
 		inval = i2c_inb(i2c_adap);
-		if (inval>=0) {
+		if (inval >= 0) {
 			*temp = inval;
 			rdcount++;
 		} else {   /* read timed out */
@@ -394,7 +416,7 @@ static int readbytes(struct i2c_adapter 
 		count--;
 
 		/* Some SMBus transactions require that we receive the
-		   transaction length as the first read byte. */
+		 * transaction length as the first read byte. */
 		if (rdcount == 1 && (flags & I2C_M_RECV_LEN)) {
 			if (inval <= 0 || inval > I2C_SMBUS_BLOCK_MAX) {
 				if (!(flags & I2C_M_NO_RD_ACK))
@@ -404,8 +426,8 @@ static int readbytes(struct i2c_adapter 
 				return -EREMOTEIO;
 			}
 			/* The original count value accounts for the extra
-			   bytes, that is, either 1 for a regular transaction,
-			   or 2 for a PEC transaction. */
+			 * bytes, that is, either 1 for a regular transaction,
+			 * or 2 for a PEC transaction. */
 			count += inval;
 			msg->len += inval;
 		}
@@ -431,7 +453,7 @@ static int readbytes(struct i2c_adapter 
  * returns:
  *  0 everything went okay, the chip ack'ed, or IGNORE_NAK flag was set
  * -x an error occurred (like: -EREMOTEIO if the device did not answer, or
- *	-ETIMEDOUT, for example if the lines are stuck...) 
+ *	-ETIMEDOUT, for example if the lines are stuck...)
  */
 static int bit_doAddress(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
 {
@@ -443,10 +465,10 @@ static int bit_doAddress(struct i2c_adap
 	int ret, retries;
 
 	retries = nak_ok ? 0 : i2c_adap->retries;
-	
-	if ( (flags & I2C_M_TEN)  ) { 
+
+	if ((flags & I2C_M_TEN)) {
 		/* a ten bit address */
-		addr = 0xf0 | (( msg->addr >> 7) & 0x03);
+		addr = 0xf0 | ((msg->addr >> 7) & 0x03);
 		bit_dbg(2, &i2c_adap->dev, "addr0: %d\n", addr);
 		/* try extended address code...*/
 		ret = try_address(i2c_adap, addr, retries);
@@ -462,7 +484,7 @@ static int bit_doAddress(struct i2c_adap
 			dev_err(&i2c_adap->dev, "died at 2nd address code\n");
 			return -EREMOTEIO;
 		}
-		if ( flags & I2C_M_RD ) {
+		if (flags & I2C_M_RD) {
 			bit_dbg(3, &i2c_adap->dev, "emitting repeated "
 				"start condition\n");
 			i2c_repstart(adap);
@@ -476,13 +498,13 @@ static int bit_doAddress(struct i2c_adap
 			}
 		}
 	} else {		/* normal 7bit address	*/
-		addr = ( msg->addr << 1 );
-		if (flags & I2C_M_RD )
+		addr = (msg->addr << 1);
+		if (flags & I2C_M_RD)
 			addr |= 1;
-		if (flags & I2C_M_REV_DIR_ADDR )
+		if (flags & I2C_M_REV_DIR_ADDR)
 			addr ^= 1;
 		ret = try_address(i2c_adap, addr, retries);
-		if ((ret!=1) && !nak_ok)
+		if ((ret != 1) && !nak_ok)
 			return -EREMOTEIO;
 	}
 
@@ -490,19 +512,18 @@ static int bit_doAddress(struct i2c_adap
 }
 
 static int bit_xfer(struct i2c_adapter *i2c_adap,
-		    struct i2c_msg msgs[], int num)
+		struct i2c_msg msgs[], int num)
 {
 	struct i2c_msg *pmsg;
 	struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
-	
 	int i,ret;
 	unsigned short nak_ok;
 
 	bit_dbg(3, &i2c_adap->dev, "emitting start condition\n");
 	i2c_start(adap);
-	for (i=0;i<num;i++) {
+	for (i = 0; i < num; i++) {
 		pmsg = &msgs[i];
-		nak_ok = pmsg->flags & I2C_M_IGNORE_NAK; 
+		nak_ok = pmsg->flags & I2C_M_IGNORE_NAK;
 		if (!(pmsg->flags & I2C_M_NOSTART)) {
 			if (i) {
 				bit_dbg(3, &i2c_adap->dev, "emitting "
@@ -517,7 +538,7 @@ static int bit_xfer(struct i2c_adapter *
 				goto bailout;
 			}
 		}
-		if (pmsg->flags & I2C_M_RD ) {
+		if (pmsg->flags & I2C_M_RD) {
 			/* read bytes into buffer*/
 			ret = readbytes(i2c_adap, pmsg);
 			if (ret >= 1)
@@ -551,7 +572,7 @@ bailout:
 
 static u32 bit_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | 
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
 	       I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
@@ -565,8 +586,8 @@ static const struct i2c_algorithm i2c_bi
 	.functionality	= bit_func,
 };
 
-/* 
- * registering functions to load algorithms at runtime 
+/*
+ * registering functions to load algorithms at runtime
  */
 static int i2c_bit_prepare_bus(struct i2c_adapter *adap)
 {



More information about the i2c mailing list