[Qemu-devel] [PATCH v2] macio: Improve shallow, half-hearted qdev'ification

[Andreas Färber]

From: Anthony Liguori <address@hidden>

Split macio into two PCIDevices with declarative device ID,
macio-oldworld and macio-newworld. Drop is_oldworld state in favor
of two separate init functions and deferred creation.

Signed-off-by: Andreas Färber <address@hidden>
Cc: Alexander Graf <address@hidden>
---
 v1 -> v2:
 * Patch was ignored for QOM second series: Rebase onto Anthony's older version.

 hw/macio.c        |  106 +++++++++++++++++++++++++++++++++++------------------
 hw/ppc_mac.h      |    8 ++--
 hw/ppc_newworld.c |    2 +-
 hw/ppc_oldworld.c |    2 +-
 4 files changed, 76 insertions(+), 42 deletions(-)

diff --git a/hw/macio.c b/hw/macio.c
index ae9db08..8e2a1f8 100644
--- a/hw/macio.c
+++ b/hw/macio.c
@@ -27,10 +27,8 @@
 #include "pci.h"
 #include "escc.h"
 
-typedef struct MacIOState
-{
+typedef struct MacIOState {
     PCIDevice parent;
-    int is_oldworld;
     MemoryRegion bar;
     MemoryRegion *pic_mem;
     MemoryRegion *dbdma_mem;
@@ -47,15 +45,6 @@ static void macio_bar_setup(MacIOState *macio_state)
     MemoryRegion *bar = &macio_state->bar;
 
     memory_region_init(bar, "macio", 0x80000);
-    if (macio_state->pic_mem) {
-        if (macio_state->is_oldworld) {
-            /* Heathrow PIC */
-            memory_region_add_subregion(bar, 0x00000, macio_state->pic_mem);
-        } else {
-            /* OpenPIC */
-            memory_region_add_subregion(bar, 0x40000, macio_state->pic_mem);
-        }
-    }
     if (macio_state->dbdma_mem) {
         memory_region_add_subregion(bar, 0x08000, macio_state->dbdma_mem);
     }
@@ -71,52 +60,92 @@ static void macio_bar_setup(MacIOState *macio_state)
                                         macio_state->ide_mem[i]);
         }
     }
-    if (macio_state->nvram != NULL)
+    if (macio_state->nvram != NULL) {
         macio_nvram_setup_bar(macio_state->nvram, bar, 0x60000);
+    }
+}
+
+static void macio_common_init(MacIOState *macio_state)
+{
+    PCIDevice *d = &macio_state->parent;
+
+    d->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt on pin 1 */
+
+    macio_bar_setup(macio_state);
+    pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &macio_state->bar);
+}
+
+static int macio_oldworld_initfn(PCIDevice *d)
+{
+    MacIOState *s = DO_UPCAST(MacIOState, parent, d);
+
+    macio_common_init(s);
+
+    if (s->pic_mem != NULL) {
+        /* Heathrow PIC */
+        memory_region_add_subregion(&s->bar, 0x00000, s->pic_mem);
+    }
+    return 0;
 }
 
-static int macio_initfn(PCIDevice *d)
+static int macio_newworld_initfn(PCIDevice *d)
 {
-    d->config[0x3d] = 0x01; // interrupt on pin 1
+    MacIOState *s = DO_UPCAST(MacIOState, parent, d);
+
+    macio_common_init(s);
+
+    if (s->pic_mem != NULL) {
+        /* OpenPIC */
+        memory_region_add_subregion(&s->bar, 0x40000, s->pic_mem);
+    }
     return 0;
 }
 
-static void macio_class_init(ObjectClass *klass, void *data)
+static void macio_oldworld_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = macio_initfn;
+    k->init = macio_oldworld_initfn;
     k->vendor_id = PCI_VENDOR_ID_APPLE;
+    k->device_id = PCI_DEVICE_ID_APPLE_343S1201;
     k->class_id = PCI_CLASS_OTHERS << 8;
 }
 
-static DeviceInfo macio_info = {
-    .name = "macio",
-    .size = sizeof(MacIOState),
-    .class_init = macio_class_init,
-};
-
-static void macio_register(void)
+static void macio_newworld_class_init(ObjectClass *klass, void *data)
 {
-    pci_qdev_register(&macio_info);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->init = macio_newworld_initfn;
+    k->vendor_id = PCI_VENDOR_ID_APPLE;
+    k->device_id = PCI_DEVICE_ID_APPLE_UNI_N_KEYL;
+    k->class_id = PCI_CLASS_OTHERS << 8;
 }
 
