Re: [PATCH] i2c: imx: test the block read flag of the message that is read

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] i2c: imx: uncleared I2CR_TXAK for I2C_M_RECV_LEN in
  i2c_imx_prepare_read() [1]
--

commit ce48b1bb4f3f90616018dca56aca9a327411019e
Author: Mehmet Fide <mehmet.fide@screeningeagle.com>

i2c: imx: test the block read flag of the message that is read

This commit changes i2c_imx_xfer_common() to check I2C_M_RECV_LEN on the
current message (msgs[i]) rather than the first message of the transfer
when deciding whether to use DMA for a read.

> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index ccf08cd63151f..9abe0a26c877a 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1713,7 +1713,7 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
>  			msgs[i].flags & I2C_M_DMA_SAFE &&
>  			!(msgs[i].flags & I2C_M_NOSTART);
>  		if (msgs[i].flags & I2C_M_RD) {
> -			int block_data = msgs->flags & I2C_M_RECV_LEN;
> +			int block_data = msgs[i].flags & I2C_M_RECV_LEN;
>  
>  			if (atomic)
>  				result = i2c_imx_atomic_read(i2c_imx, &msgs[i], is_lastmsg);

[Severity: High]
This problem wasn't introduced by this patch, but in the
i2c_imx_atomic_read() -> i2c_imx_prepare_read() path just below, does
i2c_imx_prepare_read() fail to clear I2CR_TXAK for an SMBus block read
([1])?

drivers/i2c/busses/i2c-imx.c:i2c_imx_prepare_read() {
	...
	/*
	 * Reset the I2CR_TXAK flag initially for SMBus block read since the
	 * length is unknown
	 */
	if (msgs->len - 1)
		temp &= ~I2CR_TXAK;
	...
}

i2c_smbus_xfer_emulated() initializes I2C_M_RECV_LEN read messages with
len = 1, and i2c_imx_start() sets I2CR_TXAK at the start of the transfer.
Unlike i2c_imx_isr_read(), i2c_imx_prepare_read() does not check
msgs->flags & I2C_M_RECV_LEN, so I2CR_TXAK stays set when the dummy read
of IMX_I2C_I2DR starts reception.  The controller then sends a NACK after
the initial length byte, causing the target device to abort the transfer
while the remaining payload bytes read as 0xff.

[1] https://sashiko.dev/#/bug/linux-09e3015b-fc03-4410-809b-82964cdfac9c

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113110501.1142036-1-mehmet.fide@gmail.com?part=1