1. Patchew
  2. QEMU
  3. View series

[PATCH] Aspeed: i2c: Fixed Tx and Rx error in BUFF Mode

Hang Yu via posted 1 patch 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230804081142.52539-1-1339236493@qq.com
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>
hw/i2c/aspeed_i2c.c         | 21 ++++++---------------
include/hw/i2c/aspeed_i2c.h |  5 +++--
2 files changed, 9 insertions(+), 17 deletions(-)
[PATCH] Aspeed: i2c: Fixed Tx and Rx error in BUFF Mode
Posted by Hang Yu via 9 months ago 
1. Fixed inconsistency between the bit field definition in register
I2CD_POOL_CTRL and the ast2600 datasheet
2. Fixed issue of confusing RXSIZE and RXCOUNT, as well as forgetting
to add one to TXCOUNT and RXSIZE in buff mode
3. Fixed issue with TXBUF transmission start position error in buff mode
4. Added support for the BUFFER ORGANIZATION option in reg I2CC_POOL_CTRL
After adding these changes, QEMU can support driver code:
https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v5.15
/drivers/i2c/busses/i2c-ast2600.c

Signed-off-by: Hang Yu <1339236493@qq.com>
---
 hw/i2c/aspeed_i2c.c         | 21 ++++++---------------
 include/hw/i2c/aspeed_i2c.h |  5 +++--
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 1f071a3811..0e380d0bba 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,10 +293,12 @@ 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);
+        uint8_t *pool_base ;
+        if(ARRAY_FIELD_EX32(bus->regs,I2CC_POOL_CTRL,BUF_ORGANIZATION))pool_base=aic->bus_pool_base(bus)+16;
+        else pool_base= aic->bus_pool_base(bus);
 
         for (i = 0; i < pool_rx_count; i++) {
             pool_base[i] = i2c_recv(bus->bus);
@@ -418,7 +420,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 */
@@ -449,7 +451,6 @@ 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)) {
@@ -489,16 +490,6 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
          * 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)
-                == 1) {
-                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 (bus->regs[reg_dma_len] == 0) {
                 SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 51c944efea..88b144a599 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)
@@ -162,6 +162,7 @@ REG32(I2CC_MS_TXRX_BYTE_BUF, 0x08)
     /* 15:0  shared with I2CD_BYTE_BUF[15:0] */
 REG32(I2CC_POOL_CTRL, 0x0c)
     /* 31:0 shared with I2CD_POOL_CTRL[31:0] */
+    FIELD(I2CC_POOL_CTRL, BUF_ORGANIZATION, 0, 1) /* AST2600 */
 REG32(I2CM_INTR_CTRL, 0x10)
 REG32(I2CM_INTR_STS, 0x14)
     FIELD(I2CM_INTR_STS, PKT_STATE, 28, 4)
-- 
2.39.2 (Apple Git-143)
Re: [PATCH] Aspeed: i2c: Fixed Tx and Rx error in BUFF Mode
Posted by Cédric Le Goater 8 months, 4 weeks ago 
Hello,

On 8/4/23 10:11, Hang Yu via wrote:
> 1. Fixed inconsistency between the bit field definition in register
> I2CD_POOL_CTRL and the ast2600 datasheet
> 2. Fixed issue of confusing RXSIZE and RXCOUNT, as well as forgetting
> to add one to TXCOUNT and RXSIZE in buff mode
> 3. Fixed issue with TXBUF transmission start position error in buff mode
> 4. Added support for the BUFFER ORGANIZATION option in reg I2CC_POOL_CTRL


This looks like 4 patches. Could split and resend a series please ?

Also, Cc: these persons

   Joe Komlodi <komlodi@google.com>
   Peter Delevoryas <peter@pjd.dev>

since they have been involved with the AST2600 I2C implementation.

> After adding these changes, QEMU can support driver code:
> https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v5.15
> /drivers/i2c/busses/i2c-ast2600.c
> 
> Signed-off-by: Hang Yu <1339236493@qq.com>

  This is very cryptic. Don't you have a corporate email ?

Thanks,

C.



> ---
>   hw/i2c/aspeed_i2c.c         | 21 ++++++---------------
>   include/hw/i2c/aspeed_i2c.h |  5 +++--
>   2 files changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 1f071a3811..0e380d0bba 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,10 +293,12 @@ 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);
> +        uint8_t *pool_base ;
> +        if(ARRAY_FIELD_EX32(bus->regs,I2CC_POOL_CTRL,BUF_ORGANIZATION))pool_base=aic->bus_pool_base(bus)+16;
> +        else pool_base= aic->bus_pool_base(bus);
>   
>           for (i = 0; i < pool_rx_count; i++) {
>               pool_base[i] = i2c_recv(bus->bus);
> @@ -418,7 +420,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 */
> @@ -449,7 +451,6 @@ 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)) {
> @@ -489,16 +490,6 @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value)
>            * 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)
> -                == 1) {
> -                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 (bus->regs[reg_dma_len] == 0) {
>                   SHARED_ARRAY_FIELD_DP32(bus->regs, reg_cmd, M_TX_CMD, 0);
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 51c944efea..88b144a599 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)
> @@ -162,6 +162,7 @@ REG32(I2CC_MS_TXRX_BYTE_BUF, 0x08)
>       /* 15:0  shared with I2CD_BYTE_BUF[15:0] */
>   REG32(I2CC_POOL_CTRL, 0x0c)
>       /* 31:0 shared with I2CD_POOL_CTRL[31:0] */
> +    FIELD(I2CC_POOL_CTRL, BUF_ORGANIZATION, 0, 1) /* AST2600 */
>   REG32(I2CM_INTR_CTRL, 0x10)
>   REG32(I2CM_INTR_STS, 0x14)
>       FIELD(I2CM_INTR_STS, PKT_STATE, 28, 4)

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.