On 22/04/2023 16:07, Bernhard Beschow wrote:

There is redundant code in cmd646 and via which can be extracted into the base
class. In case of piix and sii3112 this is currently unneccessary but shouldn't
interfere since the memory regions aren't mapped by those devices. In few
commits later this will be changed, i.e. those device models will also make use
of these memory regions.

Signed-off-by: Bernhard Beschow <shen...@gmail.com>
---
  hw/ide/cmd646.c | 11 -----------
  hw/ide/pci.c    | 10 ++++++++++
  hw/ide/via.c    | 11 -----------
  3 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 6fd09fe74e..85716aaf17 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -251,20 +251,9 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error 
**errp)
      dev->wmask[MRDMODE] = 0x0;
      dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
- memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
-                          &d->bus[0], "cmd646-data0", 8);
      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[0]);
-
-    memory_region_init_io(&d->cmd_bar[0], OBJECT(d), &pci_ide_cmd_le_ops,
-                          &d->bus[0], "cmd646-cmd0", 4);
      pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[0]);
-
-    memory_region_init_io(&d->data_bar[1], OBJECT(d), &pci_ide_data_le_ops,
-                          &d->bus[1], "cmd646-data1", 8);
      pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[1]);
-
-    memory_region_init_io(&d->cmd_bar[1], OBJECT(d), &pci_ide_cmd_le_ops,
-                          &d->bus[1], "cmd646-cmd1", 4);
      pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
bmdma_init_ops(d, &cmd646_bmdma_ops);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 65ed6f7f72..a9194313bd 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -543,6 +543,16 @@ static void pci_ide_init(Object *obj)
  {
      PCIIDEState *d = PCI_IDE(obj);
+ memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
+                          &d->bus[0], "pci-ide0-data-ops", 8);
+    memory_region_init_io(&d->cmd_bar[0], OBJECT(d), &pci_ide_cmd_le_ops,
+                          &d->bus[0], "pci-ide0-cmd-ops", 4);
+
+    memory_region_init_io(&d->data_bar[1], OBJECT(d), &pci_ide_data_le_ops,
+                          &d->bus[1], "pci-ide1-data-ops", 8);
+    memory_region_init_io(&d->cmd_bar[1], OBJECT(d), &pci_ide_cmd_le_ops,
+                          &d->bus[1], "pci-ide1-cmd-ops", 4);
+
      qdev_init_gpio_out(DEVICE(d), d->isa_irq, ARRAY_SIZE(d->isa_irq));
  }
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 40704e2857..704a8024cb 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -154,20 +154,9 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
      dev->wmask[PCI_INTERRUPT_LINE] = 0;
      dev->wmask[PCI_CLASS_PROG] = 5;
- memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
-                          &d->bus[0], "via-ide0-data", 8);
      pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[0]);
-
-    memory_region_init_io(&d->cmd_bar[0], OBJECT(d), &pci_ide_cmd_le_ops,
-                          &d->bus[0], "via-ide0-cmd", 4);
      pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[0]);
-
-    memory_region_init_io(&d->data_bar[1], OBJECT(d), &pci_ide_data_le_ops,
-                          &d->bus[1], "via-ide1-data", 8);
      pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_IO, &d->data_bar[1]);
-
-    memory_region_init_io(&d->cmd_bar[1], OBJECT(d), &pci_ide_cmd_le_ops,
-                          &d->bus[1], "via-ide1-cmd", 4);
      pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, &d->cmd_bar[1]);
bmdma_init_ops(d, &via_bmdma_ops);

I'd also be inclined to agree with Phil/Zoltan re: dropping the trailing "-ops" in the name, otherwise:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>


ATB,

Mark.

Reply via email to