Skip to content

Commit

Permalink
i2c-piix4: Various cleanups and minor fixes
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jean Delvare authored and Jean Delvare committed Jul 14, 2008
1 parent 9714034 commit fa63cd5
Showing 1 changed file with 15 additions and 26 deletions.
41 changes: 15 additions & 26 deletions drivers/i2c/busses/i2c-piix4.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,6 @@
#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 */
#define SMBHSTSTS (0 + piix4_smba)
#define SMBHSLVSTS (1 + piix4_smba)
Expand Down Expand Up @@ -101,8 +94,6 @@ MODULE_PARM_DESC(force_addr,
"Forcibly enable the PIIX4 at the given address. "
"EXTREMELY DANGEROUS!");

static int piix4_transaction(void);

static unsigned short piix4_smba;
static int srvrworks_csb5_delay;
static struct pci_driver piix4_driver;
Expand Down Expand Up @@ -141,8 +132,6 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
{
unsigned char temp;

dev_info(&PIIX4_dev->dev, "Found %s device\n", pci_name(PIIX4_dev));

if ((PIIX4_dev->vendor == PCI_VENDOR_ID_SERVERWORKS) &&
(PIIX4_dev->device == PCI_DEVICE_ID_SERVERWORKS_CSB5))
srvrworks_csb5_delay = 1;
Expand Down Expand Up @@ -172,17 +161,17 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
pci_read_config_word(PIIX4_dev, SMBBA, &piix4_smba);
piix4_smba &= 0xfff0;
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 "
"force_addr=0xaddr\n");
return -ENODEV;
}
}

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);
return -ENODEV;
return -EBUSY;
}

pci_read_config_byte(PIIX4_dev, SMBHSTCFG, &temp);
Expand Down Expand Up @@ -228,13 +217,13 @@ static int __devinit piix4_setup(struct pci_dev *PIIX4_dev,
"(or code out of date)!\n");

pci_read_config_byte(PIIX4_dev, SMBREV, &temp);
dev_dbg(&PIIX4_dev->dev, "SMBREV = 0x%X\n", temp);
dev_dbg(&PIIX4_dev->dev, "SMBA = 0x%X\n", piix4_smba);
dev_info(&PIIX4_dev->dev,
"SMBus Host Controller at 0x%x, revision %d\n",
piix4_smba, temp);

return 0;
}

/* Another internally used function */
static int piix4_transaction(void)
{
int temp;
Expand Down Expand Up @@ -322,27 +311,27 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
return -EOPNOTSUPP;
case I2C_SMBUS_QUICK:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
outb_p((addr << 1) | read_write,
SMBHSTADD);
size = PIIX4_QUICK;
break;
case I2C_SMBUS_BYTE:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
outb_p((addr << 1) | read_write,
SMBHSTADD);
if (read_write == I2C_SMBUS_WRITE)
outb_p(command, SMBHSTCMD);
size = PIIX4_BYTE;
break;
case I2C_SMBUS_BYTE_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
outb_p((addr << 1) | read_write,
SMBHSTADD);
outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE)
outb_p(data->byte, SMBHSTDAT0);
size = PIIX4_BYTE_DATA;
break;
case I2C_SMBUS_WORD_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
outb_p((addr << 1) | read_write,
SMBHSTADD);
outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE) {
Expand All @@ -352,15 +341,13 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
size = PIIX4_WORD_DATA;
break;
case I2C_SMBUS_BLOCK_DATA:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
outb_p((addr << 1) | read_write,
SMBHSTADD);
outb_p(command, SMBHSTCMD);
if (read_write == I2C_SMBUS_WRITE) {
len = data->block[0];
if (len < 0)
len = 0;
if (len > 32)
len = 32;
if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
return -EINVAL;
outb_p(len, SMBHSTDAT0);
i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */
for (i = 1; i <= len; i++)
Expand Down Expand Up @@ -390,6 +377,8 @@ static s32 piix4_access(struct i2c_adapter * adap, u16 addr,
break;
case PIIX4_BLOCK_DATA:
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 */
for (i = 1; i <= data->block[0]; i++)
data->block[i] = inb_p(SMBBLKDAT);
Expand Down

0 comments on commit fa63cd5

Please sign in to comment.