On Mon, 29 Jun 2020 at 09:55, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>
> 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.mayd...@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
This is just code motion, I didn't want to mess with the
existing way this enum was defined.
> > -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.
Similarly here I used smc->arm_id in-place because there
was only one user, but went for model = smc->model because
there were multiple users and it would have put extra
changed lines into the patch that I didn't think were
necessary.
thanks
-- PMM
