[i2c] [PATCH for 2.6.18-mm3][RFC] cleanup of i2c-nforce2, 2nd iteration

Hans-Frieder Vogt hfvogt at gmx.net
Tue Oct 10 23:52:44 CEST 2006


Hi Jean, hi list,

as announced in my mail that I wrote a few minutes ago, here is another patch 
to fix and cleanup the i2c-nforce2 I2C bus driver. This patch introduces a 
fix and generally cleans up the driver (thanks to Jean for his suggestions 
and support). However, the main feature is an enabled block data transfer, 
that allows up to 31 (unfortunately not 32) byte transfer.
I have created the patch against 2.6.18-mm3, but of course I can provide it 
also against 2.6.19-mm1, if not this is the same anyway.

Here is a summary of the changes:

- fixes:
   o legacy I/O region size is 64 bytes, not 8 bytes
- general cleanup:
   o removed code for the unsupported I2C block transfer mode
   o removed detail warnings about unsupported modes that are
     covered in a general warning (unsupported transaction...)
     anyway
   o removed necessity of a definition of struct i2c_adapter
   o moved definition of struct i2c_algorithm, making forward
     declarations of nforce2_access and nforce2_func unnecessary
   o introduces new function nforce2_check_status to better structure the       
      status checking and to avoid duplication of code
   o changed algorithm for status checking to make it less rigid and to enable 
      distinguishing between failed transfers and timeouts (although the 
      consequences are at the moment the same: a freezing SMBus)
   o introduce a check for the block data transfer length and gracefully fail 
      for requested transfer lengths <= 0 and > 31. This is to avoid a 
      freezing SMBus for these cases
- minor changes:
   o changes my e-mail address in MODULE_AUTHOR


 Signed-off-by: Hans-Frieder Vogt <hfvogt at gmx.net>

 i2c-nforce2.c |  133 
++++++++++++++++++++++++++++++----------------------------
 1 file changed, 69 insertions(+), 64 deletions(-)

--- linux-2.6.18-mm3/drivers/i2c/busses/i2c-nforce2.c	2006-10-08 
13:10:38.177378328 +0200
+++ linux-2.6.18-mm3-test/drivers/i2c/busses/i2c-nforce2.c	2006-10-08 
23:58:47.466748493 +0200
@@ -30,12 +30,12 @@
     nForce3 Pro150 MCP		00D4
     nForce3 250Gb MCP		00E4
     nForce4 MCP			0052
-    nForce4 MCP-04		0034
-    nForce4 MCP51		0264
-    nForce4 MCP55		0368
+    nForce MCP04		0034
+    nForce MCP51		0264
+    nForce MCP55		0368
 
     This driver supports the 2 SMBuses that are included in the MCP of the
-    nForce2/3/4 chipsets.
+    nForce2/3/4/5xx chipsets.
 */
 
 /* Note: we assume there can only be one nForce2, with two SMBus interfaces 
*/
@@ -52,8 +52,8 @@
 #include <asm/io.h>
 
 MODULE_LICENSE("GPL");
