[RESEND PATCH v1] i2c: microchip-core: re-fix fake detections w/ i2cdetect

[RESEND PATCH v1] i2c: microchip-core: re-fix fake detections w/ i2cdetect

[Conor Dooley]

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>
---

Resending cos I think it attempted a send using my korg address on a
network where that is not possible.

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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-microchip-corei2c.c b/drivers/i2c/busses/i2c-microchip-corei2c.c
index 492bf4c34722c..a4611381c4f0b 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;
@@ -505,7 +506,10 @@ static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned
 		return -EOPNOTSUPP;
 	}
 
-	mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
+	ret = mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
+	if (ret)
+		return ret;
+
 	if (read_write == I2C_SMBUS_WRITE || size <= I2C_SMBUS_BYTE_DATA)
 		return 0;
 
-- 
2.49.0

Re: [RESEND PATCH v1] i2c: microchip-core: re-fix fake detections w/ i2cdetect

[Conor Dooley]

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

On Thu, Jun 12, 2025 at 12:12:49PM +0100, Conor Dooley wrote:
> 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>
> ---
> 
> Resending cos I think it attempted a send using my korg address on a
> network where that is not possible.

Seemingly this patch has exposed an issue that causes a hang that was
not noticed previously:
# cd /sys/bus/iio/devices/iio\:device0
# cat *
VDDREG_IBUS_1
0
[   92.178899] ------------[ cut here ]------------
[   92.178921] WARNING: CPU: 0 PID: 291 at kernel/workqueue.c:2496 __queue_delayed_work+0xb4/0xbe
[   92.178981] Modules linked in: pac1934 industrialio autofs4
[   92.179024] CPU: 0 UID: 0 PID: 291 Comm: cat Not tainted 6.12.22 #1
[   92.179045] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
[   92.179055] epc : __queue_delayed_work+0xb4/0xbe
[   92.179081]  ra : mod_delayed_work_on+0x4a/0xa6
[   92.179107] epc : ffffffff8002b2f0 ra : ffffffff8002b3ba sp : ffffffc600863b70
[   92.179122]  gp : ffffffff812d6068 tp : ffffffe5a5bc24c0 t0 : 0000000000000000
[   92.179136]  t1 : 000000000000ff00 t2 : ffffffff80c01210 s0 : ffffffc600863b90
[   92.179150]  s1 : ffffffe5a2aa7568 a0 : 0000000000000040 a1 : ffffffe5a2054000
[   92.179164]  a2 : ffffffe5a2aa7568 a3 : 0000000000003a98 a4 : 0000000000000000
[   92.179178]  a5 : ffffffff8002b1fa a6 : 0000000000000000 a7 : 0000000000000000
[   92.179191]  s2 : 0000000000000040 s3 : ffffffe5a2054000 s4 : 0000000000003a98
[   92.179205]  s5 : 0000000000000000 s6 : ffffffc600863c5c s7 : ffffffc600863c58
[   92.179219]  s8 : 0000000000400cc0 s9 : fffffffffffff000 s10: 000000007ffff000
[   92.179233]  s11: ffffffe5a2806128 t3 : 0000000000ff0000 t4 : 0000000000000000
[   92.179247]  t5 : 0000000000000000 t6 : 0000000000000000
[   92.179258] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
[   92.179273] [<ffffffff8002b2f0>] __queue_delayed_work+0xb4/0xbe
[   92.179302] [<ffffffff8002b3ba>] mod_delayed_work_on+0x4a/0xa6
[   92.179331] [<ffffffff01375980>] pac1934_read_raw+0xba/0x1fc [pac1934]
[   92.179392] [<ffffffff0134942e>] iio_read_channel_info+0xae/0xc4 [industrialio]
[   92.179704] [<ffffffff80442fde>] dev_attr_show+0x14/0x46
[   92.179748] [<ffffffff8020ca3e>] sysfs_kf_seq_show+0x6c/0xe2
[   92.179788] [<ffffffff8020b15c>] kernfs_seq_show+0x18/0x20
[   92.179814] [<ffffffff801bc1ca>] seq_read_iter+0xd0/0x328
[   92.179844] [<ffffffff8020b634>] kernfs_fop_read_iter+0xfa/0x15c
[   92.179871] [<ffffffff8018e128>] vfs_read+0x1aa/0x25a
[   92.179895] [<ffffffff8018e998>] ksys_read+0x5a/0xcc
[   92.179915] [<ffffffff8018ea1e>] __riscv_sys_read+0x14/0x1c
[   92.179936] [<ffffffff807662f8>] do_trap_ecall_u+0x1ac/0x1d8
[   92.179981] [<ffffffff8076e3ba>] handle_exception+0x146/0x152
[   92.180018] ---[ end trace 0000000000000000 ]---
(The issue was detected on an internal 6.12 based branch, but the
content there is identical to what's in mainline + this patch).

Obviously adding the check for an error here doesn't cause there to be
problems with a transfer, only actually report problems that have
occurred. I have not yet been able to reproduce this on my setup, but
the reporter saw the issues going away when they disabled hardware smbus
support and used software emulation instead.

I don't know if this has any bearing on applying the patch, but I wanted
to mention it for reasons that should be apparent. I'm looking into the
issue still and hope to figure out what's going wrong.

Cheers,
Conor.

> 
> 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 | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-microchip-corei2c.c b/drivers/i2c/busses/i2c-microchip-corei2c.c
> index 492bf4c34722c..a4611381c4f0b 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;
> @@ -505,7 +506,10 @@ static int mchp_corei2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned
>  		return -EOPNOTSUPP;
>  	}
>  
> -	mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
> +	ret = mchp_corei2c_xfer(&idev->adapter, msgs, num_msgs);
> +	if (ret)
> +		return ret;
> +
>  	if (read_write == I2C_SMBUS_WRITE || size <= I2C_SMBUS_BYTE_DATA)
>  		return 0;
>  
> -- 
> 2.49.0
> 

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

