From c2f559d5df5751780c0bd3ea0bd0aa17d47c0b39 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Thu, 22 Sep 2005 22:01:07 +0200 Subject: [PATCH] [PATCH] i2c-viapro: Code cleanups Cleanups to the i2c-viapro driver: * Kill unused defines. * Kill interrupt-related code, as the driver doesn't use interrupts. * Fix broken comments (some copied from i2c-piix4.) * Centralize the unsupported command error case in vt596_access. That way we'll catch all unsupported commands, not only I2C_SMBUS_PROC_CALL. * Refactor some code. * Convert some dev_dbg into dev_err. Errors better be reported even in non-debug mode. * Do not verify that the final reset succeeded. It'll be checked at the beginning of the next transaction anyway. * Use the driver name to reserve the I/O region. * Do not print the contents of the SMBREV register, it reads 0 on all chips I've seen so far. * Some other minor fixes all over the place. Signed-off-by: Jean Delvare Signed-off-by: Greg Kroah-Hartman drivers/i2c/busses/i2c-viapro.c | 122 +++++++++++++--------------------------- 1 file changed, 41 insertions(+), 81 deletions(-) --- drivers/i2c/busses/i2c-viapro.c | 122 +++++++++++--------------------- 1 file changed, 41 insertions(+), 81 deletions(-) diff --git a/drivers/i2c/busses/i2c-viapro.c b/drivers/i2c/busses/i2c-viapro.c index b420e752b1dc..5cf8fe182806 100644 --- a/drivers/i2c/busses/i2c-viapro.c +++ b/drivers/i2c/busses/i2c-viapro.c @@ -39,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -54,34 +53,22 @@ static struct pci_dev *vt596_pdev; /* SMBus address offsets */ static unsigned short vt596_smba; #define SMBHSTSTS (vt596_smba + 0) -#define SMBHSLVSTS (vt596_smba + 1) #define SMBHSTCNT (vt596_smba + 2) #define SMBHSTCMD (vt596_smba + 3) #define SMBHSTADD (vt596_smba + 4) #define SMBHSTDAT0 (vt596_smba + 5) #define SMBHSTDAT1 (vt596_smba + 6) #define SMBBLKDAT (vt596_smba + 7) -#define SMBSLVCNT (vt596_smba + 8) -#define SMBSHDWCMD (vt596_smba + 9) -#define SMBSLVEVT (vt596_smba + 0xA) -#define SMBSLVDAT (vt596_smba + 0xC) /* PCI Address Constants */ /* SMBus data in configuration space can be found in two places, We try to select the better one */ -static unsigned short smb_cf_hstcfg = 0xD2; - -#define SMBHSTCFG (smb_cf_hstcfg) -#define SMBSLVC (smb_cf_hstcfg + 1) -#define SMBSHDW1 (smb_cf_hstcfg + 2) -#define SMBSHDW2 (smb_cf_hstcfg + 3) -#define SMBREV (smb_cf_hstcfg + 4) +static unsigned short SMBHSTCFG = 0xD2; /* Other settings */ #define MAX_TIMEOUT 500 -#define ENABLE_INT9 0 /* VT82C596 constants */ #define VT596_QUICK 0x00 @@ -107,12 +94,13 @@ MODULE_PARM_DESC(force_addr, "EXTREMELY DANGEROUS!"); +static struct pci_driver vt596_driver; static struct i2c_adapter vt596_adapter; #define FEATURE_I2CBLOCK (1<<0) static unsigned int vt596_features; -/* Another internally used function */ +/* Return -1 on error, 0 on success */ static int vt596_transaction(void) { int temp; @@ -127,23 +115,21 @@ static int vt596_transaction(void) /* Make sure the SMBus host is ready to start transmitting */ if ((temp = inb_p(SMBHSTSTS)) & 0x1F) { dev_dbg(&vt596_adapter.dev, "SMBus busy (0x%02x). " - "Resetting...\n", temp); + "Resetting... ", temp); outb_p(temp, SMBHSTSTS); if ((temp = inb_p(SMBHSTSTS)) & 0x1F) { - dev_dbg(&vt596_adapter.dev, "Failed! (0x%02x)\n", temp); - + printk("Failed! (0x%02x)\n", temp); return -1; } else { - dev_dbg(&vt596_adapter.dev, "Successfull!\n"); + printk("Successful!\n"); } } - /* start the transaction by setting bit 6 */ - outb_p(inb(SMBHSTCNT) | 0x040, SMBHSTCNT); + /* Start the transaction by setting bit 6 */ + outb_p(inb(SMBHSTCNT) | 0x40, SMBHSTCNT); - /* We will always wait for a fraction of a second! - I don't know if VIA needs this, Intel did */ + /* We will always wait for a fraction of a second */ do { msleep(1); temp = inb_p(SMBHSTSTS); @@ -152,33 +138,32 @@ static int vt596_transaction(void) /* If the SMBus is still busy, we give up */ if (timeout >= MAX_TIMEOUT) { result = -1; - dev_dbg(&vt596_adapter.dev, "SMBus Timeout!\n"); + dev_err(&vt596_adapter.dev, "SMBus timeout!\n"); } if (temp & 0x10) { result = -1; - dev_dbg(&vt596_adapter.dev, "Error: Failed bus transaction\n"); + dev_err(&vt596_adapter.dev, "Transaction failed (0x%02x)\n", + inb_p(SMBHSTCNT) & 0x3C); } if (temp & 0x08) { result = -1; - dev_info(&vt596_adapter.dev, "Bus collision! SMBus may be " - "locked until next hard\nreset. (sorry!)\n"); - /* Clock stops and slave is stuck in mid-transmission */ + dev_err(&vt596_adapter.dev, "SMBus collision!\n"); } if (temp & 0x04) { result = -1; - dev_dbg(&vt596_adapter.dev, "Error: no response!\n"); + /* Quick commands are used to probe for chips, so + errors are expected, and we don't want to frighten the + user. */ + if ((inb_p(SMBHSTCNT) & 0x3C) != VT596_QUICK) + dev_err(&vt596_adapter.dev, "Transaction error!\n"); } - if ((temp = inb_p(SMBHSTSTS)) & 0x1F) { + /* Resetting status register */ + if (temp & 0x1F) outb_p(temp, SMBHSTSTS); - if ((temp = inb_p(SMBHSTSTS)) & 0x1F) { - dev_warn(&vt596_adapter.dev, "Failed reset at end " - "of transaction (%02x)\n", temp); - } - } dev_dbg(&vt596_adapter.dev, "Transaction (post): CNT=%02x, CMD=%02x, " "ADD=%02x, DAT0=%02x, DAT1=%02x\n", inb_p(SMBHSTCNT), @@ -188,41 +173,29 @@ static int vt596_transaction(void) return result; } -/* Return -1 on error. */ +/* Return -1 on error, 0 on success */ static s32 vt596_access(struct i2c_adapter *adap, u16 addr, unsigned short flags, char read_write, u8 command, int size, union i2c_smbus_data *data) { - int i, len; + int i; switch (size) { - case I2C_SMBUS_PROC_CALL: - dev_info(&vt596_adapter.dev, - "I2C_SMBUS_PROC_CALL not supported!\n"); - return -1; case I2C_SMBUS_QUICK: - outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), - SMBHSTADD); size = VT596_QUICK; break; case I2C_SMBUS_BYTE: - outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), - SMBHSTADD); if (read_write == I2C_SMBUS_WRITE) outb_p(command, SMBHSTCMD); size = VT596_BYTE; break; case I2C_SMBUS_BYTE_DATA: - outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), - SMBHSTADD); outb_p(command, SMBHSTCMD); if (read_write == I2C_SMBUS_WRITE) outb_p(data->byte, SMBHSTDAT0); size = VT596_BYTE_DATA; break; case I2C_SMBUS_WORD_DATA: - outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), - SMBHSTADD); outb_p(command, SMBHSTCMD); if (read_write == I2C_SMBUS_WRITE) { outb_p(data->word & 0xff, SMBHSTDAT0); @@ -232,31 +205,30 @@ static s32 vt596_access(struct i2c_adapter *adap, u16 addr, break; case I2C_SMBUS_I2C_BLOCK_DATA: if (!(vt596_features & FEATURE_I2CBLOCK)) - return -1; + goto exit_unsupported; if (read_write == I2C_SMBUS_READ) outb_p(I2C_SMBUS_BLOCK_MAX, SMBHSTDAT0); /* Fall through */ case I2C_SMBUS_BLOCK_DATA: - outb_p(((addr & 0x7f) << 1) | (read_write & 0x01), - SMBHSTADD); outb_p(command, SMBHSTCMD); if (read_write == I2C_SMBUS_WRITE) { - len = data->block[0]; - if (len < 0) - len = 0; + u8 len = data->block[0]; if (len > I2C_SMBUS_BLOCK_MAX) len = I2C_SMBUS_BLOCK_MAX; outb_p(len, SMBHSTDAT0); - i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ + inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ for (i = 1; i <= len; i++) outb_p(data->block[i], SMBBLKDAT); } size = (size == I2C_SMBUS_I2C_BLOCK_DATA) ? VT596_I2C_BLOCK_DATA : VT596_BLOCK_DATA; break; + default: + goto exit_unsupported; } - outb_p((size & 0x3C) + (ENABLE_INT9 & 1), SMBHSTCNT); + outb_p(((addr & 0x7f) << 1) | read_write, SMBHSTADD); + outb_p((size & 0x3C), SMBHSTCNT); if (vt596_transaction()) /* Error in transaction */ return -1; @@ -266,12 +238,6 @@ static s32 vt596_access(struct i2c_adapter *adap, u16 addr, switch (size) { case VT596_BYTE: - /* Where is the result put? I assume here it is in - * SMBHSTDAT0 but it might just as well be in the - * SMBHSTCMD. No clue in the docs - */ - data->byte = inb_p(SMBHSTDAT0); - break; case VT596_BYTE_DATA: data->byte = inb_p(SMBHSTDAT0); break; @@ -283,12 +249,17 @@ static s32 vt596_access(struct i2c_adapter *adap, u16 addr, data->block[0] = inb_p(SMBHSTDAT0); if (data->block[0] > I2C_SMBUS_BLOCK_MAX) data->block[0] = I2C_SMBUS_BLOCK_MAX; - i = inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ + inb_p(SMBHSTCNT); /* Reset SMBBLKDAT */ for (i = 1; i <= data->block[0]; i++) data->block[i] = inb_p(SMBBLKDAT); break; } return 0; + +exit_unsupported: + dev_warn(&vt596_adapter.dev, "Unsupported command invoked! (0x%02x)\n", + size); + return -1; } static u32 vt596_func(struct i2c_adapter *adapter) @@ -311,7 +282,6 @@ static struct i2c_adapter vt596_adapter = { .owner = THIS_MODULE, .class = I2C_CLASS_HWMON, .algo = &smbus_algorithm, - .name = "unset", }; static int __devinit vt596_probe(struct pci_dev *pdev, @@ -328,12 +298,12 @@ static int __devinit vt596_probe(struct pci_dev *pdev, } if ((pci_read_config_word(pdev, id->driver_data, &vt596_smba)) || - !(vt596_smba & 0x1)) { + !(vt596_smba & 0x0001)) { /* try 2nd address and config reg. for 596 */ if (id->device == PCI_DEVICE_ID_VIA_82C596_3 && !pci_read_config_word(pdev, SMBBA2, &vt596_smba) && - (vt596_smba & 0x1)) { - smb_cf_hstcfg = 0x84; + (vt596_smba & 0x0001)) { + SMBHSTCFG = 0x84; } else { /* no matches at all */ dev_err(&pdev->dev, "Cannot configure " @@ -351,7 +321,7 @@ static int __devinit vt596_probe(struct pci_dev *pdev, } found: - if (!request_region(vt596_smba, 8, "viapro-smbus")) { + if (!request_region(vt596_smba, 8, vt596_driver.name)) { dev_err(&pdev->dev, "SMBus region 0x%x already in use!\n", vt596_smba); return -ENODEV; @@ -366,7 +336,7 @@ found: pci_write_config_byte(pdev, SMBHSTCFG, temp | 0x01); dev_warn(&pdev->dev, "WARNING: SMBus interface set to new " "address 0x%04x!\n", vt596_smba); - } else if ((temp & 1) == 0) { + } else if (!(temp & 0x01)) { if (force) { /* NOTE: This assumes I/O space and other allocations * WERE done by the Bios! Don't complain if your @@ -374,7 +344,7 @@ found: * :') Check for Bios updates before resorting to * this. */ - pci_write_config_byte(pdev, SMBHSTCFG, temp | 1); + pci_write_config_byte(pdev, SMBHSTCFG, temp | 0x01); dev_info(&pdev->dev, "Enabling SMBus device\n"); } else { dev_err(&pdev->dev, "SMBUS: Error: Host SMBus " @@ -384,16 +354,6 @@ found: } } - if ((temp & 0x0E) == 8) - dev_dbg(&pdev->dev, "using Interrupt 9 for SMBus.\n"); - else if ((temp & 0x0E) == 0) - dev_dbg(&pdev->dev, "using Interrupt SMI# for SMBus.\n"); - else - dev_dbg(&pdev->dev, "Illegal Interrupt configuration " - "(or code out of date)!\n"); - - pci_read_config_byte(pdev, SMBREV, &temp); - dev_dbg(&pdev->dev, "SMBREV = 0x%X\n", temp); dev_dbg(&pdev->dev, "VT596_smba = 0x%X\n", vt596_smba); switch (pdev->device) { -- 2.20.1