i2c-powermac: Refactor i2c_powermac_smbus_xfer

I wanted to add some error logging to the i2c-powermac driver, but found that it was very difficult due to the way the i2c_powermac_smbus_xfer function is organized. Refactor the code in this function so that each low-level function is only called once.

Signed-off-by Jean Delvare <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Cc Paul Mackerras <[email protected]>

---
This needs testing! Thanks.

 drivers/i2c/busses/i2c-powermac.c |   85 +++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 44 deletions(-)

drivers/i2c/busses/i2c-powermac.c +41 -44

@@ -49,48 +49,38 @@ static s32 i2c_powermac_smbus_xfer(	stru
 	int			rc = 0;
 	int			read = (read_write == I2C_SMBUS_READ);
 	int			addrdir = (addr << 1) | read;
+	int			mode, subsize, len;
+	u32			subaddr;
+	u8			*buf;
 	u8			local[2];
 
-	rc = pmac_i2c_open(bus, 0);
-	if (rc)
-		return rc;
+	if (size == I2C_SMBUS_QUICK || size == I2C_SMBUS_BYTE) {
+		mode = pmac_i2c_mode_std;
+		subsize = 0;
+		subaddr = 0;
+	} else {
+		mode = read ? pmac_i2c_mode_combined : pmac_i2c_mode_stdsub;
+		subsize = 1;
+		subaddr = command;
+	}
 
 	switch (size) {
         case I2C_SMBUS_QUICK:
-		rc = pmac_i2c_setmode(bus, pmac_i2c_mode_std);
-		if (rc)
-			goto bail;
-		rc = pmac_i2c_xfer(bus, addrdir, 0, 0, NULL, 0);
+		buf = NULL;
+		len = 0;
 	    	break;
         case I2C_SMBUS_BYTE:
-		rc = pmac_i2c_setmode(bus, pmac_i2c_mode_std);
-		if (rc)
-			goto bail;
-		rc = pmac_i2c_xfer(bus, addrdir, 0, 0, &data->byte, 1);
-	    	break;
         case I2C_SMBUS_BYTE_DATA:
-		rc = pmac_i2c_setmode(bus, read ?
-				      pmac_i2c_mode_combined :
-				      pmac_i2c_mode_stdsub);
-		if (rc)
-			goto bail;
-		rc = pmac_i2c_xfer(bus, addrdir, 1, command, &data->byte, 1);
+		buf = &data->byte;
+		len = 1;
 	    	break;
         case I2C_SMBUS_WORD_DATA:
-		rc = pmac_i2c_setmode(bus, read ?
-				      pmac_i2c_mode_combined :
-				      pmac_i2c_mode_stdsub);
-		if (rc)
-			goto bail;
 		if (!read) {
 			local[0] = data->word & 0xff;
 			local[1] = (data->word >> 8) & 0xff;
 		}
-		rc = pmac_i2c_xfer(bus, addrdir, 1, command, local, 2);
-		if (rc == 0 && read) {
-			data->word = ((u16)local[1]) << 8;
-			data->word |= local[0];
-		}
+		buf = local;
+		len = 2;
 	    	break;
 
 	/* Note that these are broken vs. the expected smbus API where

@@ -105,28 +95,35 @@ static s32 i2c_powermac_smbus_xfer(	stru
 	 * a repeat start/addr phase (but not stop in between)
 	 */
         case I2C_SMBUS_BLOCK_DATA:
-		rc = pmac_i2c_setmode(bus, read ?
-				      pmac_i2c_mode_combined :
-				      pmac_i2c_mode_stdsub);
-		if (rc)
-			goto bail;
-		rc = pmac_i2c_xfer(bus, addrdir, 1, command, data->block,
-				   data->block[0] + 1);
-
+		buf = data->block;
+		len = data->block[0] + 1;
 		break;
 	case I2C_SMBUS_I2C_BLOCK_DATA:
-		rc = pmac_i2c_setmode(bus, read ?
-				      pmac_i2c_mode_combined :
-				      pmac_i2c_mode_stdsub);
-		if (rc)
-			goto bail;
-		rc = pmac_i2c_xfer(bus, addrdir, 1, command,
-				   &data->block[1], data->block[0]);
+		buf = &data->block[1];
+		len = data->block[0];
 		break;
 
         default:
-	    	rc = -EINVAL;
+		return -EINVAL;
+	}
+
+	rc = pmac_i2c_open(bus, 0);
+	if (rc)
+		return rc;
+
+	rc = pmac_i2c_setmode(bus, mode);
+	if (rc)
+		goto bail;
+
+	rc = pmac_i2c_xfer(bus, addrdir, subsize, subaddr, buf, len);
+	if (rc)
+		goto bail;
+
+	if (size == I2C_SMBUS_WORD_DATA && read) {
+		data->word = ((u16)local[1]) << 8;
+		data->word |= local[0];
 	}
+
  bail:
 	pmac_i2c_close(bus);
 	return rc;

Quoted 5 lines · Jean Delvare

I wanted to add some error logging to the i2c-powermac driver, but found that it was very difficult due to the way the i2c_powermac_smbus_xfer function is organized. Refactor the code in this function so that each low-level function is only called once.

Quoted 5 lines

Signed-off-by Jean Delvare <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Cc Paul Mackerras <[email protected]>

---
This needs testing! Thanks.

Ok, will give it a go next week.