Re: [Qemu-devel] [PATCH 2/6] palmetto-bmc: replace palmetto_bmc with aspeed

[Cédric Le Goater]

On 07/28/2016 06:48 AM, Andrew Jeffery wrote:
> On Wed, 2016-07-27 at 18:46 +0200, Cédric Le Goater wrote:
>> This is mostly a name replacement to prepare ground for other socs
>> specificities. It also adds a specific TypeInfo struct for the
>> palmetto_bmc board with a custom initialization for the same reason.
> 
> I think we should rename the file, it feels a bit confusing having the
> ast2500 machine glue (added later in the series) in palmetto-bmc.c. You
> mentioned in the cover letter that moving it would break history but it
> isn't necessarily so, you can follow renames in the logs with `git log
> --follow`. It's a git switch that feels like it should be a default but
> isn't :/

Ah. nice option indeed :) I was not aware of it.

> Maybe create a commit that renames the file, then add these changes
> after?

yes. rename then add changes, I will do that in v2.

Thanks,

C. 

> Andrew
> 
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>
>>  Should we change the name of the file to aspeed.c ? I am not found of
>>  such renames as it is then difficult to track code changes.
>>
>>  hw/arm/palmetto-bmc.c | 54 
>> ++++++++++++++++++++++++++++++++++-----------------
>>  1 file changed, 36 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/arm/palmetto-bmc.c b/hw/arm/palmetto-bmc.c
>> index 1ee13d578899..f80a15733864 100644
>> --- a/hw/arm/palmetto-bmc.c
>> +++ b/hw/arm/palmetto-bmc.c
>> @@ -21,19 +21,19 @@
>>  #include "sysemu/block-backend.h"
>>  #include "sysemu/blockdev.h"
>>  
>> -static struct arm_boot_info palmetto_bmc_binfo = {
>> +static struct arm_boot_info aspeed_binfo = {
>>      .loader_start = AST2400_SDRAM_BASE,
>>      .board_id = 0,
>>      .nb_cpus = 1,
>>  };
>>  
>> -typedef struct PalmettoBMCState {
>> +typedef struct AspeedBoardState {
>>      AST2400State soc;
>>      MemoryRegion ram;
>> -} PalmettoBMCState;
>> +} AspeedBoardState;
>>  
>> -static void palmetto_bmc_init_flashes(AspeedSMCState *s, const char 
>> *flashtype,
>> -                                      Error **errp)
>> +static void aspeed_init_flashes(AspeedSMCState *s, const char *flashtype,
>> +                                Error **errp)
>>  {
>>      int i ;
>>  
>> @@ -58,11 +58,11 @@ static void palmetto_bmc_init_flashes(AspeedSMCState *s, 
>> const char *flashtype,
>>      }
>>  }
>>  
>> -static void palmetto_bmc_init(MachineState *machine)
>> +static void aspeed_init(MachineState *machine)
>>  {
>> -    PalmettoBMCState *bmc;
>> +    AspeedBoardState *bmc;
>>  
>> -    bmc = g_new0(PalmettoBMCState, 1);
>> +    bmc = g_new0(AspeedBoardState, 1);
>>      object_initialize(&bmc->soc, (sizeof(bmc->soc)), TYPE_AST2400);
>>      object_property_add_child(OBJECT(machine), "soc", OBJECT(&bmc->soc),
>>                                &error_abort);
>> @@ -79,19 +79,26 @@ static void palmetto_bmc_init(MachineState *machine)
>>      object_property_set_bool(OBJECT(&bmc->soc), true, "realized",
>>                               &error_abort);
>>  
>> -    palmetto_bmc_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
>> -    palmetto_bmc_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
>> +    aspeed_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort);
>> +    aspeed_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort);
>> +
>> +    aspeed_binfo.kernel_filename = machine->kernel_filename;
>> +    aspeed_binfo.initrd_filename = machine->initrd_filename;
>> +    aspeed_binfo.kernel_cmdline = machine->kernel_cmdline;
>> +    aspeed_binfo.ram_size = ram_size;
>> +    arm_load_kernel(ARM_CPU(first_cpu), &aspeed_binfo);
>> +}
>>  
>> -    palmetto_bmc_binfo.kernel_filename = machine->kernel_filename;
>> -    palmetto_bmc_binfo.initrd_filename = machine->initrd_filename;
>> -    palmetto_bmc_binfo.kernel_cmdline = machine->kernel_cmdline;
>> -    palmetto_bmc_binfo.ram_size = ram_size;
>> -    arm_load_kernel(ARM_CPU(first_cpu), &palmetto_bmc_binfo);
>> +static void palmetto_bmc_init(MachineState *machine)
>> +{
>> +    aspeed_init(machine);
>>  }
>>  
>> -static void palmetto_bmc_machine_init(MachineClass *mc)
>> +static void palmetto_bmc_class_init(ObjectClass *oc, void *data)
>>  {
>> -    mc->desc = "OpenPOWER Palmetto BMC";
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    mc->desc = "OpenPOWER Palmetto BMC (ARM926EJ-S)";
>>      mc->init = palmetto_bmc_init;
>>      mc->max_cpus = 1;
>>      mc->no_sdcard = 1;
>> @@ -101,4 +108,15 @@ static void palmetto_bmc_machine_init(MachineClass *mc)
>>      mc->no_parallel = 1;
>>  }
>>  
>> -DEFINE_MACHINE("palmetto-bmc", palmetto_bmc_machine_init);
>> +static const TypeInfo palmetto_bmc_type = {
>> +    .name = MACHINE_TYPE_NAME("palmetto-bmc"),
>> +    .parent = TYPE_MACHINE,
>> +    .class_init = palmetto_bmc_class_init,
>> +};
>> +
>> +static void aspeed_machine_init(void)
>> +{
>> +    type_register_static(&palmetto_bmc_type);
>> +}
>> +
>> +type_init(aspeed_machine_init)