| Message ID | 20240808024916.1262715-5-jamin_lin@aspeedtech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support I2C for AST2700 | expand |
On 8/8/24 04:49, Jamin Lin wrote: > The "Current DMA Operating Address Status(0x50)" register of > I2C new mode has been removed in AST2700. > This register is used for debugging and it is a read only register. > > To support AST2700 DMA mode, introduce a new > dma_dram_offset class attribute in AspeedI2Cbus to save the > current DMA operating address. > > ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) > And the base address of dram is "0x4 00000000" which > is 64bits address. > > Set the dma_dram_offset data type to uint64_t for > 64 bits dram address DMA support. > > Both "DMA Mode Buffer Address Register(I2CD24 old mode)" and > "DMA Operating Address Status (I2CC50 new mode)" are used for showing the > low part dram offset bits [31:0], so change to read/write both register bits [31:0] in > bus register read/write functions. > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > hw/i2c/aspeed_i2c.c | 51 +++++++++++++++++++++++-------------- > include/hw/i2c/aspeed_i2c.h | 9 +------ > 2 files changed, 33 insertions(+), 27 deletions(-) > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > index abcb1d5330..2dea3a42a0 100644 > --- a/hw/i2c/aspeed_i2c.c > +++ b/hw/i2c/aspeed_i2c.c > @@ -114,7 +114,10 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, > if (!aic->has_dma) { > qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); > value = -1; > + break; > } > + > + value = extract64(bus->dma_dram_offset, 0, 32); > break; > case A_I2CD_DMA_LEN: > if (!aic->has_dma) { > @@ -150,9 +153,7 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > case A_I2CM_DMA_TX_ADDR: > case A_I2CM_DMA_RX_ADDR: > case A_I2CM_DMA_LEN_STS: > - case A_I2CC_DMA_ADDR: > case A_I2CC_DMA_LEN: > - > case A_I2CS_DEV_ADDR: > case A_I2CS_DMA_RX_ADDR: > case A_I2CS_DMA_LEN: > @@ -161,6 +162,9 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > case A_I2CS_DMA_LEN_STS: > /* Value is already set, don't do anything. */ > break; > + case A_I2CC_DMA_ADDR: > + value = extract64(bus->dma_dram_offset, 0, 32); > + break; > case A_I2CS_INTR_STS: > break; > case A_I2CM_CMD: > @@ -210,18 +214,18 @@ static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data) > { > MemTxResult result; > AspeedI2CState *s = bus->controller; > - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); > uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); > > - result = address_space_read(&s->dram_as, bus->regs[reg_dma_addr], > + result = address_space_read(&s->dram_as, bus->dma_dram_offset, > MEMTXATTRS_UNSPECIFIED, data, 1); > if (result != MEMTX_OK) { > - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n", > - __func__, bus->regs[reg_dma_addr]); > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: DRAM read failed @%" PRIx64 "\n", > + __func__, bus->dma_dram_offset); > return -1; > } > > - bus->regs[reg_dma_addr]++; > + bus->dma_dram_offset++; > bus->regs[reg_dma_len]--; > return 0; > } > @@ -291,7 +295,6 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) > uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus); > uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus); > uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); > - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); > int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, > RX_SIZE) + 1; > > @@ -323,14 +326,17 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) > data = i2c_recv(bus->bus); > trace_aspeed_i2c_bus_recv("DMA", bus->regs[reg_dma_len], > bus->regs[reg_dma_len], data); > - result = address_space_write(&s->dram_as, bus->regs[reg_dma_addr], > + > + result = address_space_write(&s->dram_as, bus->dma_dram_offset, > MEMTXATTRS_UNSPECIFIED, &data, 1); > if (result != MEMTX_OK) { > - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n", > - __func__, bus->regs[reg_dma_addr]); > + qemu_log_mask(LOG_GUEST_ERROR, > + "%s: DRAM write failed @%" PRIx64 "\n", > + __func__, bus->dma_dram_offset); > return; > } > - bus->regs[reg_dma_addr]++; > + > + bus->dma_dram_offset++; > bus->regs[reg_dma_len]--; > /* In new mode, keep track of how many bytes we RXed */ > if (aspeed_i2c_is_new_mode(bus->controller)) { > @@ -636,14 +642,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset, > case A_I2CM_DMA_TX_ADDR: > bus->regs[R_I2CM_DMA_TX_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR, > ADDR); > - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR, ADDR); > + 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, > ADDR); > - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_RX_ADDR, ADDR); > + 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; > @@ -811,7 +821,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset, > break; > } > > - bus->regs[R_I2CD_DMA_ADDR] = value & 0x3ffffffc; > + bus->dma_dram_offset = deposit64(bus->dma_dram_offset, 0, 32, > + value & 0x3ffffffc); > break; > > case A_I2CD_DMA_LEN: > @@ -983,6 +994,7 @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { > .fields = (const VMStateField[]) { > VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG), > VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE), > + VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus), > VMSTATE_END_OF_LIST() > } > }; > @@ -1188,8 +1200,9 @@ static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus, > return -1; > } > ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0); > - bus->regs[R_I2CC_DMA_ADDR] = > - ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR); > + bus->dma_dram_offset = > + deposit64(bus->dma_dram_offset, 0, 32, > + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR)); > bus->regs[R_I2CC_DMA_LEN] = > ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1; > i2c_ack(bus->bus); > @@ -1255,10 +1268,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event) > static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data) > { > assert(address_space_write(&bus->controller->dram_as, > - bus->regs[R_I2CC_DMA_ADDR], > + bus->dma_dram_offset, > MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK); > > - bus->regs[R_I2CC_DMA_ADDR]++; > + bus->dma_dram_offset++; > bus->regs[R_I2CC_DMA_LEN]--; > ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, > ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1); > diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h > index b42c4dc584..bdaea3207d 100644 > --- a/include/hw/i2c/aspeed_i2c.h > +++ b/include/hw/i2c/aspeed_i2c.h > @@ -248,6 +248,7 @@ struct AspeedI2CBus { > > uint32_t regs[ASPEED_I2C_NEW_NUM_REG]; > uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE]; > + uint64_t dma_dram_offset; > }; > > struct AspeedI2CState { > @@ -369,14 +370,6 @@ static inline uint32_t aspeed_i2c_bus_dma_len_offset(AspeedI2CBus *bus) > return R_I2CD_DMA_LEN; > } > > -static inline uint32_t aspeed_i2c_bus_dma_addr_offset(AspeedI2CBus *bus) > -{ > - if (aspeed_i2c_is_new_mode(bus->controller)) { > - return R_I2CC_DMA_ADDR; > - } > - return R_I2CD_DMA_ADDR; > -} > - > static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus) > { > return SHARED_ARRAY_FIELD_EX32(bus->regs, aspeed_i2c_bus_ctrl_offset(bus),
Jamin,
In case you resend, would you mind changing the commit title and use
a capital letter on the first word :
hw/i2c/aspeed: Introduce a new dma_dram_offset attribute in AspeedI2Cbus
On 8/8/24 04:49, Jamin Lin wrote:
> The "Current DMA Operating Address Status(0x50)" register of
> I2C new mode has been removed in AST2700.
> This register is used for debugging and it is a read only register.
>
> To support AST2700 DMA mode, introduce a new
> dma_dram_offset class attribute in AspeedI2Cbus to save the
> current DMA operating address.
>
> ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
> And the base address of dram is "0x4 00000000" which
> is 64bits address.
>
> Set the dma_dram_offset data type to uint64_t for
> 64 bits dram address DMA support.
>
> Both "DMA Mode Buffer Address Register(I2CD24 old mode)" and
> "DMA Operating Address Status (I2CC50 new mode)" are used for showing the
> low part dram offset bits [31:0], so change to read/write both register bits [31:0] in
> bus register read/write functions.
I think it is worth mentioning that aspeed_i2c_bus_vmstate is changed
again and version is not increased because it was done earlier in the
same series.
Thanks,
C.
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
> hw/i2c/aspeed_i2c.c | 51 +++++++++++++++++++++++--------------
> include/hw/i2c/aspeed_i2c.h | 9 +------
> 2 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index abcb1d5330..2dea3a42a0 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -114,7 +114,10 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset,
> if (!aic->has_dma) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__);
> value = -1;
> + break;
> }
> +
> + value = extract64(bus->dma_dram_offset, 0, 32);
> break;
> case A_I2CD_DMA_LEN:
> if (!aic->has_dma) {
> @@ -150,9 +153,7 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
> case A_I2CM_DMA_TX_ADDR:
> case A_I2CM_DMA_RX_ADDR:
> case A_I2CM_DMA_LEN_STS:
> - case A_I2CC_DMA_ADDR:
> case A_I2CC_DMA_LEN:
> -
> case A_I2CS_DEV_ADDR:
> case A_I2CS_DMA_RX_ADDR:
> case A_I2CS_DMA_LEN:
> @@ -161,6 +162,9 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset,
> case A_I2CS_DMA_LEN_STS:
> /* Value is already set, don't do anything. */
> break;
> + case A_I2CC_DMA_ADDR:
> + value = extract64(bus->dma_dram_offset, 0, 32);
> + break;
> case A_I2CS_INTR_STS:
> break;
> case A_I2CM_CMD:
> @@ -210,18 +214,18 @@ static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data)
> {
> MemTxResult result;
> AspeedI2CState *s = bus->controller;
> - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus);
> uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
>
> - result = address_space_read(&s->dram_as, bus->regs[reg_dma_addr],
> + result = address_space_read(&s->dram_as, bus->dma_dram_offset,
> MEMTXATTRS_UNSPECIFIED, data, 1);
> if (result != MEMTX_OK) {
> - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n",
> - __func__, bus->regs[reg_dma_addr]);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: DRAM read failed @%" PRIx64 "\n",
> + __func__, bus->dma_dram_offset);
> return -1;
> }
>
> - bus->regs[reg_dma_addr]++;
> + bus->dma_dram_offset++;
> bus->regs[reg_dma_len]--;
> return 0;
> }
> @@ -291,7 +295,6 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
> uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus);
> uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus);
> uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus);
> - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus);
> int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl,
> RX_SIZE) + 1;
>
> @@ -323,14 +326,17 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus)
> data = i2c_recv(bus->bus);
> trace_aspeed_i2c_bus_recv("DMA", bus->regs[reg_dma_len],
> bus->regs[reg_dma_len], data);
> - result = address_space_write(&s->dram_as, bus->regs[reg_dma_addr],
> +
> + result = address_space_write(&s->dram_as, bus->dma_dram_offset,
> MEMTXATTRS_UNSPECIFIED, &data, 1);
> if (result != MEMTX_OK) {
> - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n",
> - __func__, bus->regs[reg_dma_addr]);
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: DRAM write failed @%" PRIx64 "\n",
> + __func__, bus->dma_dram_offset);
> return;
> }
> - bus->regs[reg_dma_addr]++;
> +
> + bus->dma_dram_offset++;
> bus->regs[reg_dma_len]--;
> /* In new mode, keep track of how many bytes we RXed */
> if (aspeed_i2c_is_new_mode(bus->controller)) {
> @@ -636,14 +642,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> case A_I2CM_DMA_TX_ADDR:
> bus->regs[R_I2CM_DMA_TX_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR,
> ADDR);
> - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR, ADDR);
> + 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,
> ADDR);
> - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_RX_ADDR, ADDR);
> + 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;
> @@ -811,7 +821,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset,
> break;
> }
>
> - bus->regs[R_I2CD_DMA_ADDR] = value & 0x3ffffffc;
> + bus->dma_dram_offset = deposit64(bus->dma_dram_offset, 0, 32,
> + value & 0x3ffffffc);
> break;
>
> case A_I2CD_DMA_LEN:
> @@ -983,6 +994,7 @@ static const VMStateDescription aspeed_i2c_bus_vmstate = {
> .fields = (const VMStateField[]) {
> VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG),
> VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE),
> + VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus),
> VMSTATE_END_OF_LIST()
> }
> };
> @@ -1188,8 +1200,9 @@ static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus,
> return -1;
> }
> ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0);
> - bus->regs[R_I2CC_DMA_ADDR] =
> - ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR);
> + bus->dma_dram_offset =
> + deposit64(bus->dma_dram_offset, 0, 32,
> + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR));
> bus->regs[R_I2CC_DMA_LEN] =
> ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1;
> i2c_ack(bus->bus);
> @@ -1255,10 +1268,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event)
> static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data)
> {
> assert(address_space_write(&bus->controller->dram_as,
> - bus->regs[R_I2CC_DMA_ADDR],
> + bus->dma_dram_offset,
> MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK);
>
> - bus->regs[R_I2CC_DMA_ADDR]++;
> + bus->dma_dram_offset++;
> bus->regs[R_I2CC_DMA_LEN]--;
> ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN,
> ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1);
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index b42c4dc584..bdaea3207d 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -248,6 +248,7 @@ struct AspeedI2CBus {
>
> uint32_t regs[ASPEED_I2C_NEW_NUM_REG];
> uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE];
> + uint64_t dma_dram_offset;
> };
>
> struct AspeedI2CState {
> @@ -369,14 +370,6 @@ static inline uint32_t aspeed_i2c_bus_dma_len_offset(AspeedI2CBus *bus)
> return R_I2CD_DMA_LEN;
> }
>
> -static inline uint32_t aspeed_i2c_bus_dma_addr_offset(AspeedI2CBus *bus)
> -{
> - if (aspeed_i2c_is_new_mode(bus->controller)) {
> - return R_I2CC_DMA_ADDR;
> - }
> - return R_I2CD_DMA_ADDR;
> -}
> -
> static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus)
> {
> return SHARED_ARRAY_FIELD_EX32(bus->regs, aspeed_i2c_bus_ctrl_offset(bus),
Hi Cedric, > Subject: Re: [PATCH v2 04/11] hw/i2c/aspeed: introduce a new > dma_dram_offset attribute in AspeedI2Cbus > > Jamin, > > In case you resend, would you mind changing the commit title and use a > capital letter on the first word : > Okay, I will change all commit title to use a capital letter on the first word in the next patch series(v3) > hw/i2c/aspeed: Introduce a new dma_dram_offset attribute in > AspeedI2Cbus > > On 8/8/24 04:49, Jamin Lin wrote: > > The "Current DMA Operating Address Status(0x50)" register of I2C new > > mode has been removed in AST2700. > > This register is used for debugging and it is a read only register. > > > > To support AST2700 DMA mode, introduce a new dma_dram_offset class > > attribute in AspeedI2Cbus to save the current DMA operating address. > > > > ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) And the > > base address of dram is "0x4 00000000" which is 64bits address. > > > > Set the dma_dram_offset data type to uint64_t for > > 64 bits dram address DMA support. > > > > Both "DMA Mode Buffer Address Register(I2CD24 old mode)" and "DMA > > Operating Address Status (I2CC50 new mode)" are used for showing the > > low part dram offset bits [31:0], so change to read/write both > > register bits [31:0] in bus register read/write functions. > > I think it is worth mentioning that aspeed_i2c_bus_vmstate is changed again > and version is not increased because it was done earlier in the same series. > Got it will add this description in this commit message. Thanks for review and suggestion. Jamin > > Thanks, > > C. > > > > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> > > --- > > hw/i2c/aspeed_i2c.c | 51 > +++++++++++++++++++++++-------------- > > include/hw/i2c/aspeed_i2c.h | 9 +------ > > 2 files changed, 33 insertions(+), 27 deletions(-) > > > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index > > abcb1d5330..2dea3a42a0 100644 > > --- a/hw/i2c/aspeed_i2c.c > > +++ b/hw/i2c/aspeed_i2c.c > > @@ -114,7 +114,10 @@ static uint64_t > aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, > > if (!aic->has_dma) { > > qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA > support\n", __func__); > > value = -1; > > + break; > > } > > + > > + value = extract64(bus->dma_dram_offset, 0, 32); > > break; > > case A_I2CD_DMA_LEN: > > if (!aic->has_dma) { > > @@ -150,9 +153,7 @@ static uint64_t > aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > > case A_I2CM_DMA_TX_ADDR: > > case A_I2CM_DMA_RX_ADDR: > > case A_I2CM_DMA_LEN_STS: > > - case A_I2CC_DMA_ADDR: > > case A_I2CC_DMA_LEN: > > - > > case A_I2CS_DEV_ADDR: > > case A_I2CS_DMA_RX_ADDR: > > case A_I2CS_DMA_LEN: > > @@ -161,6 +162,9 @@ static uint64_t > aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > > case A_I2CS_DMA_LEN_STS: > > /* Value is already set, don't do anything. */ > > break; > > + case A_I2CC_DMA_ADDR: > > + value = extract64(bus->dma_dram_offset, 0, 32); > > + break; > > case A_I2CS_INTR_STS: > > break; > > case A_I2CM_CMD: > > @@ -210,18 +214,18 @@ static int aspeed_i2c_dma_read(AspeedI2CBus > *bus, uint8_t *data) > > { > > MemTxResult result; > > AspeedI2CState *s = bus->controller; > > - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); > > uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); > > > > - result = address_space_read(&s->dram_as, bus->regs[reg_dma_addr], > > + result = address_space_read(&s->dram_as, bus->dma_dram_offset, > > MEMTXATTRS_UNSPECIFIED, data, > 1); > > if (result != MEMTX_OK) { > > - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed > @%08x\n", > > - __func__, bus->regs[reg_dma_addr]); > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: DRAM read failed @%" PRIx64 "\n", > > + __func__, bus->dma_dram_offset); > > return -1; > > } > > > > - bus->regs[reg_dma_addr]++; > > + bus->dma_dram_offset++; > > bus->regs[reg_dma_len]--; > > return 0; > > } > > @@ -291,7 +295,6 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) > > uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus); > > uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus); > > uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); > > - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); > > int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, > reg_pool_ctrl, > > RX_SIZE) + 1; > > > > @@ -323,14 +326,17 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus > *bus) > > data = i2c_recv(bus->bus); > > trace_aspeed_i2c_bus_recv("DMA", > bus->regs[reg_dma_len], > > bus->regs[reg_dma_len], > data); > > - result = address_space_write(&s->dram_as, > bus->regs[reg_dma_addr], > > + > > + result = address_space_write(&s->dram_as, > > + bus->dma_dram_offset, > > > MEMTXATTRS_UNSPECIFIED, &data, 1); > > if (result != MEMTX_OK) { > > - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write > failed @%08x\n", > > - __func__, bus->regs[reg_dma_addr]); > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: DRAM write failed @%" PRIx64 > "\n", > > + __func__, bus->dma_dram_offset); > > return; > > } > > - bus->regs[reg_dma_addr]++; > > + > > + bus->dma_dram_offset++; > > bus->regs[reg_dma_len]--; > > /* In new mode, keep track of how many bytes we RXed */ > > if (aspeed_i2c_is_new_mode(bus->controller)) { @@ > > -636,14 +642,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus > *bus, hwaddr offset, > > case A_I2CM_DMA_TX_ADDR: > > bus->regs[R_I2CM_DMA_TX_ADDR] = FIELD_EX32(value, > I2CM_DMA_TX_ADDR, > > ADDR); > > - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, > I2CM_DMA_TX_ADDR, ADDR); > > + 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, > > ADDR); > > - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, > I2CM_DMA_RX_ADDR, ADDR); > > + 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; > > @@ -811,7 +821,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus > *bus, hwaddr offset, > > break; > > } > > > > - bus->regs[R_I2CD_DMA_ADDR] = value & 0x3ffffffc; > > + bus->dma_dram_offset = deposit64(bus->dma_dram_offset, 0, > 32, > > + value & 0x3ffffffc); > > break; > > > > case A_I2CD_DMA_LEN: > > @@ -983,6 +994,7 @@ static const VMStateDescription > aspeed_i2c_bus_vmstate = { > > .fields = (const VMStateField[]) { > > VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, > ASPEED_I2C_NEW_NUM_REG), > > VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, > > ASPEED_I2C_BUS_POOL_SIZE), > > + VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus), > > VMSTATE_END_OF_LIST() > > } > > }; > > @@ -1188,8 +1200,9 @@ static int > aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus, > > return -1; > > } > > ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, > 0); > > - bus->regs[R_I2CC_DMA_ADDR] = > > - ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR); > > + bus->dma_dram_offset = > > + deposit64(bus->dma_dram_offset, 0, 32, > > + ARRAY_FIELD_EX32(bus->regs, > I2CS_DMA_RX_ADDR, > > + ADDR)); > > bus->regs[R_I2CC_DMA_LEN] = > > ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, > RX_BUF_LEN) + 1; > > i2c_ack(bus->bus); > > @@ -1255,10 +1268,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave > *slave, enum i2c_event event) > > static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, > uint8_t data) > > { > > assert(address_space_write(&bus->controller->dram_as, > > - bus->regs[R_I2CC_DMA_ADDR], > > + bus->dma_dram_offset, > > MEMTXATTRS_UNSPECIFIED, &data, > 1) == > > MEMTX_OK); > > > > - bus->regs[R_I2CC_DMA_ADDR]++; > > + bus->dma_dram_offset++; > > bus->regs[R_I2CC_DMA_LEN]--; > > ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, > > ARRAY_FIELD_EX32(bus->regs, > I2CS_DMA_LEN_STS, > > RX_LEN) + 1); diff --git a/include/hw/i2c/aspeed_i2c.h > > b/include/hw/i2c/aspeed_i2c.h index b42c4dc584..bdaea3207d 100644 > > --- a/include/hw/i2c/aspeed_i2c.h > > +++ b/include/hw/i2c/aspeed_i2c.h > > @@ -248,6 +248,7 @@ struct AspeedI2CBus { > > > > uint32_t regs[ASPEED_I2C_NEW_NUM_REG]; > > uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE]; > > + uint64_t dma_dram_offset; > > }; > > > > struct AspeedI2CState { > > @@ -369,14 +370,6 @@ static inline uint32_t > aspeed_i2c_bus_dma_len_offset(AspeedI2CBus *bus) > > return R_I2CD_DMA_LEN; > > } > > > > -static inline uint32_t aspeed_i2c_bus_dma_addr_offset(AspeedI2CBus > > *bus) -{ > > - if (aspeed_i2c_is_new_mode(bus->controller)) { > > - return R_I2CC_DMA_ADDR; > > - } > > - return R_I2CD_DMA_ADDR; > > -} > > - > > static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus) > > { > > return SHARED_ARRAY_FIELD_EX32(bus->regs, > > aspeed_i2c_bus_ctrl_offset(bus),
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index abcb1d5330..2dea3a42a0 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -114,7 +114,10 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, if (!aic->has_dma) { qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); value = -1; + break; } + + value = extract64(bus->dma_dram_offset, 0, 32); break; case A_I2CD_DMA_LEN: if (!aic->has_dma) { @@ -150,9 +153,7 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, case A_I2CM_DMA_TX_ADDR: case A_I2CM_DMA_RX_ADDR: case A_I2CM_DMA_LEN_STS: - case A_I2CC_DMA_ADDR: case A_I2CC_DMA_LEN: - case A_I2CS_DEV_ADDR: case A_I2CS_DMA_RX_ADDR: case A_I2CS_DMA_LEN: @@ -161,6 +162,9 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, case A_I2CS_DMA_LEN_STS: /* Value is already set, don't do anything. */ break; + case A_I2CC_DMA_ADDR: + value = extract64(bus->dma_dram_offset, 0, 32); + break; case A_I2CS_INTR_STS: break; case A_I2CM_CMD: @@ -210,18 +214,18 @@ static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data) { MemTxResult result; AspeedI2CState *s = bus->controller; - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); - result = address_space_read(&s->dram_as, bus->regs[reg_dma_addr], + result = address_space_read(&s->dram_as, bus->dma_dram_offset, MEMTXATTRS_UNSPECIFIED, data, 1); if (result != MEMTX_OK) { - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n", - __func__, bus->regs[reg_dma_addr]); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: DRAM read failed @%" PRIx64 "\n", + __func__, bus->dma_dram_offset); return -1; } - bus->regs[reg_dma_addr]++; + bus->dma_dram_offset++; bus->regs[reg_dma_len]--; return 0; } @@ -291,7 +295,6 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) uint32_t reg_pool_ctrl = aspeed_i2c_bus_pool_ctrl_offset(bus); uint32_t reg_byte_buf = aspeed_i2c_bus_byte_buf_offset(bus); uint32_t reg_dma_len = aspeed_i2c_bus_dma_len_offset(bus); - uint32_t reg_dma_addr = aspeed_i2c_bus_dma_addr_offset(bus); int pool_rx_count = SHARED_ARRAY_FIELD_EX32(bus->regs, reg_pool_ctrl, RX_SIZE) + 1; @@ -323,14 +326,17 @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) data = i2c_recv(bus->bus); trace_aspeed_i2c_bus_recv("DMA", bus->regs[reg_dma_len], bus->regs[reg_dma_len], data); - result = address_space_write(&s->dram_as, bus->regs[reg_dma_addr], + + result = address_space_write(&s->dram_as, bus->dma_dram_offset, MEMTXATTRS_UNSPECIFIED, &data, 1); if (result != MEMTX_OK) { - qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n", - __func__, bus->regs[reg_dma_addr]); + qemu_log_mask(LOG_GUEST_ERROR, + "%s: DRAM write failed @%" PRIx64 "\n", + __func__, bus->dma_dram_offset); return; } - bus->regs[reg_dma_addr]++; + + bus->dma_dram_offset++; bus->regs[reg_dma_len]--; /* In new mode, keep track of how many bytes we RXed */ if (aspeed_i2c_is_new_mode(bus->controller)) { @@ -636,14 +642,18 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset, case A_I2CM_DMA_TX_ADDR: bus->regs[R_I2CM_DMA_TX_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR, ADDR); - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_TX_ADDR, ADDR); + 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, ADDR); - bus->regs[R_I2CC_DMA_ADDR] = FIELD_EX32(value, I2CM_DMA_RX_ADDR, ADDR); + 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; @@ -811,7 +821,8 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, hwaddr offset, break; } - bus->regs[R_I2CD_DMA_ADDR] = value & 0x3ffffffc; + bus->dma_dram_offset = deposit64(bus->dma_dram_offset, 0, 32, + value & 0x3ffffffc); break; case A_I2CD_DMA_LEN: @@ -983,6 +994,7 @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { .fields = (const VMStateField[]) { VMSTATE_UINT32_ARRAY(regs, AspeedI2CBus, ASPEED_I2C_NEW_NUM_REG), VMSTATE_UINT8_ARRAY(pool, AspeedI2CBus, ASPEED_I2C_BUS_POOL_SIZE), + VMSTATE_UINT64(dma_dram_offset, AspeedI2CBus), VMSTATE_END_OF_LIST() } }; @@ -1188,8 +1200,9 @@ static int aspeed_i2c_bus_new_slave_event(AspeedI2CBus *bus, return -1; } ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, 0); - bus->regs[R_I2CC_DMA_ADDR] = - ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR); + bus->dma_dram_offset = + deposit64(bus->dma_dram_offset, 0, 32, + ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_RX_ADDR, ADDR)); bus->regs[R_I2CC_DMA_LEN] = ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN, RX_BUF_LEN) + 1; i2c_ack(bus->bus); @@ -1255,10 +1268,10 @@ static int aspeed_i2c_bus_slave_event(I2CSlave *slave, enum i2c_event event) static void aspeed_i2c_bus_new_slave_send_async(AspeedI2CBus *bus, uint8_t data) { assert(address_space_write(&bus->controller->dram_as, - bus->regs[R_I2CC_DMA_ADDR], + bus->dma_dram_offset, MEMTXATTRS_UNSPECIFIED, &data, 1) == MEMTX_OK); - bus->regs[R_I2CC_DMA_ADDR]++; + bus->dma_dram_offset++; bus->regs[R_I2CC_DMA_LEN]--; ARRAY_FIELD_DP32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN, ARRAY_FIELD_EX32(bus->regs, I2CS_DMA_LEN_STS, RX_LEN) + 1); diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index b42c4dc584..bdaea3207d 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -248,6 +248,7 @@ struct AspeedI2CBus { uint32_t regs[ASPEED_I2C_NEW_NUM_REG]; uint8_t pool[ASPEED_I2C_BUS_POOL_SIZE]; + uint64_t dma_dram_offset; }; struct AspeedI2CState { @@ -369,14 +370,6 @@ static inline uint32_t aspeed_i2c_bus_dma_len_offset(AspeedI2CBus *bus) return R_I2CD_DMA_LEN; } -static inline uint32_t aspeed_i2c_bus_dma_addr_offset(AspeedI2CBus *bus) -{ - if (aspeed_i2c_is_new_mode(bus->controller)) { - return R_I2CC_DMA_ADDR; - } - return R_I2CD_DMA_ADDR; -} - static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus) { return SHARED_ARRAY_FIELD_EX32(bus->regs, aspeed_i2c_bus_ctrl_offset(bus),
The "Current DMA Operating Address Status(0x50)" register of I2C new mode has been removed in AST2700. This register is used for debugging and it is a read only register. To support AST2700 DMA mode, introduce a new dma_dram_offset class attribute in AspeedI2Cbus to save the current DMA operating address. ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) And the base address of dram is "0x4 00000000" which is 64bits address. Set the dma_dram_offset data type to uint64_t for 64 bits dram address DMA support. Both "DMA Mode Buffer Address Register(I2CD24 old mode)" and "DMA Operating Address Status (I2CC50 new mode)" are used for showing the low part dram offset bits [31:0], so change to read/write both register bits [31:0] in bus register read/write functions. Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com> --- hw/i2c/aspeed_i2c.c | 51 +++++++++++++++++++++++-------------- include/hw/i2c/aspeed_i2c.h | 9 +------ 2 files changed, 33 insertions(+), 27 deletions(-)