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

[Cédric Le Goater]

On 07/28/2016 07:11 AM, Andrew Jeffery wrote:
> 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.

yes. I feel the same also.

> 
>>  };
>>  
>>  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?

So you want me send to an updated version of :

        http://lists.nongnu.org/archive/html/qemu-arm/2016-06/msg00698.html

as a prereq ? 

Now that we have done the cleanups in U-Boot, we can pull from :

https://github.com/openbmc/u-boot/blob/v2016.07-aspeed-openbmc/arch/arm/include/asm/arch-aspeed/regs-scu.h

to get the definitions. I will add that.
 
> Finally, checkpatch complained here too regarding the whitespace, I ran
> into the issue replacing the strap value.

ok.

>>  };
>>  
>>  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...

That was a blind copy paste. I will remove the extra sdcard.

Thanks,

C. 

> 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 */