-----Original Message-----
From: Jamin Lin <jamin_...@aspeedtech.com>
Sent: Tuesday, April 8, 2025 4:25 PM
To: Cédric Le Goater <c...@kaod.org>; Peter Maydell
<peter.mayd...@linaro.org>; Steven Lee <steven_...@aspeedtech.com>; Troy
Lee <leet...@gmail.com>; Andrew Jeffery <and...@codeconstruct.com.au>;
Joel Stanley <j...@jms.id.au>; open list:ASPEED BMCs
<qemu-...@nongnu.org>; open list:All patches CC here
<qemu-devel@nongnu.org>
Cc: Jamin Lin <jamin_...@aspeedtech.com>; Troy Lee
<troy_...@aspeedtech.com>
Subject: [PATCH v2] hw/i2c/aspeed: Fix wrong I2CC_DMA_LEN when
I2CM_DMA_TX/RX_ADDR set first
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
