Re: [Qemu-arm] [Qemu-devel] [PATCH 05/23] hw/arm/iotkit: Refactor into abstract base class and subclass

[Philippe Mathieu-Daudé]

On 1/21/19 7:51 PM, Peter Maydell wrote:
> The Arm SSE-200 Subsystem for Embedded is a revised and
> extended version of the older IoTKit SoC. Prepare for
> adding a model of it by refactoring the IoTKit code into
> an abstract base class which contains the functionality,
> driven by a class data block specific to each subclass.
> (This is the same approach used by the existing bcm283x
> SoC family implementation.)
> 
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  include/hw/arm/iotkit.h | 22 +++++++++++++++++-----
>  hw/arm/iotkit.c         | 34 +++++++++++++++++++++++++++++-----
>  2 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/include/hw/arm/iotkit.h b/include/hw/arm/iotkit.h
> index 9701738ec75..521d1f73757 100644
> --- a/include/hw/arm/iotkit.h
> +++ b/include/hw/arm/iotkit.h
> @@ -74,15 +74,15 @@
>  #include "hw/or-irq.h"
>  #include "hw/core/split-irq.h"
>  
> -#define TYPE_ARMSSE "iotkit"
> +#define TYPE_ARMSSE "arm-sse"
>  #define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
>  
>  /*
> - * For the moment TYPE_IOTKIT is a synonym for TYPE_ARMSSE (and the
> - * latter's underlying name is left as "iotkit"); in a later
> - * commit it will become a subclass of TYPE_ARMSSE.
> + * These type names are for specific IoTKit subsystems; other than
> + * instantiating them, code using these devices should always handle
> + * them via the ARMSSE base class, so they have no IOTKIT() etc macros.
>   */
> -#define TYPE_IOTKIT TYPE_ARMSSE
> +#define TYPE_IOTKIT "iotkit"
>  
>  /* We have an IRQ splitter and an OR gate input for each external PPC
>   * and the 2 internal PPCs
> @@ -143,4 +143,16 @@ typedef struct ARMSSE {
>      uint32_t mainclk_frq;
>  } ARMSSE;
>  
> +typedef struct ARMSSEInfo ARMSSEInfo;
> +
> +typedef struct ARMSSEClass {
> +    DeviceClass parent_class;
> +    const ARMSSEInfo *info;
> +} ARMSSEClass;
> +
> +#define ARMSSE_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(ARMSSEClass, (klass), TYPE_ARMSSE)
> +#define ARMSSE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(ARMSSEClass, (obj), TYPE_ARMSSE)
> +
>  #endif
> diff --git a/hw/arm/iotkit.c b/hw/arm/iotkit.c
> index 9360053184e..d5b172933c3 100644
> --- a/hw/arm/iotkit.c
> +++ b/hw/arm/iotkit.c
> @@ -18,6 +18,16 @@
>  #include "hw/arm/iotkit.h"
>  #include "hw/arm/arm.h"
>  
> +struct ARMSSEInfo {
> +    const char *name;
> +};
> +
> +static const ARMSSEInfo armsse_variants[] = {
> +    {
> +        .name = TYPE_IOTKIT,
> +    },
> +};
> +
>  /* Clock frequency in HZ of the 32KHz "slow clock" */
>  #define S32KCLK (32 * 1000)
>  
> @@ -732,29 +742,43 @@ static void iotkit_class_init(ObjectClass *klass, void 
> *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      IDAUInterfaceClass *iic = IDAU_INTERFACE_CLASS(klass);
> +    ARMSSEClass *asc = ARMSSE_CLASS(klass);
>  
>      dc->realize = iotkit_realize;
>      dc->vmsd = &iotkit_vmstate;
>      dc->props = iotkit_properties;
>      dc->reset = iotkit_reset;
>      iic->check = iotkit_idau_check;
> +    asc->info = data;
>  }
>  
> -static const TypeInfo iotkit_info = {
> +static const TypeInfo armsse_info = {
>      .name = TYPE_ARMSSE,
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(ARMSSE),
>      .instance_init = iotkit_init,
> -    .class_init = iotkit_class_init,
> +    .abstract = true,
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_IDAU_INTERFACE },
>          { }
>      }
>  };
>  
> -static void iotkit_register_types(void)
> +static void armsse_register_types(void)
>  {
> -    type_register_static(&iotkit_info);
> +    int i;
> +
> +    type_register_static(&armsse_info);
> +
> +    for (i = 0; i < ARRAY_SIZE(armsse_variants); i++) {
> +        TypeInfo ti = {
> +            .name = armsse_variants[i].name,
> +            .parent = TYPE_ARMSSE,
> +            .class_init = iotkit_class_init,
> +            .class_data = (void *)&armsse_variants[i],
> +        };
> +        type_register(&ti);
> +    }
>  }
>  
> -type_init(iotkit_register_types);
> +type_init(armsse_register_types);
> 

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>