Message ID | 20240808024916.1262715-7-jamin_lin@aspeedtech.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | support I2C for AST2700 | expand |
Jamin, Please change commit title to hw/i2c/aspeed: Add support for Tx/Rx buffer 64 bits address So more proposals to improve the commit log below, On 8/8/24 04:49, Jamin Lin wrote: > ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) > And the base address of dram is "0x4 00000000" which and > is 64bits address. > > It have "Master DMA Mode Tx Buffer Base Address[39:32](0x60)" it has > and "Master DMA Mode Rx Buffer Base Address[39:32](0x64)" ... registers > to save the high part physical address of Tx/Rx buffer address > for master mode. > > It have "Slave DMA Mode Tx Buffer Base Address[39:32](0x68)" and It has > "Slave DMA Mode Rx Buffer Base Address[39:32](0x6C)" registers > to save the high part physical address of Tx/Rx buffer address > for slave mode. > > Ex: Tx buffer address for master mode [39:0] > The "Master DMA Mode Tx Buffer Base Address[39:32](0x60)" > bits [7:0] which corresponds the bits [39:32] of the 64 bits address of > the Tx buffer address. > The "Master DMA Mode Tx Buffer Base Address(0x30)" bits [31:0] > which corresponds the bits [31:0] of the 64 bits address > of the Tx buffer address. > > Introduce a new has_dma64 class attribute and new registers of ... new registers for the > new mode to support DMA 64 bits dram address. > Update new mode register number to 28. 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> > Reviewed-by: Cédric Le Goater <clg@redhat.com> > --- > hw/i2c/aspeed_i2c.c | 48 +++++++++++++++++++++++++++++++++++++ > include/hw/i2c/aspeed_i2c.h | 12 +++++++++- > 2 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c > index 819a875839..c1ff80b1cf 100644 > --- a/hw/i2c/aspeed_i2c.c > +++ b/hw/i2c/aspeed_i2c.c > @@ -140,6 +140,7 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, > static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > unsigned size) > { > + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); > uint64_t value = bus->regs[offset / sizeof(*bus->regs)]; > > switch (offset) { > @@ -170,6 +171,16 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > case A_I2CM_CMD: > value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, i2c_bus_busy(bus->bus)); > break; > + case A_I2CM_DMA_TX_ADDR_HI: > + case A_I2CM_DMA_RX_ADDR_HI: > + case A_I2CS_DMA_TX_ADDR_HI: > + case A_I2CS_DMA_RX_ADDR_HI: > + if (!aic->has_dma64) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", > + __func__); > + value = -1; > + } > + break; > default: > qemu_log_mask(LOG_GUEST_ERROR, > "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); > @@ -731,6 +742,42 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset, > qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not implemented\n", > __func__); > break; > + > + case A_I2CM_DMA_TX_ADDR_HI: > + if (!aic->has_dma64) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", > + __func__); > + break; > + } > + bus->regs[R_I2CM_DMA_TX_ADDR_HI] = FIELD_EX32(value, > + I2CM_DMA_TX_ADDR_HI, > + ADDR_HI); > + break; > + case A_I2CM_DMA_RX_ADDR_HI: > + if (!aic->has_dma64) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", > + __func__); > + break; > + } > + bus->regs[R_I2CM_DMA_RX_ADDR_HI] = FIELD_EX32(value, > + I2CM_DMA_RX_ADDR_HI, > + ADDR_HI); > + break; > + case A_I2CS_DMA_TX_ADDR_HI: > + qemu_log_mask(LOG_UNIMP, > + "%s: Slave mode DMA TX Addr high is not implemented\n", > + __func__); > + break; > + case A_I2CS_DMA_RX_ADDR_HI: > + if (!aic->has_dma64) { > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", > + __func__); > + break; > + } > + bus->regs[R_I2CS_DMA_RX_ADDR_HI] = FIELD_EX32(value, > + I2CS_DMA_RX_ADDR_HI, > + ADDR_HI); > + break; > default: > qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", > __func__, offset); > @@ -1554,6 +1601,7 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data) > aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; > aic->has_dma = true; > aic->mem_size = 0x2000; > + aic->has_dma64 = true; > } > > static const TypeInfo aspeed_2700_i2c_info = { > diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h > index 4f23dc10c3..2c4c81bd20 100644 > --- a/include/hw/i2c/aspeed_i2c.h > +++ b/include/hw/i2c/aspeed_i2c.h > @@ -38,7 +38,7 @@ OBJECT_DECLARE_TYPE(AspeedI2CState, AspeedI2CClass, ASPEED_I2C) > #define ASPEED_I2C_SHARE_POOL_SIZE 0x800 > #define ASPEED_I2C_BUS_POOL_SIZE 0x20 > #define ASPEED_I2C_OLD_NUM_REG 11 > -#define ASPEED_I2C_NEW_NUM_REG 22 > +#define ASPEED_I2C_NEW_NUM_REG 28 > > #define A_I2CD_M_STOP_CMD BIT(5) > #define A_I2CD_M_RX_CMD BIT(3) > @@ -227,6 +227,15 @@ REG32(I2CS_DMA_LEN_STS, 0x4c) > FIELD(I2CS_DMA_LEN_STS, TX_LEN, 0, 13) > REG32(I2CC_DMA_ADDR, 0x50) > REG32(I2CC_DMA_LEN, 0x54) > +/* DMA 64bits */ > +REG32(I2CM_DMA_TX_ADDR_HI, 0x60) > + FIELD(I2CM_DMA_TX_ADDR_HI, ADDR_HI, 0, 7) > +REG32(I2CM_DMA_RX_ADDR_HI, 0x64) > + FIELD(I2CM_DMA_RX_ADDR_HI, ADDR_HI, 0, 7) > +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) > > struct AspeedI2CState; > > @@ -292,6 +301,7 @@ struct AspeedI2CClass { > bool has_dma; > bool has_share_pool; > uint64_t mem_size; > + bool has_dma64; > }; > > static inline bool aspeed_i2c_is_new_mode(AspeedI2CState *s)
Hi Cedric, > Subject: Re: [PATCH v2 06/11] hw/i2c/aspeed: support Tx/Rx buffer 64 bits > address > > Jamin, > > Please change commit title to > Thanks for review and suggestion. Will fix them in v3 patch. Jamin > hw/i2c/aspeed: Add support for Tx/Rx buffer 64 bits address > > So more proposals to improve the commit log below, > > On 8/8/24 04:49, Jamin Lin wrote: > > ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) And the > > base address of dram is "0x4 00000000" which > > and > > > is 64bits address. > > > > It have "Master DMA Mode Tx Buffer Base Address[39:32](0x60)" > > it has > > > and "Master DMA Mode Rx Buffer Base Address[39:32](0x64)" > > ... registers > > > to save the high part physical address of Tx/Rx buffer address for > > master mode. > > > > It have "Slave DMA Mode Tx Buffer Base Address[39:32](0x68)" and > > It has > > > "Slave DMA Mode Rx Buffer Base Address[39:32](0x6C)" > > registers > > > to save the high part physical address of Tx/Rx buffer address for > > slave mode. > > > > Ex: Tx buffer address for master mode [39:0] The "Master DMA Mode Tx > > Buffer Base Address[39:32](0x60)" > > bits [7:0] which corresponds the bits [39:32] of the 64 bits address > > of the Tx buffer address. > > The "Master DMA Mode Tx Buffer Base Address(0x30)" bits [31:0] which > > corresponds the bits [31:0] of the 64 bits address of the Tx buffer > > address. > > > > Introduce a new has_dma64 class attribute and new registers of > > ... new registers for the > > > new mode to support DMA 64 bits dram address. > > Update new mode register number to 28. > > > 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> > > Reviewed-by: Cédric Le Goater <clg@redhat.com> > > --- > > hw/i2c/aspeed_i2c.c | 48 > +++++++++++++++++++++++++++++++++++++ > > include/hw/i2c/aspeed_i2c.h | 12 +++++++++- > > 2 files changed, 59 insertions(+), 1 deletion(-) > > > > diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index > > 819a875839..c1ff80b1cf 100644 > > --- a/hw/i2c/aspeed_i2c.c > > +++ b/hw/i2c/aspeed_i2c.c > > @@ -140,6 +140,7 @@ static uint64_t > aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, > > static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr > offset, > > unsigned size) > > { > > + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); > > uint64_t value = bus->regs[offset / sizeof(*bus->regs)]; > > > > switch (offset) { > > @@ -170,6 +171,16 @@ static uint64_t > aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, > > case A_I2CM_CMD: > > value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, > i2c_bus_busy(bus->bus)); > > break; > > + case A_I2CM_DMA_TX_ADDR_HI: > > + case A_I2CM_DMA_RX_ADDR_HI: > > + case A_I2CS_DMA_TX_ADDR_HI: > > + case A_I2CS_DMA_RX_ADDR_HI: > > + if (!aic->has_dma64) { > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits > support\n", > > + __func__); > > + value = -1; > > + } > > + break; > > default: > > qemu_log_mask(LOG_GUEST_ERROR, > > "%s: Bad offset 0x%" HWADDR_PRIx "\n", > > __func__, offset); @@ -731,6 +742,42 @@ static void > aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset, > > qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not > implemented\n", > > __func__); > > break; > > + > > + case A_I2CM_DMA_TX_ADDR_HI: > > + if (!aic->has_dma64) { > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits > support\n", > > + __func__); > > + break; > > + } > > + bus->regs[R_I2CM_DMA_TX_ADDR_HI] = FIELD_EX32(value, > > + > I2CM_DMA_TX_ADDR_HI, > > + > ADDR_HI); > > + break; > > + case A_I2CM_DMA_RX_ADDR_HI: > > + if (!aic->has_dma64) { > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits > support\n", > > + __func__); > > + break; > > + } > > + bus->regs[R_I2CM_DMA_RX_ADDR_HI] = FIELD_EX32(value, > > + > I2CM_DMA_RX_ADDR_HI, > > + > ADDR_HI); > > + break; > > + case A_I2CS_DMA_TX_ADDR_HI: > > + qemu_log_mask(LOG_UNIMP, > > + "%s: Slave mode DMA TX Addr high is not > implemented\n", > > + __func__); > > + break; > > + case A_I2CS_DMA_RX_ADDR_HI: > > + if (!aic->has_dma64) { > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits > support\n", > > + __func__); > > + break; > > + } > > + bus->regs[R_I2CS_DMA_RX_ADDR_HI] = FIELD_EX32(value, > > + > I2CS_DMA_RX_ADDR_HI, > > + > ADDR_HI); > > + break; > > default: > > qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" > HWADDR_PRIx "\n", > > __func__, offset); @@ -1554,6 +1601,7 @@ > > static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data) > > aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; > > aic->has_dma = true; > > aic->mem_size = 0x2000; > > + aic->has_dma64 = true; > > } > > > > static const TypeInfo aspeed_2700_i2c_info = { diff --git > > a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index > > 4f23dc10c3..2c4c81bd20 100644 > > --- a/include/hw/i2c/aspeed_i2c.h > > +++ b/include/hw/i2c/aspeed_i2c.h > > @@ -38,7 +38,7 @@ OBJECT_DECLARE_TYPE(AspeedI2CState, > AspeedI2CClass, ASPEED_I2C) > > #define ASPEED_I2C_SHARE_POOL_SIZE 0x800 > > #define ASPEED_I2C_BUS_POOL_SIZE 0x20 > > #define ASPEED_I2C_OLD_NUM_REG 11 > > -#define ASPEED_I2C_NEW_NUM_REG 22 > > +#define ASPEED_I2C_NEW_NUM_REG 28 > > > > #define A_I2CD_M_STOP_CMD BIT(5) > > #define A_I2CD_M_RX_CMD BIT(3) > > @@ -227,6 +227,15 @@ REG32(I2CS_DMA_LEN_STS, 0x4c) > > FIELD(I2CS_DMA_LEN_STS, TX_LEN, 0, 13) > > REG32(I2CC_DMA_ADDR, 0x50) > > REG32(I2CC_DMA_LEN, 0x54) > > +/* DMA 64bits */ > > +REG32(I2CM_DMA_TX_ADDR_HI, 0x60) > > + FIELD(I2CM_DMA_TX_ADDR_HI, ADDR_HI, 0, 7) > > +REG32(I2CM_DMA_RX_ADDR_HI, 0x64) > > + FIELD(I2CM_DMA_RX_ADDR_HI, ADDR_HI, 0, 7) > > +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) > > > > struct AspeedI2CState; > > > > @@ -292,6 +301,7 @@ struct AspeedI2CClass { > > bool has_dma; > > bool has_share_pool; > > uint64_t mem_size; > > + bool has_dma64; > > }; > > > > static inline bool aspeed_i2c_is_new_mode(AspeedI2CState *s)
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index 819a875839..c1ff80b1cf 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -140,6 +140,7 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, hwaddr offset, static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, unsigned size) { + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); uint64_t value = bus->regs[offset / sizeof(*bus->regs)]; switch (offset) { @@ -170,6 +171,16 @@ static uint64_t aspeed_i2c_bus_new_read(AspeedI2CBus *bus, hwaddr offset, case A_I2CM_CMD: value = SHARED_FIELD_DP32(value, BUS_BUSY_STS, i2c_bus_busy(bus->bus)); break; + case A_I2CM_DMA_TX_ADDR_HI: + case A_I2CM_DMA_RX_ADDR_HI: + case A_I2CS_DMA_TX_ADDR_HI: + case A_I2CS_DMA_RX_ADDR_HI: + if (!aic->has_dma64) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", + __func__); + value = -1; + } + break; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -731,6 +742,42 @@ static void aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset, qemu_log_mask(LOG_UNIMP, "%s: Slave mode DMA TX is not implemented\n", __func__); break; + + case A_I2CM_DMA_TX_ADDR_HI: + if (!aic->has_dma64) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", + __func__); + break; + } + bus->regs[R_I2CM_DMA_TX_ADDR_HI] = FIELD_EX32(value, + I2CM_DMA_TX_ADDR_HI, + ADDR_HI); + break; + case A_I2CM_DMA_RX_ADDR_HI: + if (!aic->has_dma64) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", + __func__); + break; + } + bus->regs[R_I2CM_DMA_RX_ADDR_HI] = FIELD_EX32(value, + I2CM_DMA_RX_ADDR_HI, + ADDR_HI); + break; + case A_I2CS_DMA_TX_ADDR_HI: + qemu_log_mask(LOG_UNIMP, + "%s: Slave mode DMA TX Addr high is not implemented\n", + __func__); + break; + case A_I2CS_DMA_RX_ADDR_HI: + if (!aic->has_dma64) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64 bits support\n", + __func__); + break; + } + bus->regs[R_I2CS_DMA_RX_ADDR_HI] = FIELD_EX32(value, + I2CS_DMA_RX_ADDR_HI, + ADDR_HI); + break; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -1554,6 +1601,7 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data) aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; aic->has_dma = true; aic->mem_size = 0x2000; + aic->has_dma64 = true; } static const TypeInfo aspeed_2700_i2c_info = { diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index 4f23dc10c3..2c4c81bd20 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -38,7 +38,7 @@ OBJECT_DECLARE_TYPE(AspeedI2CState, AspeedI2CClass, ASPEED_I2C) #define ASPEED_I2C_SHARE_POOL_SIZE 0x800 #define ASPEED_I2C_BUS_POOL_SIZE 0x20 #define ASPEED_I2C_OLD_NUM_REG 11 -#define ASPEED_I2C_NEW_NUM_REG 22 +#define ASPEED_I2C_NEW_NUM_REG 28 #define A_I2CD_M_STOP_CMD BIT(5) #define A_I2CD_M_RX_CMD BIT(3) @@ -227,6 +227,15 @@ REG32(I2CS_DMA_LEN_STS, 0x4c) FIELD(I2CS_DMA_LEN_STS, TX_LEN, 0, 13) REG32(I2CC_DMA_ADDR, 0x50) REG32(I2CC_DMA_LEN, 0x54) +/* DMA 64bits */ +REG32(I2CM_DMA_TX_ADDR_HI, 0x60) + FIELD(I2CM_DMA_TX_ADDR_HI, ADDR_HI, 0, 7) +REG32(I2CM_DMA_RX_ADDR_HI, 0x64) + FIELD(I2CM_DMA_RX_ADDR_HI, ADDR_HI, 0, 7) +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) struct AspeedI2CState; @@ -292,6 +301,7 @@ struct AspeedI2CClass { bool has_dma; bool has_share_pool; uint64_t mem_size; + bool has_dma64; }; static inline bool aspeed_i2c_is_new_mode(AspeedI2CState *s)