Re: [PATCH 3/5] aspeed: Add a DRAM memory region at the SoC level

[Philippe Mathieu-Daudé]

On 10/16/19 10:50 AM, Cédric Le Goater wrote:
> Currently, we link the DRAM memory region to the FMC model (for DMAs)
> through a property alias at the SoC level. The I2C model will need a
> similar region for DMA support, add a DRAM region property at the SoC
> level for both model to use.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>   include/hw/arm/aspeed_soc.h | 1 +
>   hw/arm/aspeed_ast2600.c     | 7 +++++--
>   hw/arm/aspeed_soc.c         | 9 +++++++--
>   3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index cccb684a19bb..3375ef91607f 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -40,6 +40,7 @@ typedef struct AspeedSoCState {
>       ARMCPU cpu[ASPEED_CPUS_NUM];
>       uint32_t num_cpus;
>       A15MPPrivState     a7mpcore;
> +    MemoryRegion *dram_mr;
>       MemoryRegion sram;
>       AspeedVICState vic;
>       AspeedRtcState rtc;
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 931887ac681f..a403c2aae067 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -158,8 +158,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
>                             typename);
>       object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
>                                 &error_abort);
> -    object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram",
> -                              &error_abort);
>   
>       for (i = 0; i < sc->spis_num; i++) {
>           snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, 
> socname);
> @@ -362,6 +360,11 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, 
> Error **errp)
>       }
>   
>       /* FMC, The number of CS is set at the board level */
> +    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", 
> &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>       object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
>                               "sdram-base", &err);
>       if (err) {
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index f4fe243458fd..dd1ee0e3336d 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -175,8 +175,6 @@ static void aspeed_soc_init(Object *obj)
>                             typename);
>       object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
>                                 &error_abort);
> -    object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram",
> -                              &error_abort);
>   
>       for (i = 0; i < sc->spis_num; i++) {
>           snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, 
> socname);
> @@ -323,6 +321,11 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
> **errp)
>                          aspeed_soc_get_irq(s, ASPEED_I2C));
>   
>       /* FMC, The number of CS is set at the board level */
> +    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", 
> &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>       object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
>                               "sdram-base", &err);
>       if (err) {
> @@ -429,6 +432,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
> **errp)
>   }
>   static Property aspeed_soc_properties[] = {
>       DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
> +    DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
> +                     MemoryRegion *),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>