[PATCH v2 3/4] i2c: rtl9300: Add missing count byte for SMBus Block Ops

Sven Eckelmann posted 4 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v2 3/4] i2c: rtl9300: Add missing count byte for SMBus Block Ops
Posted by Sven Eckelmann 1 month, 1 week ago
The expected on-wire format of an SMBus Block Write is

  S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P

Everything starting from the Count byte is provided by the I2C subsystem in
the array data->block. But the driver was skipping the Count byte
(data->block[0]) when sending it to the RTL93xx I2C controller.

Only the actual data could be seen on the wire:

  S Addr Wr [A] Comm [A] Data [A] Data [A] ... [A] Data [A] P

This wire format is not SMBus Block Write compatible but matches the format
of an I2C Block Write. Simply adding the count byte to the buffer for the
I2C controller enough to fix the transmission.

This also affects read because the I2C controller must receive the count
byte + $count * data bytes.

Cc: <stable@vger.kernel.org>
Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/i2c/busses/i2c-rtl9300.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
index a10e5e6e00075fabb8906d56f09f5b9141fbc06e..4e0844d97607f64386a6d7a7c4086a81fdd89d6c 100644
--- a/drivers/i2c/busses/i2c-rtl9300.c
+++ b/drivers/i2c/busses/i2c-rtl9300.c
@@ -284,15 +284,15 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
 		ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
 		if (ret)
 			goto out_unlock;
-		ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
+		ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0] + 1);
 		if (ret)
 			goto out_unlock;
 		if (read_write == I2C_SMBUS_WRITE) {
-			ret = rtl9300_i2c_write(i2c, &data->block[1], data->block[0]);
+			ret = rtl9300_i2c_write(i2c, &data->block[0], data->block[0] + 1);
 			if (ret)
 				goto out_unlock;
 		}
-		len = data->block[0];
+		len = data->block[0] + 1;
 		break;
 
 	default:

-- 
2.47.2
Re: [PATCH v2 3/4] i2c: rtl9300: Add missing count byte for SMBus Block Ops
Posted by Chris Packham 1 month, 1 week ago
Hi Sven, Andi,

On 04/08/2025 04:54, Sven Eckelmann wrote:
> The expected on-wire format of an SMBus Block Write is
>
>    S Addr Wr [A] Comm [A] Count [A] Data [A] Data [A] ... [A] Data [A] P
>
> Everything starting from the Count byte is provided by the I2C subsystem in
> the array data->block. But the driver was skipping the Count byte
> (data->block[0]) when sending it to the RTL93xx I2C controller.
>
> Only the actual data could be seen on the wire:
>
>    S Addr Wr [A] Comm [A] Data [A] Data [A] ... [A] Data [A] P
>
> This wire format is not SMBus Block Write compatible but matches the format
> of an I2C Block Write. Simply adding the count byte to the buffer for the
> I2C controller enough to fix the transmission.
>
> This also affects read because the I2C controller must receive the count
> byte + $count * data bytes.
>
> Cc: <stable@vger.kernel.org>
> Fixes: c366be720235 ("i2c: Add driver for the RTL9300 I2C controller")
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

This conflicts with commit f46867c08a22 ("i2c: rtl9300: Fix 
out-of-bounds bug in rtl9300_i2c_smbus_xfer") which didn't get a Fixes 
tag. The merge conflict is reasonably straight forward to resolve. Not 
sure how you want to handle the patch for stable.

> ---
>   drivers/i2c/busses/i2c-rtl9300.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rtl9300.c b/drivers/i2c/busses/i2c-rtl9300.c
> index a10e5e6e00075fabb8906d56f09f5b9141fbc06e..4e0844d97607f64386a6d7a7c4086a81fdd89d6c 100644
> --- a/drivers/i2c/busses/i2c-rtl9300.c
> +++ b/drivers/i2c/busses/i2c-rtl9300.c
> @@ -284,15 +284,15 @@ static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned s
>   		ret = rtl9300_i2c_reg_addr_set(i2c, command, 1);
>   		if (ret)
>   			goto out_unlock;
> -		ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0]);
> +		ret = rtl9300_i2c_config_xfer(i2c, chan, addr, data->block[0] + 1);
>   		if (ret)
>   			goto out_unlock;
>   		if (read_write == I2C_SMBUS_WRITE) {
> -			ret = rtl9300_i2c_write(i2c, &data->block[1], data->block[0]);
> +			ret = rtl9300_i2c_write(i2c, &data->block[0], data->block[0] + 1);
>   			if (ret)
>   				goto out_unlock;
>   		}
> -		len = data->block[0];
> +		len = data->block[0] + 1;
>   		break;
>   
>   	default:
>