Hi Cedric,
> Subject: Re: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for
> DMA 64 bits
>
> On 9/3/24 05:06, Jamin Lin wrote:
> > Hi Cedric,
> >
> >> Subject: Re: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram
> >> offset for DMA 64 bits
> >>
> >> Jamin,
> >>
> >> Please adjust commit title
> >
> > What do you think if I change the commit title as following.
> >
> > hw/i2c/aspeed: Add support for dma_dram_offset attribute bits 33 and 32.
>
> How about ?
>
> "hw/i2c/aspeed: Add support for 64 bit addresses"
>
Got it. Will add.
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
> >
> > Thanks-Jamin
> >>
> >> 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 is 64bits address.
> >>>
> >>> The AST2700 support the maximum DRAM size is 8 GB.
> >>> The DRAM physical address range is from "0x4_0000_0000" to
> >>> "0x5_FFFF_FFFF".
> >>>
> >>> The DRAM offset range is from "0x0_0000_0000" to "0x1_FFFF_FFFF" and
> >>> it is enough to use bits [33:0] saving the dram offset.
> >>>
> >>> Therefore, save the high part physical address bit[1:0] of Tx/Rx
> >>> buffer address as dma_dram_offset bit[33:32].
> >>> It does not need to decrease the dram physical high part address for
> >>> DMA operation.
> >>> (high part physical address bit[7:0] – 4)
> >>>
> >>> Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
> >>> Reviewed-by: Cédric Le Goater <c...@redhat.com>
> >>> ---
> >>> hw/i2c/aspeed_i2c.c | 14 ++++++++++++++
> >>> 1 file changed, 14 insertions(+)
> >>>
> >>> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index
> >>> c1ff80b1cf..44c3c39233 100644
> >>> --- a/hw/i2c/aspeed_i2c.c
> >>> +++ b/hw/i2c/aspeed_i2c.c
> >>> @@ -743,6 +743,14 @@ static void
> >> aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> >>> __func__);
> >>> break;
> >>>
> >>> + /*
> >>> + * The AST2700 support the maximum DRAM size is 8 GB.
> >>> + * The DRAM offset range is from 0x0_0000_0000 to
> >>> + * 0x1_FFFF_FFFF and it is enough to use bits [33:0]
> >>> + * saving the dram offset.
> >>> + * Therefore, save the high part physical address bit[1:0]
> >>> + * of Tx/Rx buffer address as dma_dram_offset bit[33:32].
> >>> + */
> >>> case A_I2CM_DMA_TX_ADDR_HI:
> >>> if (!aic->has_dma64) {
> >>> qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA 64
> bits
> >>> support\n", @@ -752,6 +760,8 @@ static void
> >> aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> >>> bus->regs[R_I2CM_DMA_TX_ADDR_HI] = FIELD_EX32(value,
> >>>
> >> I2CM_DMA_TX_ADDR_HI,
> >>>
> >> ADDR_HI);
> >>> + bus->dma_dram_offset = deposit64(bus->dma_dram_offset,
> 32,
> >> 32,
> >>> + extract32(value, 0, 2));
> >>> break;
> >>> case A_I2CM_DMA_RX_ADDR_HI:
> >>> if (!aic->has_dma64) {
> >>> @@ -762,6 +772,8 @@ static void
> >> aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> >>> bus->regs[R_I2CM_DMA_RX_ADDR_HI] =
> FIELD_EX32(value,
> >>>
> >> I2CM_DMA_RX_ADDR_HI,
> >>>
> >> ADDR_HI);
> >>> + bus->dma_dram_offset = deposit64(bus->dma_dram_offset,
> 32,
> >> 32,
> >>> + extract32(value, 0, 2));
> >>> break;
> >>> case A_I2CS_DMA_TX_ADDR_HI:
> >>> qemu_log_mask(LOG_UNIMP,
> >>> @@ -777,6 +789,8 @@ static void
> >> aspeed_i2c_bus_new_write(AspeedI2CBus *bus, hwaddr offset,
> >>> bus->regs[R_I2CS_DMA_RX_ADDR_HI] = FIELD_EX32(value,
> >>>
> >> I2CS_DMA_RX_ADDR_HI,
> >>>
> >> ADDR_HI);
> >>> + bus->dma_dram_offset = deposit64(bus->dma_dram_offset,
> 32,
> >> 32,
> >>> + extract32(value, 0, 2));
> >>> break;
> >>> default:
> >>> qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%"
> >>> HWADDR_PRIx "\n",
> >
