Skip to content

Commit

Permalink
i2c: Bus drivers return -Errno not -1
Browse files Browse the repository at this point in the history 
Tighten error paths used by various i2c adapters (mostly x86) so
they return real fault/errno codes instead of a "-1" (which is
most often interpreted as "-EPERM").  Build tested, with eyeball
review.

One minor initial goal is to have adapters consistently return
the code "-ENXIO" when addressing a device doesn't get an ACK
response, at least in the probe paths where they are already
good at stifling related logspam.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Jean Delvare <khali@linux-fr.org>
  • Loading branch information
David Brownell authored and Jean Delvare committed Jul 14, 2008
1 parent 6ea438e commit 9714034
Show file tree
Hide file tree
Showing 15 changed files with 191 additions and 148 deletions.
4 changes: 2 additions & 2 deletions drivers/i2c/algos/i2c-algo-bit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ static int try_address(struct i2c_adapter *i2c_adap,
unsigned char addr, int retries)
{
struct i2c_algo_bit_data *adap = i2c_adap->algo_data;
int i, ret = -1;
int i, ret = 0;

for (i = 0; i <= retries; i++) {
ret = i2c_outb(i2c_adap, addr);
Expand Down Expand Up @@ -508,7 +508,7 @@ static int bit_doAddress(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
addr ^= 1;
ret = try_address(i2c_adap, addr, retries);
if ((ret != 1) && !nak_ok)
return -EREMOTEIO;
return -ENXIO;
}

return 0;
Expand Down
22 changes: 10 additions & 12 deletions drivers/i2c/busses/i2c-ali1535.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ static int ali1535_transaction(struct i2c_adapter *adap)
dev_err(&adap->dev,
"SMBus reset failed! (0x%02x) - controller or "
"device on bus is probably hung\n", temp);
return -1;
return -EBUSY;
}
} else {
/* check and clear done bit */
Expand All @@ -281,12 +281,12 @@ static int ali1535_transaction(struct i2c_adapter *adap)

/* If the SMBus is still busy, we give up */
if (timeout >= MAX_TIMEOUT) {
result = -1;
result = -ETIMEDOUT;
dev_err(&adap->dev, "SMBus Timeout!\n");
}

if (temp & ALI1535_STS_FAIL) {
result = -1;
result = -EIO;
dev_dbg(&adap->dev, "Error: Failed bus transaction\n");
}

Expand All @@ -295,21 +295,21 @@ static int ali1535_transaction(struct i2c_adapter *adap)
* do a printk. This means that bus collisions go unreported.
*/
if (temp & ALI1535_STS_BUSERR) {
result = -1;
result = -ENXIO;
dev_dbg(&adap->dev,
"Error: no response or bus collision ADD=%02x\n",
inb_p(SMBHSTADD));
}

/* haven't ever seen this */
if (temp & ALI1535_STS_DEV) {
result = -1;
result = -EIO;
dev_err(&adap->dev, "Error: device error\n");
}

/* check to see if the "command complete" indication is set */
if (!(temp & ALI1535_STS_DONE)) {
result = -1;
result = -ETIMEDOUT;
dev_err(&adap->dev, "Error: command never completed\n");
}

Expand All @@ -332,7 +332,7 @@ static int ali1535_transaction(struct i2c_adapter *adap)
return result;
}

