tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
fork atom

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>

authored by

Julia Lawall and committed by
Jean Delvare 9cb2c272 ef9d9b8f

+113 -50
+113 -50
drivers/i2c/busses/i2c-amd8111.c
··· 69 69 * ACPI 2.0 chapter 13 access of registers of the EC 70 70 */ 71 71 72 - static unsigned int amd_ec_wait_write(struct amd_smbus *smbus) 72 + static int amd_ec_wait_write(struct amd_smbus *smbus) 73 73 { 74 74 int timeout = 500; 75 75 ··· 85 85 return 0; 86 86 } 87 87 88 - static unsigned int amd_ec_wait_read(struct amd_smbus *smbus) 88 + static int amd_ec_wait_read(struct amd_smbus *smbus) 89 89 { 90 90 int timeout = 500; 91 91 ··· 101 101 return 0; 102 102 } 103 103 104 - static unsigned int amd_ec_read(struct amd_smbus *smbus, unsigned char address, 104 + static int amd_ec_read(struct amd_smbus *smbus, unsigned char address, 105 105 unsigned char *data) 106 106 { 107 107 int status; ··· 124 124 return 0; 125 125 } 126 126 127 - static unsigned int amd_ec_write(struct amd_smbus *smbus, unsigned char address, 127 + static int amd_ec_write(struct amd_smbus *smbus, unsigned char address, 128 128 unsigned char data) 129 129 { 130 130 int status; ··· 196 196 { 197 197 struct amd_smbus *smbus = adap->algo_data; 198 198 unsigned char protocol, len, pec, temp[2]; 199 - int i; 199 + int i, status; 200 200 201 201 protocol = (read_write == I2C_SMBUS_READ) ? AMD_SMB_PRTCL_READ 202 202 : AMD_SMB_PRTCL_WRITE; ··· 209 209 break; 210 210 211 211 case I2C_SMBUS_BYTE: 212 - if (read_write == I2C_SMBUS_WRITE) 213 - amd_ec_write(smbus, AMD_SMB_CMD, command); 212 + if (read_write == I2C_SMBUS_WRITE) { 213 + status = amd_ec_write(smbus, AMD_SMB_CMD, 214 + command); 215 + if (status) 216 + return status; 217 + } 214 218 protocol |= AMD_SMB_PRTCL_BYTE; 215 219 break; 216 220 217 221 case I2C_SMBUS_BYTE_DATA: 218 - amd_ec_write(smbus, AMD_SMB_CMD, command); 219 - if (read_write == I2C_SMBUS_WRITE) 220 - amd_ec_write(smbus, AMD_SMB_DATA, data->byte); 222 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 223 + if (status) 224 + return status; 225 + if (read_write == I2C_SMBUS_WRITE) { 226 + status = amd_ec_write(smbus, AMD_SMB_DATA, 227 + data->byte); 228 + if (status) 229 + return status; 230 + } 221 231 protocol |= AMD_SMB_PRTCL_BYTE_DATA; 222 232 break; 223 233 224 234 case I2C_SMBUS_WORD_DATA: 225 - amd_ec_write(smbus, AMD_SMB_CMD, command); 235 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 236 + if (status) 237 + return status; 226 238 if (read_write == I2C_SMBUS_WRITE) { 227 - amd_ec_write(smbus, AMD_SMB_DATA, 228 - data->word & 0xff); 229 - amd_ec_write(smbus, AMD_SMB_DATA + 1, 230 - data->word >> 8); 239 + status = amd_ec_write(smbus, AMD_SMB_DATA, 240 + data->word & 0xff); 241 + if (status) 242 + return status; 243 + status = amd_ec_write(smbus, AMD_SMB_DATA + 1, 244 + data->word >> 8); 245 + if (status) 246 + return status; 231 247 } 232 248 protocol |= AMD_SMB_PRTCL_WORD_DATA | pec; 233 249 break; 234 250 235 251 case I2C_SMBUS_BLOCK_DATA: 236 - amd_ec_write(smbus, AMD_SMB_CMD, command); 252 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 253 + if (status) 254 + return status; 237 255 if (read_write == I2C_SMBUS_WRITE) { 238 256 len = min_t(u8, data->block[0], 239 257 I2C_SMBUS_BLOCK_MAX); 240 - amd_ec_write(smbus, AMD_SMB_BCNT, len); 241 - for (i = 0; i < len; i++) 242 - amd_ec_write(smbus, AMD_SMB_DATA + i, 243 - data->block[i + 1]); 258 + status = amd_ec_write(smbus, AMD_SMB_BCNT, len); 259 + if (status) 260 + return status; 261 + for (i = 0; i < len; i++) { 262 + status = 263 + amd_ec_write(smbus, AMD_SMB_DATA + i, 264 + data->block[i + 1]); 265 + if (status) 266 + return status; 267 + } 244 268 } 245 269 protocol |= AMD_SMB_PRTCL_BLOCK_DATA | pec; 246 270 break; ··· 272 248 case I2C_SMBUS_I2C_BLOCK_DATA: 273 249 len = min_t(u8, data->block[0], 274 250 I2C_SMBUS_BLOCK_MAX); 275 - amd_ec_write(smbus, AMD_SMB_CMD, command); 276 - amd_ec_write(smbus, AMD_SMB_BCNT, len); 251 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 252 + if (status) 253 + return status; 254 + status = amd_ec_write(smbus, AMD_SMB_BCNT, len); 255 + if (status) 256 + return status; 277 257 if (read_write == I2C_SMBUS_WRITE) 278 - for (i = 0; i < len; i++) 279 - amd_ec_write(smbus, AMD_SMB_DATA + i, 280 - data->block[i + 1]); 258 + for (i = 0; i < len; i++) { 259 + status = 260 + amd_ec_write(smbus, AMD_SMB_DATA + i, 261 + data->block[i + 1]); 262 + if (status) 263 + return status; 264 + } 281 265 protocol |= AMD_SMB_PRTCL_I2C_BLOCK_DATA; 282 266 break; 283 267 284 268 case I2C_SMBUS_PROC_CALL: 285 - amd_ec_write(smbus, AMD_SMB_CMD, command); 286 - amd_ec_write(smbus, AMD_SMB_DATA, data->word & 0xff); 287 - amd_ec_write(smbus, AMD_SMB_DATA + 1, data->word >> 8); 269 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 270 + if (status) 271 + return status; 272 + status = amd_ec_write(smbus, AMD_SMB_DATA, 273 + data->word & 0xff); 274 + if (status) 275 + return status; 276 + status = amd_ec_write(smbus, AMD_SMB_DATA + 1, 277 + data->word >> 8); 278 + if (status) 279 + return status; 288 280 protocol = AMD_SMB_PRTCL_PROC_CALL | pec; 289 281 read_write = I2C_SMBUS_READ; 290 282 break; ··· 308 268 case I2C_SMBUS_BLOCK_PROC_CALL: 309 269 len = min_t(u8, data->block[0], 310 270 I2C_SMBUS_BLOCK_MAX - 1); 311 - amd_ec_write(smbus, AMD_SMB_CMD, command); 312 - amd_ec_write(smbus, AMD_SMB_BCNT, len); 313 - for (i = 0; i < len; i++) 314 - amd_ec_write(smbus, AMD_SMB_DATA + i, 315 - data->block[i + 1]); 271 + status = amd_ec_write(smbus, AMD_SMB_CMD, command); 272 + if (status) 273 + return status; 274 + status = amd_ec_write(smbus, AMD_SMB_BCNT, len); 275 + if (status) 276 + return status; 277 + for (i = 0; i < len; i++) { 278 + status = amd_ec_write(smbus, AMD_SMB_DATA + i, 279 + data->block[i + 1]); 280 + if (status) 281 + return status; 282 + } 316 283 protocol = AMD_SMB_PRTCL_BLOCK_PROC_CALL | pec; 317 284 read_write = I2C_SMBUS_READ; 318 285 break; ··· 329 282 return -EOPNOTSUPP; 330 283 } 331 284 332 - amd_ec_write(smbus, AMD_SMB_ADDR, addr << 1); 333 - amd_ec_write(smbus, AMD_SMB_PRTCL, protocol); 285 + status = amd_ec_write(smbus, AMD_SMB_ADDR, addr << 1); 286 + if (status) 287 + return status; 288 + status = amd_ec_write(smbus, AMD_SMB_PRTCL, protocol); 289 + if (status) 290 + return status; 334 291 335 - /* FIXME this discards status from ec_read(); so temp[0] will 336 - * hold stack garbage ... the rest of this routine will act 337 - * nonsensically. Ignored ec_write() status might explain 338 - * some such failures... 339 - */ 340 - amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 292 + status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 293 + if (status) 294 + return status; 341 295 342 296 if (~temp[0] & AMD_SMB_STS_DONE) { 343 297 udelay(500); 344 - amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 298 + status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 299 + if (status) 300 + return status; 345 301 } 346 302 347 303 if (~temp[0] & AMD_SMB_STS_DONE) { 348 304 msleep(1); 349 - amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 305 + status = amd_ec_read(smbus, AMD_SMB_STS, temp + 0); 306 + if (status) 307 + return status; 350 308 } 351 309 352 310 if ((~temp[0] & AMD_SMB_STS_DONE) || (temp[0] & AMD_SMB_STS_STATUS)) ··· 363 311 switch (size) { 364 312 case I2C_SMBUS_BYTE: 365 313 case I2C_SMBUS_BYTE_DATA: 366 - amd_ec_read(smbus, AMD_SMB_DATA, &data->byte); 314 + status = amd_ec_read(smbus, AMD_SMB_DATA, &data->byte); 315 + if (status) 316 + return status; 367 317 break; 368 318 369 319 case I2C_SMBUS_WORD_DATA: 370 320 case I2C_SMBUS_PROC_CALL: 371 - amd_ec_read(smbus, AMD_SMB_DATA, temp + 0); 372 - amd_ec_read(smbus, AMD_SMB_DATA + 1, temp + 1); 321 + status = amd_ec_read(smbus, AMD_SMB_DATA, temp + 0); 322 + if (status) 323 + return status; 324 + status = amd_ec_read(smbus, AMD_SMB_DATA + 1, temp + 1); 325 + if (status) 326 + return status; 373 327 data->word = (temp[1] << 8) | temp[0]; 374 328 break; 375 329 376 330 case I2C_SMBUS_BLOCK_DATA: 377 331 case I2C_SMBUS_BLOCK_PROC_CALL: 378 - amd_ec_read(smbus, AMD_SMB_BCNT, &len); 332 + status = amd_ec_read(smbus, AMD_SMB_BCNT, &len); 333 + if (status) 334 + return status; 379 335 len = min_t(u8, len, I2C_SMBUS_BLOCK_MAX); 380 336 case I2C_SMBUS_I2C_BLOCK_DATA: 381 - for (i = 0; i < len; i++) 382 - amd_ec_read(smbus, AMD_SMB_DATA + i, 383 - data->block + i + 1); 337 + for (i = 0; i < len; i++) { 338 + status = amd_ec_read(smbus, AMD_SMB_DATA + i, 339 + data->block + i + 1); 340 + if (status) 341 + return status; 342 + } 384 343 data->block[0] = len; 385 344 break; 386 345 }