[PATCH v2] hw/i2c/aspeed_i2c: Latch received bytes for SMBus block reads

[Jamin Lin <jamin_lin@aspeedtech.com>]

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 and latch received bytes into it. The pool
buffer, DMA-to-pool and DMA-to-DRAM paths latch their first byte, the byte
mode path latches every byte. Each latch also updates the receive byte
buffer unless RX_DMA_EN is set, which is the datasheet condition and does
not depend on FUNC_CFG_DMA_EN. The byte data log only exists on AST2700
and AST1040, so it is gated on a class flag.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
Tested-by: Mikail Sadic <mikail.sadic@ibm.com>
---
 hw/i2c/aspeed_i2c.c         | 38 +++++++++++++++++++++++++++++++++++--
 include/hw/i2c/aspeed_i2c.h |  3 +++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index 68bdcd0e25..0bc4bb6fbe 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,27 @@ static int aspeed_i2c_bus_send_dma_pool(AspeedI2CBus *bus)
     return ret;
 }
 
+/*
+ * Latch a received byte where firmware reads it back from: the receive byte
+ * buffer, only valid while the DMA buffer is disabled, and the byte data log,
+ * which AST2700 uses instead. Buffer and DMA transfers latch only the first
+ * byte, read back as the SMBus block length.
+ */
+static void aspeed_i2c_bus_latch_rx_byte(AspeedI2CBus *bus, uint8_t data)
+{
+    AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller);
+    uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
+    uint32_t reg_cmd = aspeed_i2c_bus_cmd_offset(bus);
+
+    if (aic->has_byte_data_log) {
+        ARRAY_FIELD_DP32(bus->regs, I2CC_BYTE_DATA_LOG, RX_BUF, data);
+    }
+
+    if (!SHARED_ARRAY_FIELD_EX32(bus->regs, reg_cmd, RX_DMA_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 +371,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_byte(bus, pool_base[offset]);
+        }
         bus->regs[reg_dma_len]--;
         ARRAY_FIELD_DP32(bus->regs, I2CM_DMA_LEN_STS, RX_LEN, i + 1);
     }
@@ -443,6 +468,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_byte(bus, pool_base[0]);
+            }
         }
 
         /* Update RX count */
@@ -460,7 +488,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 +504,10 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
                 return;
             }
 
+            if (i == 0) {
+                aspeed_i2c_bus_latch_rx_byte(bus, data);
+            }
+
             bus->dma_dram_offset++;
             bus->regs[reg_dma_len]--;
             /* In new mode, keep track of how many bytes we RXed */
@@ -489,7 +521,7 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
     } else {
         data = i2c_recv(bus->bus);
         trace_aspeed_i2c_bus_recv("BYTE", 1, 1, bus->regs[reg_byte_buf]);
-        SHARED_ARRAY_FIELD_DP32(bus->regs, reg_byte_buf, RX_BUF, data);
+        aspeed_i2c_bus_latch_rx_byte(bus, data);
     }
 }
 
@@ -1758,6 +1790,7 @@ static void aspeed_1040_i2c_class_init(ObjectClass *klass, const void *data)
     aic->has_dma = true;
     aic->mem_size = 0x2000;
     aic->has_dma64 = true;
+    aic->has_byte_data_log = true;
     aic->dma_addr_lo_mask = 0x00ffffff;
 }
 
@@ -1780,6 +1813,7 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, const void *data)
     aic->has_dma = true;
     aic->mem_size = 0x2000;
     aic->has_dma64 = true;
+    aic->has_byte_data_log = true;
     aic->dma_addr_lo_mask = 0xffffffff;
 }
 
diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 05937a7a0b..480c6418fe 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)
 
@@ -302,6 +304,7 @@ struct AspeedI2CClass {
     bool has_share_pool;
     uint64_t mem_size;
     bool has_dma64;
+    bool has_byte_data_log;
     uint32_t dma_addr_lo_mask;
 };
 
-- 
2.53.0