[PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits

Jamin Lin via posted 11 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits
Posted by Jamin Lin via 3 months, 2 weeks ago
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_lin@aspeedtech.com>
Reviewed-by: Cédric Le Goater <clg@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",
-- 
2.34.1


Re: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits
Posted by Cédric Le Goater 2 months, 3 weeks ago
Jamin,

Please adjust commit title

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_lin@aspeedtech.com>
> Reviewed-by: Cédric Le Goater <clg@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",


RE: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits
Posted by Jamin Lin 2 months, 3 weeks ago
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.

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_lin@aspeedtech.com>
> > Reviewed-by: Cédric Le Goater <clg@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",

Re: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits
Posted by Cédric Le Goater 2 months, 3 weeks ago
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"
  

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_lin@aspeedtech.com>
>>> Reviewed-by: Cédric Le Goater <clg@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",
> 


RE: [PATCH v2 07/11] hw/i2c/aspeed: support high part dram offset for DMA 64 bits
Posted by Jamin Lin 2 months, 3 weeks ago
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_lin@aspeedtech.com>
> >>> Reviewed-by: Cédric Le Goater <clg@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",
> >