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

[Mika Westerberg]

Hi,

On Sat, Oct 31, 2026 at 07:54:12PM +0900, 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.
> 
> Fixes: 69f035c480d7 ("i2c: designware: Handle invalid SMBus block data response length value")
> Signed-off-by: Baek Kyu, Kim <pocj8ur4in@xxxxxxxxx>
> ---
> The DesignWare receive functions were exercised with stubbed register
> input for block counts 1, 32, 33 and 255. Counts 33 and 255 became 1
> before the change and remained unchanged after it; both versions
> scheduled one extra read for an invalid count. No physical controller,
> STOP signal, PEC result or end-to-end SMBus return was measured.
> 
>  drivers/i2c/busses/i2c-designware-master.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
> index 7e1d9a2e77332..f38ffc5b0a347 100644
> --- a/drivers/i2c/busses/i2c-designware-master.c
> +++ b/drivers/i2c/busses/i2c-designware-master.c
> @@ -563,7 +563,7 @@ 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 */
> +			/* Limit the extra read, but keep the raw count for i2c-core */
>  			if (flags & I2C_M_RECV_LEN) {
>  				/*
>  				 * if IC_EMPTYFIFO_HOLD_MASTER_EN is set, which cannot be
> @@ -574,10 +574,8 @@ 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;
> -
> -				len = i2c_dw_recv_len(dev, tmp);
> +				len = i2c_dw_recv_len(dev,
> +						  (!tmp || tmp > I2C_SMBUS_BLOCK_MAX) ? 1 : tmp);

I suggest using helper variable here. This is pretty hard to understand
otherwise.

>  			}
>  			*buf++ = tmp;
>  			dev->rx_outstanding--;
> -- 
> 2.55.0