| Message ID | 20250225113314.12353-1-anindya.sg@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c-algo-bit: Added the bus lock during a i2c transaction | expand |
Hi, On Tue, Feb 25, 2025 at 05:03:14PM +0530, Anindya Sundar Gayen wrote: > Earlier i2c-bus lock was not available to i2c-algo-bit driver > for that reason if there are back to back i2c transaction occurs > it may be possible to occur a race condition. > > To avoid the race condition we added a mutex lock mechanism which will > help to protect the i2c_outb()with a proper lock. what is the issue you are trying to fix? Can you refer to a practical failure you received? Thanks, Andi
Hi Anindya,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anindya-Sundar-Gayen/i2c-algo-bit-Added-the-bus-lock-during-a-i2c-transaction/20250225-201732
base: https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link: https://lore.kernel.org/r/20250225113314.12353-1-anindya.sg%40samsung.com
patch subject: [PATCH] i2c-algo-bit: Added the bus lock during a i2c transaction
config: powerpc-randconfig-r071-20250228 (https://download.01.org/0day-ci/archive/20250228/202502281946.tIwDrE9k-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 204dcafec0ecf0db81d420d2de57b02ada6b09ec)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202502281946.tIwDrE9k-lkp@intel.com/
smatch warnings:
drivers/i2c/algos/i2c-algo-bit.c:385 sendbytes() warn: inconsistent returns 'global &i2c_bus_lock'.
vim +385 drivers/i2c/algos/i2c-algo-bit.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 344 static int sendbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg)
^1da177e4c3f41 Linus Torvalds 2005-04-16 345 {
494dbb64dc5b36 Jean Delvare 2007-05-01 346 const unsigned char *temp = msg->buf;
^1da177e4c3f41 Linus Torvalds 2005-04-16 347 int count = msg->len;
^1da177e4c3f41 Linus Torvalds 2005-04-16 348 unsigned short nak_ok = msg->flags & I2C_M_IGNORE_NAK;
^1da177e4c3f41 Linus Torvalds 2005-04-16 349 int retval;
^1da177e4c3f41 Linus Torvalds 2005-04-16 350 int wrcount = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 351
8eb6c9a270e167 Anindya Sundar Gayen 2025-02-25 352 /* Aquire the lock before accessing the I2C bus */
8eb6c9a270e167 Anindya Sundar Gayen 2025-02-25 353 mutex_lock(&i2c_bus_lock);
^1da177e4c3f41 Linus Torvalds 2005-04-16 354 while (count > 0) {
494dbb64dc5b36 Jean Delvare 2007-05-01 355 retval = i2c_outb(i2c_adap, *temp);
cf978ab2846d86 David Brownell 2008-01-27 356
cf978ab2846d86 David Brownell 2008-01-27 357 /* OK/ACK; or ignored NAK */
cf978ab2846d86 David Brownell 2008-01-27 358 if ((retval > 0) || (nak_ok && (retval == 0))) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 359 count--;
^1da177e4c3f41 Linus Torvalds 2005-04-16 360 temp++;
^1da177e4c3f41 Linus Torvalds 2005-04-16 361 wrcount++;
bf3e2d1d9b8605 David Brownell 2008-01-27 362
bf3e2d1d9b8605 David Brownell 2008-01-27 363 /* A slave NAKing the master means the slave didn't like
bf3e2d1d9b8605 David Brownell 2008-01-27 364 * something about the data it saw. For example, maybe
bf3e2d1d9b8605 David Brownell 2008-01-27 365 * the SMBus PEC was wrong.
bf3e2d1d9b8605 David Brownell 2008-01-27 366 */
bf3e2d1d9b8605 David Brownell 2008-01-27 367 } else if (retval == 0) {
bf3e2d1d9b8605 David Brownell 2008-01-27 368 dev_err(&i2c_adap->dev, "sendbytes: NAK bailout.\n");
bf3e2d1d9b8605 David Brownell 2008-01-27 369 return -EIO;
mutex_unlock(&i2c_bus_lock);
bf3e2d1d9b8605 David Brownell 2008-01-27 370
bf3e2d1d9b8605 David Brownell 2008-01-27 371 /* Timeout; or (someday) lost arbitration
bf3e2d1d9b8605 David Brownell 2008-01-27 372 *
bf3e2d1d9b8605 David Brownell 2008-01-27 373 * FIXME Lost ARB implies retrying the transaction from
bf3e2d1d9b8605 David Brownell 2008-01-27 374 * the first message, after the "winning" master issues
bf3e2d1d9b8605 David Brownell 2008-01-27 375 * its STOP. As a rule, upper layer code has no reason
bf3e2d1d9b8605 David Brownell 2008-01-27 376 * to know or care about this ... it is *NOT* an error.
bf3e2d1d9b8605 David Brownell 2008-01-27 377 */
bf3e2d1d9b8605 David Brownell 2008-01-27 378 } else {
bf3e2d1d9b8605 David Brownell 2008-01-27 379 dev_err(&i2c_adap->dev, "sendbytes: error %d\n",
bf3e2d1d9b8605 David Brownell 2008-01-27 380 retval);
bf3e2d1d9b8605 David Brownell 2008-01-27 381 return retval;
And here too.
^1da177e4c3f41 Linus Torvalds 2005-04-16 382 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 383 }
8eb6c9a270e167 Anindya Sundar Gayen 2025-02-25 384 mutex_unlock(&i2c_bus_lock);
^1da177e4c3f41 Linus Torvalds 2005-04-16 @385 return wrcount;
^1da177e4c3f41 Linus Torvalds 2005-04-16 386 }
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c index eddf25b90ca8..18021dd9eef3 100644 --- a/drivers/i2c/algos/i2c-algo-bit.c +++ b/drivers/i2c/algos/i2c-algo-bit.c @@ -31,7 +31,7 @@ #endif /* DEBUG */ /* ----- global variables --------------------------------------------- */ - +static DEFINE_MUTEX(i2c_bus_lock); static int bit_test; /* see if the line-setting functions work */ module_param(bit_test, int, S_IRUGO); MODULE_PARM_DESC(bit_test, "lines testing - 0 off; 1 report; 2 fail if stuck"); @@ -349,6 +349,8 @@ static int sendbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg) int retval; int wrcount = 0; + /* Aquire the lock before accessing the I2C bus */ + mutex_lock(&i2c_bus_lock); while (count > 0) { retval = i2c_outb(i2c_adap, *temp); @@ -379,6 +381,7 @@ static int sendbytes(struct i2c_adapter *i2c_adap, struct i2c_msg *msg) return retval; } } + mutex_unlock(&i2c_bus_lock); return wrcount; }
Earlier i2c-bus lock was not available to i2c-algo-bit driver for that reason if there are back to back i2c transaction occurs it may be possible to occur a race condition. To avoid the race condition we added a mutex lock mechanism which will help to protect the i2c_outb()with a proper lock. Signed-off-by: Anindya Sundar Gayen <anindya.sg@samsung.com> --- drivers/i2c/algos/i2c-algo-bit.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)