Re: [PATCH 01/44] Split out common part of BCM283X classes

[Peter Maydell]

On Wed, 26 Jul 2023 at 14:43, Sergey Kambalin <serg.oker@gmail.com> wrote:
>
> Signed-off-by: Sergey Kambalin <sergey.kambalin@auriga.com>
> ---
>  hw/arm/bcm2836.c         | 102 ++++++++++++++++++++++-----------------
>  hw/arm/raspi.c           |   2 +-
>  include/hw/arm/bcm2836.h |  26 +++++++++-
>  3 files changed, 83 insertions(+), 47 deletions(-)

> @@ -230,11 +238,17 @@ static const TypeInfo bcm283x_types[] = {
>  #endif
>      }, {
>          .name           = TYPE_BCM283X,
> -        .parent         = TYPE_DEVICE,
> +        .parent         = TYPE_BCM283X_BASE,
>          .instance_size  = sizeof(BCM283XState),
> -        .instance_init  = bcm2836_init,
> -        .class_size     = sizeof(BCM283XClass),
> -        .class_init     = bcm283x_class_init,
> +        .instance_init  = bcm283x_init,
> +        .abstract       = true,
> +    }, {
> +        .name           = TYPE_BCM283X_BASE,
> +        .parent         = TYPE_DEVICE,
> +        .instance_size  = sizeof(BCM283XBaseState),
> +        .instance_init  = bcm283x_base_init,
> +        .class_size     = sizeof(BCM283XBaseClass),
> +        .class_init     = bcm283x_base_class_init,
>          .abstract       = true,
>      }
>  };
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> +
> +struct BCM283XBaseClass {
> +    /*< private >*/
> +    DeviceClass parent_class;
> +    /*< public >*/
> +    const char *name;
> +    const char *cpu_type;
> +    unsigned core_count;
> +    hwaddr peri_base; /* Peripheral base address seen by the CPU */
> +    hwaddr ctrl_base; /* Interrupt controller and mailboxes etc. */
> +    int clusterid;
> +};
> +
> +struct BCM283XState {
> +    /*< private >*/
> +    BCM283XBaseState parent_obj;
> +    /*< public >*/
>      BCM2835PeripheralState peripherals;
>  };
>

This gives us a slightly odd class hierarchy where we have
two "common between bcmxxxx SoCs" types:

   TYPE_BCM283X_BASE --> TYPE_BCM283X --> TYPE_BCM2835
                     |                |-> TYPE_BCM2836
                     |                \-> TYPE_BCM2837
                     \-> TYPE_BCM2838

The only thing TYPE_BCM283X seems to be doing here that
TYPE_BCM283X_BASE is not is handling the BCM2835PeripheralState
object. Would it be clearer to keep the existing
class hierarchy where everything inherits from
TYPE_BCM283X, and accept a little code duplication for
the 3 subclasses that use the same BCM2835PeripheralState?
I'm not sure...

thanks
-- PMM