Re: [PATCH 05/13] hw/ide: Extract pci_ide_class_init()

Sat, 22 Apr 2023 10:36:56 -0700 

On Sat, 22 Apr 2023, Bernhard Beschow wrote:

Resolves redundant code in every PCI IDE device model.



This patch could be broken up a bit more as it seems to do unrelated 
changes. Such as setting DEVICE_CATEGORY_STORAGE in a different way could 
be a separate patch to make it simpler to review.


Regards,
BALATON Zoltan


---
include/hw/ide/pci.h |  1 -
hw/ide/cmd646.c      | 15 ---------------
hw/ide/pci.c         | 25 ++++++++++++++++++++++++-
hw/ide/piix.c        | 19 -------------------
hw/ide/sii3112.c     |  3 ++-
hw/ide/via.c         | 15 ---------------
6 files changed, 26 insertions(+), 52 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 74c127e32f..7bc4e53d02 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -61,7 +61,6 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
extern MemoryRegionOps bmdma_addr_ioport_ops;
void pci_ide_create_devs(PCIDevice *dev);

-extern const VMStateDescription vmstate_ide_pci;
extern const MemoryRegionOps pci_ide_cmd_le_ops;
extern const MemoryRegionOps pci_ide_data_le_ops;
#endif
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a094a6e12a..9aabf80e52 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -301,17 +301,6 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error 
**errp)
    }
}

-static void pci_cmd646_ide_exitfn(PCIDevice *dev)
-{
-    PCIIDEState *d = PCI_IDE(dev);
-    unsigned i;
-
-    for (i = 0; i < 2; ++i) {
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-    }
-}
-
static Property cmd646_ide_properties[] = {
    DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
    DEFINE_PROP_END_OF_LIST(),
@@ -323,17 +312,13 @@ static void cmd646_ide_class_init(ObjectClass *klass, 
void *data)
    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);

    dc->reset = cmd646_reset;
-    dc->vmsd = &vmstate_ide_pci;
    k->realize = pci_cmd646_ide_realize;
-    k->exit = pci_cmd646_ide_exitfn;
    k->vendor_id = PCI_VENDOR_ID_CMD;
    k->device_id = PCI_DEVICE_ID_CMD_646;
    k->revision = 0x07;
-    k->class_id = PCI_CLASS_STORAGE_IDE;
    k->config_read = cmd646_pci_config_read;
    k->config_write = cmd646_pci_config_write;
    device_class_set_props(dc, cmd646_ide_properties);
-    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}

static const TypeInfo cmd646_ide_info = {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 67e0998ff0..8bea92e394 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -467,7 +467,7 @@ static int ide_pci_post_load(void *opaque, int version_id)
    return 0;
}

-const VMStateDescription vmstate_ide_pci = {
+static const VMStateDescription vmstate_ide_pci = {
    .name = "ide",
    .version_id = 3,
    .minimum_version_id = 0,
@@ -530,11 +530,34 @@ static void pci_ide_init(Object *obj)
    qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq));
}

+static void pci_ide_exitfn(PCIDevice *dev)
+{
+    PCIIDEState *d = PCI_IDE(dev);
+    unsigned i;
+
+    for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
+        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
+        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
+    }
+}
+
+static void pci_ide_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_ide_pci;
+    k->exit = pci_ide_exitfn;
+    k->class_id = PCI_CLASS_STORAGE_IDE;
+    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+}
+
static const TypeInfo pci_ide_type_info = {
    .name = TYPE_PCI_IDE,
    .parent = TYPE_PCI_DEVICE,
    .instance_size = sizeof(PCIIDEState),
    .instance_init = pci_ide_init,
+    .class_init = pci_ide_class_init,
    .abstract = true,
    .interfaces = (InterfaceInfo[]) {
        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a32f7ccece..4e6ca99123 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -159,8 +159,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
    bmdma_setup_bar(d);
    pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &d->bmdma_bar);

-    vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
-
    for (unsigned i = 0; i < 2; i++) {
        if (!pci_piix_init_bus(d, i, errp)) {
            return;
@@ -168,17 +166,6 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
    }
}

-static void pci_piix_ide_exitfn(PCIDevice *dev)
-{
-    PCIIDEState *d = PCI_IDE(dev);
-    unsigned i;
-
-    for (i = 0; i < 2; ++i) {
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-    }
-}
-
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -187,11 +174,8 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)

    dc->reset = piix_ide_reset;
    k->realize = pci_piix_ide_realize;
-    k->exit = pci_piix_ide_exitfn;
    k->vendor_id = PCI_VENDOR_ID_INTEL;
    k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
-    k->class_id = PCI_CLASS_STORAGE_IDE;
-    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
    dc->hotpluggable = false;
}

@@ -209,11 +193,8 @@ static void piix4_ide_class_init(ObjectClass *klass, void 
*data)

    dc->reset = piix_ide_reset;
    k->realize = pci_piix_ide_realize;
-    k->exit = pci_piix_ide_exitfn;
    k->vendor_id = PCI_VENDOR_ID_INTEL;
    k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
-    k->class_id = PCI_CLASS_STORAGE_IDE;
-    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
    dc->hotpluggable = false;
}

diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 5dd3d03c29..0af897a9ef 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -301,9 +301,10 @@ static void sii3112_pci_class_init(ObjectClass *klass, 
void *data)
    pd->class_id = PCI_CLASS_STORAGE_RAID;
    pd->revision = 1;
    pd->realize = sii3112_pci_realize;
+    pd->exit = NULL;
    dc->reset = sii3112_reset;
+    dc->vmsd = NULL;
    dc->desc = "SiI3112A SATA controller";
-    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}

static const TypeInfo sii3112_pci_info = {
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 91253fa4ef..287143a005 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -200,34 +200,19 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
    }
}

-static void via_ide_exitfn(PCIDevice *dev)
-{
-    PCIIDEState *d = PCI_IDE(dev);
-    unsigned i;
-
-    for (i = 0; i < ARRAY_SIZE(d->bmdma); ++i) {
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].extra_io);
-        memory_region_del_subregion(&d->bmdma_bar, &d->bmdma[i].addr_ioport);
-    }
-}
-
static void via_ide_class_init(ObjectClass *klass, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(klass);
    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);

    dc->reset = via_ide_reset;
-    dc->vmsd = &vmstate_ide_pci;
    /* Reason: only works as function of VIA southbridge */
    dc->user_creatable = false;

    k->realize = via_ide_realize;
-    k->exit = via_ide_exitfn;
    k->vendor_id = PCI_VENDOR_ID_VIA;
    k->device_id = PCI_DEVICE_ID_VIA_IDE;
    k->revision = 0x06;
-    k->class_id = PCI_CLASS_STORAGE_IDE;
-    set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
}

static const TypeInfo via_ide_info = {


























					Reply via email to