Re: [Qemu-devel] [PATCH 6/6] arm: add support for an ast2500 evaluation board

[Andrew Jeffery]

On Wed, 2016-07-27 at 18:46 +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  hw/arm/palmetto-bmc.c    | 32 +++++++++++++++++++++++++++++++-
>  include/hw/arm/ast2400.h |  5 +++++
>  2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
> index cd8aa59756b9..8d8bfeb571e2 100644
> --- a/hw/arm/palmetto-bmc.c
> +++ b/hw/arm/palmetto-bmc.c
> @@ -37,12 +37,15 @@ typedef struct AspeedBoardConfig {
>  } AspeedBoardConfig;
>  
>  enum {
> -    PALMETTO_BMC
> +    PALMETTO_BMC,
> +    AST2500_EDK

It was called 'ast2500-edk' in the out-of-tree patches, but can we
rename it 'ast2500-evb'? This would make it consistent with patches we
have in our Linux trees.

>  };
>  
>  static const AspeedBoardConfig aspeed_boards[] = {
>      [ PALMETTO_BMC ] = { 0x120CE416, AST2400_A0_SILICON_REV,
>                           AST2400_SDRAM_BASE },
> +    [ AST2500_EDK ]  = { 0x00000200, AST2500_A1_SILICON_REV,
> +                         AST2500_SDRAM_BASE },

Can we include the strap value from the board for completeness?

Also, the meaning of the bits have changed from the AST2400 - they
probably should be documented somewhere?

Finally, checkpatch complained here too regarding the whitespace, I ran
into the issue replacing the strap value.

>  };
>  
>  static void aspeed_init_flashes(AspeedSMCState *s, const char *flashtype,
> @@ -133,9 +136,36 @@ static const TypeInfo palmetto_bmc_type = {
>      .class_init = palmetto_bmc_class_init,
>  };
>  
> +static void ast2500_edk_init(MachineState *machine)
> +{
> +    machine->cpu_model = "arm1176";
> +    aspeed_init(machine, AST2500_EDK);
> +}
> +
> +static void ast2500_edk_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "Aspeed AST2500 EDK (ARM1176)";
> +    mc->init = ast2500_edk_init;
> +    mc->max_cpus = 1;
> +    mc->no_sdcard = 1;
> +    mc->no_floppy = 1;
> +    mc->no_cdrom = 1;
> +    mc->no_sdcard = 1;

mc->no_sdcard is already assigned a couple of lines up. I think this
may be the case for palmetto config as well...

Cheers,

Andrew

> +    mc->no_parallel = 1;
> +}
> +
> +static const TypeInfo ast2500_edk_type = {
> +    .name = MACHINE_TYPE_NAME("ast2500-edk"),
> +    .parent = TYPE_MACHINE,
> +    .class_init = ast2500_edk_class_init,
> +};
> +
>  static void aspeed_machine_init(void)
>  {
>      type_register_static(&palmetto_bmc_type);
> +    type_register_static(&ast2500_edk_type);
>  }
>  
>  type_init(aspeed_machine_init)
> diff --git a/include/hw/arm/ast2400.h b/include/hw/arm/ast2400.h
> index e68807d475b7..2e6864f88790 100644
> --- a/include/hw/arm/ast2400.h
> +++ b/include/hw/arm/ast2400.h
> @@ -41,4 +41,9 @@ typedef struct AST2400State {
>  
>  #define AST2400_SDRAM_BASE       0x40000000
>  
> +/*
> + * for Aspeed AST2500 SOC and higher
> + */
> +#define AST2500_SDRAM_BASE       0x80000000
> +
>  #endif /* AST2400_H */

signature.asc
Description: This is a digitally signed message part