Re: [PATCH v1 3/8] aspeed/sdmc: Add AST2700 support

[Philippe Mathieu-Daudé]

Hi Jamin,

On 29/2/24 08:23, Jamin Lin via 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.
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/misc/aspeed_sdmc.c         | 215 ++++++++++++++++++++++++++++++----
>   include/hw/misc/aspeed_sdmc.h |   4 +-
>   2 files changed, 198 insertions(+), 21 deletions(-)


> @@ -231,7 +270,10 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error 
> **errp)
>       AspeedSDMCState *s = ASPEED_SDMC(dev);
>       AspeedSDMCClass *asc = ASPEED_SDMC_GET_CLASS(s);
>   
> -    assert(asc->max_ram_size < 4 * GiB); /* 32-bit address bus */
> +    if (!asc->is_aarch64) {

Maybe name it 'bus64bit'? Because this isn't really related
to Aarch64.

> +        assert(asc->max_ram_size < 4 * GiB); /* 32-bit address bus */
> +    }


> +static void aspeed_2700_sdmc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    AspeedSDMCClass *asc = ASPEED_SDMC_CLASS(klass);
> +
> +    dc->desc = "ASPEED 2700 SDRAM Memory Controller";
> +    dc->reset = aspeed_2700_sdmc_reset;
> +
> +    asc->is_aarch64 = true;
> +    asc->max_ram_size = 8 * GiB;
> +    asc->compute_conf = aspeed_2700_sdmc_compute_conf;
> +    asc->write = aspeed_2700_sdmc_write;
> +    asc->valid_ram_sizes = aspeed_2700_ram_sizes;
> +}


> @@ -51,6 +52,7 @@ struct AspeedSDMCClass {
>       const uint64_t *valid_ram_sizes;
>       uint32_t (*compute_conf)(AspeedSDMCState *s, uint32_t data);
>       void (*write)(AspeedSDMCState *s, uint32_t reg, uint32_t data);
> +    uint32_t is_aarch64;

bool.

>   };
>   
>   #endif /* ASPEED_SDMC_H */