qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 07/13] hw/ide: Extract pci_ide_{cmd, data}_le_ops initializat


From: Mark Cave-Ayland
Subject: Re: [PATCH 07/13] hw/ide: Extract pci_ide_{cmd, data}_le_ops initialization into base class constructor
Date: Wed, 26 Apr 2023 12:16:02 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

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 <shentey@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-ayland@ilande.co.uk>


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]