Re: [PATCH v3 4/8] i2c/aspeed: Fix DMA receive first-byte handling for block reads

Jamin,

I would appreciate a feedback from you on this change.

On 8/10/26 20:57, Mikail Sadic wrote:
> An SMBus block read (I2C_M_RECV_LEN) reads the block length from the
> first received byte. The Linux/U-Boot aspeed I2C driver obtains that
> first byte from the I2CC_STS_AND_BUFF register (modelled here as
> reg_byte_buf), even when the transfer uses DMA. The DMA receive path,
> however, only wrote received data to DRAM and never updated
> reg_byte_buf, so block reads read a stale/zero length.
> 
> Mirror the first DMA-received byte into reg_byte_buf so that
> I2C_M_RECV_LEN transfers using DMA report the correct block length.
> This is required for the ucd9000 driver, which uses
> i2c_smbus_read_block_data().
> 
> Signed-off-by: Mikail Sadic <[email protected]>

Does this change deserve a Fixes: tag ?

Thanks,

C.

> ---
>   hw/i2c/aspeed_i2c.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 27afcaecee..facb54d27e 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -365,6 +365,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
>       uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus);
>       uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
>       uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
> +    bool first_dma_byte;
>       int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl,
>                                                   RX_SIZE) + 1;
>   
> @@ -391,6 +392,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
>           }
>   
>           aspeed_i2c_set_rx_dma_dram_offset(bus);
> +        first_dma_byte = true;
>           while (bus->regs[reg_dma_len]) {
>               MemTxResult result;
>   
> @@ -407,6 +409,11 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
>                   return;
>               }
>   
> +            /* Mirror first byte to reg_byte_buf for I2C_M_RECV_LEN. */
> +            if (first_dma_byte) {
> +                SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
> +                first_dma_byte = false;
> +            }
>               bus->dma_dram_offset++;
>               bus->regs[reg_dma_len]--;
>               /* In new mode, keep track of how many bytes we RXed */