Re: [PATCH v2] hw/arm/aspeed: Add machine properties to define the flash models

[Lei Yu]

Tested-by: Lei YU <yulei.sh@bytedance.com>

Tested on ast2500-evb with 64MiB flash on fmc and it works as expected.

On Tue, Sep 15, 2020 at 1:49 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> Some machines don't have much differences a part from the flash model
> being used. Introduce new machine properties to change them from the
> command line.
>
> For instance, to start the ast2500-evb machine with a different FMC
> chip and a 64M SPI chip, use :
>
>   -M ast2500-evb,fmc-model=mx25l25635e,spi-model=mx66u51235f
>
> Cc: 郁雷 <yulei.sh@bytedance.com>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Message-Id: <20200910200715.1920318-1-clg@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  docs/system/arm/aspeed.rst | 18 +++++++++++++++
>  hw/arm/aspeed.c            | 45 ++++++++++++++++++++++++++++++++++++--
>  2 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 45f891eb3cad..2d33d023453e 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -83,3 +83,21 @@ The image should be attached as an MTD drive. Run :
>
>    $ qemu-system-arm -M romulus-bmc -nic user \
>         -drive file=flash-romulus,format=raw,if=mtd -nographic
> +
> +Options specific to Aspeed machines are :
> +
> + * ``execute-in-place`` which emulates the boot from the CE0 flash
> +   device by using the FMC controller to load the instructions, and
> +   not simply from RAM. This takes a little longer.
> +
> + * ``fmc-model`` to change the FMC Flash model. FW needs support for
> +   the chip model to boot.
> +
> + * ``spi-model`` to change the SPI Flash model.
> +
> +For instance, to start the ``ast2500-evb`` machine with a different
> +FMC chip and a bigger (64M) SPI chip, use :
> +
> +.. code-block:: bash
> +
> +  -M ast2500-evb,fmc-model=mx25l25635e,spi-model=mx66u51235f
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 8bfb1c79ddc5..bdb981d2f878 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -41,6 +41,8 @@ struct AspeedMachineState {
>      MemoryRegion ram_container;
>      MemoryRegion max_ram;
>      bool mmio_exec;
> +    char *fmc_model;
> +    char *spi_model;
>  };
>
>  /* Palmetto hardware value: 0x120CE416 */
> @@ -332,8 +334,10 @@ static void aspeed_machine_init(MachineState *machine)
>                            "max_ram", max_ram_size  - ram_size);
>      memory_region_add_subregion(&bmc->ram_container, ram_size, 
> &bmc->max_ram);
>
> -    aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model);
> -    aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model);
> +    aspeed_board_init_flashes(&bmc->soc.fmc, bmc->fmc_model ?
> +                              bmc->fmc_model : amc->fmc_model);
> +    aspeed_board_init_flashes(&bmc->soc.spi[0], bmc->spi_model ?
> +                              bmc->spi_model : amc->spi_model);
>
>      /* Install first FMC flash content as a boot rom. */
>      if (drive0) {
> @@ -570,6 +574,34 @@ static void aspeed_machine_instance_init(Object *obj)
>      ASPEED_MACHINE(obj)->mmio_exec = false;
>  }
>
> +static char *aspeed_get_fmc_model(Object *obj, Error **errp)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
> +    return g_strdup(bmc->fmc_model);
> +}
> +
> +static void aspeed_set_fmc_model(Object *obj, const char *value, Error 
> **errp)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
> +
> +    g_free(bmc->fmc_model);
> +    bmc->fmc_model = g_strdup(value);
> +}
> +
> +static char *aspeed_get_spi_model(Object *obj, Error **errp)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
> +    return g_strdup(bmc->spi_model);
> +}
> +
> +static void aspeed_set_spi_model(Object *obj, const char *value, Error 
> **errp)
> +{
> +    AspeedMachineState *bmc = ASPEED_MACHINE(obj);
> +
> +    g_free(bmc->spi_model);
> +    bmc->spi_model = g_strdup(value);
> +}
> +
>  static void aspeed_machine_class_props_init(ObjectClass *oc)
>  {
>      object_class_property_add_bool(oc, "execute-in-place",
> @@ -577,6 +609,15 @@ static void aspeed_machine_class_props_init(ObjectClass 
> *oc)
>                                     aspeed_set_mmio_exec);
>      object_class_property_set_description(oc, "execute-in-place",
>                             "boot directly from CE0 flash device");
> +
> +    object_class_property_add_str(oc, "fmc-model", aspeed_get_fmc_model,
> +                                   aspeed_set_fmc_model);
> +    object_class_property_set_description(oc, "fmc-model",
> +                                          "Change the FMC Flash model");
> +    object_class_property_add_str(oc, "spi-model", aspeed_get_spi_model,
> +                                   aspeed_set_spi_model);
> +    object_class_property_set_description(oc, "spi-model",
> +                                          "Change the SPI Flash model");
>  }
>
>  static int aspeed_soc_num_cpus(const char *soc_name)
> --
> 2.25.4
>


-- 
BRs,
Lei YU