RE: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus

[Carlos Song (OSS)]

> -----Original Message-----
> From: Vincent Jardin <vjardin@xxxxxxx>
> Sent: Tuesday, May 26, 2026 4:12 PM
> To: Carlos Song (OSS) <carlos.song@xxxxxxxxxxx>
> Cc: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Andi Shyti <andi.shyti@xxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Fabio
> Estevam <festevam@xxxxxxxxx>; Wolfram Sang <wsa@xxxxxxxxxx>; Kaushal
> Butala <kaushalkernelmailinglist@xxxxxxxxx>; Shawn Guo
> <shawn.guo@xxxxxxxxxxxxx>; Stefan Eichenberger
> <stefan.eichenberger@xxxxxxxxxxx>; linux-i2c@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 0/2] i2c: imx: fix SMBus block-read of 0 locking the bus
> 
> [You don't often get email from vjardin@xxxxxxx. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Carlos,
> 
> > Thanks for working on this fix, this looks good to me.
> 
> thanks for checking it. It took some times to isolate this issue and then find a
> fix.
> 
> > SMBus block reads with a length of 0 seem quite uncommon in practice.
> > Was this triggered by a specific device behavior, or mainly found
> > during boundary / compliance testing?
> 
> It is trigger by the usage of a mpq8785 on the i2c bus: when the kernel
> attaches on it using its pmsbus/hwmon framework, then the i2c bus get
> locked on lx2160 !
> 
> > Regarding the handling of len == 0,
> > I see that the patch sets:
> >
> >     msg->buf[0] = 0;
> >     msg->len = 2;
> >
> > It relies on the last-byte STOP handling together with TXAK. It will
> > help I2C-IMX generate NACK + STOP and release the bus, right?
> 
> Yes, exactly. Reading I2DR for the length byte has already armed the next byte,
> so we set TXAK to NACK it and extend msg->len to 2.
> Next then i2c_imx_isr_read_continue() at msg_buf_idx == msg->len - 1, ie the
> normal last-byte path, which clears MSTA to emit STOP. So NACK + STOP, and
> THEN the bus is released. I do not see any other means to handle it.
> 
> > len = 0 is a legal behavior, So it go into a successful path.
> 
> Yes. count == 0 is legal (SMBus 3.1 6.5.7), so the transfer reaches
> STATE_DONE and returns success.
> 
> > But len > I2C_SMBUS_BLOCK_MAX is abnormal behavior. So it go into a fail
> path.
> 
> Correct, and it is a protocol error, so it needs to end up with a -EPROTO while
> a count of 0 is an ok case.
> 
> > Do I understand it right?
> 
> yes. I do not see any other means to handle it.
> 
> > Also, if possible could you briefly describe how you validated this
> > change (e.g. test setup or steps, with and without the fix)?
> 
> On a lx2160a board, on its i2c, bind a mpq8785, and enable the Kernel
> pmbus/hwmon framework, then the i2c bus becomes un-useable. Using a
> scope, we can confirm that the lx21260a i2c cannot recover.
> 
Hi, Vincent

Thank you very much!

Acked-by: Carlos Song <carlos.song@xxxxxxx>

> Best regards,
>   Vincent