Commit 88b9e750 authored by Jean Delvare's avatar Jean Delvare Committed by Jean Delvare

i2c-amd8111: Proposed cleanups

Proposed cleanups to the i2c-amd8111 SMBus driver:
* Fold long lines.
* Add an explicit mask when writing the low byte of a word.
* Use I2C_SMBUS_BLOCK_MAX instead of hardcoding 32.
* Discard extra blank lines.
* Use boolean not instead of bitwise not for bit tests, it's clearer.
* Return -EBUSY rather than -1 on I/O resource conflict.
* Fix a race on device registration, initialization should be done
  before the bus is registered.
Signed-off-by: default avatarJean Delvare <khali@linux-fr.org>
parent 55249cf7
...@@ -76,7 +76,8 @@ static unsigned int amd_ec_wait_write(struct amd_smbus *smbus) ...@@ -76,7 +76,8 @@ static unsigned int amd_ec_wait_write(struct amd_smbus *smbus)
udelay(1); udelay(1);
if (!timeout) { if (!timeout) {
dev_warn(&smbus->dev->dev, "Timeout while waiting for IBF to clear\n"); dev_warn(&smbus->dev->dev,
"Timeout while waiting for IBF to clear\n");
return -1; return -1;
} }
...@@ -91,14 +92,16 @@ static unsigned int amd_ec_wait_read(struct amd_smbus *smbus) ...@@ -91,14 +92,16 @@ static unsigned int amd_ec_wait_read(struct amd_smbus *smbus)
udelay(1); udelay(1);
if (!timeout) { if (!timeout) {
dev_warn(&smbus->dev->dev, "Timeout while waiting for OBF to set\n"); dev_warn(&smbus->dev->dev,
"Timeout while waiting for OBF to set\n");
return -1; return -1;
} }
return 0; return 0;
} }
static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address, unsigned char *data) static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address,
unsigned char *data)
{ {
if (amd_ec_wait_write(smbus)) if (amd_ec_wait_write(smbus))
return -1; return -1;
...@@ -115,7 +118,8 @@ static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address, ...@@ -115,7 +118,8 @@ static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address,
return 0; return 0;
} }
static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address, unsigned char data) static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address,
unsigned char data)
{ {
if (amd_ec_wait_write(smbus)) if (amd_ec_wait_write(smbus))
return -1; return -1;
...@@ -175,18 +179,19 @@ static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address, ...@@ -175,18 +179,19 @@ static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address,
#define AMD_SMB_PRTCL_PEC 0x80 #define AMD_SMB_PRTCL_PEC 0x80
static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short flags, static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
char read_write, u8 command, int size, union i2c_smbus_data * data) unsigned short flags, char read_write, u8 command, int size,
union i2c_smbus_data * data)
{ {
struct amd_smbus *smbus = adap->algo_data; struct amd_smbus *smbus = adap->algo_data;
unsigned char protocol, len, pec, temp[2]; unsigned char protocol, len, pec, temp[2];
int i; int i;
protocol = (read_write == I2C_SMBUS_READ) ? AMD_SMB_PRTCL_READ : AMD_SMB_PRTCL_WRITE; protocol = (read_write == I2C_SMBUS_READ) ? AMD_SMB_PRTCL_READ
: AMD_SMB_PRTCL_WRITE;
pec = (flags & I2C_CLIENT_PEC) ? AMD_SMB_PRTCL_PEC : 0; pec = (flags & I2C_CLIENT_PEC) ? AMD_SMB_PRTCL_PEC : 0;
switch (size) { switch (size) {
case I2C_SMBUS_QUICK: case I2C_SMBUS_QUICK:
protocol |= AMD_SMB_PRTCL_QUICK; protocol |= AMD_SMB_PRTCL_QUICK;
read_write = I2C_SMBUS_WRITE; read_write = I2C_SMBUS_WRITE;
...@@ -208,8 +213,10 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -208,8 +213,10 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
case I2C_SMBUS_WORD_DATA: case I2C_SMBUS_WORD_DATA:
amd_ec_write(smbus, AMD_SMB_CMD, command); amd_ec_write(smbus, AMD_SMB_CMD, command);
if (read_write == I2C_SMBUS_WRITE) { if (read_write == I2C_SMBUS_WRITE) {
amd_ec_write(smbus, AMD_SMB_DATA, data->word); amd_ec_write(smbus, AMD_SMB_DATA,
amd_ec_write(smbus, AMD_SMB_DATA + 1, data->word >> 8); data->word & 0xff);
amd_ec_write(smbus, AMD_SMB_DATA + 1,
data->word >> 8);
} }
protocol |= AMD_SMB_PRTCL_WORD_DATA | pec; protocol |= AMD_SMB_PRTCL_WORD_DATA | pec;
break; break;
...@@ -217,27 +224,31 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -217,27 +224,31 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
case I2C_SMBUS_BLOCK_DATA: case I2C_SMBUS_BLOCK_DATA:
amd_ec_write(smbus, AMD_SMB_CMD, command); amd_ec_write(smbus, AMD_SMB_CMD, command);
if (read_write == I2C_SMBUS_WRITE) { if (read_write == I2C_SMBUS_WRITE) {
len = min_t(u8, data->block[0], 32); len = min_t(u8, data->block[0],
I2C_SMBUS_BLOCK_MAX);
amd_ec_write(smbus, AMD_SMB_BCNT, len); amd_ec_write(smbus, AMD_SMB_BCNT, len);
for (i = 0; i < len; i++) for (i = 0; i < len; i++)
amd_ec_write(smbus, AMD_SMB_DATA + i, data->block[i + 1]); amd_ec_write(smbus, AMD_SMB_DATA + i,
data->block[i + 1]);
} }
protocol |= AMD_SMB_PRTCL_BLOCK_DATA | pec; protocol |= AMD_SMB_PRTCL_BLOCK_DATA | pec;
break; break;
case I2C_SMBUS_I2C_BLOCK_DATA: case I2C_SMBUS_I2C_BLOCK_DATA:
len = min_t(u8, data->block[0], 32); len = min_t(u8, data->block[0],
I2C_SMBUS_BLOCK_MAX);
amd_ec_write(smbus, AMD_SMB_CMD, command); amd_ec_write(smbus, AMD_SMB_CMD, command);
amd_ec_write(smbus, AMD_SMB_BCNT, len); amd_ec_write(smbus, AMD_SMB_BCNT, len);
if (read_write == I2C_SMBUS_WRITE) if (read_write == I2C_SMBUS_WRITE)
for (i = 0; i < len; i++) for (i = 0; i < len; i++)
amd_ec_write(smbus, AMD_SMB_DATA + i, data->block[i + 1]); amd_ec_write(smbus, AMD_SMB_DATA + i,
data->block[i + 1]);
protocol |= AMD_SMB_PRTCL_I2C_BLOCK_DATA; protocol |= AMD_SMB_PRTCL_I2C_BLOCK_DATA;
break; break;
case I2C_SMBUS_PROC_CALL: case I2C_SMBUS_PROC_CALL:
amd_ec_write(smbus, AMD_SMB_CMD, command); amd_ec_write(smbus, AMD_SMB_CMD, command);
amd_ec_write(smbus, AMD_SMB_DATA, data->word); amd_ec_write(smbus, AMD_SMB_DATA, data->word & 0xff);
amd_ec_write(smbus, AMD_SMB_DATA + 1, data->word >> 8); amd_ec_write(smbus, AMD_SMB_DATA + 1, data->word >> 8);
protocol = AMD_SMB_PRTCL_PROC_CALL | pec; protocol = AMD_SMB_PRTCL_PROC_CALL | pec;
read_write = I2C_SMBUS_READ; read_write = I2C_SMBUS_READ;
...@@ -248,7 +259,8 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -248,7 +259,8 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
amd_ec_write(smbus, AMD_SMB_CMD, command); amd_ec_write(smbus, AMD_SMB_CMD, command);
amd_ec_write(smbus, AMD_SMB_BCNT, len); amd_ec_write(smbus, AMD_SMB_BCNT, len);
for (i = 0; i < len; i++) for (i = 0; i < len; i++)
amd_ec_write(smbus, AMD_SMB_DATA + i, data->block[i + 1]); amd_ec_write(smbus, AMD_SMB_DATA + i,
data->block[i + 1]);
protocol = AMD_SMB_PRTCL_BLOCK_PROC_CALL | pec; protocol = AMD_SMB_PRTCL_BLOCK_PROC_CALL | pec;
read_write = I2C_SMBUS_READ; read_write = I2C_SMBUS_READ;
break; break;
...@@ -280,7 +292,6 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -280,7 +292,6 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
return 0; return 0;
switch (size) { switch (size) {
case I2C_SMBUS_BYTE: case I2C_SMBUS_BYTE:
case I2C_SMBUS_BYTE_DATA: case I2C_SMBUS_BYTE_DATA:
amd_ec_read(smbus, AMD_SMB_DATA, &data->byte); amd_ec_read(smbus, AMD_SMB_DATA, &data->byte);
...@@ -296,10 +307,11 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -296,10 +307,11 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
case I2C_SMBUS_BLOCK_DATA: case I2C_SMBUS_BLOCK_DATA:
case I2C_SMBUS_BLOCK_PROC_CALL: case I2C_SMBUS_BLOCK_PROC_CALL:
amd_ec_read(smbus, AMD_SMB_BCNT, &len); amd_ec_read(smbus, AMD_SMB_BCNT, &len);
len = min_t(u8, len, 32); len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
case I2C_SMBUS_I2C_BLOCK_DATA: case I2C_SMBUS_I2C_BLOCK_DATA:
for (i = 0; i < len; i++) for (i = 0; i < len; i++)
amd_ec_read(smbus, AMD_SMB_DATA + i, data->block + i + 1); amd_ec_read(smbus, AMD_SMB_DATA + i,
data->block + i + 1);
data->block[0] = len; data->block[0] = len;
break; break;
} }
...@@ -310,7 +322,8 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl ...@@ -310,7 +322,8 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr, unsigned short fl
static u32 amd8111_func(struct i2c_adapter *adapter) static u32 amd8111_func(struct i2c_adapter *adapter)
{ {
return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA | return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE |
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_DATA |
I2C_FUNC_SMBUS_PROC_CALL | I2C_FUNC_SMBUS_BLOCK_PROC_CALL | I2C_FUNC_SMBUS_PROC_CALL | I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
I2C_FUNC_SMBUS_I2C_BLOCK | I2C_FUNC_SMBUS_HWPEC_CALC; I2C_FUNC_SMBUS_I2C_BLOCK | I2C_FUNC_SMBUS_HWPEC_CALC;
...@@ -329,12 +342,13 @@ static struct pci_device_id amd8111_ids[] = { ...@@ -329,12 +342,13 @@ static struct pci_device_id amd8111_ids[] = {
MODULE_DEVICE_TABLE (pci, amd8111_ids); MODULE_DEVICE_TABLE (pci, amd8111_ids);
static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_id *id) static int __devinit amd8111_probe(struct pci_dev *dev,
const struct pci_device_id *id)
{ {
struct amd_smbus *smbus; struct amd_smbus *smbus;
int error = -ENODEV; int error;
if (~pci_resource_flags(dev, 0) & IORESOURCE_IO) if (!(pci_resource_flags(dev, 0) & IORESOURCE_IO))
return -ENODEV; return -ENODEV;
smbus = kzalloc(sizeof(struct amd_smbus), GFP_KERNEL); smbus = kzalloc(sizeof(struct amd_smbus), GFP_KERNEL);
...@@ -345,8 +359,10 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_ ...@@ -345,8 +359,10 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_
smbus->base = pci_resource_start(dev, 0); smbus->base = pci_resource_start(dev, 0);
smbus->size = pci_resource_len(dev, 0); smbus->size = pci_resource_len(dev, 0);
if (!request_region(smbus->base, smbus->size, amd8111_driver.name)) if (!request_region(smbus->base, smbus->size, amd8111_driver.name)) {
error = -EBUSY;
goto out_kfree; goto out_kfree;
}
smbus->adapter.owner = THIS_MODULE; smbus->adapter.owner = THIS_MODULE;
snprintf(smbus->adapter.name, I2C_NAME_SIZE, snprintf(smbus->adapter.name, I2C_NAME_SIZE,
...@@ -359,11 +375,11 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_ ...@@ -359,11 +375,11 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_
/* set up the driverfs linkage to our parent device */ /* set up the driverfs linkage to our parent device */
smbus->adapter.dev.parent = &dev->dev; smbus->adapter.dev.parent = &dev->dev;
pci_write_config_dword(smbus->dev, AMD_PCI_MISC, 0);
error = i2c_add_adapter(&smbus->adapter); error = i2c_add_adapter(&smbus->adapter);
if (error) if (error)
goto out_release_region; goto out_release_region;
pci_write_config_dword(smbus->dev, AMD_PCI_MISC, 0);
pci_set_drvdata(dev, smbus); pci_set_drvdata(dev, smbus);
return 0; return 0;
...@@ -371,10 +387,9 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_ ...@@ -371,10 +387,9 @@ static int __devinit amd8111_probe(struct pci_dev *dev, const struct pci_device_
release_region(smbus->base, smbus->size); release_region(smbus->base, smbus->size);
out_kfree: out_kfree:
kfree(smbus); kfree(smbus);
return -1; return error;
} }
static void __devexit amd8111_remove(struct pci_dev *dev) static void __devexit amd8111_remove(struct pci_dev *dev)
{ {
struct amd_smbus *smbus = pci_get_drvdata(dev); struct amd_smbus *smbus = pci_get_drvdata(dev);
...@@ -396,7 +411,6 @@ static int __init i2c_amd8111_init(void) ...@@ -396,7 +411,6 @@ static int __init i2c_amd8111_init(void)
return pci_register_driver(&amd8111_driver); return pci_register_driver(&amd8111_driver);
} }
static void __exit i2c_amd8111_exit(void) static void __exit i2c_amd8111_exit(void)
{ {
pci_unregister_driver(&amd8111_driver); pci_unregister_driver(&amd8111_driver);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment