i2c-amd8111: Add proper error handling

The functions the functions amd_ec_wait_write and amd_ec_wait_read have an
unsigned return type, but return a negative constant to indicate an error
condition.

A sematic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@exists@
identifier f;
constant C;
@@

 unsigned f(...)
 { <+...
*  return -C;
 ...+> }
// </smpl>

Fixing amd_ec_wait_write and amd_ec_wait_read leads to the need to adjust
the return type of the functions amd_ec_write and amd_ec_read, which are
the only functions that call amd_ec_wait_write and amd_ec_wait_read.
amd_ec_write and amd_ec_read, in turn, are only called from within the
function amd8111_access, which already returns a signed typed value.  Each
of the calls to amd_ec_write and amd_ec_read are updated using the
following semantic patch:

// <smpl>
@@
@@

+ status = amd_ec_write
- amd_ec_write
  (...);
+ if (status) return status;

@@
@@

+ status = amd_ec_read
- amd_ec_read
  (...);
+ if (status) return status;
// </smpl>

The patch also adds the declaration of the status variable.

Signed-off-by: Julia Lawall <julia@diku.dk>
Signed-off-by: Jean Delvare <khali@linux-fr.org>

1 files changed, 113 insertions, 50 deletions

diff --git a/drivers/i2c/busses/i2c-amd8111.c b/drivers/i2c/busses/i2c-amd8111.c
index af1e5e254b7b..6b6a6b1d7025 100644
--- a/drivers/i2c/busses/i2c-amd8111.c
+++ b/drivers/i2c/busses/i2c-amd8111.c
@@ -69,7 +69,7 @@ static struct pci_driver amd8111_driver;
  * ACPI 2.0 chapter 13 access of registers of the EC
  */
 
-static unsigned int amd_ec_wait_write(struct amd_smbus *smbus)
+static int amd_ec_wait_write(struct amd_smbus *smbus)
 {
         int timeout = 500;
 
@@ -85,7 +85,7 @@ static unsigned int amd_ec_wait_write(struct amd_smbus *smbus)
         return 0;
 }
 
-static unsigned int amd_ec_wait_read(struct amd_smbus *smbus)
+static int amd_ec_wait_read(struct amd_smbus *smbus)
 {
         int timeout = 500;
 
@@ -101,7 +101,7 @@ static unsigned int amd_ec_wait_read(struct amd_smbus *smbus)
         return 0;
 }
 
-static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address,
+static int amd_ec_read(struct amd_smbus *smbus, unsigned char address,
                 unsigned char *data)
 {
         int status;
@@ -124,7 +124,7 @@ static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address,
         return 0;
 }
 
-static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address,
+static int amd_ec_write(struct amd_smbus *smbus, unsigned char address,
                 unsigned char data)
 {
         int status;
@@ -196,7 +196,7 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
 {
         struct amd_smbus *smbus = adap->algo_data;
         unsigned char protocol, len, pec, temp[2];
-        int i;
+        int i, status;
 
         protocol = (read_write == I2C_SMBUS_READ) ? AMD_SMB_PRTCL_READ
                                                   : AMD_SMB_PRTCL_WRITE;
@@ -209,38 +209,62 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
                         break;
 
                 case I2C_SMBUS_BYTE:
-                        if (read_write == I2C_SMBUS_WRITE)
-                                amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (read_write == I2C_SMBUS_WRITE) {
+                                status = amd_ec_write(smbus, AMD_SMB_CMD,
+                                                      command);
+                                if (status)
+                                        return status;
+                        }
                         protocol |= AMD_SMB_PRTCL_BYTE;
                         break;
 
                 case I2C_SMBUS_BYTE_DATA:
-                        amd_ec_write(smbus, AMD_SMB_CMD, command);
-                        if (read_write == I2C_SMBUS_WRITE)
-                                amd_ec_write(smbus, AMD_SMB_DATA, data->byte);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
+                        if (read_write == I2C_SMBUS_WRITE) {
+                                status = amd_ec_write(smbus, AMD_SMB_DATA,
+                                                      data->byte);
+                                if (status)
+                                        return status;
+                        }
                         protocol |= AMD_SMB_PRTCL_BYTE_DATA;
                         break;
 
                 case I2C_SMBUS_WORD_DATA:
-                        amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
                         if (read_write == I2C_SMBUS_WRITE) {
-                                amd_ec_write(smbus, AMD_SMB_DATA,
-                                             data->word & 0xff);
-                                amd_ec_write(smbus, AMD_SMB_DATA + 1,
-                                             data->word >> 8);
+                                status = amd_ec_write(smbus, AMD_SMB_DATA,
+                                                      data->word & 0xff);
+                                if (status)
+                                        return status;
+                                status = amd_ec_write(smbus, AMD_SMB_DATA + 1,
+                                                      data->word >> 8);
+                                if (status)
+                                        return status;
                         }
                         protocol |= AMD_SMB_PRTCL_WORD_DATA | pec;
                         break;
 
                 case I2C_SMBUS_BLOCK_DATA:
-                        amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
                         if (read_write == I2C_SMBUS_WRITE) {
                                 len = min_t(u8, data->block[0],
                                             I2C_SMBUS_BLOCK_MAX);
-                                amd_ec_write(smbus, AMD_SMB_BCNT, len);
-                                for (i = 0; i < len; i++)
-                                        amd_ec_write(smbus, AMD_SMB_DATA + i,
-                                                     data->block[i + 1]);
+                                status = amd_ec_write(smbus, AMD_SMB_BCNT, len);
+                                if (status)
+                                        return status;
+                                for (i = 0; i < len; i++) {
+                                        status =
+                                          amd_ec_write(smbus, AMD_SMB_DATA + i,
+                                                       data->block[i + 1]);
+                                        if (status)
+                                                return status;
+                                }
                         }
                         protocol |= AMD_SMB_PRTCL_BLOCK_DATA | pec;
                         break;
@@ -248,19 +272,35 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
                 case I2C_SMBUS_I2C_BLOCK_DATA:
                         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_BCNT, len);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
+                        status = amd_ec_write(smbus, AMD_SMB_BCNT, len);
+                        if (status)
+                                return status;
                         if (read_write == I2C_SMBUS_WRITE)
-                                for (i = 0; i < len; i++)
-                                        amd_ec_write(smbus, AMD_SMB_DATA + i,
-                                                     data->block[i + 1]);
+                                for (i = 0; i < len; i++) {
+                                        status =
+                                          amd_ec_write(smbus, AMD_SMB_DATA + i,
+                                                       data->block[i + 1]);
+                                        if (status)
+                                                return status;
+                                }
                         protocol |= AMD_SMB_PRTCL_I2C_BLOCK_DATA;
                         break;
 
                 case I2C_SMBUS_PROC_CALL:
-                        amd_ec_write(smbus, AMD_SMB_CMD, command);
-                        amd_ec_write(smbus, AMD_SMB_DATA, data->word & 0xff);
-                        amd_ec_write(smbus, AMD_SMB_DATA + 1, data->word >> 8);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
+                        status = amd_ec_write(smbus, AMD_SMB_DATA,
+                                              data->word & 0xff);
+                        if (status)
+                                return status;
+                        status = amd_ec_write(smbus, AMD_SMB_DATA + 1,
+                                              data->word >> 8);
+                        if (status)
+                                return status;
                         protocol = AMD_SMB_PRTCL_PROC_CALL | pec;
                         read_write = I2C_SMBUS_READ;
                         break;
@@ -268,11 +308,18 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
                 case I2C_SMBUS_BLOCK_PROC_CALL:
                         len = min_t(u8, data->block[0],
                                     I2C_SMBUS_BLOCK_MAX - 1);
-                        amd_ec_write(smbus, AMD_SMB_CMD, command);
-                        amd_ec_write(smbus, AMD_SMB_BCNT, len);
-                        for (i = 0; i < len; i++)
-                                amd_ec_write(smbus, AMD_SMB_DATA + i,
-                                             data->block[i + 1]);
+                        status = amd_ec_write(smbus, AMD_SMB_CMD, command);
+                        if (status)
+                                return status;
+                        status = amd_ec_write(smbus, AMD_SMB_BCNT, len);
+                        if (status)
+                                return status;
+                        for (i = 0; i < len; i++) {
+                                status = amd_ec_write(smbus, AMD_SMB_DATA + i,
+                                                      data->block[i + 1]);
+                                if (status)
+                                        return status;
+                        }
                         protocol = AMD_SMB_PRTCL_BLOCK_PROC_CALL | pec;
                         read_write = I2C_SMBUS_READ;
                         break;
@@ -282,24 +329,29 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
                         return -EOPNOTSUPP;
         }
 