-device_init(macio_register);
+static DeviceInfo macio_oldworld_info = {
+    .name = "macio-oldworld",
+    .size = sizeof(MacIOState),
+    .class_init = macio_oldworld_class_init,
+};
+
+static DeviceInfo macio_newworld_info = {
+    .name = "macio-newworld",
+    .size = sizeof(MacIOState),
+    .class_init = macio_newworld_class_init,
+};
 
-void macio_init (PCIBus *bus, int device_id, int is_oldworld,
-                 MemoryRegion *pic_mem, MemoryRegion *dbdma_mem,
-                 MemoryRegion *cuda_mem, void *nvram,
-                 int nb_ide, MemoryRegion **ide_mem,
-                 MemoryRegion *escc_mem)
+void macio_init(PCIBus *bus, bool is_oldworld,
+                MemoryRegion *pic_mem, MemoryRegion *dbdma_mem,
+                MemoryRegion *cuda_mem, void *nvram,
+                int nb_ide, MemoryRegion **ide_mem,
+                MemoryRegion *escc_mem)
 {
     PCIDevice *d;
     MacIOState *macio_state;
     int i;
 
-    d = pci_create_simple(bus, -1, "macio");
+    d = pci_create(bus, -1, is_oldworld ? "macio-oldworld" : "macio-newworld");
 
     macio_state = DO_UPCAST(MacIOState, parent, d);
-    macio_state->is_oldworld = is_oldworld;
     macio_state->pic_mem = pic_mem;
     macio_state->dbdma_mem = dbdma_mem;
     macio_state->cuda_mem = cuda_mem;
@@ -132,8 +161,13 @@ void macio_init (PCIBus *bus, int device_id, int 
is_oldworld,
     /* Note: this code is strongly inspirated from the corresponding code
        in PearPC */
 
-    pci_config_set_device_id(d->config, device_id);
+    qdev_init_nofail(&d->qdev);
+}
 
-    macio_bar_setup(macio_state);
-    pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &macio_state->bar);
+static void macio_register(void)
+{
+    pci_qdev_register(&macio_oldworld_info);
+    pci_qdev_register(&macio_newworld_info);
 }
+
+device_init(macio_register)
diff --git a/hw/ppc_mac.h b/hw/ppc_mac.h
index af75e45..c165096 100644
--- a/hw/ppc_mac.h
+++ b/hw/ppc_mac.h
@@ -45,10 +45,10 @@
 void cuda_init (MemoryRegion **cuda_mem, qemu_irq irq);
 
 /* MacIO */
-void macio_init (PCIBus *bus, int device_id, int is_oldworld,
-                 MemoryRegion *pic_mem, MemoryRegion *dbdma_mem,
-                 MemoryRegion *cuda_mem, void *nvram,
-                 int nb_ide, MemoryRegion **ide_mem, MemoryRegion *escc_mem);
+void macio_init(PCIBus *bus, bool is_oldworld,
+                MemoryRegion *pic_mem, MemoryRegion *dbdma_mem,
+                MemoryRegion *cuda_mem, void *nvram,
+                int nb_ide, MemoryRegion **ide_mem, MemoryRegion *escc_mem);
 
 /* Heathrow PIC */
 qemu_irq *heathrow_pic_init(MemoryRegion **pmem,
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index 506187b..b7a5553 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -348,7 +348,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
     adb_kbd_init(&adb_bus);
     adb_mouse_init(&adb_bus);
 
-    macio_init(pci_bus, PCI_DEVICE_ID_APPLE_UNI_N_KEYL, 0, pic_mem,
+    macio_init(pci_bus, false, pic_mem,
                dbdma_mem, cuda_mem, NULL, 3, ide_mem, escc_bar);
 
     if (usb_enabled) {
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 9295a34..7a74b61 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -274,7 +274,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
     nvr = macio_nvram_init(0x2000, 4);
     pmac_format_nvram_partition(nvr, 0x2000);
 
-    macio_init(pci_bus, PCI_DEVICE_ID_APPLE_343S1201, 1, pic_mem,
+    macio_init(pci_bus, true, pic_mem,
                dbdma_mem, cuda_mem, nvr, 2, ide_mem, escc_bar);
 
     if (usb_enabled) {
-- 
1.7.7