-MODULE_AUTHOR ("Hans-Frieder Vogt <hfvogt at arcor.de>");
-MODULE_DESCRIPTION("nForce2 SMBus driver");
+MODULE_AUTHOR ("Hans-Frieder Vogt <hfvogt at gmx.net>");
+MODULE_DESCRIPTION("nForce2/3/4/5xx SMBus driver");
 
 
 struct nforce2_smbus {
@@ -98,29 +98,37 @@ struct nforce2_smbus {
 #define NVIDIA_SMB_PRTCL_BLOCK_DATA		0x0a
 #define NVIDIA_SMB_PRTCL_PROC_CALL		0x0c
 #define NVIDIA_SMB_PRTCL_BLOCK_PROC_CALL	0x0d
-#define NVIDIA_SMB_PRTCL_I2C_BLOCK_DATA		0x4a
 #define NVIDIA_SMB_PRTCL_PEC			0x80
 
+/* Misc definitions */
+#define MAX_TIMEOUT	1000
+
+
 static struct pci_driver nforce2_driver;
 
-static s32 nforce2_access(struct i2c_adapter *adap, u16 addr,
-		       unsigned short flags, char read_write,
-		       u8 command, int size, union i2c_smbus_data *data);
-static u32 nforce2_func(struct i2c_adapter *adapter);
 
+static int nforce2_check_status (struct i2c_adapter * adap) {
+	struct nforce2_smbus *smbus = adap->algo_data;
+	int timeout = 0;
+	unsigned char temp;
 
-static const struct i2c_algorithm smbus_algorithm = {
-	.smbus_xfer = nforce2_access,
-	.functionality = nforce2_func,
-};
+	do {
+		msleep(1);
+		temp = inb_p(NVIDIA_SMB_STS);
+	} while ((!temp) && (timeout++ < MAX_TIMEOUT));
+
+	if ((~temp & NVIDIA_SMB_STS_DONE) && (timeout >= MAX_TIMEOUT)) {
+		dev_dbg(&adap->dev, "SMBus Timeout (0x%02x)!\n", temp);
+		return -1;
+	}
+	if ((~temp & NVIDIA_SMB_STS_DONE) || (temp & NVIDIA_SMB_STS_STATUS)) {
+		dev_dbg(&adap->dev, "Transaction failed (0x%02x)!\n", temp);
+		return -1;
+	}
+	return 0;
+}
 
-static struct i2c_adapter nforce2_adapter = {
-	.owner          = THIS_MODULE,
-	.class          = I2C_CLASS_HWMON,
-	.algo           = &smbus_algorithm,
-};
 
-/* Return -1 on error. See smbus.h for more information */
 static s32 nforce2_access(struct i2c_adapter * adap, u16 addr,
 		unsigned short flags, char read_write,
 		u8 command, int size, union i2c_smbus_data * data)
@@ -164,34 +172,42 @@ static s32 nforce2_access(struct i2c_ada
 			break;
 
 		case I2C_SMBUS_BLOCK_DATA:
-			outb_p(command, NVIDIA_SMB_CMD);
 			if (read_write == I2C_SMBUS_WRITE) {
-				len = min_t(u8, data->block[0], 32);
+				outb_p(command, NVIDIA_SMB_CMD);
+				len = data->block[0];
+				/* nforce2+ SMBus controller seems to support
+				   only lengths between 1 and 31 */
+				if ((len == 0) || (len > 0x1f)) {
+					dev_err(&adap->dev, "Transaction failed (requested block size: 
0x%02x)\n", len);
+					return -1;
+				}
 				outb_p(len, NVIDIA_SMB_BCNT);
 				for (i = 0; i < len; i++)
 					outb_p(data->block[i + 1], NVIDIA_SMB_DATA+i);
+			} else {
+				/* Work around to prevent the smbus to block in
+				   case of requests for blocks sizes with 0 and
+				   > 31 bytes.
+				   These requests are still not accepted, but
+				   at least now they gracefully fail */
+				outb_p(command, NVIDIA_SMB_CMD);
+				outb_p((addr & 0x7f) << 1, NVIDIA_SMB_ADDR);
+				outb_p(NVIDIA_SMB_PRTCL_BYTE_DATA |
+				       NVIDIA_SMB_PRTCL_READ, NVIDIA_SMB_PRTCL);
+				if (nforce2_check_status(adap))
+					return -1;
+				temp = inb_p(NVIDIA_SMB_DATA);
+				if ((temp == 0) || (temp > 0x1f)) {
+					dev_err(&adap->dev, "Transaction failed (requested block size: 
0x%02x)\n", temp);
+					return -1;
+				}
+				/* Work around finished, therefore continue with
+				   requested function */
+				outb_p(command, NVIDIA_SMB_CMD);
 			}
 			protocol |= NVIDIA_SMB_PRTCL_BLOCK_DATA | pec;
 			break;
 
-		case I2C_SMBUS_I2C_BLOCK_DATA:
-			len = min_t(u8, data->block[0], 32);
-			outb_p(command, NVIDIA_SMB_CMD);
-			outb_p(len, NVIDIA_SMB_BCNT);
-			if (read_write == I2C_SMBUS_WRITE)
-				for (i = 0; i < len; i++)
-					outb_p(data->block[i + 1], NVIDIA_SMB_DATA+i);
-			protocol |= NVIDIA_SMB_PRTCL_I2C_BLOCK_DATA;
-			break;
-
-		case I2C_SMBUS_PROC_CALL:
-			dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
-			return -1;
-
-		case I2C_SMBUS_BLOCK_PROC_CALL:
-			dev_err(&adap->dev, "I2C_SMBUS_BLOCK_PROC_CALL not supported!\n");
-			return -1;
-
 		default:
 			dev_err(&adap->dev, "Unsupported transaction %d\n", size);
 			return -1;
@@ -200,21 +216,8 @@ static s32 nforce2_access(struct i2c_ada
 	outb_p((addr & 0x7f) << 1, NVIDIA_SMB_ADDR);
 	outb_p(protocol, NVIDIA_SMB_PRTCL);
 
-	temp = inb_p(NVIDIA_SMB_STS);
-
-	if (~temp & NVIDIA_SMB_STS_DONE) {
-		udelay(500);
-		temp = inb_p(NVIDIA_SMB_STS);
-	}
-	if (~temp & NVIDIA_SMB_STS_DONE) {
-		msleep(10);
-		temp = inb_p(NVIDIA_SMB_STS);
-	}
-
-	if ((~temp & NVIDIA_SMB_STS_DONE) || (temp & NVIDIA_SMB_STS_STATUS)) {
-		dev_dbg(&adap->dev, "SMBus Timeout! (0x%02x)\n", temp);
+	if (nforce2_check_status(adap))
 		return -1;
-	}
 
 	if (read_write == I2C_SMBUS_WRITE)
 		return 0;
@@ -227,15 +230,11 @@ static s32 nforce2_access(struct i2c_ada
 			break;
 
 		case I2C_SMBUS_WORD_DATA:
-		/* case I2C_SMBUS_PROC_CALL: not supported */
 			data->word = inb_p(NVIDIA_SMB_DATA) | (inb_p(NVIDIA_SMB_DATA+1) << 8);
 			break;
 
 		case I2C_SMBUS_BLOCK_DATA:
-		/* case I2C_SMBUS_BLOCK_PROC_CALL: not supported */
-			len = inb_p(NVIDIA_SMB_BCNT);
-			len = min_t(u8, len, 32);
-		case I2C_SMBUS_I2C_BLOCK_DATA:
+			len = inb_p(NVIDIA_SMB_BCNT) & 0x1f;
 			for (i = 0; i < len; i++)
 				data->block[i+1] = inb_p(NVIDIA_SMB_DATA + i);
 			data->block[0] = len;
@@ -250,10 +249,14 @@ static u32 nforce2_func(struct i2c_adapt
 {
 	/* other functionality might be possible, but is not tested */
 	return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
-	    I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA /* |
-	    I2C_FUNC_SMBUS_BLOCK_DATA */;
+	    I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA |
+	    I2C_FUNC_SMBUS_BLOCK_DATA;
 }
 
+static struct i2c_algorithm smbus_algorithm = {
+	.smbus_xfer = nforce2_access,
+	.functionality = nforce2_func,
+};
 
 static struct pci_device_id nforce2_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE2_SMBUS) },
@@ -291,7 +294,7 @@ static int __devinit nforce2_probe_smb (
 		}
 
 		smbus->base = iobase & PCI_BASE_ADDRESS_IO_MASK;
-		smbus->size = 8;
+		smbus->size = 64;
 	}
 	smbus->dev = dev;
 
@@ -300,7 +303,9 @@ static int __devinit nforce2_probe_smb (
 			smbus->base, smbus->base+smbus->size-1, name);
 		return -1;
 	}
-	smbus->adapter = nforce2_adapter;
+	smbus->adapter.owner = THIS_MODULE;
+	smbus->adapter.class = I2C_CLASS_HWMON;
+	smbus->adapter.algo = &smbus_algorithm;
 	smbus->adapter.algo_data = smbus;
 	smbus->adapter.dev.parent = &dev->dev;
 	snprintf(smbus->adapter.name, I2C_NAME_SIZE,


-- 
--
Hans-Frieder Vogt                 e-mail: hfvogt <at> arcor .dot. de
                                          hfvogt <at> gmx .dot. net



More information about the i2c mailing list