diff mbox series

[2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c

Message ID 20240219104912.378211-3-thuth@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h | expand

Commit Message

Thomas Huth Feb. 19, 2024, 10:49 a.m. UTC
qdev.c is a mixture between IDE bus specific functions and IDE device
functions. Let's split it up to make it more obvious which part is
related to bus handling and which part is related to device handling.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ide/ide-bus.c             | 111 +++++++++++++++++++++++++++++++++++
 hw/ide/{qdev.c => ide-dev.c} |  87 +--------------------------
 hw/arm/Kconfig               |   2 +
 hw/ide/Kconfig               |  30 ++++++----
 hw/ide/meson.build           |   3 +-
 5 files changed, 134 insertions(+), 99 deletions(-)
 create mode 100644 hw/ide/ide-bus.c
 rename hw/ide/{qdev.c => ide-dev.c} (78%)

Comments

BALATON Zoltan Feb. 19, 2024, 11:45 a.m. UTC | #1
On Mon, 19 Feb 2024, Thomas Huth wrote:
> qdev.c is a mixture between IDE bus specific functions and IDE device
> functions. Let's split it up to make it more obvious which part is
> related to bus handling and which part is related to device handling.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/ide/ide-bus.c             | 111 +++++++++++++++++++++++++++++++++++
> hw/ide/{qdev.c => ide-dev.c} |  87 +--------------------------
> hw/arm/Kconfig               |   2 +
> hw/ide/Kconfig               |  30 ++++++----
> hw/ide/meson.build           |   3 +-
> 5 files changed, 134 insertions(+), 99 deletions(-)
> create mode 100644 hw/ide/ide-bus.c
> rename hw/ide/{qdev.c => ide-dev.c} (78%)
[...]
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 29abe1da29..b372b819a4 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -275,6 +275,8 @@ config SBSA_REF
>     select USB_XHCI_SYSBUS
>     select WDT_SBSA
>     select BOCHS_DISPLAY
> +    select IDE_BUS
> +    select IDE_DEV
>
> config SABRELITE
>     bool
> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
> index b93d6743d5..6dfc5a2129 100644
> --- a/hw/ide/Kconfig
> +++ b/hw/ide/Kconfig
> @@ -1,51 +1,58 @@
> config IDE_CORE
>     bool
>
> -config IDE_QDEV
> +config IDE_BUS
>     bool
>     select IDE_CORE

Maybe we can assume if something has an IDE bus it also wants to connect 
IDE devices to it so just select IDE_DEV here and not at every place 
IDE_BUS is selected? Or is there a place that only wants IDE_BUS?

Regards,
BALATON Zoltan

> +config IDE_DEV
> +    bool
> +    depends on IDE_BUS
> +
> config IDE_PCI
>     bool
>     depends on PCI
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_ISA
>     bool
>     depends on ISA_BUS
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_PIIX
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_CMD646
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_MACIO
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_MMIO
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_VIA
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config MICRODRIVE
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>     depends on PCMCIA
>
> config AHCI
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config AHCI_ICH9
>     bool
> @@ -56,8 +63,7 @@ config AHCI_ICH9
> config IDE_SII3112
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_CF
>     bool
> -    default y if IDE_QDEV
> +    default y if IDE_BUS
> diff --git a/hw/ide/meson.build b/hw/ide/meson.build
> index d2e5b45c9e..d09705cac0 100644
> --- a/hw/ide/meson.build
> +++ b/hw/ide/meson.build
> @@ -1,15 +1,16 @@
> system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
> system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
> system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
> +system_ss.add(when: 'CONFIG_IDE_BUS', if_true: files('ide-bus.c'))
> system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
> system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
> system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
> +system_ss.add(when: 'CONFIG_IDE_DEV', if_true: files('ide-dev.c'))
> system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
> system_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
> system_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
> system_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
> system_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
> -system_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
> system_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
> system_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
> system_ss.add(when: 'CONFIG_MICRODRIVE', if_true: files('microdrive.c'))
>
Thomas Huth Feb. 19, 2024, 6:31 p.m. UTC | #2
On 19/02/2024 12.45, BALATON Zoltan wrote:
> On Mon, 19 Feb 2024, Thomas Huth wrote:
>> qdev.c is a mixture between IDE bus specific functions and IDE device
>> functions. Let's split it up to make it more obvious which part is
>> related to bus handling and which part is related to device handling.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> hw/ide/ide-bus.c             | 111 +++++++++++++++++++++++++++++++++++
>> hw/ide/{qdev.c => ide-dev.c} |  87 +--------------------------
>> hw/arm/Kconfig               |   2 +
>> hw/ide/Kconfig               |  30 ++++++----
>> hw/ide/meson.build           |   3 +-
>> 5 files changed, 134 insertions(+), 99 deletions(-)
>> create mode 100644 hw/ide/ide-bus.c
>> rename hw/ide/{qdev.c => ide-dev.c} (78%)
> [...]
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index 29abe1da29..b372b819a4 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -275,6 +275,8 @@ config SBSA_REF
>>     select USB_XHCI_SYSBUS
>>     select WDT_SBSA
>>     select BOCHS_DISPLAY
>> +    select IDE_BUS
>> +    select IDE_DEV
>>
>> config SABRELITE
>>     bool
>> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
>> index b93d6743d5..6dfc5a2129 100644
>> --- a/hw/ide/Kconfig
>> +++ b/hw/ide/Kconfig
>> @@ -1,51 +1,58 @@
>> config IDE_CORE
>>     bool
>>
>> -config IDE_QDEV
>> +config IDE_BUS
>>     bool
>>     select IDE_CORE
> 
> Maybe we can assume if something has an IDE bus it also wants to connect IDE 
> devices to it so just select IDE_DEV here and not at every place IDE_BUS is 
> selected? Or is there a place that only wants IDE_BUS?

Currently not, but I think it's conceptually much cleaner if we are explicit 
here.

  Thomas
diff mbox series

Patch

diff --git a/hw/ide/ide-bus.c b/hw/ide/ide-bus.c
new file mode 100644
index 0000000000..57fe67b29c
--- /dev/null
+++ b/hw/ide/ide-bus.c
@@ -0,0 +1,111 @@ 
+/*
+ * ide bus support for qdev.
+ *
+ * Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "hw/ide/internal.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/blockdev.h"
+#include "sysemu/runstate.h"
+
+static char *idebus_get_fw_dev_path(DeviceState *dev);
+static void idebus_unrealize(BusState *qdev);
+
+static void ide_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->get_fw_dev_path = idebus_get_fw_dev_path;
+    k->unrealize = idebus_unrealize;
+}
+
+static void idebus_unrealize(BusState *bus)
+{
+    IDEBus *ibus = IDE_BUS(bus);
+
+    if (ibus->vmstate) {
+        qemu_del_vm_change_state_handler(ibus->vmstate);
+    }
+}
+
+static const TypeInfo ide_bus_info = {
+    .name = TYPE_IDE_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(IDEBus),
+    .class_init = ide_bus_class_init,
+};
+
+void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
+                 int bus_id, int max_units)
+{
+    qbus_init(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
+    idebus->bus_id = bus_id;
+    idebus->max_units = max_units;
+}
+
+static char *idebus_get_fw_dev_path(DeviceState *dev)
+{
+    char path[30];
+
+    snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
+             ((IDEBus *)dev->parent_bus)->bus_id);
+
+    return g_strdup(path);
+}
+
+IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
+{
+    DeviceState *dev;
+
+    dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
+    qdev_prop_set_uint32(dev, "unit", unit);
+    qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
+                            &error_fatal);
+    qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
+    return DO_UPCAST(IDEDevice, qdev, dev);
+}
+
+int ide_get_geometry(BusState *bus, int unit,
+                     int16_t *cyls, int8_t *heads, int8_t *secs)
+{
+    IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
+
+    if (s->drive_kind != IDE_HD || !s->blk) {
+        return -1;
+    }
+
+    *cyls = s->cylinders;
+    *heads = s->heads;
+    *secs = s->sectors;
+    return 0;
+}
+
+int ide_get_bios_chs_trans(BusState *bus, int unit)
+{
+    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
+}
+
+static void ide_bus_register_type(void)
+{
+    type_register_static(&ide_bus_info);
+}
+
+type_init(ide_bus_register_type)
diff --git a/hw/ide/qdev.c b/hw/ide/ide-dev.c
similarity index 78%
rename from hw/ide/qdev.c
rename to hw/ide/ide-dev.c
index 4189313d30..15d088fd06 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/ide-dev.c
@@ -1,5 +1,5 @@ 
 /*
- * ide bus support for qdev.
+ * IDE device functions
  *
  * Copyright (c) 2009 Gerd Hoffmann <kraxel@redhat.com>
  *
@@ -18,71 +18,21 @@ 
  */
 
 #include "qemu/osdep.h"
-#include "sysemu/dma.h"
 #include "qapi/error.h"
 #include "qapi/qapi-types-block.h"
 #include "qemu/error-report.h"
-#include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "hw/ide/ide-dev.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
-#include "sysemu/runstate.h"
 #include "qapi/visitor.h"
 
-/* --------------------------------- */
-
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-static void idebus_unrealize(BusState *qdev);
-
 static Property ide_props[] = {
     DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->get_fw_dev_path = idebus_get_fw_dev_path;
-    k->unrealize = idebus_unrealize;
-}
-
-static void idebus_unrealize(BusState *bus)
-{
-    IDEBus *ibus = IDE_BUS(bus);
-
-    if (ibus->vmstate) {
-        qemu_del_vm_change_state_handler(ibus->vmstate);
-    }
-}
-
-static const TypeInfo ide_bus_info = {
-    .name = TYPE_IDE_BUS,
-    .parent = TYPE_BUS,
-    .instance_size = sizeof(IDEBus),
-    .class_init = ide_bus_class_init,
-};
-
-void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
-                 int bus_id, int max_units)
-{
-    qbus_init(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
-    idebus->bus_id = bus_id;
-    idebus->max_units = max_units;
-}
-
-static char *idebus_get_fw_dev_path(DeviceState *dev)
-{
-    char path[30];
-
-    snprintf(path, sizeof(path), "%s@%x", qdev_fw_name(dev),
-             ((IDEBus*)dev->parent_bus)->bus_id);
-
-    return g_strdup(path);
-}
-
 static void ide_qdev_realize(DeviceState *qdev, Error **errp)
 {
     IDEDevice *dev = IDE_DEVICE(qdev);
@@ -121,40 +71,6 @@  static void ide_qdev_realize(DeviceState *qdev, Error **errp)
     dc->realize(dev, errp);
 }
 
-IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
-{
-    DeviceState *dev;
-
-    dev = qdev_new(drive->media_cd ? "ide-cd" : "ide-hd");
-    qdev_prop_set_uint32(dev, "unit", unit);
-    qdev_prop_set_drive_err(dev, "drive", blk_by_legacy_dinfo(drive),
-                            &error_fatal);
-    qdev_realize_and_unref(dev, &bus->qbus, &error_fatal);
-    return DO_UPCAST(IDEDevice, qdev, dev);
-}
-
-int ide_get_geometry(BusState *bus, int unit,
-                     int16_t *cyls, int8_t *heads, int8_t *secs)
-{
-    IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
-
-    if (s->drive_kind != IDE_HD || !s->blk) {
-        return -1;
-    }
-
-    *cyls = s->cylinders;
-    *heads = s->heads;
-    *secs = s->sectors;
-    return 0;
-}
-
-int ide_get_bios_chs_trans(BusState *bus, int unit)
-{
-    return DO_UPCAST(IDEBus, qbus, bus)->ifs[unit].chs_trans;
-}
-
-/* --------------------------------- */
-
 void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
 {
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
@@ -347,7 +263,6 @@  static const TypeInfo ide_device_type_info = {
 
 static void ide_register_types(void)
 {
-    type_register_static(&ide_bus_info);
     type_register_static(&ide_hd_info);
     type_register_static(&ide_cd_info);
     type_register_static(&ide_device_type_info);
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 29abe1da29..b372b819a4 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -275,6 +275,8 @@  config SBSA_REF
     select USB_XHCI_SYSBUS
     select WDT_SBSA
     select BOCHS_DISPLAY
+    select IDE_BUS
+    select IDE_DEV
 
 config SABRELITE
     bool
diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
index b93d6743d5..6dfc5a2129 100644
--- a/hw/ide/Kconfig
+++ b/hw/ide/Kconfig
@@ -1,51 +1,58 @@ 
 config IDE_CORE
     bool
 
-config IDE_QDEV
+config IDE_BUS
     bool
     select IDE_CORE
 
+config IDE_DEV
+    bool
+    depends on IDE_BUS
+
 config IDE_PCI
     bool
     depends on PCI
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
 
 config IDE_ISA
     bool
     depends on ISA_BUS
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
 
 config IDE_PIIX
     bool
     select IDE_PCI
-    select IDE_QDEV
 
 config IDE_CMD646
     bool
     select IDE_PCI
-    select IDE_QDEV
 
 config IDE_MACIO
     bool
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
 
 config IDE_MMIO
     bool
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
 
 config IDE_VIA
     bool
     select IDE_PCI
-    select IDE_QDEV
 
 config MICRODRIVE
     bool
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
     depends on PCMCIA
 
 config AHCI
     bool
-    select IDE_QDEV
+    select IDE_BUS
+    select IDE_DEV
 
 config AHCI_ICH9
     bool
@@ -56,8 +63,7 @@  config AHCI_ICH9
 config IDE_SII3112
     bool
     select IDE_PCI
-    select IDE_QDEV
 
 config IDE_CF
     bool
-    default y if IDE_QDEV
+    default y if IDE_BUS
diff --git a/hw/ide/meson.build b/hw/ide/meson.build
index d2e5b45c9e..d09705cac0 100644
--- a/hw/ide/meson.build
+++ b/hw/ide/meson.build
@@ -1,15 +1,16 @@ 
 system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
 system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
 system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
+system_ss.add(when: 'CONFIG_IDE_BUS', if_true: files('ide-bus.c'))
 system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
 system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
 system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
+system_ss.add(when: 'CONFIG_IDE_DEV', if_true: files('ide-dev.c'))
 system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
 system_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
 system_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
 system_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
 system_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
-system_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
 system_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
 system_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
 system_ss.add(when: 'CONFIG_MICRODRIVE', if_true: files('microdrive.c'))