Re: [RESEND PATCH v1] i2c: microchip-core: re-fix fake detections w/ i2cdetect

[Andi Shyti]

Hi Conor,

On Wed, Jun 25, 2025 at 02:27:51PM +0100, Conor Dooley wrote:
> On Thu, Jun 12, 2025 at 12:12:49PM +0100, Conor Dooley wrote:
> > 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>
> > ---
> > 
> > Resending cos I think it attempted a send using my korg address on a
> > network where that is not possible.
> 
> Seemingly this patch has exposed an issue that causes a hang that was
> not noticed previously:
> # cd /sys/bus/iio/devices/iio\:device0
> # cat *
> VDDREG_IBUS_1
> 0
> [   92.178899] ------------[ cut here ]------------
> [   92.178921] WARNING: CPU: 0 PID: 291 at kernel/workqueue.c:2496 __queue_delayed_work+0xb4/0xbe
> [   92.178981] Modules linked in: pac1934 industrialio autofs4
> [   92.179024] CPU: 0 UID: 0 PID: 291 Comm: cat Not tainted 6.12.22 #1
> [   92.179045] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> [   92.179055] epc : __queue_delayed_work+0xb4/0xbe
> [   92.179081]  ra : mod_delayed_work_on+0x4a/0xa6
> [   92.179107] epc : ffffffff8002b2f0 ra : ffffffff8002b3ba sp : ffffffc600863b70
> [   92.179122]  gp : ffffffff812d6068 tp : ffffffe5a5bc24c0 t0 : 0000000000000000
> [   92.179136]  t1 : 000000000000ff00 t2 : ffffffff80c01210 s0 : ffffffc600863b90
> [   92.179150]  s1 : ffffffe5a2aa7568 a0 : 0000000000000040 a1 : ffffffe5a2054000
> [   92.179164]  a2 : ffffffe5a2aa7568 a3 : 0000000000003a98 a4 : 0000000000000000
> [   92.179178]  a5 : ffffffff8002b1fa a6 : 0000000000000000 a7 : 0000000000000000
> [   92.179191]  s2 : 0000000000000040 s3 : ffffffe5a2054000 s4 : 0000000000003a98
> [   92.179205]  s5 : 0000000000000000 s6 : ffffffc600863c5c s7 : ffffffc600863c58
> [   92.179219]  s8 : 0000000000400cc0 s9 : fffffffffffff000 s10: 000000007ffff000
> [   92.179233]  s11: ffffffe5a2806128 t3 : 0000000000ff0000 t4 : 0000000000000000
> [   92.179247]  t5 : 0000000000000000 t6 : 0000000000000000
> [   92.179258] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
> [   92.179273] [<ffffffff8002b2f0>] __queue_delayed_work+0xb4/0xbe
> [   92.179302] [<ffffffff8002b3ba>] mod_delayed_work_on+0x4a/0xa6
> [   92.179331] [<ffffffff01375980>] pac1934_read_raw+0xba/0x1fc [pac1934]
> [   92.179392] [<ffffffff0134942e>] iio_read_channel_info+0xae/0xc4 [industrialio]
> [   92.179704] [<ffffffff80442fde>] dev_attr_show+0x14/0x46
> [   92.179748] [<ffffffff8020ca3e>] sysfs_kf_seq_show+0x6c/0xe2
> [   92.179788] [<ffffffff8020b15c>] kernfs_seq_show+0x18/0x20
> [   92.179814] [<ffffffff801bc1ca>] seq_read_iter+0xd0/0x328
> [   92.179844] [<ffffffff8020b634>] kernfs_fop_read_iter+0xfa/0x15c
> [   92.179871] [<ffffffff8018e128>] vfs_read+0x1aa/0x25a
> [   92.179895] [<ffffffff8018e998>] ksys_read+0x5a/0xcc
> [   92.179915] [<ffffffff8018ea1e>] __riscv_sys_read+0x14/0x1c
> [   92.179936] [<ffffffff807662f8>] do_trap_ecall_u+0x1ac/0x1d8
> [   92.179981] [<ffffffff8076e3ba>] handle_exception+0x146/0x152
> [   92.180018] ---[ end trace 0000000000000000 ]---
> (The issue was detected on an internal 6.12 based branch, but the
> content there is identical to what's in mainline + this patch).
> 
> Obviously adding the check for an error here doesn't cause there to be
> problems with a transfer, only actually report problems that have
> occurred. I have not yet been able to reproduce this on my setup, but
> the reporter saw the issues going away when they disabled hardware smbus
> support and used software emulation instead.
> 
> I don't know if this has any bearing on applying the patch, but I wanted
> to mention it for reasons that should be apparent. I'm looking into the
> issue still and hope to figure out what's going wrong.

Thanks for letting me know. I'll hold off on this for now. Please
ping me once you have some results.

Andi

Re: [RESEND PATCH v1] i2c: microchip-core: re-fix fake detections w/ i2cdetect

[Conor Dooley]

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

On Wed, Jun 25, 2025 at 11:50:28PM +0200, Andi Shyti wrote:
> Hi Conor,
> 
> On Wed, Jun 25, 2025 at 02:27:51PM +0100, Conor Dooley wrote:
> > On Thu, Jun 12, 2025 at 12:12:49PM +0100, Conor Dooley wrote:
> > > 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>
> > > ---
> > > 
> > > Resending cos I think it attempted a send using my korg address on a
> > > network where that is not possible.
> > 
> > Seemingly this patch has exposed an issue that causes a hang that was
> > not noticed previously:
> > # cd /sys/bus/iio/devices/iio\:device0
> > # cat *
> > VDDREG_IBUS_1
> > 0
> > [   92.178899] ------------[ cut here ]------------
> > [   92.178921] WARNING: CPU: 0 PID: 291 at kernel/workqueue.c:2496 __queue_delayed_work+0xb4/0xbe
> > [   92.178981] Modules linked in: pac1934 industrialio autofs4
> > [   92.179024] CPU: 0 UID: 0 PID: 291 Comm: cat Not tainted 6.12.22 #1
> > [   92.179045] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> > [   92.179055] epc : __queue_delayed_work+0xb4/0xbe
> > [   92.179081]  ra : mod_delayed_work_on+0x4a/0xa6
> > [   92.179107] epc : ffffffff8002b2f0 ra : ffffffff8002b3ba sp : ffffffc600863b70
> > [   92.179122]  gp : ffffffff812d6068 tp : ffffffe5a5bc24c0 t0 : 0000000000000000
> > [   92.179136]  t1 : 000000000000ff00 t2 : ffffffff80c01210 s0 : ffffffc600863b90
> > [   92.179150]  s1 : ffffffe5a2aa7568 a0 : 0000000000000040 a1 : ffffffe5a2054000
> > [   92.179164]  a2 : ffffffe5a2aa7568 a3 : 0000000000003a98 a4 : 0000000000000000
> > [   92.179178]  a5 : ffffffff8002b1fa a6 : 0000000000000000 a7 : 0000000000000000
> > [   92.179191]  s2 : 0000000000000040 s3 : ffffffe5a2054000 s4 : 0000000000003a98
> > [   92.179205]  s5 : 0000000000000000 s6 : ffffffc600863c5c s7 : ffffffc600863c58
> > [   92.179219]  s8 : 0000000000400cc0 s9 : fffffffffffff000 s10: 000000007ffff000
> > [   92.179233]  s11: ffffffe5a2806128 t3 : 0000000000ff0000 t4 : 0000000000000000
> > [   92.179247]  t5 : 0000000000000000 t6 : 0000000000000000
> > [   92.179258] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
> > [   92.179273] [<ffffffff8002b2f0>] __queue_delayed_work+0xb4/0xbe
> > [   92.179302] [<ffffffff8002b3ba>] mod_delayed_work_on+0x4a/0xa6
> > [   92.179331] [<ffffffff01375980>] pac1934_read_raw+0xba/0x1fc [pac1934]
> > [   92.179392] [<ffffffff0134942e>] iio_read_channel_info+0xae/0xc4 [industrialio]
> > [   92.179704] [<ffffffff80442fde>] dev_attr_show+0x14/0x46
> > [   92.179748] [<ffffffff8020ca3e>] sysfs_kf_seq_show+0x6c/0xe2
> > [   92.179788] [<ffffffff8020b15c>] kernfs_seq_show+0x18/0x20
> > [   92.179814] [<ffffffff801bc1ca>] seq_read_iter+0xd0/0x328
> > [   92.179844] [<ffffffff8020b634>] kernfs_fop_read_iter+0xfa/0x15c
> > [   92.179871] [<ffffffff8018e128>] vfs_read+0x1aa/0x25a
> > [   92.179895] [<ffffffff8018e998>] ksys_read+0x5a/0xcc
> > [   92.179915] [<ffffffff8018ea1e>] __riscv_sys_read+0x14/0x1c
> > [   92.179936] [<ffffffff807662f8>] do_trap_ecall_u+0x1ac/0x1d8
> > [   92.179981] [<ffffffff8076e3ba>] handle_exception+0x146/0x152
> > [   92.180018] ---[ end trace 0000000000000000 ]---
> > (The issue was detected on an internal 6.12 based branch, but the
> > content there is identical to what's in mainline + this patch).
> > 
> > Obviously adding the check for an error here doesn't cause there to be
> > problems with a transfer, only actually report problems that have
> > occurred. I have not yet been able to reproduce this on my setup, but
> > the reporter saw the issues going away when they disabled hardware smbus
> > support and used software emulation instead.
> > 
> > I don't know if this has any bearing on applying the patch, but I wanted
> > to mention it for reasons that should be apparent. I'm looking into the
> > issue still and hope to figure out what's going wrong.
> 
> Thanks for letting me know. I'll hold off on this for now. Please
> ping me once you have some results.

Well, I made a mistake in my patch, so I'll be sending a v2. I still do
not know what is causing there to be issues with corruption of the
workqueue in the client driver. Nothing really makes sense, as the bug
in my patch causes read date to never get populated into the rx data
array rather than writing to something invalid. There's nothing obvious
in the client driver either that is blindly writing into some address
based on the result (there are some reads indexed by it). I'll continue
to look at that more, but I have a v2 that fixes the patch here.

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