i2c-piix4: Various cleanups and minor fixes

The i2c-piix4 driver was used recently as a model to write a new SMBus
host controller driver and this made me realize that the code of this
old driver wasn't exactly good. So, here are many cleanups and minor
fixes to this driver, so that these minor mistakes aren't duplicated
again:

* Delete unused structure.
* Delete needless forward function declaration.
* Properly announce the SMBus host controller as we find it.
* Spell it SMBus not SMB.
* Return -EBUSY instead of -ENODEV when the I/O region is already in
  use.
* Drop useless masks on the 7-bit address and the R/W bit.
* Reject block transaction requests with an invalid block length.
* Check and report block transaction replies with an invalid block
  length.
* Delete a useless comment.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
This commit is contained in:
Jean Delvare 2008-07-14 22:38:25 +02:00 committed by Jean Delvare
parent 97140342e6
commit fa63cd56d2

View File

@ -42,13 +42,6 @@
#include <asm/io.h> #include <asm/io.h>
struct sd {
const unsigned short mfr;
const unsigned short dev;
const unsigned char fn;
const char *name;
};
/* PIIX4 SMBus address offsets */ /* PIIX4 SMBus address offsets */
#define SMBHSTSTS (0 + piix4_smba) #define SMBHSTSTS (0 + piix4_smba)
#define SMBHSLVSTS (1 + piix4_smba) #define SMBHSLVSTS (1 + piix4_smba)
@ -101,8 +94,6 @@ MODULE_PARM_DESC(force_addr,
"Forcibly enable the PIIX4 at the given address. " "Forcibly enable the PIIX4 at the given address. "
"EXTREMELY DANGEROUS!"); "EXTREMELY DANGEROUS!");
static int piix4_transaction(void);
static unsigned short piix4_smba; static unsigned short piix4_smba;
static int srvrworks_csb5_delay; static int srvrworks_csb5_delay;
static struct pci_driver piix4_driver; static struct pci_driver piix4_driver;
@ -141,8 +132,6 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
{ {
unsigned char temp; unsigned char temp;
dev_info(&PIIX4_dev->dev, "Found %s device\n", pci_name(PIIX4_dev));
if ((PIIX4_dev->vendor == PCI_VENDOR_ID_SERVERWORKS) && if ((PIIX4_dev->vendor == PCI_VENDOR_ID_SERVERWORKS) &&
(PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5)) (PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5))
srvrworks_csb5_delay = 1; srvrworks_csb5_delay = 1;
@ -172,7 +161,7 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba); pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
piix4_smba &= 0xfff0; piix4_smba &= 0xfff0;
if(piix4_smba == 0) { if(piix4_smba == 0) {
dev_err(&PIIX4_dev->dev, "SMB base address " dev_err(&PIIX4_dev->dev, "SMBus base address "
"uninitialized - upgrade BIOS or use " "uninitialized - upgrade BIOS or use "
"force_addr=0xaddr\n"); "force_addr=0xaddr\n");
return -ENODEV; return -ENODEV;
@ -180,9 +169,9 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
} }
if (!request_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) { if (!request_region(piix4_smba, SMBIOSIZE, piix4_driver.name)) {
dev_err(&PIIX4_dev->dev, "SMB region 0x%x already in use!\n", dev_err(&PIIX4_dev->dev, "SMBus region 0x%x already in use!\n",
piix4_smba); piix4_smba);
return -ENODEV; return -EBUSY;
} }
pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp); pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp);
@ -228,13 +217,13 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
"(or code out of date)!\n"); "(or code out of date)!\n");
pci_read_config_byte(PIIX4_dev, SMBREV, &temp); pci_read_config_byte(PIIX4_dev, SMBREV, &temp);
dev_dbg(&PIIX4_dev->dev, "SMBREV = 0x%X\n", temp); dev_info(&PIIX4_dev->dev,
dev_dbg(&PIIX4_dev->dev, "SMBA = 0x%X\n", piix4_smba); "SMBus Host Controller at 0x%x, revision %d\n",
piix4_smba, temp);
return 0; return 0;
} }
/* Another internally used function */
static int piix4_transaction(void) static int piix4_transaction(void)
{ {
int temp; int temp;
@ -322,19 +311,19 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n"); dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
return -EOPNOTSUPP; return -EOPNOTSUPP;
case I2C_SMBUS_QUICK: case I2C_SMBUS_QUICK:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), outb_p((addr << 1) | read_write,
SMBHSTADD); SMBHSTADD);
size = PIIX4_QUICK; size = PIIX4_QUICK;
break; break;
case I2C_SMBUS_BYTE: case I2C_SMBUS_BYTE:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), outb_p((addr << 1) | read_write,
SMBHSTADD); SMBHSTADD);
if (read_write == I2C_SMBUS_WRITE) if (read_write == I2C_SMBUS_WRITE)
outb_p(command, SMBHSTCMD); outb_p(command, SMBHSTCMD);
size = PIIX4_BYTE; size = PIIX4_BYTE;
break; break;
case I2C_SMBUS_BYTE_DATA: case I2C_SMBUS_BYTE_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), outb_p((addr << 1) | read_write,
SMBHSTADD); SMBHSTADD);
outb_p(command, SMBHSTCMD); outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE) if (read_write == I2C_SMBUS_WRITE)
@ -342,7 +331,7 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
size = PIIX4_BYTE_DATA; size = PIIX4_BYTE_DATA;
break; break;
case I2C_SMBUS_WORD_DATA: case I2C_SMBUS_WORD_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), outb_p((addr << 1) | read_write,
SMBHSTADD); SMBHSTADD);
outb_p(command, SMBHSTCMD); outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE) { if (read_write == I2C_SMBUS_WRITE) {
@ -352,15 +341,13 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
size = PIIX4_WORD_DATA; size = PIIX4_WORD_DATA;
break; break;
case I2C_SMBUS_BLOCK_DATA: case I2C_SMBUS_BLOCK_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), outb_p((addr << 1) | read_write,
SMBHSTADD); SMBHSTADD);
outb_p(command, SMBHSTCMD); outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE) { if (read_write == I2C_SMBUS_WRITE) {
len = data->block[0]; len = data->block[0];
if (len < 0) if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
len = 0; return -EINVAL;
if (len > 32)
len = 32;
outb_p(len, SMBHSTDAT0); outb_p(len, SMBHSTDAT0);
i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */
for (i = 1; i <= len; i++) for (i = 1; i <= len; i++)
@ -390,6 +377,8 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
break; break;
case PIIX4_BLOCK_DATA: case PIIX4_BLOCK_DATA:
data->block[0] = inb_p(SMBHSTDAT0); data->block[0] = inb_p(SMBHSTDAT0);
if (data->block[0] == 0 || data->block[0] > I2C_SMBUS_BLOCK_MAX)
return -EPROTO;
i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */
for (i = 1; i <= data->block[0]; i++) for (i = 1; i <= data->block[0]; i++)
data->block[i] = inb_p(SMBBLKDAT); data->block[i] = inb_p(SMBBLKDAT);