1. Patchew
  2. linux
  3. View series

[PATCH] i2c-algo-bit: Added the bus lock during a i2c transaction

Anindya Sundar Gayen posted 1 patch 11 months, 2 weeks ago 
drivers/i2c/algos/i2c-algo-bit.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] i2c-algo-bit: Added the bus lock during a i2c transaction
Posted by Anindya Sundar Gayen 11 months, 2 weeks ago 
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(-)

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;
 }
 
-- 
2.17.1
Re: [PATCH] i2c-algo-bit: Added the bus lock during a i2c transaction
Posted by Dan Carpenter 11 months, 2 weeks ago 
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  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] i2c-algo-bit: Added the bus lock during a i2c transaction
Posted by Andi Shyti 11 months, 2 weeks ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.