-        amd_ec_write(smbus, AMD_SMB_ADDR, addr << 1);
-        amd_ec_write(smbus, AMD_SMB_PRTCL, protocol);
+        status = amd_ec_write(smbus, AMD_SMB_ADDR, addr << 1);
+        if (status)
+                return status;
+        status = amd_ec_write(smbus, AMD_SMB_PRTCL, protocol);
+        if (status)
+                return status;
 
-        /* FIXME this discards status from ec_read(); so temp[0] will
-         * hold stack garbage ... the rest of this routine will act
-         * nonsensically.  Ignored ec_write() status might explain
-         * some such failures...
-         */
-        amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+        status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+        if (status)
+                return status;
 
         if (~temp[0] & AMD_SMB_STS_DONE) {
                 udelay(500);
-                amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+                status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+                if (status)
+                        return status;
         }
 
         if (~temp[0] & AMD_SMB_STS_DONE) {
                 msleep(1);
-                amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+                status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0);
+                if (status)
+                        return status;
         }
 
         if ((~temp[0] & AMD_SMB_STS_DONE) || (temp[0] & AMD_SMB_STS_STATUS))
@@ -311,24 +363,35 @@ static s32 amd8111_access(struct i2c_adapter * adap, u16 addr,
         switch (size) {
                 case I2C_SMBUS_BYTE:
                 case I2C_SMBUS_BYTE_DATA:
-                        amd_ec_read(smbus, AMD_SMB_DATA, &data->byte);
+                        status = amd_ec_read(smbus, AMD_SMB_DATA, &data->byte);
+                        if (status)
+                                return status;
                         break;
 
                 case I2C_SMBUS_WORD_DATA:
                 case I2C_SMBUS_PROC_CALL:
-                        amd_ec_read(smbus, AMD_SMB_DATA, temp + 0);
-                        amd_ec_read(smbus, AMD_SMB_DATA + 1, temp + 1);
+                        status = amd_ec_read(smbus, AMD_SMB_DATA, temp + 0);
+                        if (status)
+                                return status;
+                        status = amd_ec_read(smbus, AMD_SMB_DATA + 1, temp + 1);
+                        if (status)
+                                return status;
                         data->word = (temp[1] << 8) | temp[0];
                         break;
 
                 case I2C_SMBUS_BLOCK_DATA:
                 case I2C_SMBUS_BLOCK_PROC_CALL:
-                        amd_ec_read(smbus, AMD_SMB_BCNT, &len);
+                        status = amd_ec_read(smbus, AMD_SMB_BCNT, &len);
+                        if (status)
+                                return status;
                         len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX);
                 case I2C_SMBUS_I2C_BLOCK_DATA:
-                        for (i = 0; i < len; i++)
-                                amd_ec_read(smbus, AMD_SMB_DATA + i,
-                                            data->block + i + 1);
+                        for (i = 0; i < len; i++) {
+                                status = amd_ec_read(smbus, AMD_SMB_DATA + i,
+                                                     data->block + i + 1);
+                                if (status)
+                                        return status;
+                        }
                         data->block[0] = len;
                         break;
         }