/* Return -1 on error. */
/* Return negative errno on error. */
static s32 ali1535_access(struct i2c_adapter *adap, u16 addr,
unsigned short flags, char read_write, u8 command,
int size, union i2c_smbus_data *data)
Expand All @@ -359,7 +359,7 @@ static s32 ali1535_access(struct i2c_adapter *adap, u16 addr,
switch (size) {
case I2C_SMBUS_PROC_CALL:
dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
result = -1;
result = -EOPNOTSUPP;
goto EXIT;
case I2C_SMBUS_QUICK:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
Expand Down Expand Up @@ -420,11 +420,9 @@ static s32 ali1535_access(struct i2c_adapter *adap, u16 addr,
break;
}

if (ali1535_transaction(adap)) {
/* Error in transaction */
result = -1;
result = ali1535_transaction(adap);
if (result)
goto EXIT;
}

if ((read_write == I2C_SMBUS_WRITE) || (size == ALI1535_QUICK)) {
result = 0;
Expand Down
23 changes: 17 additions & 6 deletions drivers/i2c/busses/i2c-ali1563.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ static int ali1563_transaction(struct i2c_adapter * a, int size)
{
u32 data;
int timeout;
int status = -EIO;

dev_dbg(&a->dev, "Transaction (pre): STS=%02x, CNTL1=%02x, "
"CNTL2=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
Expand Down Expand Up @@ -103,13 +104,15 @@ static int ali1563_transaction(struct i2c_adapter * a, int size)
/* Issue 'kill' to host controller */
outb_p(HST_CNTL2_KILL,SMB_HST_CNTL2);
data = inb_p(SMB_HST_STS);
status = -ETIMEDOUT;
}

/* device error - no response, ignore the autodetection case */
if ((data & HST_STS_DEVERR) && (size != HST_CNTL2_QUICK)) {
dev_err(&a->dev, "Device error!\n");
if (data & HST_STS_DEVERR) {
if (size != HST_CNTL2_QUICK)
dev_err(&a->dev, "Device error!\n");
status = -ENXIO;
}

/* bus collision */
if (data & HST_STS_BUSERR) {
dev_err(&a->dev, "Bus collision!\n");
Expand All @@ -122,13 +125,14 @@ static int ali1563_transaction(struct i2c_adapter * a, int size)
outb_p(0x0,SMB_HST_CNTL2);
}

return -1;
return status;
}

static int ali1563_block_start(struct i2c_adapter * a)
{
u32 data;
int timeout;
int status = -EIO;

dev_dbg(&a->dev, "Block (pre): STS=%02x, CNTL1=%02x, "
"CNTL2=%02x, CMD=%02x, ADD=%02x, DAT0=%02x, DAT1=%02x\n",
Expand Down Expand Up @@ -164,13 +168,20 @@ static int ali1563_block_start(struct i2c_adapter * a)

if (timeout && !(data & HST_STS_BAD))
return 0;

if (timeout == 0)
status = -ETIMEDOUT;

if (data & HST_STS_DEVERR)
status = -ENXIO;

dev_err(&a->dev, "SMBus Error: %s%s%s%s%s\n",
timeout ? "Timeout " : "",
timeout ? "" : "Timeout ",
data & HST_STS_FAIL ? "Transaction Failed " : "",
data & HST_STS_BUSERR ? "No response or Bus Collision " : "",
data & HST_STS_DEVERR ? "Device Error " : "",
!(data & HST_STS_DONE) ? "Transaction Never Finished " : "");
return -1;
return status;
}

static int ali1563_block(struct i2c_adapter * a, union i2c_smbus_data * data, u8 rw)
Expand Down
19 changes: 10 additions & 9 deletions drivers/i2c/busses/i2c-ali15x3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ static int ali15x3_transaction(struct i2c_adapter *adap)
dev_err(&adap->dev, "SMBus reset failed! (0x%02x) - "
"controller or device on bus is probably hung\n",
temp);
return -1;
return -EBUSY;
}
} else {
/* check and clear done bit */
Expand All @@ -304,12 +304,12 @@ static int ali15x3_transaction(struct i2c_adapter *adap)

/* If the SMBus is still busy, we give up */
if (timeout >= MAX_TIMEOUT) {
result = -1;
result = -ETIMEDOUT;
dev_err(&adap->dev, "SMBus Timeout!\n");
}

if (temp & ALI15X3_STS_TERM) {
result = -1;
result = -EIO;
dev_dbg(&adap->dev, "Error: Failed bus transaction\n");
}

Expand All @@ -320,15 +320,15 @@ static int ali15x3_transaction(struct i2c_adapter *adap)
This means that bus collisions go unreported.
*/
if (temp & ALI15X3_STS_COLL) {
result = -1;
result = -ENXIO;
dev_dbg(&adap->dev,
"Error: no response or bus collision ADD=%02x\n",
inb_p(SMBHSTADD));
}

/* haven't ever seen this */
if (temp & ALI15X3_STS_DEV) {
result = -1;
result = -EIO;
dev_err(&adap->dev, "Error: device error\n");
}
dev_dbg(&adap->dev, "Transaction (post): STS=%02x, CNT=%02x, CMD=%02x, "
Expand All @@ -338,7 +338,7 @@ static int ali15x3_transaction(struct i2c_adapter *adap)
return result;
}

/* Return -1 on error. */
/* Return negative errno on error. */
static s32 ali15x3_access(struct i2c_adapter * adap, u16 addr,
unsigned short flags, char read_write, u8 command,
int size, union i2c_smbus_data * data)
Expand All @@ -364,7 +364,7 @@ static s32 ali15x3_access(struct i2c_adapter * adap, u16 addr,
switch (size) {
case I2C_SMBUS_PROC_CALL:
dev_err(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
return -1;
return -EOPNOTSUPP;
case I2C_SMBUS_QUICK:
outb_p(((addr & 0x7f) << 1) | (read_write & 0x01),
SMBHSTADD);
Expand Down Expand Up @@ -421,8 +421,9 @@ static s32 ali15x3_access(struct i2c_adapter * adap, u16 addr,

outb_p(size, SMBHSTCNT); /* output command */

if (ali15x3_transaction(adap)) /* Error in transaction */
return -1;
temp = ali15x3_transaction(adap);
if (temp)
return temp;

if ((read_write == I2C_SMBUS_WRITE) || (size == ALI15X3_QUICK))
return 0;
Expand Down
4 changes: 2 additions & 2 deletions drivers/i2c/busses/i2c-amd756-s4882.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static s32 amd756_access_virt0(struct i2c_adapter * adap, u16 addr,
/* We exclude the multiplexed addresses */
if (addr == 0x4c || (addr & 0xfc) == 0x50 || (addr & 0xfc) == 0x30
|| addr == 0x18)
return -1;
return -ENXIO;

mutex_lock(&amd756_lock);

Expand Down Expand Up @@ -86,7 +86,7 @@ static inline s32 amd756_access_channel(struct i2c_adapter * adap, u16 addr,

/* We exclude the non-multiplexed addresses */
if (addr != 0x4c && (addr & 0xfc) != 0x50 && (addr & 0xfc) != 0x30)
return -1;
return -ENXIO;

mutex_lock(&amd756_lock);

Expand Down
18 changes: 10 additions & 8 deletions drivers/i2c/busses/i2c-amd756.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,17 @@ static int amd756_transaction(struct i2c_adapter *adap)
}

if (temp & GS_PRERR_STS) {
result = -1;
result = -ENXIO;
dev_dbg(&adap->dev, "SMBus Protocol error (no response)!\n");
}

if (temp & GS_COL_STS) {
result = -1;
result = -EIO;
dev_warn(&adap->dev, "SMBus collision!\n");
}

if (temp & GS_TO_STS) {
result = -1;
result = -ETIMEDOUT;
dev_dbg(&adap->dev, "SMBus protocol timeout!\n");
}

Expand Down Expand Up @@ -189,22 +189,23 @@ static int amd756_transaction(struct i2c_adapter *adap)
outw_p(inw(SMB_GLOBAL_ENABLE) | GE_ABORT, SMB_GLOBAL_ENABLE);
msleep(100);
outw_p(GS_CLEAR_STS, SMB_GLOBAL_STATUS);
return -1;
return -EIO;
}

/* Return -1 on error. */
/* Return negative errno on error. */
static s32 amd756_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 status;

/** TODO: Should I supporte the 10-bit transfers? */
switch (size) {
case I2C_SMBUS_PROC_CALL:
dev_dbg(&adap->dev, "I2C_SMBUS_PROC_CALL not supported!\n");
/* TODO: Well... It is supported, I'm just not sure what to do here... */
return -1;
return -EOPNOTSUPP;
case I2C_SMBUS_QUICK:
outw_p(((addr & 0x7f) << 1) | (read_write & 0x01),
SMB_HOST_ADDRESS);
Expand Down Expand Up @@ -256,8 +257,9 @@ static s32 amd756_access(struct i2c_adapter * adap, u16 addr,
/* How about enabling interrupts... */
outw_p(size & GE_CYC_TYPE_MASK, SMB_GLOBAL_ENABLE);

if (amd756_transaction(adap)) /* Error in transaction */
return -1;
status = amd756_transaction(adap);
if (status)
return status;

if ((read_write == I2C_SMBUS_WRITE) || (size == AMD756_QUICK))
return 0;
Expand Down
Loading

0 comments on commit 9714034

Please sign in to comment.