Re: [PATCH 3/4] hw/isa/piix: Unify QOM type name of PIIX ISA function

[Bernhard Beschow]

Am 10. Februar 2023 16:57:53 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>Mechanical change doing:
>
>  $ sed -i -e 's/PIIX4_PCI_DEVICE/PIIX4_ISA/g' $(git grep -l PIIX4_PCI_DEVICE)
>  $ sed -i -e 's/PIIX3_XEN_DEVICE/PIIX3_ISA_XEN/g' $(git grep -l 
> PIIX3_XEN_DEVICE)
>  $ sed -i -e 's/PIIX3_DEVICE/PIIX3_ISA/g' $(git grep -l PIIX3_DEVICE)
>  $ sed -i -e 's/PIIX3_PCI_DEVICE/PIIX_ISA/g' $(git grep -l PIIX3_PCI_DEVICE)
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/i386/pc_piix.c             |  5 ++---
> hw/isa/piix3.c                | 20 ++++++++++----------
> hw/isa/piix4.c                | 10 +++++-----
> hw/mips/malta.c               |  2 +-
> include/hw/southbridge/piix.h | 10 +++++-----
> 5 files changed, 23 insertions(+), 24 deletions(-)
>
>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>index 7c48ba30e0..afef5ed115 100644
>--- a/hw/i386/pc_piix.c
>+++ b/hw/i386/pc_piix.c
>@@ -220,8 +220,7 @@ static void pc_init1(MachineState *machine,
>     if (pcmc->pci_enabled) {
>         PIIX3State *piix3;
>         PCIDevice *pci_dev;
>-        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
>-                                         : TYPE_PIIX3_DEVICE;
>+        const char *type = xen_enabled() ? TYPE_PIIX3_ISA_XEN : 
>TYPE_PIIX3_ISA;
> 
>         pci_bus = i440fx_init(pci_type,
>                               i440fx_host,
>@@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
>         pcms->bus = pci_bus;
> 
>         pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
>-        piix3 = PIIX3_PCI_DEVICE(pci_dev);
>+        piix3 = PIIX3_ISA(pci_dev);
>         piix3->pic = x86ms->gsi;
>         piix3_devfn = piix3->dev.devfn;
>         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
>diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
>index 0ee94a2313..38e0c269ae 100644
>--- a/hw/isa/piix3.c
>+++ b/hw/isa/piix3.c
>@@ -112,7 +112,7 @@ static void piix3_write_config(PCIDevice *dev,
> {
>     pci_default_write_config(dev, address, val, len);
>     if (ranges_overlap(address, len, PIIX_PIRQCA, 4)) {
>-        PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
>+        PIIX3State *piix3 = PIIX3_ISA(dev);
>         int pic_irq;
> 
>         pci_bus_fire_intx_routing_notifier(pci_get_bus(&piix3->dev));
>@@ -145,7 +145,7 @@ static void piix3_write_config_xen(PCIDevice *dev,
> 
> static void piix3_reset(DeviceState *dev)
> {
>-    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>+    PIIX3State *d = PIIX3_ISA(dev);
>     uint8_t *pci_conf = d->dev.config;
> 
>     pci_conf[0x04] = 0x07; /* master, memory and I/O */
>@@ -286,7 +286,7 @@ static const MemoryRegionOps rcr_ops = {
> 
> static void pci_piix3_realize(PCIDevice *dev, Error **errp)
> {
>-    PIIX3State *d = PIIX3_PCI_DEVICE(dev);
>+    PIIX3State *d = PIIX3_ISA(dev);
>     ISABus *isa_bus;
> 
>     isa_bus = isa_bus_new(DEVICE(d), pci_address_space(dev),
>@@ -349,7 +349,7 @@ static void pci_piix3_class_init(ObjectClass *klass, void 
>*data)
> static void piix3_realize(PCIDevice *dev, Error **errp)
> {
>     ERRP_GUARD();
>-    PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
>+    PIIX3State *piix3 = PIIX3_ISA(dev);
>     PCIBus *pci_bus = pci_get_bus(dev);
> 
>     pci_piix3_realize(dev, errp);
>@@ -372,7 +372,7 @@ static void piix3_class_init(ObjectClass *klass, void 
>*data)
> static void piix3_xen_realize(PCIDevice *dev, Error **errp)
> {
>     ERRP_GUARD();
>-    PIIX3State *piix3 = PIIX3_PCI_DEVICE(dev);
>+    PIIX3State *piix3 = PIIX3_ISA(dev);
>     PCIBus *pci_bus = pci_get_bus(dev);
> 
>     pci_piix3_realize(dev, errp);
>@@ -399,7 +399,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void 
>*data)
> 
> static const TypeInfo piix_isa_types[] = {
>     {
>-        .name           = TYPE_PIIX3_PCI_DEVICE,
>+        .name           = TYPE_PIIX_ISA,
>         .parent         = TYPE_PCI_DEVICE,
>         .instance_size  = sizeof(PIIX3State),
>         .class_init     = pci_piix3_class_init,
>@@ -410,12 +410,12 @@ static const TypeInfo piix_isa_types[] = {
>             { },
>         },
>     }, {
>-        .name           = TYPE_PIIX3_DEVICE,
>-        .parent         = TYPE_PIIX3_PCI_DEVICE,
>+        .name           = TYPE_PIIX3_ISA,
>+        .parent         = TYPE_PIIX_ISA,
>         .class_init     = piix3_class_init,
>     }, {
>-        .name           = TYPE_PIIX3_XEN_DEVICE,
>-        .parent         = TYPE_PIIX3_PCI_DEVICE,
>+        .name           = TYPE_PIIX3_ISA_XEN,
>+        .parent         = TYPE_PIIX_ISA,
>         .class_init     = piix3_xen_class_init,
>     }
> };
>diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>index ef24826993..8c51b523e5 100644
>--- a/hw/isa/piix4.c
>+++ b/hw/isa/piix4.c
>@@ -56,7 +56,7 @@ struct PIIX4State {
>     uint8_t rcr;
> };
> 
>-OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_PCI_DEVICE)
>+OBJECT_DECLARE_SIMPLE_TYPE(PIIX4State, PIIX4_ISA)
> 
> static void piix4_set_irq(void *opaque, int irq_num, int level)
> {
>@@ -81,7 +81,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int 
>level)
> 
> static void piix4_isa_reset(DeviceState *dev)
> {
>-    PIIX4State *d = PIIX4_PCI_DEVICE(dev);
>+    PIIX4State *d = PIIX4_ISA(dev);

PIIX4State holds the state of the whole south bridge and not just the ISA 
function. I therefore tend to find PIIX4_ISA() slightly misleading...

The same will apply to PIIX3 once it got modernized like PIIX4 got already.

Best regards,
Bernhard

>     uint8_t *pci_conf = d->dev.config;
> 
>     pci_conf[0x04] = 0x07; // master, memory and I/O
>@@ -186,7 +186,7 @@ static const MemoryRegionOps piix4_rcr_ops = {
> 
> static void piix4_realize(PCIDevice *dev, Error **errp)
> {
>-    PIIX4State *s = PIIX4_PCI_DEVICE(dev);
>+    PIIX4State *s = PIIX4_ISA(dev);
>     PCIBus *pci_bus = pci_get_bus(dev);
>     ISABus *isa_bus;
>     qemu_irq *i8259_out_irq;
>@@ -251,7 +251,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
> 
> static void piix4_init(Object *obj)
> {
>-    PIIX4State *s = PIIX4_PCI_DEVICE(obj);
>+    PIIX4State *s = PIIX4_ISA(obj);
> 
>     object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
>     object_initialize_child(obj, "ide", &s->ide, TYPE_PIIX4_IDE);
>@@ -283,7 +283,7 @@ static void piix4_class_init(ObjectClass *klass, void 
>*data)
> }
> 
> static const TypeInfo piix4_info = {
>-    .name          = TYPE_PIIX4_PCI_DEVICE,
>+    .name          = TYPE_PIIX4_ISA,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(PIIX4State),
>     .instance_init = piix4_init,
>diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>index ec172b111a..5aefeba581 100644
>--- a/hw/mips/malta.c
>+++ b/hw/mips/malta.c
>@@ -1255,7 +1255,7 @@ void mips_malta_init(MachineState *machine)
> 
>     /* Southbridge */
>     piix4 = pci_create_simple_multifunction(pci_bus, PIIX4_PCI_DEVFN, true,
>-                                            TYPE_PIIX4_PCI_DEVICE);
>+                                            TYPE_PIIX4_ISA);
>     isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix4), "isa.0"));
> 
>     dev = DEVICE(object_resolve_path_component(OBJECT(piix4), "ide"));
>diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
>index a58bf13a41..71a82ef266 100644
>--- a/include/hw/southbridge/piix.h
>+++ b/include/hw/southbridge/piix.h
>@@ -58,11 +58,11 @@ struct PIIX3State {
>     MemoryRegion rcr_mem;
> };
> 
>-#define TYPE_PIIX3_PCI_DEVICE "pci-piix3"
>-#define TYPE_PIIX3_DEVICE "PIIX3"
>-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>-#define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>+#define TYPE_PIIX_ISA       "pci-piix3"
>+#define TYPE_PIIX3_ISA      "PIIX3"
>+#define TYPE_PIIX3_ISA_XEN  "PIIX3-xen"
>+#define TYPE_PIIX4_ISA      "piix4-isa"
> 
>-OBJECT_DECLARE_SIMPLE_TYPE(PIIX3State, PIIX3_PCI_DEVICE)
>+OBJECT_DECLARE_SIMPLE_TYPE(PIIX3State, PIIX3_ISA)
> 
> #endif