| Message ID | e10a8d11ea424aa8fa727936b2ad6c2fe439b3ad.1663368422.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc ppc/mac machines clean up | expand |
On 17/09/2022 00:07, BALATON Zoltan wrote: > It is only used by mac_oldworld anyway and it already instantiates > a few devices by name so this allows reducing the shared header further. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/pci-host/grackle.c | 1 + > hw/ppc/mac.h | 3 --- > hw/ppc/mac_oldworld.c | 2 +- > 3 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c > index b05facf463..5282123004 100644 > --- a/hw/pci-host/grackle.c > +++ b/hw/pci-host/grackle.c > @@ -34,6 +34,7 @@ > #include "trace.h" > #include "qom/object.h" > > +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" > OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE) > > struct GrackleState { > diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h > index 55cb02c990..fe77a6c6db 100644 > --- a/hw/ppc/mac.h > +++ b/hw/ppc/mac.h > @@ -35,9 +35,6 @@ > #define KERNEL_LOAD_ADDR 0x01000000 > #define KERNEL_GAP 0x00100000 > > -/* Grackle PCI */ > -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" > - > /* Mac NVRAM */ > #define TYPE_MACIO_NVRAM "macio-nvram" > OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM) > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index f323a49d7a..a4094226bc 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine) > } > > /* Grackle PCI host bridge */ > - grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE); > + grackle_dev = qdev_new("grackle-pcihost"); > qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000); > s = SYS_BUS_DEVICE(grackle_dev); > sysbus_realize_and_unref(s, &error_fatal); This is the wrong way around - we want to move towards using TYPE_ macros everywhere for device instantiation instead of hardcoded strings. What's really missing here is that the QOM structs and definitions for grackle.c should be moved to a new include/hw/pci-host/grackle.h file from mac.h and included where necessary. ATB, Mark.
On Sun, 25 Sep 2022, Mark Cave-Ayland wrote: > On 17/09/2022 00:07, BALATON Zoltan wrote: >> It is only used by mac_oldworld anyway and it already instantiates >> a few devices by name so this allows reducing the shared header further. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/pci-host/grackle.c | 1 + >> hw/ppc/mac.h | 3 --- >> hw/ppc/mac_oldworld.c | 2 +- >> 3 files changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c >> index b05facf463..5282123004 100644 >> --- a/hw/pci-host/grackle.c >> +++ b/hw/pci-host/grackle.c >> @@ -34,6 +34,7 @@ >> #include "trace.h" >> #include "qom/object.h" >> +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" >> OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE) >> struct GrackleState { >> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h >> index 55cb02c990..fe77a6c6db 100644 >> --- a/hw/ppc/mac.h >> +++ b/hw/ppc/mac.h >> @@ -35,9 +35,6 @@ >> #define KERNEL_LOAD_ADDR 0x01000000 >> #define KERNEL_GAP 0x00100000 >> -/* Grackle PCI */ >> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" >> - >> /* Mac NVRAM */ >> #define TYPE_MACIO_NVRAM "macio-nvram" >> OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM) >> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c >> index f323a49d7a..a4094226bc 100644 >> --- a/hw/ppc/mac_oldworld.c >> +++ b/hw/ppc/mac_oldworld.c >> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine) >> } >> /* Grackle PCI host bridge */ >> - grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE); >> + grackle_dev = qdev_new("grackle-pcihost"); >> qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000); >> s = SYS_BUS_DEVICE(grackle_dev); >> sysbus_realize_and_unref(s, &error_fatal); > > This is the wrong way around - we want to move towards using TYPE_ macros > everywhere for device instantiation instead of hardcoded strings. > > What's really missing here is that the QOM structs and definitions for > grackle.c should be moved to a new include/hw/pci-host/grackle.h file from > mac.h and included where necessary. That could be done any time later, this patch is good enough for now, there are other devices instantiated this way in mac_oldworld now. I don't want to chnage grackle here, just clean up the mac.h. Regards, BALATON Zoltan
On 25/09/2022 10:26, BALATON Zoltan wrote: > On Sun, 25 Sep 2022, Mark Cave-Ayland wrote: >> On 17/09/2022 00:07, BALATON Zoltan wrote: >>> It is only used by mac_oldworld anyway and it already instantiates >>> a few devices by name so this allows reducing the shared header further. >>> >>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>> --- >>> hw/pci-host/grackle.c | 1 + >>> hw/ppc/mac.h | 3 --- >>> hw/ppc/mac_oldworld.c | 2 +- >>> 3 files changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c >>> index b05facf463..5282123004 100644 >>> --- a/hw/pci-host/grackle.c >>> +++ b/hw/pci-host/grackle.c >>> @@ -34,6 +34,7 @@ >>> #include "trace.h" >>> #include "qom/object.h" >>> +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" >>> OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE) >>> struct GrackleState { >>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h >>> index 55cb02c990..fe77a6c6db 100644 >>> --- a/hw/ppc/mac.h >>> +++ b/hw/ppc/mac.h >>> @@ -35,9 +35,6 @@ >>> #define KERNEL_LOAD_ADDR 0x01000000 >>> #define KERNEL_GAP 0x00100000 >>> -/* Grackle PCI */ >>> -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" >>> - >>> /* Mac NVRAM */ >>> #define TYPE_MACIO_NVRAM "macio-nvram" >>> OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM) >>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c >>> index f323a49d7a..a4094226bc 100644 >>> --- a/hw/ppc/mac_oldworld.c >>> +++ b/hw/ppc/mac_oldworld.c >>> @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine) >>> } >>> /* Grackle PCI host bridge */ >>> - grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE); >>> + grackle_dev = qdev_new("grackle-pcihost"); >>> qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000); >>> s = SYS_BUS_DEVICE(grackle_dev); >>> sysbus_realize_and_unref(s, &error_fatal); >> >> This is the wrong way around - we want to move towards using TYPE_ macros >> everywhere for device instantiation instead of hardcoded strings. >> >> What's really missing here is that the QOM structs and definitions for grackle.c >> should be moved to a new include/hw/pci-host/grackle.h file from mac.h and included >> where necessary. > > That could be done any time later, this patch is good enough for now, there are other > devices instantiated this way in mac_oldworld now. I don't want to chnage grackle > here, just clean up the mac.h. It is a long-standing philosophy for QEMU that if outdated code is touched then there should be a best effort to update it to the latest coding standards. Moving the QOM definition to a separate header file is not too dissimilar to patch 10, so will be a fairly trivial change. ATB, Mark.
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c index b05facf463..5282123004 100644 --- a/hw/pci-host/grackle.c +++ b/hw/pci-host/grackle.c @@ -34,6 +34,7 @@ #include "trace.h" #include "qom/object.h" +#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" OBJECT_DECLARE_SIMPLE_TYPE(GrackleState, GRACKLE_PCI_HOST_BRIDGE) struct GrackleState { diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h index 55cb02c990..fe77a6c6db 100644 --- a/hw/ppc/mac.h +++ b/hw/ppc/mac.h @@ -35,9 +35,6 @@ #define KERNEL_LOAD_ADDR 0x01000000 #define KERNEL_GAP 0x00100000 -/* Grackle PCI */ -#define TYPE_GRACKLE_PCI_HOST_BRIDGE "grackle-pcihost" - /* Mac NVRAM */ #define TYPE_MACIO_NVRAM "macio-nvram" OBJECT_DECLARE_SIMPLE_TYPE(MacIONVRAMState, MACIO_NVRAM) diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index f323a49d7a..a4094226bc 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -214,7 +214,7 @@ static void ppc_heathrow_init(MachineState *machine) } /* Grackle PCI host bridge */ - grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE); + grackle_dev = qdev_new("grackle-pcihost"); qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x80000000); s = SYS_BUS_DEVICE(grackle_dev); sysbus_realize_and_unref(s, &error_fatal);
It is only used by mac_oldworld anyway and it already instantiates a few devices by name so this allows reducing the shared header further. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/pci-host/grackle.c | 1 + hw/ppc/mac.h | 3 --- hw/ppc/mac_oldworld.c | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-)