i2c-i801: Fix handling of error conditions
Move the check of pre-transaction and post-transaction conditions to
separate functions, and adjust them a bit. Having dedicated functions
for that ensures that errors are handled in a consistent way.
Bit HOST_BUSY of the status register is read-only, so writing to it is
certainly not going to clear it. If this bit is set then we simply
don't want to start the transaction, as it means that somebody else
(ACPI, SMM?) is already using the controller.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
index 73bd855..46aa41f 100644
--- a/drivers/i2c/busses/i2c-i801.c
+++ b/drivers/i2c/busses/i2c-i801.c
@@ -4,7 +4,7 @@
Copyright (c) 1998 - 2002 Frodo Looijaard <frodol@dds.nl>,
Philip Edelbrock <phil@netroedge.com>, and Mark D. Studebaker
<mdsxyz123@yahoo.com>
- Copyright (C) 2007 Jean Delvare <khali@linux-fr.org>
+ Copyright (C) 2007, 2008 Jean Delvare <khali@linux-fr.org>
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
@@ -121,6 +121,10 @@
#define SMBHSTSTS_INTR 0x02
#define SMBHSTSTS_HOST_BUSY 0x01
+#define STATUS_FLAGS (SMBHSTSTS_BYTE_DONE | SMBHSTSTS_FAILED | \
+ SMBHSTSTS_BUS_ERR | SMBHSTSTS_DEV_ERR | \
+ SMBHSTSTS_INTR)
+
static unsigned long i801_smba;
static unsigned char i801_original_hstcfg;
static struct pci_driver i801_driver;
@@ -132,25 +136,95 @@
#define FEATURE_I2C_BLOCK_READ (1 << 3)
static unsigned int i801_features;
+/* Make sure the SMBus host is ready to start transmitting.
+ Return 0 if it is, -EBUSY if it is not. */
+static int i801_check_pre(void)
+{
+ int status;
+
+ status = inb_p(SMBHSTSTS);
+ if (status & SMBHSTSTS_HOST_BUSY) {
+ dev_err(&I801_dev->dev, "SMBus is busy, can't use it!\n");
+ return -EBUSY;
+ }
+
+ status &= STATUS_FLAGS;
+ if (status) {
+ dev_dbg(&I801_dev->dev, "Clearing status flags (%02x)\n",
+ status);
+ outb_p(status, SMBHSTSTS);
+ status = inb_p(SMBHSTSTS) & STATUS_FLAGS;
+ if (status) {
+ dev_err(&I801_dev->dev,
+ "Failed clearing status flags (%02x)\n",
+ status);
+ return -EBUSY;
+ }
+ }
+
+ return 0;
+}
+
+/* Convert the status register to an error code, and clear it. */
+static int i801_check_post(int status, int timeout)
+{
+ int result = 0;
+
+ /* If the SMBus is still busy, we give up */
+ if (timeout) {
+ dev_err(&I801_dev->dev, "Transaction timeout\n");
+ /* try to stop the current command */
+ dev_dbg(&I801_dev->dev, "Terminating the current operation\n");
+ outb_p(inb_p(SMBHSTCNT) | SMBHSTCNT_KILL, SMBHSTCNT);
+ msleep(1);
+ outb_p(inb_p(SMBHSTCNT) & (~SMBHSTCNT_KILL), SMBHSTCNT);
+
+ /* Check if it worked */
+ status = inb_p(SMBHSTSTS);
+ if ((status & SMBHSTSTS_HOST_BUSY) ||
+ !(status & SMBHSTSTS_FAILED))
+ dev_err(&I801_dev->dev,
+ "Failed terminating the transaction\n");
+ outb_p(STATUS_FLAGS, SMBHSTSTS);
+ return -ETIMEDOUT;
+ }
+
+ if (status & SMBHSTSTS_FAILED) {
+ result = -EIO;
+ dev_err(&I801_dev->dev, "Transaction failed\n");
+ }
+ if (status & SMBHSTSTS_DEV_ERR) {
+ result = -ENXIO;
+ dev_dbg(&I801_dev->dev, "No response\n");
+ }
+ if (status & SMBHSTSTS_BUS_ERR) {
+ result = -EAGAIN;
+ dev_dbg(&I801_dev->dev, "Lost arbitration\n");
+ }
+
+ if (result) {
+ /* Clear error flags */
+ outb_p(status & STATUS_FLAGS, SMBHSTSTS);
+ status = inb_p(SMBHSTSTS) & STATUS_FLAGS;
+ if (status) {
+ dev_warn(&I801_dev->dev, "Failed clearing status "
+ "flags at end of transaction (%02x)\n",
+ status);
+ }
+ }
+
+ return result;
+}
+
static int i801_transaction(int xact)
{
int status;
- int result = 0;
+ int result;
int timeout = 0;
- /* Make sure the SMBus host is ready to start transmitting */
- /* 0x1f = Failed, Bus_Err, Dev_Err, Intr, Host_Busy */
- if ((status = (0x1f & inb_p(SMBHSTSTS))) != 0x00) {
- dev_dbg(&I801_dev->dev, "SMBus busy (%02x). Resetting...\n",
- status);
- outb_p(status, SMBHSTSTS);
- if ((status = (0x1f & inb_p(SMBHSTSTS))) != 0x00) {
- dev_dbg(&I801_dev->dev, "Failed! (%02x)\n", status);
- return -EBUSY;
- } else {
- dev_dbg(&I801_dev->dev, "Successful!\n");
- }
- }
+ result = i801_check_pre();
+ if (result < 0)
+ return result;
/* the current contents of SMBHSTCNT can be overwritten, since PEC,
* INTREN, SMBSCMD are passed in xact */
@@ -162,40 +236,12 @@
status = inb_p(SMBHSTSTS);
} while ((status & SMBHSTSTS_HOST_BUSY) && (timeout++ < MAX_TIMEOUT));
- /* If the SMBus is still busy, we give up */
- if (timeout >= MAX_TIMEOUT) {
- dev_dbg(&I801_dev->dev, "SMBus Timeout!\n");
- result = -ETIMEDOUT;
- /* try to stop the current command */
- dev_dbg(&I801_dev->dev, "Terminating the current operation\n");
- outb_p(inb_p(SMBHSTCNT) | SMBHSTCNT_KILL, SMBHSTCNT);
- msleep(1);
- outb_p(inb_p(SMBHSTCNT) & (~SMBHSTCNT_KILL), SMBHSTCNT);
- }
+ result = i801_check_post(status, timeout >= MAX_TIMEOUT);
+ if (result < 0)
+ return result;
- if (status & SMBHSTSTS_FAILED) {
- result = -EIO;
- dev_dbg(&I801_dev->dev, "Error: Failed bus transaction\n");
- }
-
- if (status & SMBHSTSTS_BUS_ERR) {
- result = -EAGAIN;
- dev_dbg(&I801_dev->dev, "Lost arbitration\n");
- }
-
- if (status & SMBHSTSTS_DEV_ERR) {
- result = -ENXIO;
- dev_dbg(&I801_dev->dev, "Error: no response!\n");
- }
-
- if ((inb_p(SMBHSTSTS) & 0x1f) != 0x00)
- outb_p(inb(SMBHSTSTS), SMBHSTSTS);
-
- if ((status = (0x1f & inb_p(SMBHSTSTS))) != 0x00) {
- dev_dbg(&I801_dev->dev, "Failed reset at end of transaction "
- "(%02x)\n", status);
- }
- return result;
+ outb_p(SMBHSTSTS_INTR, SMBHSTSTS);
+ return 0;
}
/* wait for INTR bit as advised by Intel */
@@ -256,9 +302,12 @@
int i, len;
int smbcmd;
int status;
- int result = 0;
+ int result;
int timeout;
- unsigned char errmask;
+
+ result = i801_check_pre();
+ if (result < 0)
+ return result;
len = data->block[0];
@@ -282,31 +331,6 @@
}
outb_p(smbcmd | ENABLE_INT9, SMBHSTCNT);
- /* Make sure the SMBus host is ready to start transmitting */
- status = inb_p(SMBHSTSTS);
- if (i == 1) {
- /* Erroneous conditions before transaction:
- * Byte_Done, Failed, Bus_Err, Dev_Err, Intr, Host_Busy */
- errmask = 0x9f;
- } else {
- /* Erroneous conditions during transaction:
- * Failed, Bus_Err, Dev_Err, Intr */
- errmask = 0x1e;
- }
- if (status & errmask) {
- dev_dbg(&I801_dev->dev, "SMBus busy (%02x). "
- "Resetting...\n", status);
- outb_p(status, SMBHSTSTS);
- if (((status = inb_p(SMBHSTSTS)) & errmask) != 0x00) {
- dev_err(&I801_dev->dev,
- "Reset failed! (%02x)\n", status);
- return -EBUSY;
- }
- if (i != 1)
- /* if die in middle of block transaction, fail */
- return -EIO;
- }
-
if (i == 1)
outb_p(inb(SMBHSTCNT) | I801_START, SMBHSTCNT);
@@ -319,36 +343,23 @@
while ((!(status & SMBHSTSTS_BYTE_DONE))
&& (timeout++ < MAX_TIMEOUT));
- /* If the SMBus is still busy, we give up */
- if (timeout >= MAX_TIMEOUT) {
- /* try to stop the current command */
- dev_dbg(&I801_dev->dev, "Terminating the current "
- "operation\n");
- outb_p(inb_p(SMBHSTCNT) | SMBHSTCNT_KILL, SMBHSTCNT);
- msleep(1);
- outb_p(inb_p(SMBHSTCNT) & (~SMBHSTCNT_KILL),
- SMBHSTCNT);
- result = -ETIMEDOUT;
- dev_dbg(&I801_dev->dev, "SMBus Timeout!\n");
- }
-
- if (status & SMBHSTSTS_FAILED) {
- result = -EIO;
- dev_dbg(&I801_dev->dev,
- "Error: Failed bus transaction\n");
- } else if (status & SMBHSTSTS_BUS_ERR) {
- result = -EAGAIN;
- dev_dbg(&I801_dev->dev, "Lost arbitration\n");
- } else if (status & SMBHSTSTS_DEV_ERR) {
- result = -ENXIO;
- dev_dbg(&I801_dev->dev, "Error: no response!\n");
- }
+ result = i801_check_post(status, timeout >= MAX_TIMEOUT);
+ if (result < 0)
+ return result;
if (i == 1 && read_write == I2C_SMBUS_READ
&& command != I2C_SMBUS_I2C_BLOCK_DATA) {
len = inb_p(SMBHSTDAT0);
- if (len < 1 || len > I2C_SMBUS_BLOCK_MAX)
+ if (len < 1 || len > I2C_SMBUS_BLOCK_MAX) {
+ dev_err(&I801_dev->dev,
+ "Illegal SMBus block read size %d\n",
+ len);
+ /* Recover */
+ while (inb_p(SMBHSTSTS) & SMBHSTSTS_HOST_BUSY)
+ outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS);
+ outb_p(SMBHSTSTS_INTR, SMBHSTSTS);
return -EPROTO;
+ }
data->block[0] = len;
}
@@ -357,19 +368,12 @@
data->block[i] = inb_p(SMBBLKDAT);
if (read_write == I2C_SMBUS_WRITE && i+1 <= len)
outb_p(data->block[i+1], SMBBLKDAT);
- if ((status & 0x9e) != 0x00)
- outb_p(status, SMBHSTSTS); /* signals SMBBLKDAT ready */
- if ((status = (0x1e & inb_p(SMBHSTSTS))) != 0x00) {
- dev_dbg(&I801_dev->dev,
- "Bad status (%02x) at end of transaction\n",
- status);
- }
-
- if (result < 0)
- return result;
+ /* signals SMBBLKDAT ready */
+ outb_p(SMBHSTSTS_BYTE_DONE | SMBHSTSTS_INTR, SMBHSTSTS);
}
- return result;
+
+ return 0;
}
static int i801_set_block_buffer_mode(void)