Hi Mikail, Could you please review this patch and verify that it works with the UCD9000 driver?
Thanks, Jamin > -----Original Message----- > From: Jamin Lin <[email protected]> > Sent: Thursday, August 13, 2026 5:02 PM > To: Cédric Le Goater <[email protected]>; Peter Maydell > <[email protected]>; Steven Lee <[email protected]>; Troy > Lee <[email protected]>; Kane Chen <[email protected]>; > Andrew Jeffery <[email protected]>; Joel Stanley > <[email protected]>; open list:ASPEED BMCs <[email protected]>; open > list:All patches CC here <[email protected]> > Cc: Jamin Lin <[email protected]>; Troy Lee > <[email protected]>; [email protected] > Subject: [PATCH v1] hw/i2c/aspeed_i2c: Latch first received byte for SMBus > block reads > > 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
