Re: [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState

[Cédric Le Goater]

On 6/23/20 9:21 AM, Philippe Mathieu-Daudé wrote:
> AspeedMachineState seems crippled. We use incorrectly 2
> different structures to do the same thing. Merge them
> altogether:
> - Move AspeedMachine fields to AspeedMachineState
> - AspeedMachineState is now QOM
> - Remove unused AspeedMachine structure
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  include/hw/arm/aspeed.h |  8 +-------
>  hw/arm/aspeed.c         | 11 +++++++----
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 5114ba0bd4..09da9d9acc 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -15,13 +15,7 @@ typedef struct AspeedMachineState AspeedMachineState;
>  
>  #define TYPE_ASPEED_MACHINE       MACHINE_TYPE_NAME("aspeed")
>  #define ASPEED_MACHINE(obj) \
> -    OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE)
> -
> -typedef struct AspeedMachine {
> -    MachineState parent_obj;
> -
> -    bool mmio_exec;
> -} AspeedMachine;
> +    OBJECT_CHECK(AspeedMachineState, (obj), TYPE_ASPEED_MACHINE)
>  
>  #define ASPEED_MAC0_ON   (1 << 0)
>  #define ASPEED_MAC1_ON   (1 << 1)
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 680345beca..ccf127b328 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -33,9 +33,14 @@ static struct arm_boot_info aspeed_board_binfo = {
>  };
>  
>  struct AspeedMachineState {
> +    /* Private */
> +    MachineState parent_obj;
> +    /* Public */
> +
>      AspeedSoCState soc;
>      MemoryRegion ram_container;
>      MemoryRegion max_ram;
> +    bool mmio_exec;
>  };
>  
>  /* Palmetto hardware value: 0x120CE416 */
> @@ -253,7 +258,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, 
> DriveInfo *dinfo)
>  
>  static void aspeed_machine_init(MachineState *machine)
>  {
> -    AspeedMachineState *bmc;
> +    AspeedMachineState *bmc = ASPEED_MACHINE(machine);
>      AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
>      AspeedSoCClass *sc;
>      DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> @@ -261,8 +266,6 @@ static void aspeed_machine_init(MachineState *machine)
>      int i;
>      NICInfo *nd = &nd_table[0];
>  
> -    bmc = g_new0(AspeedMachineState, 1);
> -
>      memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
>                         4 * GiB);
>      memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
> @@ -751,7 +754,7 @@ static const TypeInfo aspeed_machine_types[] = {
>      }, {
>          .name          = TYPE_ASPEED_MACHINE,
>          .parent        = TYPE_MACHINE,
> -        .instance_size = sizeof(AspeedMachine),
> +        .instance_size = sizeof(AspeedMachineState),
>          .instance_init = aspeed_machine_instance_init,
>          .class_size    = sizeof(AspeedMachineClass),
>          .class_init    = aspeed_machine_class_init,
>