Re: [PATCH v2] i2c: designware: preserve invalid SMBus block count

[Aniket Limaye <a-limaye@ti.com>]

On 07/11/26 13:41, Baek Kyu, Kim wrote:
> When a slave reports an invalid SMBus block count, i2c_dw_read() uses
> one extra read to reach STOP. It currently replaces the received count
> with 1 for scheduling and also stores 1 in the message buffer. A count
> above 32 is therefore exposed as a valid one-byte response.
> 
> Keep the received byte in the buffer and limit only the count passed to
> i2c_dw_recv_len(). This preserves the extra read for STOP while retaining
> the invalid count for the caller to reject.
> 
> The DesignWare receive functions were exercised with stubbed register
> input for block counts 0, 1, 32, 33 and 255, with PEC both off and on.
> The original code replaced invalid counts in the message buffer with 1;
> v2 retains the received byte while scheduling the same extra read.
> The arm64 i2c-designware-master.o build passed with W=1.
> No physical controller or end-to-end SMBus result was measured.
> 
> Fixes: 69f035c480d7 ("i2c: designware: Handle invalid SMBus block data response length value")
> Signed-off-by: Baek Kyu, Kim <pocj8ur4in@gmail.com>

Reviewed-by: Aniket Limaye <a-limaye@ti.com>

> ---
> Changes in v2:
>    - Use a separate count variable for the extra read, as Mika Westerberg suggested.
> 
> Link to v1: https://marc.info/?l=linux-i2c&m=179344386454087&w=2
> 
>   drivers/i2c/busses/i2c-designware-master.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index a1bcc3797e4ff..49e9e36340c84 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -542,8 +542,9 @@ i2c_dw_read(struct dw_i2c_dev *dev)
>   		for (; len > 0 && rx_valid > 0; len--, rx_valid--) {
>   			regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
>   			tmp &= DW_IC_DATA_CMD_DAT;
> -			/* Ensure length byte is a valid value */
>   			if (flags & I2C_M_RECV_LEN) {
> +				unsigned int count = tmp;
> +
>   				/*
>   				 * if IC_EMPTYFIFO_HOLD_MASTER_EN is set, which cannot be
>   				 * detected from the registers, the controller can be
> @@ -553,10 +554,10 @@ i2c_dw_read(struct dw_i2c_dev *dev)
>   				 * another byte with STOP bit set when the block data
>   				 * response length is invalid to complete the transaction.
>   				 */
> -				if (!tmp || tmp > I2C_SMBUS_BLOCK_MAX)
> -					tmp = 1;
> +				if (!count || count > I2C_SMBUS_BLOCK_MAX)
> +					count = 1;
>   
> -				len = i2c_dw_recv_len(dev, tmp);
> +				len = i2c_dw_recv_len(dev, count);
>   			}
>   			*buf++ = tmp;
>   			dev->rx_outstanding--;
>