linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect
@ 2025-06-26 16:15 Conor Dooley
  2025-06-28 14:28 ` kernel test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Conor Dooley @ 2025-06-26 16:15 UTC (permalink / raw)
  To: linux-i2c; +Cc: conor, Conor Dooley, Daire McNamara, Andi Shyti, linux-kernel

From: Conor Dooley <conor.dooley@microchip.com>

Introducing support for smbus re-broke i2cdetect, causing it to detect
devices at every i2c address, just as it did prior to being fixed in
commit 49e1f0fd0d4cb ("i2c: microchip-core: fix "ghost" detections").
This was caused by an oversight, where the new smbus code failed to
check the return value of mchp_corei2c_xfer(). Check it, and propagate
any errors.

Fixes: d6ceb40538263 ("i2c: microchip-corei2c: add smbus support")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
v2:
- replace "if (ret)" with "if (ret < 0)" because the xfer callback,
  which the smbus code re-uses, can return positive numbers in success
  cases.

CC: Conor Dooley <conor.dooley@microchip.com>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: Andi Shyti <andi.shyti@kernel.org>
CC: linux-i2c@vger.kernel.org
CC: linux-kernel@vger.kernel.org
---
 drivers/i2c/busses/i2c-microchip-corei2c.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/i2c/busses/i2c-microchip-corei2c.c b/drivers/i2c/busses/i2c-microchip-corei2c.c
index 492bf4c34722c..7505bce3a06cd 100644
--- a/drivers/i2c/busses/i2c-microchip-corei2c.c
+++ b/drivers/i2c/busses/i2c-microchip-corei2c.c
@@ -435,6 +435,7 @@ static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned
 	u8 tx_buf[I2C_SMBUS_BLOCK_MAX + 2];
 	u8 rx_buf[I2C_SMBUS_BLOCK_MAX + 1];
 	int num_msgs = 1;
+	int ret;
 
 	msgs[CORE_I2C_SMBUS_MSG_WR].addr = addr;
 	msgs[CORE_I2C_SMBUS_MSG_WR].flags = 0;
@@ -506,6 +507,9 @@ static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned
 	}
 
 	mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
+	if (ret < 0)
+		return ret;
+
 	if (read_write == I2C_SMBUS_WRITE || size <= I2C_SMBUS_BYTE_DATA)
 		return 0;
 
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect
  2025-06-26 16:15 [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect Conor Dooley
@ 2025-06-28 14:28 ` kernel test robot
  2025-06-30 15:04   ` Conor Dooley
  0 siblings, 1 reply; 3+ messages in thread
From: kernel test robot @ 2025-06-28 14:28 UTC (permalink / raw)
  To: Conor Dooley, linux-i2c
  Cc: llvm, oe-kbuild-all, conor, Conor Dooley, Daire McNamara,
	Andi Shyti, linux-kernel

Hi Conor,

kernel test robot noticed the following build warnings:

[auto build test WARNING on andi-shyti/i2c/i2c-host]
[also build test WARNING on linus/master v6.16-rc3 next-20250627]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/i2c-microchip-core-re-fix-fake-detections-w-i2cdetect/20250627-001626
base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link:    https://lore.kernel.org/r/20250626-unusable-excess-da94ebc218e8%40spud
patch subject: [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect
config: riscv-randconfig-002-20250628 (https://download.01.org/0day-ci/archive/20250628/202506282209.FXWbPIPz-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250628/202506282209.FXWbPIPz-lkp@intel.com/reproduce)

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>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506282209.FXWbPIPz-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-microchip-corei2c.c:510:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
           if (ret < 0)
               ^~~
   drivers/i2c/busses/i2c-microchip-corei2c.c:438:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +/ret +510 drivers/i2c/busses/i2c-microchip-corei2c.c

   428	
   429	static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags,
   430					   char read_write, u8 command,
   431					   int size, union i2c_smbus_data *data)
   432	{
   433		struct i2c_msg msgs[2];
   434		struct mchp_corei2c_dev *idev = i2c_get_adapdata(adap);
   435		u8 tx_buf[I2C_SMBUS_BLOCK_MAX + 2];
   436		u8 rx_buf[I2C_SMBUS_BLOCK_MAX + 1];
   437		int num_msgs = 1;
   438		int ret;
   439	
   440		msgs[CORE_I2C_SMBUS_MSG_WR].addr = addr;
   441		msgs[CORE_I2C_SMBUS_MSG_WR].flags = 0;
   442	
   443		if (read_write == I2C_SMBUS_READ && size <= I2C_SMBUS_BYTE)
   444			msgs[CORE_I2C_SMBUS_MSG_WR].flags = I2C_M_RD;
   445	
   446		if (read_write == I2C_SMBUS_WRITE && size <= I2C_SMBUS_WORD_DATA)
   447			msgs[CORE_I2C_SMBUS_MSG_WR].len = size;
   448	
   449		if (read_write == I2C_SMBUS_WRITE && size > I2C_SMBUS_BYTE) {
   450			msgs[CORE_I2C_SMBUS_MSG_WR].buf = tx_buf;
   451			msgs[CORE_I2C_SMBUS_MSG_WR].buf[0] = command;
   452		}
   453	
   454		if (read_write == I2C_SMBUS_READ && size >= I2C_SMBUS_BYTE_DATA) {
   455			msgs[CORE_I2C_SMBUS_MSG_WR].buf = tx_buf;
   456			msgs[CORE_I2C_SMBUS_MSG_WR].buf[0] = command;
   457			msgs[CORE_I2C_SMBUS_MSG_RD].addr = addr;
   458			msgs[CORE_I2C_SMBUS_MSG_RD].flags = I2C_M_RD;
   459			num_msgs = 2;
   460		}
   461	
   462		if (read_write == I2C_SMBUS_READ && size > I2C_SMBUS_QUICK)
   463			msgs[CORE_I2C_SMBUS_MSG_WR].len = 1;
   464	
   465		switch (size) {
   466		case I2C_SMBUS_QUICK:
   467			msgs[CORE_I2C_SMBUS_MSG_WR].buf = NULL;
   468			return 0;
   469		case I2C_SMBUS_BYTE:
   470			if (read_write == I2C_SMBUS_WRITE)
   471				msgs[CORE_I2C_SMBUS_MSG_WR].buf = &command;
   472			else
   473				msgs[CORE_I2C_SMBUS_MSG_WR].buf = &data->byte;
   474			break;
   475		case I2C_SMBUS_BYTE_DATA:
   476			if (read_write == I2C_SMBUS_WRITE) {
   477				msgs[CORE_I2C_SMBUS_MSG_WR].buf[1] = data->byte;
   478			} else {
   479				msgs[CORE_I2C_SMBUS_MSG_RD].len = size - 1;
   480				msgs[CORE_I2C_SMBUS_MSG_RD].buf = &data->byte;
   481			}
   482			break;
   483		case I2C_SMBUS_WORD_DATA:
   484			if (read_write == I2C_SMBUS_WRITE) {
   485				msgs[CORE_I2C_SMBUS_MSG_WR].buf[1] = data->word & 0xFF;
   486				msgs[CORE_I2C_SMBUS_MSG_WR].buf[2] = (data->word >> 8) & 0xFF;
   487			} else {
   488				msgs[CORE_I2C_SMBUS_MSG_RD].len = size - 1;
   489				msgs[CORE_I2C_SMBUS_MSG_RD].buf = rx_buf;
   490			}
   491			break;
   492		case I2C_SMBUS_BLOCK_DATA:
   493			if (read_write == I2C_SMBUS_WRITE) {
   494				int data_len;
   495	
   496				data_len = data->block[0];
   497				msgs[CORE_I2C_SMBUS_MSG_WR].len = data_len + 2;
   498				for (int i = 0; i <= data_len; i++)
   499					msgs[CORE_I2C_SMBUS_MSG_WR].buf[i + 1] = data->block[i];
   500			} else {
   501				msgs[CORE_I2C_SMBUS_MSG_RD].len = I2C_SMBUS_BLOCK_MAX + 1;
   502				msgs[CORE_I2C_SMBUS_MSG_RD].buf = rx_buf;
   503			}
   504			break;
   505		default:
   506			return -EOPNOTSUPP;
   507		}
   508	
   509		mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
 > 510		if (ret < 0)
   511			return ret;
   512	
   513		if (read_write == I2C_SMBUS_WRITE || size <= I2C_SMBUS_BYTE_DATA)
   514			return 0;
   515	
   516		switch (size) {
   517		case I2C_SMBUS_WORD_DATA:
   518			data->word = (rx_buf[0] | (rx_buf[1] << 8));
   519			break;
   520		case I2C_SMBUS_BLOCK_DATA:
   521			if (rx_buf[0] > I2C_SMBUS_BLOCK_MAX)
   522				rx_buf[0] = I2C_SMBUS_BLOCK_MAX;
   523			/* As per protocol first member of block is size of the block. */
   524			for (int i = 0; i <= rx_buf[0]; i++)
   525				data->block[i] = rx_buf[i];
   526			break;
   527		}
   528	
   529		return 0;
   530	}
   531	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect
  2025-06-28 14:28 ` kernel test robot
