Re: [PATCH 02/17] hw/arm/spitz: Create SpitzMachineClass abstract base class

[Philippe Mathieu-Daudé]

On 6/28/20 4:24 PM, Peter Maydell wrote:
> For the four Spitz-family machines (akita, borzoi, spitz, terrier)
> create a proper abstract class SpitzMachineClass which encapsulates
> the common behaviour, rather than having them all derive directly
> from TYPE_MACHINE:
>  * instead of each machine class setting mc->init to a wrapper
>    function which calls spitz_common_init() with parameters,
>    put that data in the SpitzMachineClass and make spitz_common_init
>    the SpitzMachineClass machine-init function
>  * move the settings of mc->block_default_type and
>    mc->ignore_memory_transaction_failures into the SpitzMachineClass
>    class init rather than repeating them in each machine's class init
> 
> (The motivation is that we're going to want to keep some state in
> the SpitzMachineState so we can connect GPIOs between devices created
> in one sub-function of the machine init to devices created in a
> different sub-function.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/spitz.c | 91 ++++++++++++++++++++++++++++++--------------------
>  1 file changed, 55 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
> index 9eaedab79b5..c70e912a33d 100644
> --- a/hw/arm/spitz.c
> +++ b/hw/arm/spitz.c
> @@ -33,6 +33,26 @@
>  #include "exec/address-spaces.h"
>  #include "cpu.h"
>  
> +enum spitz_model_e { spitz, akita, borzoi, terrier };
> +
> +typedef struct {
> +    MachineClass parent;
> +    enum spitz_model_e model;

Nitpick, I'd drop the not very useful typedef and use
directly ...:

       enum { spitz, akita, borzoi, terrier } model

> +    int arm_id;
> +} SpitzMachineClass;
> +
> +typedef struct {
> +    MachineState parent;
> +} SpitzMachineState;
> +
> +#define TYPE_SPITZ_MACHINE "spitz-common"
> +#define SPITZ_MACHINE(obj) \
> +    OBJECT_CHECK(SpitzMachineState, obj, TYPE_SPITZ_MACHINE)
> +#define SPITZ_MACHINE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(SpitzMachineClass, obj, TYPE_SPITZ_MACHINE)
> +#define SPITZ_MACHINE_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(SpitzMachineClass, klass, TYPE_SPITZ_MACHINE)
> +
>  #undef REG_FMT
>  #define REG_FMT                         "0x%02lx"
>  
> @@ -905,8 +925,6 @@ static void spitz_gpio_setup(PXA2xxState *cpu, int slots)
>  }
>  
>  /* Board init.  */
> -enum spitz_model_e { spitz, akita, borzoi, terrier };
> -
>  #define SPITZ_RAM       0x04000000
>  #define SPITZ_ROM       0x00800000
>  
> @@ -915,9 +933,10 @@ static struct arm_boot_info spitz_binfo = {
>      .ram_size = 0x04000000,
>  };
>  
> -static void spitz_common_init(MachineState *machine,
> -                              enum spitz_model_e model, int arm_id)
> +static void spitz_common_init(MachineState *machine)
>  {
> +    SpitzMachineClass *smc = SPITZ_MACHINE_GET_CLASS(machine);
> +    enum spitz_model_e model = smc->model;

... and use 'smc->model' in place.

Patch easier to review with 'git-diff -W' [*].

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[*] Content of my .gitattributes:

$ cat .gitattributes
*.c         diff=c
*.cpp       diff=cpp
*.m         text diff=objc
*.h         diff=c
*.py        diff=python
*.json      text
*.pl        text diff=perl
*.sh        text eol=lf
*.xml       text
*.yml       text
*.bz2       binary