Re: [PATCH v3 1/3] hw/i2c/aspeed: Fix Tx count and Rx size error in buffer pool mode

["Cédric Le Goater" <clg@kaod.org>]

On 8/12/23 08:52, Hang Yu wrote:
> Fixed inconsistency between the regisiter bit field definition header file
> and the ast2600 datasheet. The reg name is I2CD1C:Pool Buffer Control
> Register in old register mode and  I2CC0C: Master/Slave Pool Buffer Control
> Register in new register mode. They share bit field
> [12:8]:Transmit Data Byte Count and bit field
> [29:24]:Actual Received Pool Buffer Size according to the datasheet.
> According to the ast2600 datasheet,the actual Tx count is
> Transmit Data Byte Count plus 1, and the max Rx size is
> Receive Pool Buffer Size plus 1, both in Pool Buffer Control Register.
> The version before forgot to plus 1, and mistake Rx count for Rx size.
> 
> Signed-off-by: Hang Yu <francis_yuu@stu.pku.edu.cn>
> Fixes: 3be3d6ccf2ad ("aspeed: i2c: Migrate to registerfields API")

This is -stable material with the following patch. It fixes support for
the v08.06 SDK.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
> v2-->v3:
> 1. Merged patch1 and patch2 in v2
> 2. added fixes tag
> 3. Fixed typos
> 
>   hw/i2c/aspeed_i2c.c         | 8 ++++----
>   include/hw/i2c/aspeed_i2c.h | 4 ++--
>   2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 1f071a3811..e485d8bfb8 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -236,7 +236,7 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
>       uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
>       uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
>       int pool_tx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl,
> -                                                TX_COUNT);
> +                                                TX_COUNT) + 1;
>   
>       if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_BUFF_EN)) {
>           for (i = pool_start; i < pool_tx_count; i++) {
> @@ -293,7 +293,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
>       uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
>       uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus);
>       int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl,
> -                                                RX_COUNT);
> +                                                RX_SIZE) + 1;
>   
>       if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, RX_BUFF_EN)) {
>           uint8_t *pool_base = aic->bus_pool_base(bus);
> @@ -418,7 +418,7 @@ static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus)
>       uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
>       uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
>       if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, RX_BUFF_EN)) {
> -        count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, TX_COUNT);
> +        count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, TX_COUNT) + 1;
>       } else if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, RX_DMA_EN)) {
>           count = bus->regs[reg_dma_len];
>       } else { /* BYTE mode */
> @@ -490,7 +490,7 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>            */
>           if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_BUFF_EN)) {
>               if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, TX_COUNT)
> -                == 1) {
> +                == 0) {
>                   SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
>               } else {
>                   /*
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 51c944efea..2e1e15aaf0 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -139,9 +139,9 @@ REG32(I2CD_CMD, 0x14) /* I2CD Command/Status */
>   REG32(I2CD_DEV_ADDR, 0x18) /* Slave Device Address */
>       SHARED_FIELD(SLAVE_DEV_ADDR1, 0, 7)
>   REG32(I2CD_POOL_CTRL, 0x1C) /* Pool Buffer Control */
> -    SHARED_FIELD(RX_COUNT, 24, 5)
> +    SHARED_FIELD(RX_COUNT, 24, 6)
>       SHARED_FIELD(RX_SIZE, 16, 5)
> -    SHARED_FIELD(TX_COUNT, 9, 5)
> +    SHARED_FIELD(TX_COUNT, 8, 5)
>       FIELD(I2CD_POOL_CTRL, OFFSET, 2, 6) /* AST2400 */
>   REG32(I2CD_BYTE_BUF, 0x20) /* Transmit/Receive Byte Buffer */
>       SHARED_FIELD(RX_BUF, 8, 8)