[v3,2/3] hw/i2c/aspeed: Fix TXBUF transmission start position error

According to the ast2600 datasheet and the linux aspeed i2c driver,
the TXBUF transmission start position should be TXBUF[0] instead
of TXBUF[1],so the arg pool_start is useless,and the address is not
included in TXBUF.So even if Tx Count equals zero,there is at least
1 byte data needs to be transmitted,and M_TX_CMD should not be cleared
at this condition.The driver url is:
https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v5.15/drivers/i2c/busses/i2c-ast2600.c

Signed-off-by: Hang Yu <francis_yuu@stu.pku.edu.cn>
---
v2-->v3:
No change.

 hw/i2c/aspeed_i2c.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

On 8/12/23 08:52, Hang Yu wrote:
> According to the ast2600 datasheet and the linux aspeed i2c driver,
> the TXBUF transmission start position should be TXBUF[0] instead
> of TXBUF[1],so the arg pool_start is useless,and the address is not
> included in TXBUF.So even if Tx Count equals zero,there is at least
> 1 byte data needs to be transmitted,and M_TX_CMD should not be cleared
> at this condition.The driver url is:
> https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v5.15/drivers/i2c/busses/i2c-ast2600.c
> 
> Signed-off-by: Hang Yu <francis_yuu@stu.pku.edu.cn>

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

Fixes: 6054fc73e8f4 ("aspeed/i2c: Add support for pool buffer transfers")

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

Thanks,

C.


> ---
> v2-->v3:
> No change.
> 
>   hw/i2c/aspeed_i2c.c | 30 ++++++------------------------
>   1 file changed, 6 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index e485d8bfb8..44905d7899 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -226,7 +226,7 @@ static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data)
>       return 0;
>   }
>   
> -static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
> +static int aspeed_i2c_bus_send(AspeedI2CBus *bus)
>   {
>       AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
>       int ret = -1;
> @@ -239,7 +239,7 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
>                                                   TX_COUNT) + 1;
>   
>       if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_BUFF_EN)) {
> -        for (i = pool_start; i < pool_tx_count; i++) {
> +        for (i = 0; i < pool_tx_count; i++) {
>               uint8_t *pool_base = aic->bus_pool_base(bus);
>   
>               trace_aspeed_i2c_bus_send("BUF", i + 1, pool_tx_count,
> @@ -273,7 +273,7 @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start)
>           }
>           SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, TX_DMA_EN, 0);
>       } else {
> -        trace_aspeed_i2c_bus_send("BYTE", pool_start, 1,
> +        trace_aspeed_i2c_bus_send("BYTE", 0, 1,
>                                     bus->regs[reg_byte_buf]);
>           ret = i2c_send(bus->bus, bus->regs[reg_byte_buf]);
>       }
> @@ -446,10 +446,8 @@ static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus)
>    */
>   static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>   {
> -    uint8_t pool_start = 0;
>       uint32_t reg_intr_sts = aspeed_i2c_bus_intr_sts_offset(bus);
>       uint32_t reg_cmd = aspeed_i2c_bus_cmd_offset(bus);
> -    uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus);
>       uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
>   
>       if (!aspeed_i2c_check_sram(bus)) {
> @@ -483,27 +481,11 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>   
>           SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_START_CMD, 0);
>   
> -        /*
> -         * The START command is also a TX command, as the slave
> -         * address is sent on the bus. Drop the TX flag if nothing
> -         * else needs to be sent in this sequence.
> -         */
> -        if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_BUFF_EN)) {
> -            if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, TX_COUNT)
> -                == 0) {
> -                SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
> -            } else {
> -                /*
> -                 * Increase the start index in the TX pool buffer to
> -                 * skip the address byte.
> -                 */
> -                pool_start++;
> -            }
> -        } else if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_DMA_EN)) {
> +        if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_DMA_EN)) {
>               if (bus->regs[reg_dma_len] == 0) {
>                   SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
>               }
> -        } else {
> +        } else if (!SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, TX_BUFF_EN)) {
>               SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
>           }
>   
> @@ -520,7 +502,7 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>   
>       if (SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, M_TX_CMD)) {
>           aspeed_i2c_set_state(bus, I2CD_MTXD);
> -        if (aspeed_i2c_bus_send(bus, pool_start)) {
> +        if (aspeed_i2c_bus_send(bus)) {
>               SHARED_ARRAY_FIELD_DP32(bus->regs, reg_intr_sts, TX_NAK, 1);
>               i2c_end_transfer(bus->bus);
>           } else {