Re: [PATCH v4 05/16] aspeed/sdmc: Add AST2700 support

["Cédric Le Goater" <clg@kaod.org>]

On 5/28/24 03:26, Jamin Lin wrote:
> Hi Philippe, Cedric
> 
>> On 27/5/24 13:18, Cédric Le Goater wrote:
>>> On 5/27/24 12:24, Philippe Mathieu-Daudé wrote:
>>>> Hi Jamin,
>>>>
>>>> On 27/5/24 10:02, Jamin Lin wrote:
>>>>> The SDRAM memory controller(DRAMC) controls the access to external
>>>>> DDR4 and DDR5 SDRAM and power up to DDR4 and DDR5 PHY.
>>>>>
>>>>> The DRAM memory controller of AST2700 is not backward compatible to
>>>>> previous chips such AST2600, AST2500 and AST2400.
>>>>>
>>>>> Max memory is now 8GiB on the AST2700. Introduce new
>>>>> aspeed_2700_sdmc and class with read/write operation and reset
>>>>> handlers.
>>>>>
>>>>> Define DRAMC necessary protected registers and unprotected registers
>>>>> for AST2700 and increase the register set to 0x1000.
>>>>>
>>>>> Add unlocked property to change controller protected status.
>>>>>
>>>>> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
>>>>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
>>>>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>>>>> ---
>>>>>    hw/misc/aspeed_sdmc.c         | 190
>>>>> +++++++++++++++++++++++++++++++++-
>>>>>    include/hw/misc/aspeed_sdmc.h |   5 +-
>>>>>    2 files changed, 193 insertions(+), 2 deletions(-)
>>>>
>>>>
>>>>> diff --git a/include/hw/misc/aspeed_sdmc.h
>>>>> b/include/hw/misc/aspeed_sdmc.h index ec2d59a14f..61c979583a 100644
>>>>> --- a/include/hw/misc/aspeed_sdmc.h
>>>>> +++ b/include/hw/misc/aspeed_sdmc.h
>>>>> @@ -17,6 +17,7 @@ OBJECT_DECLARE_TYPE(AspeedSDMCState,
>>>>> AspeedSDMCClass, ASPEED_SDMC)
>>>>>    #define TYPE_ASPEED_2400_SDMC TYPE_ASPEED_SDMC "-ast2400"
>>>>>    #define TYPE_ASPEED_2500_SDMC TYPE_ASPEED_SDMC "-ast2500"
>>>>>    #define TYPE_ASPEED_2600_SDMC TYPE_ASPEED_SDMC "-ast2600"
>>>>> +#define TYPE_ASPEED_2700_SDMC TYPE_ASPEED_SDMC "-ast2700"
>>>>>    /*
>>>>>     * SDMC has 174 documented registers. In addition the u-boot
>>>>> device tree @@ -29,7 +30,7 @@
>> OBJECT_DECLARE_TYPE(AspeedSDMCState,
>>>>> AspeedSDMCClass, ASPEED_SDMC)
>>>>>     * time, and the other is in the DDR-PHY IP which is used during
>>>>> DDR-PHY
>>>>>     * training.
>>>>>     */
>>>>> -#define ASPEED_SDMC_NR_REGS (0x500 >> 2)
>>>>> +#define ASPEED_SDMC_NR_REGS (0x1000 >> 2)
>>>>
>>>> This change breaks the migration stream.
>>>
>>> Do you mean migration compat ? We never cared much about that for the
>>> Aspeed machines.
>>
>> So let's just remove the VMSTATE to reduce code burden?
>>
>> Otherwise incrementing the vmstate.version is enough.
>>
>> Regards,
>>
>> Phil.
> If you both okay, I will remove it.
> Do I need to create a new patch or just update in this patch?

I don't think this is necessary to do so now. Possibly, increase the
version number in the vmstate when resending a v5.

Also, all Aspeed models should be addressed and that's beyond the scope
of this series.


Thanks,

C.