@ 2025-06-30 15:04   ` Conor Dooley
  0 siblings, 0 replies; 3+ messages in thread
From: Conor Dooley @ 2025-06-30 15:04 UTC (permalink / raw)
  To: kernel test robot
  Cc: linux-i2c, llvm, oe-kbuild-all, Conor Dooley, Daire McNamara,
	Andi Shyti, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2180 bytes --]

On Sat, Jun 28, 2025 at 10:28:45PM +0800, kernel test robot wrote:
> Hi Conor,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on andi-shyti/i2c/i2c-host]
> [also build test WARNING on linus/master v6.16-rc3 next-20250627]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Conor-Dooley/i2c-microchip-core-re-fix-fake-detections-w-i2cdetect/20250627-001626
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
> patch link:    https://lore.kernel.org/r/20250626-unusable-excess-da94ebc218e8%40spud
> patch subject: [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect
> config: riscv-randconfig-002-20250628 (https://download.01.org/0day-ci/archive/20250628/202506282209.FXWbPIPz-lkp@intel.com/config)
> compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250628/202506282209.FXWbPIPz-lkp@intel.com/reproduce)
> 
> 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>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202506282209.FXWbPIPz-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/i2c/busses/i2c-microchip-corei2c.c:510:6: warning: variable 'ret' is uninitialized when used here [-Wuninitialized]
>            if (ret < 0)
>                ^~~
>    drivers/i2c/busses/i2c-microchip-corei2c.c:438:9: note: initialize the variable 'ret' to silence this warning
>            int ret;
>                   ^
>                    = 0
>    1 warning generated.

Oh damn it, I accidentally committed a version that I was using to
provoke the WARN while trying to understand why this was corrupting
the workqueue.
V3 incoming.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-30 15:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-26 16:15 [PATCH v2] i2c: microchip-core: re-fix fake detections w/ i2cdetect Conor Dooley
2025-06-28 14:28 ` kernel test robot
2025-06-30 15:04   ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).