[i2c] [PATCH 1of3] change i2c-i801 driver to use internal 32-byte buffer on ICH4+
Jean Delvare
khali at linux-fr.org
Fri Jun 15 16:10:00 CEST 2007
Hi Oleg,
On Mon, 11 Jun 2007 04:00:05 -0700, Oleg Ryjkov wrote:
> This is the first part of the patch to i2c-i801.c. It only does some
> cosmetic changes(adds some new defines and fixes several wrong
> indentations and moves HWPEC to a separate function).
>
> Signed-ff-by: Oleg Ryjkov <olegr at google.com>
Applied, with the following minor changes:
> --- linux-2.6-untouched/drivers/i2c/busses/i2c-i801.c 2007-05-17 16:10:33.000000000 -0700
> +++ linux-2.6/drivers/i2c/busses/i2c-i801.c 2007-06-11 02:48:34.000000000 -0700
> @@ -22,10 +22,10 @@
>
> /*
> SUPPORTED DEVICES PCI ID
> - 82801AA 2413
> - 82801AB 2423
> - 82801BA 2443
> - 82801CA/CAM 2483
> + 82801AA 2413
> + 82801AB 2423
> + 82801BA 2443
> + 82801CA/CAM 2483
> 82801DB 24C3 (HW PEC supported, 32 byte buffer not supported)
> 82801EB 24D3 (HW PEC supported, 32 byte buffer not supported)
> 6300ESB 25A4
> @@ -74,6 +74,13 @@
> #define SMBHSTCFG_SMB_SMI_EN 2
> #define SMBHSTCFG_I2C_EN 4
>
> +/* Auxillary control register bits, ICH4+ only */
> +#define SMBAUXCTL_E32B 2
> +#define SMBAUXCTL_CRC 0x01
I made it "1" for consistency.
> +
> +/* kill bit for SMBHSTCNT */
> +#define SMBHSTCNT_KILL 2
> +
> /* Other settings */
> #define MAX_TIMEOUT 100
> #define ENABLE_INT9 0 /* set to 0x01 to enable - untested */
> @@ -91,10 +98,15 @@
> #define I801_START 0x40
> #define I801_PEC_EN 0x80 /* ICH4 only */
>
> -
> -static int i801_transaction(void);
> -static int i801_block_transaction(union i2c_smbus_data *data, char read_write,
> - int command, int hwpec);
> +/* I801 Hosts Status register bits */
> +#define SMBHSTSTS_BYTE_DONE 0x80
> +#define SMBHSTSTS_INUSE_STS 0x40
> +#define SMBHSTSTS_SMBALERT_STS 0x20
> +#define SMBHSTSTS_FAILED 0x10
> +#define SMBHSTSTS_BUS_ERR 0x08
> +#define SMBHSTSTS_DEV_ERR 0x04
> +#define SMBHSTSTS_INTR 0x02
> +#define SMBHSTSTS_HOST_BUSY 0x01
>
> static unsigned long i801_smba;
> static unsigned char i801_original_hstcfg;
> @@ -133,27 +145,32 @@
> do {
> msleep(1);
> temp = inb_p(SMBHSTSTS);
> - } while ((temp & 0x01) && (timeout++ < MAX_TIMEOUT));
> + } while ((temp & 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 = -1;
> + /* 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);
> }
>
> - if (temp & 0x10) {
> + if (temp & SMBHSTSTS_FAILED) {
> result = -1;
> dev_dbg(&I801_dev->dev, "Error: Failed bus transaction\n");
> }
>
> - if (temp & 0x08) {
> + if (temp & SMBHSTSTS_BUS_ERR) {
> result = -1;
> dev_err(&I801_dev->dev, "Bus collision! SMBus may be locked "
> "until next hard reset. (sorry!)\n");
> /* Clock stops and slave is stuck in mid-transmission */
> }
>
> - if (temp & 0x04) {
> + if (temp & SMBHSTSTS_DEV_ERR) {
> result = -1;
> dev_dbg(&I801_dev->dev, "Error: no response!\n");
> }
> @@ -172,6 +189,24 @@
> return result;
> }
>
> +/* wait for INTR bit as advised by Intel */
> +static void i801_wait_hwpec(void)
> +{
> + int timeout = 0;
> + int temp;
> +
> + do {
> + msleep(1);
> + temp = inb_p(SMBHSTSTS);
> + } while ((!(temp & SMBHSTSTS_INTR))
> + && (timeout++ < MAX_TIMEOUT));
> +
> + if (timeout >= MAX_TIMEOUT) {
> + dev_dbg(&I801_dev->dev, "PEC Timeout!\n");
> + }
> + outb_p(temp, SMBHSTSTS);
> +}
> +
> /* All-inclusive block transaction function */
> static int i801_block_transaction(union i2c_smbus_data *data, char read_write,
> int command, int hwpec)
> @@ -227,13 +262,13 @@
> /* Make sure the SMBus host is ready to start transmitting */
> temp = inb_p(SMBHSTSTS);
> if (i == 1) {
> - /* Erronenous conditions before transaction:
> + /* Erronenous conditions before transaction:
> * Byte_Done, Failed, Bus_Err, Dev_Err, Intr, Host_Busy */
> - errmask=0x9f;
> + errmask=0x9f;
> } else {
> - /* Erronenous conditions during transaction:
> + /* Erronenous conditions during transaction:
> * Failed, Bus_Err, Dev_Err, Intr */
> - errmask=0x1e;
> + errmask=0x1e;
> }
> if (temp & errmask) {
> dev_dbg(&I801_dev->dev, "SMBus busy (%02x). "
> @@ -243,7 +278,7 @@
> dev_err(&I801_dev->dev,
> "Reset failed! (%02x)\n", temp);
> result = -1;
> - goto END;
> + goto END;
> }
> if (i != 1) {
> /* if die in middle of block transaction, fail */
> @@ -261,33 +296,40 @@
> msleep(1);
> temp = inb_p(SMBHSTSTS);
> }
> - while ((!(temp & 0x80))
> - && (timeout++ < MAX_TIMEOUT));
> + while ((!(temp & 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 = -1;
> dev_dbg(&I801_dev->dev, "SMBus Timeout!\n");
> }
>
> - if (temp & 0x10) {
> + if (temp & SMBHSTSTS_FAILED) {
> result = -1;
> dev_dbg(&I801_dev->dev,
> "Error: Failed bus transaction\n");
> - } else if (temp & 0x08) {
> + } else if (temp & SMBHSTSTS_BUS_ERR) {
> result = -1;
> dev_err(&I801_dev->dev, "Bus collision!\n");
> - } else if (temp & 0x04) {
> + } else if (temp & SMBHSTSTS_DEV_ERR) {
> result = -1;
> dev_dbg(&I801_dev->dev, "Error: no response!\n");
> }
>
> if (i == 1 && read_write == I2C_SMBUS_READ) {
> len = inb_p(SMBHSTDAT0);
> - if (len < 1)
> - len = 1;
> - if (len > 32)
> - len = 32;
> + if (len < 1 || len > 32) {
Replaced 32 with I2C_SMBUS_BLOCK_MAX.
> + result = -1;
> + goto END;
> + }
> data->block[0] = len;
> }
>
> @@ -313,20 +355,9 @@
> goto END;
> }
>
> - if (hwpec) {
> - /* wait for INTR bit as advised by Intel */
> - timeout = 0;
> - do {
> - msleep(1);
> - temp = inb_p(SMBHSTSTS);
> - } while ((!(temp & 0x02))
> - && (timeout++ < MAX_TIMEOUT));
> + if (hwpec)
> + i801_wait_hwpec();
>
> - if (timeout >= MAX_TIMEOUT) {
> - dev_dbg(&I801_dev->dev, "PEC Timeout!\n");
> - }
> - outb_p(temp, SMBHSTSTS);
> - }
> result = 0;
> END:
> if (command == I2C_SMBUS_I2C_BLOCK_DATA) {
> @@ -393,7 +424,9 @@
> return -1;
> }
>
> - outb_p(hwpec, SMBAUXCTL); /* enable/disable hardware PEC */
> + if (hwpec)
> + outb_p(inb_p(SMBAUXCTL) | hwpec, SMBAUXCTL); /* enable/disable
> + hardware PEC */
I don't like this change, as it assumes that you found the chip in a
given state (PEC disabled), which could not be the case. As a matter of
fact, it wasn't for me when I tested. We want to set or clear the PEC
bit each time to make sure it is what we want. So I replaced with:
if (hwpec) /* enable/disable hardware PEC */
outb_p(inb_p(SMBAUXCTL) | SMBAUXCTL_CRC, SMBAUXCTL);
else
outb_p(inb_p(SMBAUXCTL) & (~SMBAUXCTL_CRC), SMBAUXCTL);
>
> if(block)
> ret = i801_block_transaction(data, read_write, size, hwpec);
> @@ -405,7 +438,7 @@
> /* Some BIOSes don't like it when PEC is enabled at reboot or resume
> time, so we forcibly disable it after every transaction. */
> if (hwpec)
> - outb_p(0, SMBAUXCTL);
> + outb_p(inb_p(SMBAUXCTL) & (~SMBAUXCTL_CRC), SMBAUXCTL);
>
> if(block)
> return ret;
Thanks,
--
Jean Delvare
More information about the i2c
mailing list