[PATCH v1] hw/i2c/aspeed_i2c: Latch first received byte for SMBus block reads

Jamin Lin <[email protected]>
Newsgroups org.nongnu.qemu-arm,org.nongnu.qemu-devel
Message-ID <[email protected]>
An SMBus block read takes the block length from the first byte of the
transfer, and firmware reads that byte back from a register rather than
from the transfer buffer. The receive paths never updated those
registers, so block reads reported a bogus length.

On AST2600 the driver reads the length from the receive byte buffer,
I2CC_MS_TXRX_BYTE_BUF[15:8]. The datasheet documents that field as valid
while the DMA buffer is not enabled. The byte mode receive path already
updated it, but the pool buffer path did not, and the driver selects
buffer mode by default.

On AST2700 the driver reads the length from offset 0x84 instead.

Add I2CC_BYTE_DATA_LOG at 0x84. Latch the first received byte from the
three receive paths: the pool buffer path, the DMA-to-pool path and the
DMA-to-DRAM path. Each latch updates the byte data log, and updates the
receive byte buffer as well while the DMA buffer is not enabled. The byte
data log is outside the register window of the earlier SoCs, so it is only
visible on AST2700/AST1040.

Signed-off-by: Jamin Lin <[email protected]>
---
 hw/i2c/aspeed_i2c.c         | 31 ++++++++++++++++++++++++++++++-
 include/hw/i2c/aspeed_i2c.h |  2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 68bdcd0e25..e6d0eee816 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -159,6 +159,7 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
     case A_I2CS_INTR_CTRL:
     case A_I2CS_DMA_LEN_STS:
     case A_I2CS_INTR_STS:
+    case A_I2CC_BYTE_DATA_LOG:
     case A_I2CC_VERSION_CTRL:
         value = bus->regs[offset / sizeof(*bus->regs)];
         break;
@@ -334,6 +335,24 @@ static int aspeed_i2c_bus_send_dma_pool(AspeedI2CBus *bus)
     return ret;
 }
 
+/*
+ * Latch the first received byte, which firmware reads back as the SMBus block
+ * length. AST2600 reads it from the receive byte buffer, only valid while the
+ * DMA buffer is disabled; AST2700 reads it from the byte data log, a register
+ * the earlier SoCs do not expose.
+ */
+static void aspeed_i2c_bus_latch_rx_len(AspeedI2CBus *bus, bool dma_buf_en,
+                                        uint8_t data)
+{
+    uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
+
+    ARRAY_FIELD_DP32(bus->regs, I2CC_BYTE_DATA_LOG, RX_BUF, data);
+
+    if (!dma_buf_en) {
+        SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
+    }
+}
+
 static void aspeed_i2c_bus_recv_dma_pool(AspeedI2CBus *bus)
 {
     AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
@@ -349,6 +368,9 @@ static void aspeed_i2c_bus_recv_dma_pool(AspeedI2CBus *bus)
         pool_base[offset + i] = i2c_recv(bus->bus);
         trace_aspeed_i2c_bus_recv("BUFF", i + 1, bus->regs[reg_dma_len],
                                   pool_base[offset + i]);
+        if (i == 0) {
+            aspeed_i2c_bus_latch_rx_len(bus, true, pool_base[offset]);
+        }
         bus->regs[reg_dma_len]--;
         ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN_STS, RX_LEN, i + 1);
     }
@@ -443,6 +465,9 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
             pool_base[i] = i2c_recv(bus->bus);
             trace_aspeed_i2c_bus_recv("BUF", i + 1, pool_rx_count,
                                       pool_base[i]);
+            if (i == 0) {
+                aspeed_i2c_bus_latch_rx_len(bus, false, pool_base[0]);
+            }
         }
 
         /* Update RX count */
@@ -460,7 +485,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
         }
 
         aspeed_i2c_set_rx_dma_dram_offset(bus);
-        while (bus->regs[reg_dma_len]) {
+        for (i = 0; bus->regs[reg_dma_len]; i++) {
             MemTxResult result;
 
             data = i2c_recv(bus->bus);
@@ -476,6 +501,10 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
                 return;
             }
 
+            if (i == 0) {
+                aspeed_i2c_bus_latch_rx_len(bus, true, data);
+            }
+
             bus->dma_dram_offset++;
             bus->regs[reg_dma_len]--;
             /* In new mode, keep track of how many bytes we RXed */
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 05937a7a0b..c8e6ea54ad 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -231,6 +231,8 @@ REG32(I2CS_DMA_TX_ADDR_HI, 0x68)
     FIELD(I2CS_DMA_TX_ADDR_HI, ADDR_HI, 0, 7)
 REG32(I2CS_DMA_RX_ADDR_HI, 0x6c)
     FIELD(I2CS_DMA_RX_ADDR_HI, ADDR_HI, 0, 7)
+REG32(I2CC_BYTE_DATA_LOG, 0x84)
+    FIELD(I2CC_BYTE_DATA_LOG, RX_BUF, 0, 8)
 REG32(I2CC_VERSION_CTRL, 0x94)
     FIELD(I2CC_VERSION_CTRL, FUNC_CFG_DMA_EN, 2, 1)
 
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.