diff mbox series

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

Message ID 20230812065230.8839-2-francis_yuu@stu.pku.edu.cn
State New
Headers show
Series hw/i2c/aspeed: Fix Tx and Rx error | expand

Commit Message

Hang Yu Aug. 12, 2023, 6:52 a.m. UTC
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")
---
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(-)

Comments

Cédric Le Goater Aug. 16, 2023, 8:32 a.m. UTC | #1
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)
Hang Yu Aug. 16, 2023, 9:44 a.m. UTC | #2
Hello! Thank you for your review!Sorry I forgot to cc other maintainers so I resend this mail.


From: "Cédric Le Goater" <clg@kaod.org>
Date: 2023-08-16 16:32:58
To:  Hang Yu <francis_yuu@stu.pku.edu.cn>,qemu-devel@nongnu.org
Cc:  komlodi@google.com,peter@pjd.dev,Peter Maydell <peter.maydell@linaro.org>,Andrew Jeffery <andrew@aj.id.au>,Joel Stanley <joel@jms.id.au>,"open list:ASPEED BMCs" <qemu-arm@nongnu.org>,qemu-stable@nongnu.org
Subject: Re: [PATCH v3 1/3] hw/i2c/aspeed: Fix Tx count and Rx size error in buffer pool mode>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.
Should I add this line into the commit in next version?

>
>Reviewed-by: Cédric Le Goater <clg@kaod.org>
Should I add your Reviewed-by tag and send v4 now?Or just wait for 
other maintainers to reply?


Thanks,
Hang.
>
>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)
>
Cédric Le Goater Aug. 28, 2023, 8:54 a.m. UTC | #3
On 8/16/23 11:44, Hang Yu wrote:
> Hello! Thank you for your review!Sorry I forgot to cc other maintainers so I resend this mail.
> 
> From: "Cédric Le Goater" <clg@kaod.org>
> Date: 2023-08-16 16:32:58
> To:  Hang Yu <francis_yuu@stu.pku.edu.cn>,qemu-devel@nongnu.org
> Cc:  komlodi@google.com,peter@pjd.dev,Peter Maydell <peter.maydell@linaro.org>,Andrew Jeffery <andrew@aj.id.au>,Joel Stanley <joel@jms.id.au>,"open list:ASPEED BMCs" <qemu-arm@nongnu.org>,qemu-stable@nongnu.org
> Subject: Re: [PATCH v3 1/3] hw/i2c/aspeed: Fix Tx count and Rx size error in buffer pool mode>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.
> Should I add this line into the commit in next version?
>>
>  >Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Should I add your Reviewed-by tag and send v4 now?Or just wait for
> other maintainers to reply?

No need for a v4. The tags are pulled automatically with the tooling
we use, b4, pwclient, etc.

I will prepare an aspeed PR for QEMU 8.1 with these patches. Then,
the first two could be backported to -stable.

Thanks,

C.

> 
> Thanks,
> Hang.
>>
>>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)
>>
> 
>
diff mbox series

Patch

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)