| Message ID | 20220803132844.2370514-3-clg@kaod.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc: QOM'ify 405 board | expand |
On Wed, 3 Aug 2022, Cédric Le Goater wrote: > We will use this machine as a base to define the ref405ep and possibly > the PPC405 hotfoot board as found in the Linux kernel. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/ppc405_boards.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c > index 1a4e7588c584..4c269b6526a5 100644 > --- a/hw/ppc/ppc405_boards.c > +++ b/hw/ppc/ppc405_boards.c > @@ -50,6 +50,15 @@ > > #define USE_FLASH_BIOS > > +struct Ppc405MachineState { > + /* Private */ > + MachineState parent_obj; > + /* Public */ > +}; > + > +#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405") > +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE); In other patches the declaration of the state struct comes after the OBJECT_DECLARE macro so here instead of above. It would be better to write it like that here too for consistency and also because then the DECLARE macro starts the object declaration and everything belonging to the object are together below it. Declaring the structure before is kind of outside the object, although this is only cosmetic and may be a matter of style. Regards, BALATON Zoltan > + > /*****************************************************************************/ > /* PPC405EP reference board (IBM) */ > /* Standalone board with: > @@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void *data) > > mc->desc = "ref405ep"; > mc->init = ref405ep_init; > - mc->default_ram_size = 0x08000000; > - mc->default_ram_id = "ef405ep.ram"; > } > > static const TypeInfo ref405ep_type = { > .name = MACHINE_TYPE_NAME("ref405ep"), > - .parent = TYPE_MACHINE, > + .parent = TYPE_PPC405_MACHINE, > .class_init = ref405ep_class_init, > }; > > +static void ppc405_machine_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->desc = "PPC405 generic machine"; > + mc->default_ram_size = 0x08000000; > + mc->default_ram_id = "ppc405.ram"; > +} > + > +static const TypeInfo ppc405_machine_type = { > + .name = TYPE_PPC405_MACHINE, > + .parent = TYPE_MACHINE, > + .instance_size = sizeof(Ppc405MachineState), > + .class_init = ppc405_machine_class_init, > + .abstract = true, > +}; > + > static void ppc405_machine_init(void) > { > + type_register_static(&ppc405_machine_type); > type_register_static(&ref405ep_type); > } > >
On 8/3/22 14:03, BALATON Zoltan wrote: > On Wed, 3 Aug 2022, Cédric Le Goater wrote: >> We will use this machine as a base to define the ref405ep and possibly >> the PPC405 hotfoot board as found in the Linux kernel. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/ppc405_boards.c | 31 ++++++++++++++++++++++++++++--- >> 1 file changed, 28 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >> index 1a4e7588c584..4c269b6526a5 100644 >> --- a/hw/ppc/ppc405_boards.c >> +++ b/hw/ppc/ppc405_boards.c >> @@ -50,6 +50,15 @@ >> >> #define USE_FLASH_BIOS >> >> +struct Ppc405MachineState { >> + /* Private */ >> + MachineState parent_obj; >> + /* Public */ >> +}; >> + >> +#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405") >> +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE); > > In other patches the declaration of the state struct comes after the OBJECT_DECLARE macro so here instead of above. It would be better to write it like that here too for consistency and also because then the DECLARE macro starts the object declaration and everything belonging to the object are together below it. Declaring the structure before is kind of outside the object, although this is only cosmetic and may be a matter of style. Good point. I moved the struct declaration to after the OBJECT_DECLARE macro. Thanks, Daniel > > Regards, > BALATON Zoltan > >> + >> /*****************************************************************************/ >> /* PPC405EP reference board (IBM) */ >> /* Standalone board with: >> @@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void *data) >> >> mc->desc = "ref405ep"; >> mc->init = ref405ep_init; >> - mc->default_ram_size = 0x08000000; >> - mc->default_ram_id = "ef405ep.ram"; >> } >> >> static const TypeInfo ref405ep_type = { >> .name = MACHINE_TYPE_NAME("ref405ep"), >> - .parent = TYPE_MACHINE, >> + .parent = TYPE_PPC405_MACHINE, >> .class_init = ref405ep_class_init, >> }; >> >> +static void ppc405_machine_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + >> + mc->desc = "PPC405 generic machine"; >> + mc->default_ram_size = 0x08000000; >> + mc->default_ram_id = "ppc405.ram"; >> +} >> + >> +static const TypeInfo ppc405_machine_type = { >> + .name = TYPE_PPC405_MACHINE, >> + .parent = TYPE_MACHINE, >> + .instance_size = sizeof(Ppc405MachineState), >> + .class_init = ppc405_machine_class_init, >> + .abstract = true, >> +}; >> + >> static void ppc405_machine_init(void) >> { >> + type_register_static(&ppc405_machine_type); >> type_register_static(&ref405ep_type); >> } >> >>
On Wed, 3 Aug 2022, Cédric Le Goater wrote: > We will use this machine as a base to define the ref405ep and possibly > the PPC405 hotfoot board as found in the Linux kernel. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/ppc405_boards.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c > index 1a4e7588c584..4c269b6526a5 100644 > --- a/hw/ppc/ppc405_boards.c > +++ b/hw/ppc/ppc405_boards.c > @@ -50,6 +50,15 @@ > > #define USE_FLASH_BIOS > > +struct Ppc405MachineState { > + /* Private */ > + MachineState parent_obj; > + /* Public */ > +}; > + > +#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405") > +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE); > + > /*****************************************************************************/ > /* PPC405EP reference board (IBM) */ > /* Standalone board with: > @@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void *data) > > mc->desc = "ref405ep"; > mc->init = ref405ep_init; > - mc->default_ram_size = 0x08000000; > - mc->default_ram_id = "ef405ep.ram"; > } > > static const TypeInfo ref405ep_type = { > .name = MACHINE_TYPE_NAME("ref405ep"), > - .parent = TYPE_MACHINE, > + .parent = TYPE_PPC405_MACHINE, > .class_init = ref405ep_class_init, > }; > > +static void ppc405_machine_class_init(ObjectClass *oc, void *data) > +{ > + MachineClass *mc = MACHINE_CLASS(oc); > + > + mc->desc = "PPC405 generic machine"; > + mc->default_ram_size = 0x08000000; > + mc->default_ram_id = "ppc405.ram"; Is the default RAM size a property of specific boards or the PPC405? I think it could be different for different boards so don't see why it's moved to the generic machine but maybe it has something to do with how other parts of QEMU handles this or I'm not getting what the generic PPC405 machine is for. Would it be clearer to just write 128 * MiB instead of a long hex number with extra zeros that's hard to read? It would be a good opportunity to change it here. Regards, BALATON Zoltan > +} > + > +static const TypeInfo ppc405_machine_type = { > + .name = TYPE_PPC405_MACHINE, > + .parent = TYPE_MACHINE, > + .instance_size = sizeof(Ppc405MachineState), > + .class_init = ppc405_machine_class_init, > + .abstract = true, > +}; > + > static void ppc405_machine_init(void) > { > + type_register_static(&ppc405_machine_type); > type_register_static(&ref405ep_type); > } > >
On 8/4/22 00:07, BALATON Zoltan wrote: > On Wed, 3 Aug 2022, Cédric Le Goater wrote: >> We will use this machine as a base to define the ref405ep and possibly >> the PPC405 hotfoot board as found in the Linux kernel. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> hw/ppc/ppc405_boards.c | 31 ++++++++++++++++++++++++++++--- >> 1 file changed, 28 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >> index 1a4e7588c584..4c269b6526a5 100644 >> --- a/hw/ppc/ppc405_boards.c >> +++ b/hw/ppc/ppc405_boards.c >> @@ -50,6 +50,15 @@ >> >> #define USE_FLASH_BIOS >> >> +struct Ppc405MachineState { >> + /* Private */ >> + MachineState parent_obj; >> + /* Public */ >> +}; >> + >> +#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405") >> +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE); >> + >> /*****************************************************************************/ >> /* PPC405EP reference board (IBM) */ >> /* Standalone board with: >> @@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void *data) >> >> mc->desc = "ref405ep"; >> mc->init = ref405ep_init; >> - mc->default_ram_size = 0x08000000; >> - mc->default_ram_id = "ef405ep.ram"; >> } >> >> static const TypeInfo ref405ep_type = { >> .name = MACHINE_TYPE_NAME("ref405ep"), >> - .parent = TYPE_MACHINE, >> + .parent = TYPE_PPC405_MACHINE, >> .class_init = ref405ep_class_init, >> }; >> >> +static void ppc405_machine_class_init(ObjectClass *oc, void *data) >> +{ >> + MachineClass *mc = MACHINE_CLASS(oc); >> + >> + mc->desc = "PPC405 generic machine"; >> + mc->default_ram_size = 0x08000000; >> + mc->default_ram_id = "ppc405.ram"; > > Is the default RAM size a property of specific boards or the PPC405? I think it could be different for different boards so don't see why it's moved to the generic machine but maybe it has something to do with how other parts of QEMU handles this or I'm not getting what the generic PPC405 machine is for. Well, the two QEMU PPC405 machines had 128M, so they were sharing the same definition. This can be overridden in a child class if needed but I doubt there will be any new PPC405 machines in QEMU. Let's keep it here. > > Would it be clearer to just write 128 * MiB instead of a long hex number with extra zeros that's hard to read? It would be a good opportunity to change it here. agree. Thanks, C. > > Regards, > BALATON Zoltan > >> +} >> + >> +static const TypeInfo ppc405_machine_type = { >> + .name = TYPE_PPC405_MACHINE, >> + .parent = TYPE_MACHINE, >> + .instance_size = sizeof(Ppc405MachineState), >> + .class_init = ppc405_machine_class_init, >> + .abstract = true, >> +}; >> + >> static void ppc405_machine_init(void) >> { >> + type_register_static(&ppc405_machine_type); >> type_register_static(&ref405ep_type); >> } >> >>
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index 1a4e7588c584..4c269b6526a5 100644 --- a/hw/ppc/ppc405_boards.c +++ b/hw/ppc/ppc405_boards.c @@ -50,6 +50,15 @@ #define USE_FLASH_BIOS +struct Ppc405MachineState { + /* Private */ + MachineState parent_obj; + /* Public */ +}; + +#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405") +OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE); + /*****************************************************************************/ /* PPC405EP reference board (IBM) */ /* Standalone board with: @@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void *data) mc->desc = "ref405ep"; mc->init = ref405ep_init; - mc->default_ram_size = 0x08000000; - mc->default_ram_id = "ef405ep.ram"; } static const TypeInfo ref405ep_type = { .name = MACHINE_TYPE_NAME("ref405ep"), - .parent = TYPE_MACHINE, + .parent = TYPE_PPC405_MACHINE, .class_init = ref405ep_class_init, }; +static void ppc405_machine_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->desc = "PPC405 generic machine"; + mc->default_ram_size = 0x08000000; + mc->default_ram_id = "ppc405.ram"; +} + +static const TypeInfo ppc405_machine_type = { + .name = TYPE_PPC405_MACHINE, + .parent = TYPE_MACHINE, + .instance_size = sizeof(Ppc405MachineState), + .class_init = ppc405_machine_class_init, + .abstract = true, +}; + static void ppc405_machine_init(void) { + type_register_static(&ppc405_machine_type); type_register_static(&ref405ep_type); }
We will use this machine as a base to define the ref405ep and possibly the PPC405 hotfoot board as found in the Linux kernel. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/ppc/ppc405_boards.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-)