In the previous design, the I2C model would update I2CC_DMA_LEN (0x54) based on
the value of I2CM_DMA_LEN (0x1C) when the firmware set either I2CM_DMA_TX_ADDR
(0x30) or I2CM_DMA_RX_ADDR (0x34). However, this only worked correctly if the
firmware set I2CM_DMA_LEN before setting I2CM_DMA_TX_ADDR or I2CM_DMA_RX_ADDR.
If the firmware instead set I2CM_DMA_TX_ADDR or I2CM_DMA_RX_ADDR before setting
I2CM_DMA_LEN, the value written to I2CC_DMA_LEN would be incorrect.
Ideally, this issue should be resolved by updating the model to set
I2CC_DMA_LEN (0x54) when the firmware writes to the I2CM_DMA_LEN (0x1C)
register, instead of when it writes to I2CM_DMA_TX_ADDR (0x30) or
I2CM_DMA_RX_ADDR (0x34).
Originally, the design of I2CM_DMA_LEN (0x1C) included buffer length
write-enable bits for the current command:
Bit 31 enabled the RX buffer length update
Bit 15 enabled the TX buffer length update
In other words, when the firmware set either bit 31 or bit 15, the I2C model
could safely update I2CC_DMA_LEN (0x54) with the value in I2CM_DMA_LEN (0x1C).
However, starting with the AST2700, the design of the I2CM_DMA_LEN (0x1C)
register was changed. The write-enable bits (bit 31 and bit 15) were removed,
meaning there is no longer an explicit indication of whether the firmware
intends to update the TX or RX length.
As a result, on AST2700 and newer SoCs, the model cannot reliably determine
whether a write to I2CM_DMA_LEN was meant for TX or RX. This ambiguity is
especially problematic when the value written is 0, which actually corresponds
to a DMA length of 1.
To ensure consistent behavior across all SoCs, the model now updates
I2CC_DMA_LEN when I2CM_CMD (0x18) is written, as this is the final command that
initiates a TX or RX transfer and reflects the firmware’s intent more clearly.
Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
Fixes: ba2cccd (aspeed: i2c: Add new mode support)
---
hw/i2c/aspeed_i2c.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index a8fbb9f44a..c659099e9a 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -634,6 +634,20 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus,
hwaddr offset,
break;
}
+ /* Handle DMA length */
+ if (SHARED_FIELD_EX32(value, TX_DMA_EN) &&
+ SHARED_FIELD_EX32(value, M_TX_CMD)) {
+ bus->regs[R_I2CC_DMA_LEN] = ARRAY_FIELD_EX32(bus->regs,
+ I2CM_DMA_LEN,
+ TX_BUF_LEN) + 1;
+ }
+ if (SHARED_FIELD_EX32(value, RX_DMA_EN) &&
+ SHARED_FIELD_EX32(value, M_RX_CMD)) {
+ bus->regs[R_I2CC_DMA_LEN] = ARRAY_FIELD_EX32(bus->regs,
+ I2CM_DMA_LEN,
+ RX_BUF_LEN) + 1;
+ }
+
if (bus->regs[R_I2CM_INTR_STS] & 0xffff0000) {
qemu_log_mask(LOG_UNIMP, "%s: Packet mode is not implemented\n",
__func__);
@@ -656,8 +670,6 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus,
hwaddr offset,
bus->dma_dram_offset =
deposit64(bus->dma_dram_offset, 0, 32,
FIELD_EX32(value, I2CM_DMA_TX_ADDR, ADDR));
- bus->regs[R_I2CC_DMA_LEN] = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN,
- TX_BUF_LEN) + 1;
break;
case A_I2CM_DMA_RX_ADDR:
bus->regs[R_I2CM_DMA_RX_ADDR] = FIELD_EX32(value, I2CM_DMA_RX_ADDR,
@@ -665,8 +677,6 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus,
hwaddr offset,
bus->dma_dram_offset =
deposit64(bus->dma_dram_offset, 0, 32,
FIELD_EX32(value, I2CM_DMA_RX_ADDR, ADDR));
- bus->regs[R_I2CC_DMA_LEN] = ARRAY_FIELD_EX32(bus->regs, I2CM_DMA_LEN,
- RX_BUF_LEN) + 1;
break;
case A_I2CM_DMA_LEN:
w1t = FIELD_EX32(value, I2CM_DMA_LEN, RX_BUF_LEN_W1T) ||
--
2.43.0
