[PATCH v3 00/10] Instantiate VT82xx functions in host device

[PATCH v3 00/10] Instantiate VT82xx functions in host device

[Bernhard Beschow]

v3:
* Replace pre increment by post increment in for loop (Zoltan)
* Move class defines close to where the class is defined (Zoltan)

Testing done:
* `make check-avocado`
  Passes for boot_linux_console.py for mips64el_fuloong2e
* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel morphos-3.17/boot.img`
  Boots successfully and it is possible to open games and tools.

v2:
* Keep the call to pci_ide_create_devs() in board code for consistency (Zoltan)
* Create rtc-time alias in board rather than in south bridge code
* Remove stale comments about PCI functions (Zoltan)

v1:
This series instantiates all PCI functions of the VT82xx south bridges in the south bridges themselves.
For the IDE function this is especially important since its interrupt routing is configured in the
ISA function, hence doesn't make sense to instantiate it as a "Frankenstein" device. The interrupt
routing is currently hardcoded and changing that is currently not in the scope of this series.

Testing done:
* `qemu-system-ppc -machine pegasos2 -rtc base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom morphos-3.17.iso -kernel morphos-3.17/boot.img`
  Boots successfully and it is possible to open games and tools.

* I was unable to test the fuloong2e board even before this series since it seems to be unfinished [1].
  A buildroot-baked kernel [2] booted but doesn't find its root partition, though the issues could be in the buildroot receipt I created.

[1] https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2
[2] https://github.com/shentok/buildroot/commits/fuloong2e

Bernhard Beschow (10):
  hw/isa/vt82c686: Resolve chip-specific realize methods
  hw/isa/vt82c686: Resolve unneeded attribute
  hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()
  hw/isa/vt82c686: Reuse errp
  hw/isa/vt82c686: Instantiate IDE function in host device
  hw/isa/vt82c686: Instantiate USB functions in host device
  hw/isa/vt82c686: Instantiate PM function in host device
  hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device
  hw/isa/vt82c686: Embed RTCState in host device
  hw/isa/vt82c686: Create rtc-time alias in boards instead

 configs/devices/mips64el-softmmu/default.mak |   1 -
 hw/isa/Kconfig                               |   1 +
 hw/isa/vt82c686.c                            | 120 +++++++++++++++----
 hw/mips/fuloong2e.c                          |  21 ++--
 hw/ppc/Kconfig                               |   1 -
 hw/ppc/pegasos2.c                            |  25 ++--
 include/hw/isa/vt82c686.h                    |   2 -
 7 files changed, 116 insertions(+), 55 deletions(-)

-- 
2.37.3

[PATCH v3 01/10] hw/isa/vt82c686: Resolve chip-specific realize methods

[Bernhard Beschow]

The object creation now happens in chip-specific init methods which
allows the realize methods to be consolidated into one method. Shifting
the logic into the init methods has the addidional advantage that the
parent object's init methods are called implicitly - like constructors
in object-oriented languages.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 8f656251b8..0217c98fe4 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -544,7 +544,7 @@ struct ViaISAState {
     qemu_irq cpu_intr;
     qemu_irq *isa_irqs;
     ISABus *isa_bus;
-    ViaSuperIOState *via_sio;
+    ViaSuperIOState via_sio;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -602,6 +602,11 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
             d->wmask[i] = 0;
         }
     }
+
+    /* Super I/O */
+    if (!qdev_realize(DEVICE(&s->via_sio), BUS(s->isa_bus), errp)) {
+        return;
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
@@ -615,7 +620,7 @@ static void vt82c686b_write_config(PCIDevice *d, uint32_t addr,
     pci_default_write_config(d, addr, val, len);
     if (addr == 0x85) {
         /* BIT(1): enable or disable superio config io ports */
-        via_superio_io_enable(s->via_sio, val & BIT(1));
+        via_superio_io_enable(&s->via_sio, val & BIT(1));
     }
 }
 
@@ -639,13 +644,11 @@ static void vt82c686b_isa_reset(DeviceState *dev)
     pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */
 }
 
-static void vt82c686b_realize(PCIDevice *d, Error **errp)
+static void vt82c686b_init(Object *obj)
 {
-    ViaISAState *s = VIA_ISA(d);
+    ViaISAState *s = VIA_ISA(obj);
 
-    via_isa_realize(d, errp);
-    s->via_sio = VIA_SUPERIO(isa_create_simple(s->isa_bus,
-                                               TYPE_VT82C686B_SUPERIO));
+    object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT82C686B_SUPERIO);
 }
 
 static void vt82c686b_class_init(ObjectClass *klass, void *data)
@@ -653,7 +656,7 @@ static void vt82c686b_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->realize = vt82c686b_realize;
+    k->realize = via_isa_realize;
     k->config_write = vt82c686b_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_82C686B_ISA;
@@ -670,6 +673,7 @@ static const TypeInfo vt82c686b_isa_info = {
     .name          = TYPE_VT82C686B_ISA,
     .parent        = TYPE_VIA_ISA,
     .instance_size = sizeof(ViaISAState),
+    .instance_init = vt82c686b_init,
     .class_init    = vt82c686b_class_init,
 };
 
@@ -684,7 +688,7 @@ static void vt8231_write_config(PCIDevice *d, uint32_t addr,
     pci_default_write_config(d, addr, val, len);
     if (addr == 0x50) {
         /* BIT(2): enable or disable superio config io ports */
-        via_superio_io_enable(s->via_sio, val & BIT(2));
+        via_superio_io_enable(&s->via_sio, val & BIT(2));
     }
 }
 
@@ -703,13 +707,11 @@ static void vt8231_isa_reset(DeviceState *dev)
     pci_conf[0x6b] = 0x01; /* Fast IR I/O Base */
 }
 
-static void vt8231_realize(PCIDevice *d, Error **errp)
+static void vt8231_init(Object *obj)
 {
-    ViaISAState *s = VIA_ISA(d);
+    ViaISAState *s = VIA_ISA(obj);
 
-    via_isa_realize(d, errp);
-    s->via_sio = VIA_SUPERIO(isa_create_simple(s->isa_bus,
-                                               TYPE_VT8231_SUPERIO));
+    object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT8231_SUPERIO);
 }
 
 static void vt8231_class_init(ObjectClass *klass, void *data)
@@ -717,7 +719,7 @@ static void vt8231_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->realize = vt8231_realize;
+    k->realize = via_isa_realize;
     k->config_write = vt8231_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
     k->device_id = PCI_DEVICE_ID_VIA_8231_ISA;
@@ -734,6 +736,7 @@ static const TypeInfo vt8231_isa_info = {
     .name          = TYPE_VT8231_ISA,
     .parent        = TYPE_VIA_ISA,
     .instance_size = sizeof(ViaISAState),
+    .instance_init = vt8231_init,
     .class_init    = vt8231_class_init,
 };
 
-- 
2.37.3

[PATCH v3 02/10] hw/isa/vt82c686: Resolve unneeded attribute

[Bernhard Beschow]

Now that also the super io device is realized in the common realize method,
the isa_bus attribute can be turned into a temporary.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 0217c98fe4..9d12e1cae4 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -543,7 +543,6 @@ struct ViaISAState {
     PCIDevice dev;
     qemu_irq cpu_intr;
     qemu_irq *isa_irqs;
-    ISABus *isa_bus;
     ViaSuperIOState via_sio;
 };
 
@@ -585,17 +584,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     ViaISAState *s = VIA_ISA(d);
     DeviceState *dev = DEVICE(d);
     qemu_irq *isa_irq;
+    ISABus *isa_bus;
     int i;
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
-    s->isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
+    isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
                           &error_fatal);
-    s->isa_irqs = i8259_init(s->isa_bus, *isa_irq);
-    isa_bus_irqs(s->isa_bus, s->isa_irqs);
-    i8254_pit_init(s->isa_bus, 0x40, 0, NULL);
-    i8257_dma_init(s->isa_bus, 0);
-    mc146818_rtc_init(s->isa_bus, 2000, NULL);
+    s->isa_irqs = i8259_init(isa_bus, *isa_irq);
+    isa_bus_irqs(isa_bus, s->isa_irqs);
+    i8254_pit_init(isa_bus, 0x40, 0, NULL);
+    i8257_dma_init(isa_bus, 0);
+    mc146818_rtc_init(isa_bus, 2000, NULL);
 
     for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {
         if (i < PCI_COMMAND || i >= PCI_REVISION_ID) {
@@ -604,7 +604,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     }
 
     /* Super I/O */
-    if (!qdev_realize(DEVICE(&s->via_sio), BUS(s->isa_bus), errp)) {
+    if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
         return;
     }
 }
-- 
2.37.3

[PATCH v3 03/10] hw/isa/vt82c686: Prefer pci_address_space() over get_system_memory()

[Bernhard Beschow]

Unlike get_system_memory(), pci_address_space() respects the memory tree
available to the parent device.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9d12e1cae4..5582c0b179 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -589,7 +589,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
 
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
-    isa_bus = isa_bus_new(dev, get_system_memory(), pci_address_space_io(d),
+    isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
                           &error_fatal);
     s->isa_irqs = i8259_init(isa_bus, *isa_irq);
     isa_bus_irqs(isa_bus, s->isa_irqs);
-- 
2.37.3

[PATCH v3 04/10] hw/isa/vt82c686: Reuse errp

[Bernhard Beschow]

Rather than terminating abruptly, make use of the already present errp and
propagate the error to the caller.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 5582c0b179..37e37b3855 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -590,7 +590,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     qdev_init_gpio_out(dev, &s->cpu_intr, 1);
     isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
     isa_bus = isa_bus_new(dev, pci_address_space(d), pci_address_space_io(d),
-                          &error_fatal);
+                          errp);
+
+    if (!isa_bus) {
+        return;
+    }
+
     s->isa_irqs = i8259_init(isa_bus, *isa_irq);
     isa_bus_irqs(isa_bus, s->isa_irqs);
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
-- 
2.37.3

[PATCH v3 05/10] hw/isa/vt82c686: Instantiate IDE function in host device

[Bernhard Beschow]

The IDE function is closely tied to the ISA function (e.g. the IDE
interrupt routing happens there), so it makes sense that the IDE
function is instantiated within the south bridge itself.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 configs/devices/mips64el-softmmu/default.mak |  1 -
 hw/isa/Kconfig                               |  1 +
 hw/isa/vt82c686.c                            | 17 +++++++++++++++++
 hw/mips/fuloong2e.c                          |  8 ++++----
 hw/ppc/Kconfig                               |  1 -
 hw/ppc/pegasos2.c                            |  9 ++++-----
 6 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
index c610749ac1..d5188f7ea5 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -1,7 +1,6 @@
 # Default configuration for mips64el-softmmu
 
 include ../mips-softmmu/common.mak
-CONFIG_IDE_VIA=y
 CONFIG_FULOONG=y
 CONFIG_LOONGSON3V=y
 CONFIG_ATI_VGA=y
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index d42143a991..20de7e9294 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -53,6 +53,7 @@ config VT82C686
     select I8254
     select I8257
     select I8259
+    select IDE_VIA
     select MC146818RTC
     select PARALLEL
 
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 37e37b3855..9d946cea54 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -17,6 +17,7 @@
 #include "hw/isa/vt82c686.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
+#include "hw/ide/pci.h"
 #include "hw/isa/isa.h"
 #include "hw/isa/superio.h"
 #include "hw/intc/i8259.h"
@@ -544,6 +545,7 @@ struct ViaISAState {
     qemu_irq cpu_intr;
     qemu_irq *isa_irqs;
     ViaSuperIOState via_sio;
+    PCIIDEState ide;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
     }
 };
 
+static void via_isa_init(Object *obj)
+{
+    ViaISAState *s = VIA_ISA(obj);
+
+    object_initialize_child(obj, "ide", &s->ide, "via-ide");
+}
+
 static const TypeInfo via_isa_info = {
     .name          = TYPE_VIA_ISA,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(ViaISAState),
+    .instance_init = via_isa_init,
     .abstract      = true,
     .interfaces    = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
@@ -583,6 +593,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
 {
     ViaISAState *s = VIA_ISA(d);
     DeviceState *dev = DEVICE(d);
+    PCIBus *pci_bus = pci_get_bus(d);
     qemu_irq *isa_irq;
     ISABus *isa_bus;
     int i;
@@ -612,6 +623,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
         return;
     }
+
+    /* Function 1: IDE */
+    qdev_prop_set_int32(DEVICE(&s->ide), "addr", d->devfn + 1);
+    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
+        return;
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 5ee546f5f6..32605901e7 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -199,13 +199,13 @@ static void main_cpu_reset(void *opaque)
 static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
                                        I2CBus **i2c_bus)
 {
-    PCIDevice *dev;
+    PCIDevice *dev, *via;
 
-    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
+    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
                                           TYPE_VT82C686B_ISA);
-    qdev_connect_gpio_out(DEVICE(dev), 0, intc);
+    qdev_connect_gpio_out(DEVICE(via), 0, intc);
 
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
+    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 400511c6b7..18565e966b 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -74,7 +74,6 @@ config PEGASOS2
     bool
     select MV64361
     select VT82C686
-    select IDE_VIA
     select SMBUS_EEPROM
     select VOF
 # This should come with VT82C686
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 61f4263953..8bc528a560 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -102,7 +102,7 @@ static void pegasos2_init(MachineState *machine)
     CPUPPCState *env;
     MemoryRegion *rom = g_new(MemoryRegion, 1);
     PCIBus *pci_bus;
-    PCIDevice *dev;
+    PCIDevice *dev, *via;
     I2CBus *i2c_bus;
     const char *fwname = machine->firmware ?: PROM_FILENAME;
     char *filename;
@@ -160,13 +160,12 @@ static void pegasos2_init(MachineState *machine)
 
     /* VIA VT8231 South Bridge (multifunction PCI device) */
     /* VT8231 function 0: PCI-to-ISA Bridge */
-    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
+    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
                                           TYPE_VT8231_ISA);
-    qdev_connect_gpio_out(DEVICE(dev), 0,
+    qdev_connect_gpio_out(DEVICE(via), 0,
                           qdev_get_gpio_in_named(pm->mv, "gpp", 31));
 
-    /* VT8231 function 1: IDE Controller */
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), "via-ide");
+    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
     /* VT8231 function 2-3: USB Ports */
-- 
2.37.3

[PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[Bernhard Beschow]

The USB functions can be enabled/disabled through the ISA function. Also
its interrupt routing can be influenced there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c   | 12 ++++++++++++
 hw/mips/fuloong2e.c |  3 ---
 hw/ppc/pegasos2.c   |  4 ----
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 9d946cea54..66a4b9c230 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -23,6 +23,7 @@
 #include "hw/intc/i8259.h"
 #include "hw/irq.h"
 #include "hw/dma/i8257.h"
+#include "hw/usb/hcd-uhci.h"
 #include "hw/timer/i8254.h"
 #include "hw/rtc/mc146818rtc.h"
 #include "migration/vmstate.h"
@@ -546,6 +547,7 @@ struct ViaISAState {
     qemu_irq *isa_irqs;
     ViaSuperIOState via_sio;
     PCIIDEState ide;
+    UHCIState uhci[2];
 };
 
 static const VMStateDescription vmstate_via = {
@@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
     ViaISAState *s = VIA_ISA(obj);
 
     object_initialize_child(obj, "ide", &s->ide, "via-ide");
+    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
+    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
 }
 
 static const TypeInfo via_isa_info = {
@@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
         return;
     }
+
+    /* Functions 2-3: USB Ports */
+    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
+        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
+        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
+            return;
+        }
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 32605901e7..dc92223b76 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
-
     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
 
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 8bc528a560..85cca6f7a6 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    /* VT8231 function 2-3: USB Ports */
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
-
     /* VT8231 function 4: Power Management Controller */
     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
-- 
2.37.3

[PATCH v3 07/10] hw/isa/vt82c686: Instantiate PM function in host device

[Bernhard Beschow]

The PM controller has activity bits which monitor activity of other
built-in devices in the host device.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c         | 13 +++++++++++++
 hw/mips/fuloong2e.c       |  2 +-
 hw/ppc/pegasos2.c         |  3 +--
 include/hw/isa/vt82c686.h |  2 --
 4 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 66a4b9c230..fcc9894e8b 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -250,6 +250,8 @@ static const ViaPMInitInfo vt82c686b_pm_init_info = {
     .device_id = PCI_DEVICE_ID_VIA_82C686B_PM,
 };
 
+#define TYPE_VT82C686B_PM "vt82c686b-pm"
+
 static const TypeInfo vt82c686b_pm_info = {
     .name          = TYPE_VT82C686B_PM,
     .parent        = TYPE_VIA_PM,
@@ -261,6 +263,8 @@ static const ViaPMInitInfo vt8231_pm_init_info = {
     .device_id = PCI_DEVICE_ID_VIA_8231_PM,
 };
 
+#define TYPE_VT8231_PM "vt8231-pm"
+
 static const TypeInfo vt8231_pm_info = {
     .name          = TYPE_VT8231_PM,
     .parent        = TYPE_VIA_PM,
@@ -548,6 +552,7 @@ struct ViaISAState {
     ViaSuperIOState via_sio;
     PCIIDEState ide;
     UHCIState uhci[2];
+    ViaPMState pm;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -641,6 +646,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
             return;
         }
     }
+
+    /* Function 4: Power Management */
+    qdev_prop_set_int32(DEVICE(&s->pm), "addr", d->devfn + 4);
+    if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
+        return;
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
@@ -683,6 +694,7 @@ static void vt82c686b_init(Object *obj)
     ViaISAState *s = VIA_ISA(obj);
 
     object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT82C686B_SUPERIO);
+    object_initialize_child(obj, "pm", &s->pm, TYPE_VT82C686B_PM);
 }
 
 static void vt82c686b_class_init(ObjectClass *klass, void *data)
@@ -746,6 +758,7 @@ static void vt8231_init(Object *obj)
     ViaISAState *s = VIA_ISA(obj);
 
     object_initialize_child(obj, "sio", &s->via_sio, TYPE_VT8231_SUPERIO);
+    object_initialize_child(obj, "pm", &s->pm, TYPE_VT8231_PM);
 }
 
 static void vt8231_class_init(ObjectClass *klass, void *data)
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index dc92223b76..377108d313 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -208,7 +208,7 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
+    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
 
     /* Audio support */
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 85cca6f7a6..e32944ee2b 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -168,8 +168,7 @@ static void pegasos2_init(MachineState *machine)
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
     pci_ide_create_devs(dev);
 
-    /* VT8231 function 4: Power Management Controller */
-    dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
+    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
     spd_data = spd_data_generate(DDR, machine->ram_size);
     smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
diff --git a/include/hw/isa/vt82c686.h b/include/hw/isa/vt82c686.h
index 56ac141be3..559f7c8926 100644
--- a/include/hw/isa/vt82c686.h
+++ b/include/hw/isa/vt82c686.h
@@ -4,9 +4,7 @@
 #include "hw/pci/pci.h"
 
 #define TYPE_VT82C686B_ISA "vt82c686b-isa"
-#define TYPE_VT82C686B_PM "vt82c686b-pm"
 #define TYPE_VT8231_ISA "vt8231-isa"
-#define TYPE_VT8231_PM "vt8231-pm"
 #define TYPE_VIA_AC97 "via-ac97"
 #define TYPE_VIA_MC97 "via-mc97"
 
-- 
2.37.3

[PATCH v3 08/10] hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

[Bernhard Beschow]

The AC97 function's wakeup status is wired to the PM function and both
the AC97 and MC97 interrupt routing is determined by the ISA function.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c   | 16 ++++++++++++++++
 hw/mips/fuloong2e.c |  4 ----
 hw/ppc/pegasos2.c   |  5 -----
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index fcc9894e8b..691a467b2c 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -553,6 +553,8 @@ struct ViaISAState {
     PCIIDEState ide;
     UHCIState uhci[2];
     ViaPMState pm;
+    PCIDevice ac97;
+    PCIDevice mc97;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -572,6 +574,8 @@ static void via_isa_init(Object *obj)
     object_initialize_child(obj, "ide", &s->ide, "via-ide");
     object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
     object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
+    object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
+    object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
 }
 
 static const TypeInfo via_isa_info = {
@@ -652,6 +656,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
         return;
     }
+
+    /* Function 5: AC97 Audio */
+    qdev_prop_set_int32(DEVICE(&s->ac97), "addr", d->devfn + 5);
+    if (!qdev_realize(DEVICE(&s->ac97), BUS(pci_bus), errp)) {
+        return;
+    }
+
+    /* Function 6: AC97 Modem */
+    qdev_prop_set_int32(DEVICE(&s->mc97), "addr", d->devfn + 6);
+    if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
+        return;
+    }
 }
 
 /* TYPE_VT82C686B_ISA */
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 377108d313..2d8723ab74 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -210,10 +210,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
 
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
-
-    /* Audio support */
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
-    pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
 }
 
 /* Network support */
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index e32944ee2b..09fdb7557f 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -159,7 +159,6 @@ static void pegasos2_init(MachineState *machine)
     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
 
     /* VIA VT8231 South Bridge (multifunction PCI device) */
-    /* VT8231 function 0: PCI-to-ISA Bridge */
     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
                                           TYPE_VT8231_ISA);
     qdev_connect_gpio_out(DEVICE(via), 0,
@@ -173,10 +172,6 @@ static void pegasos2_init(MachineState *machine)
     spd_data = spd_data_generate(DDR, machine->ram_size);
     smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
 
-    /* VT8231 function 5-6: AC97 Audio & Modem */
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 5), TYPE_VIA_AC97);
-    pci_create_simple(pci_bus, PCI_DEVFN(12, 6), TYPE_VIA_MC97);
-
     /* other PC hardware */
     pci_vga_init(pci_bus);
 
-- 
2.37.3

[PATCH v3 09/10] hw/isa/vt82c686: Embed RTCState in host device

[Bernhard Beschow]

Embed the rtc in the host device, analoguous to the other child devices
and analoguous to PIIX4.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 691a467b2c..0ddb04b433 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -550,6 +550,7 @@ struct ViaISAState {
     qemu_irq cpu_intr;
     qemu_irq *isa_irqs;
     ViaSuperIOState via_sio;
+    RTCState rtc;
     PCIIDEState ide;
     UHCIState uhci[2];
     ViaPMState pm;
@@ -571,6 +572,7 @@ static void via_isa_init(Object *obj)
 {
     ViaISAState *s = VIA_ISA(obj);
 
+    object_initialize_child(obj, "rtc", &s->rtc, TYPE_MC146818_RTC);
     object_initialize_child(obj, "ide", &s->ide, "via-ide");
     object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
     object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
@@ -624,7 +626,15 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     isa_bus_irqs(isa_bus, s->isa_irqs);
     i8254_pit_init(isa_bus, 0x40, 0, NULL);
     i8257_dma_init(isa_bus, 0);
-    mc146818_rtc_init(isa_bus, 2000, NULL);
+
+    /* RTC */
+    qdev_prop_set_int32(DEVICE(&s->rtc), "base_year", 2000);
+    if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
+        return;
+    }
+    object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(&s->rtc),
+                              "date");
+    isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
 
     for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {
         if (i < PCI_COMMAND || i >= PCI_REVISION_ID) {
-- 
2.37.3

[PATCH v3 10/10] hw/isa/vt82c686: Create rtc-time alias in boards instead

[Bernhard Beschow]

According to good QOM practice, an object should only deal with objects
of its own sub tree. Having devices create an alias on the machine
object doesn't respect this good practice. To resolve this, create the
alias in the machine's code.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/isa/vt82c686.c   | 2 --
 hw/mips/fuloong2e.c | 4 ++++
 hw/ppc/pegasos2.c   | 4 ++++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 0ddb04b433..d5200af0ff 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -632,8 +632,6 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
     if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
         return;
     }
-    object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(&s->rtc),
-                              "date");
     isa_connect_gpio_out(ISA_DEVICE(&s->rtc), 0, s->rtc.isairq);
 
     for (i = 0; i < PCI_CONFIG_HEADER_SIZE; i++) {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 2d8723ab74..0f4cfe1188 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -203,6 +203,10 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
 
     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
                                           TYPE_VT82C686B_ISA);
+    object_property_add_alias(qdev_get_machine(), "rtc-time",
+                              object_resolve_path_component(OBJECT(via),
+                                                            "rtc"),
+                              "date");
     qdev_connect_gpio_out(DEVICE(via), 0, intc);
 
     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 09fdb7557f..f50e1d8b3f 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -161,6 +161,10 @@ static void pegasos2_init(MachineState *machine)
     /* VIA VT8231 South Bridge (multifunction PCI device) */
     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
                                           TYPE_VT8231_ISA);
+    object_property_add_alias(qdev_get_machine(), "rtc-time",
+                              object_resolve_path_component(OBJECT(via),
+                                                            "rtc"),
+                              "date");
     qdev_connect_gpio_out(DEVICE(via), 0,
                           qdev_get_gpio_in_named(pm->mv, "gpp", 31));
 
-- 
2.37.3

Re: [PATCH v3 05/10] hw/isa/vt82c686: Instantiate IDE function in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> The IDE function is closely tied to the ISA function (e.g. the IDE
> interrupt routing happens there), so it makes sense that the IDE
> function is instantiated within the south bridge itself.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> configs/devices/mips64el-softmmu/default.mak |  1 -
> hw/isa/Kconfig                               |  1 +
> hw/isa/vt82c686.c                            | 17 +++++++++++++++++
> hw/mips/fuloong2e.c                          |  8 ++++----
> hw/ppc/Kconfig                               |  1 -
> hw/ppc/pegasos2.c                            |  9 ++++-----
> 6 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
> index c610749ac1..d5188f7ea5 100644
> --- a/configs/devices/mips64el-softmmu/default.mak
> +++ b/configs/devices/mips64el-softmmu/default.mak
> @@ -1,7 +1,6 @@
> # Default configuration for mips64el-softmmu
>
> include ../mips-softmmu/common.mak
> -CONFIG_IDE_VIA=y
> CONFIG_FULOONG=y
> CONFIG_LOONGSON3V=y
> CONFIG_ATI_VGA=y
> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
> index d42143a991..20de7e9294 100644
> --- a/hw/isa/Kconfig
> +++ b/hw/isa/Kconfig
> @@ -53,6 +53,7 @@ config VT82C686
>     select I8254
>     select I8257
>     select I8259
> +    select IDE_VIA
>     select MC146818RTC
>     select PARALLEL
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 37e37b3855..9d946cea54 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -17,6 +17,7 @@
> #include "hw/isa/vt82c686.h"
> #include "hw/pci/pci.h"
> #include "hw/qdev-properties.h"
> +#include "hw/ide/pci.h"
> #include "hw/isa/isa.h"
> #include "hw/isa/superio.h"
> #include "hw/intc/i8259.h"
> @@ -544,6 +545,7 @@ struct ViaISAState {
>     qemu_irq cpu_intr;
>     qemu_irq *isa_irqs;
>     ViaSuperIOState via_sio;
> +    PCIIDEState ide;
> };
>
> static const VMStateDescription vmstate_via = {
> @@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
>     }
> };
>
> +static void via_isa_init(Object *obj)
> +{
> +    ViaISAState *s = VIA_ISA(obj);
> +
> +    object_initialize_child(obj, "ide", &s->ide, "via-ide");
> +}
> +
> static const TypeInfo via_isa_info = {
>     .name          = TYPE_VIA_ISA,
>     .parent        = TYPE_PCI_DEVICE,
>     .instance_size = sizeof(ViaISAState),
> +    .instance_init = via_isa_init,

Did you verify this is actually called? I guess you could add a debug 
printf in the init method above or check the output of info qom-tree to 
see if ide apears below via-isa. I'm not sure because I think QOM does not 
call superclass methods if you override them, that's why the subclass 
realize methods called via_isa_realize before. In this case it may not 
cause a problem because ide-via does not have an init method so it will 
work with just realize called so the only effect may be that qom-tree is 
not like it should be. Or if this is called then I still don't get QOM.

Regards,
BALATON Zoltan

>     .abstract      = true,
>     .interfaces    = (InterfaceInfo[]) {
>         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> @@ -583,6 +593,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
> {
>     ViaISAState *s = VIA_ISA(d);
>     DeviceState *dev = DEVICE(d);
> +    PCIBus *pci_bus = pci_get_bus(d);
>     qemu_irq *isa_irq;
>     ISABus *isa_bus;
>     int i;
> @@ -612,6 +623,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
>         return;
>     }
> +
> +    /* Function 1: IDE */
> +    qdev_prop_set_int32(DEVICE(&s->ide), "addr", d->devfn + 1);
> +    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
> +        return;
> +    }
> }
>
> /* TYPE_VT82C686B_ISA */
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 5ee546f5f6..32605901e7 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -199,13 +199,13 @@ static void main_cpu_reset(void *opaque)
> static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>                                        I2CBus **i2c_bus)
> {
> -    PCIDevice *dev;
> +    PCIDevice *dev, *via;
>
> -    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
> +    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
>                                           TYPE_VT82C686B_ISA);
> -    qdev_connect_gpio_out(DEVICE(dev), 0, intc);
> +    qdev_connect_gpio_out(DEVICE(via), 0, intc);
>
> -    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> +    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
>     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 400511c6b7..18565e966b 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -74,7 +74,6 @@ config PEGASOS2
>     bool
>     select MV64361
>     select VT82C686
> -    select IDE_VIA
>     select SMBUS_EEPROM
>     select VOF
> # This should come with VT82C686
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index 61f4263953..8bc528a560 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -102,7 +102,7 @@ static void pegasos2_init(MachineState *machine)
>     CPUPPCState *env;
>     MemoryRegion *rom = g_new(MemoryRegion, 1);
>     PCIBus *pci_bus;
> -    PCIDevice *dev;
> +    PCIDevice *dev, *via;
>     I2CBus *i2c_bus;
>     const char *fwname = machine->firmware ?: PROM_FILENAME;
>     char *filename;
> @@ -160,13 +160,12 @@ static void pegasos2_init(MachineState *machine)
>
>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>     /* VT8231 function 0: PCI-to-ISA Bridge */
> -    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
> +    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>                                           TYPE_VT8231_ISA);
> -    qdev_connect_gpio_out(DEVICE(dev), 0,
> +    qdev_connect_gpio_out(DEVICE(via), 0,
>                           qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>
> -    /* VT8231 function 1: IDE Controller */
> -    dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), "via-ide");
> +    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
>     /* VT8231 function 2-3: USB Ports */
>

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> The USB functions can be enabled/disabled through the ISA function. Also
> its interrupt routing can be influenced there.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/isa/vt82c686.c   | 12 ++++++++++++
> hw/mips/fuloong2e.c |  3 ---
> hw/ppc/pegasos2.c   |  4 ----
> 3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 9d946cea54..66a4b9c230 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -23,6 +23,7 @@
> #include "hw/intc/i8259.h"
> #include "hw/irq.h"
> #include "hw/dma/i8257.h"
> +#include "hw/usb/hcd-uhci.h"
> #include "hw/timer/i8254.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "migration/vmstate.h"
> @@ -546,6 +547,7 @@ struct ViaISAState {
>     qemu_irq *isa_irqs;
>     ViaSuperIOState via_sio;
>     PCIIDEState ide;
> +    UHCIState uhci[2];
> };
>
> static const VMStateDescription vmstate_via = {
> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>     ViaISAState *s = VIA_ISA(obj);
>
>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");

Sorry for not saying this yesterday, this can also be done separately so 
no need for another version of this series if not needed for another 
reason but could we add a define for vt82c686b-usb-uhci in 
include/hw/isa/vt82c686.h and use that here and in 
hw/usb/vt82c686-uhci-pci.c ?

Regards,
BALATON Zoltan

> }
>
> static const TypeInfo via_isa_info = {
> @@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
>         return;
>     }
> +
> +    /* Functions 2-3: USB Ports */
> +    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
> +        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
> +        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
> +            return;
> +        }
> +    }
> }
>
> /* TYPE_VT82C686B_ISA */
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 32605901e7..dc92223b76 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
> -
>     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index 8bc528a560..85cca6f7a6 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>     pci_ide_create_devs(dev);
>
> -    /* VT8231 function 2-3: USB Ports */
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
> -
>     /* VT8231 function 4: Power Management Controller */
>     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
>     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>

Re: [PATCH v3 08/10] hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> The AC97 function's wakeup status is wired to the PM function and both
> the AC97 and MC97 interrupt routing is determined by the ISA function.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/isa/vt82c686.c   | 16 ++++++++++++++++
> hw/mips/fuloong2e.c |  4 ----
> hw/ppc/pegasos2.c   |  5 -----
> 3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index fcc9894e8b..691a467b2c 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -553,6 +553,8 @@ struct ViaISAState {
>     PCIIDEState ide;
>     UHCIState uhci[2];
>     ViaPMState pm;
> +    PCIDevice ac97;
> +    PCIDevice mc97;
> };
>
> static const VMStateDescription vmstate_via = {
> @@ -572,6 +574,8 @@ static void via_isa_init(Object *obj)
>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>     object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>     object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
> +    object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
> +    object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
> }
>
> static const TypeInfo via_isa_info = {
> @@ -652,6 +656,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>     if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
>         return;
>     }
> +
> +    /* Function 5: AC97 Audio */
> +    qdev_prop_set_int32(DEVICE(&s->ac97), "addr", d->devfn + 5);
> +    if (!qdev_realize(DEVICE(&s->ac97), BUS(pci_bus), errp)) {
> +        return;
> +    }
> +
> +    /* Function 6: AC97 Modem */

Is this MC97 Modem instead?

Regards,
BALATON Zoltan

> +    qdev_prop_set_int32(DEVICE(&s->mc97), "addr", d->devfn + 6);
> +    if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
> +        return;
> +    }
> }
>
> /* TYPE_VT82C686B_ISA */
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 377108d313..2d8723ab74 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -210,10 +210,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>
>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
> -
> -    /* Audio support */
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
> }
>
> /* Network support */
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index e32944ee2b..09fdb7557f 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -159,7 +159,6 @@ static void pegasos2_init(MachineState *machine)
>     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
>
>     /* VIA VT8231 South Bridge (multifunction PCI device) */
> -    /* VT8231 function 0: PCI-to-ISA Bridge */
>     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>                                           TYPE_VT8231_ISA);
>     qdev_connect_gpio_out(DEVICE(via), 0,
> @@ -173,10 +172,6 @@ static void pegasos2_init(MachineState *machine)
>     spd_data = spd_data_generate(DDR, machine->ram_size);
>     smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
>
> -    /* VT8231 function 5-6: AC97 Audio & Modem */
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 5), TYPE_VIA_AC97);
> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 6), TYPE_VIA_MC97);
> -
>     /* other PC hardware */
>     pci_vga_init(pci_bus);
>
>

Re: [PATCH v3 05/10] hw/isa/vt82c686: Instantiate IDE function in host device

[BB]

Am 31. August 2022 15:12:26 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>> The IDE function is closely tied to the ISA function (e.g. the IDE
>> interrupt routing happens there), so it makes sense that the IDE
>> function is instantiated within the south bridge itself.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> configs/devices/mips64el-softmmu/default.mak |  1 -
>> hw/isa/Kconfig                               |  1 +
>> hw/isa/vt82c686.c                            | 17 +++++++++++++++++
>> hw/mips/fuloong2e.c                          |  8 ++++----
>> hw/ppc/Kconfig                               |  1 -
>> hw/ppc/pegasos2.c                            |  9 ++++-----
>> 6 files changed, 26 insertions(+), 11 deletions(-)
>> 
>> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
>> index c610749ac1..d5188f7ea5 100644
>> --- a/configs/devices/mips64el-softmmu/default.mak
>> +++ b/configs/devices/mips64el-softmmu/default.mak
>> @@ -1,7 +1,6 @@
>> # Default configuration for mips64el-softmmu
>> 
>> include ../mips-softmmu/common.mak
>> -CONFIG_IDE_VIA=y
>> CONFIG_FULOONG=y
>> CONFIG_LOONGSON3V=y
>> CONFIG_ATI_VGA=y
>> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>> index d42143a991..20de7e9294 100644
>> --- a/hw/isa/Kconfig
>> +++ b/hw/isa/Kconfig
>> @@ -53,6 +53,7 @@ config VT82C686
>>     select I8254
>>     select I8257
>>     select I8259
>> +    select IDE_VIA
>>     select MC146818RTC
>>     select PARALLEL
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 37e37b3855..9d946cea54 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -17,6 +17,7 @@
>> #include "hw/isa/vt82c686.h"
>> #include "hw/pci/pci.h"
>> #include "hw/qdev-properties.h"
>> +#include "hw/ide/pci.h"
>> #include "hw/isa/isa.h"
>> #include "hw/isa/superio.h"
>> #include "hw/intc/i8259.h"
>> @@ -544,6 +545,7 @@ struct ViaISAState {
>>     qemu_irq cpu_intr;
>>     qemu_irq *isa_irqs;
>>     ViaSuperIOState via_sio;
>> +    PCIIDEState ide;
>> };
>> 
>> static const VMStateDescription vmstate_via = {
>> @@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
>>     }
>> };
>> 
>> +static void via_isa_init(Object *obj)
>> +{
>> +    ViaISAState *s = VIA_ISA(obj);
>> +
>> +    object_initialize_child(obj, "ide", &s->ide, "via-ide");
>> +}
>> +
>> static const TypeInfo via_isa_info = {
>>     .name          = TYPE_VIA_ISA,
>>     .parent        = TYPE_PCI_DEVICE,
>>     .instance_size = sizeof(ViaISAState),
>> +    .instance_init = via_isa_init,
>
>Did you verify this is actually called? I guess you could add a debug printf in the init method above or check the output of info qom-tree to see if ide apears below via-isa. I'm not sure because I think QOM does not call superclass methods if you override them, that's why the subclass realize methods called via_isa_realize before. In this case it may not cause a problem because ide-via does not have an init method so it will work with just realize called so the only effect may be that qom-tree is not like it should be. Or if this is called then I still don't get QOM.

We discussed the semantics of init() vs. realize() when discussing patch 1 in v1 which consolidates realize() methods. My understanding is that init() behaves like C++ constructors which are called implicitly parent first, child next. OTOH realize() methods behave like virtual methods which get replaced by the most specific one, i.e. one needs to call the parent implementation explicitly.

Anyway, via_isa_init() must be called, otherwise the realize() method would abort due to trying to realize the non-initialized ide attribute.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>>     .abstract      = true,
>>     .interfaces    = (InterfaceInfo[]) {
>>         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
>> @@ -583,6 +593,7 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>> {
>>     ViaISAState *s = VIA_ISA(d);
>>     DeviceState *dev = DEVICE(d);
>> +    PCIBus *pci_bus = pci_get_bus(d);
>>     qemu_irq *isa_irq;
>>     ISABus *isa_bus;
>>     int i;
>> @@ -612,6 +623,12 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->via_sio), BUS(isa_bus), errp)) {
>>         return;
>>     }
>> +
>> +    /* Function 1: IDE */
>> +    qdev_prop_set_int32(DEVICE(&s->ide), "addr", d->devfn + 1);
>> +    if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
>> +        return;
>> +    }
>> }
>> 
>> /* TYPE_VT82C686B_ISA */
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 5ee546f5f6..32605901e7 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -199,13 +199,13 @@ static void main_cpu_reset(void *opaque)
>> static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>>                                        I2CBus **i2c_bus)
>> {
>> -    PCIDevice *dev;
>> +    PCIDevice *dev, *via;
>> 
>> -    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
>> +    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(slot, 0), true,
>>                                           TYPE_VT82C686B_ISA);
>> -    qdev_connect_gpio_out(DEVICE(dev), 0, intc);
>> +    qdev_connect_gpio_out(DEVICE(via), 0, intc);
>> 
>> -    dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
>> +    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>>     pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index 400511c6b7..18565e966b 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -74,7 +74,6 @@ config PEGASOS2
>>     bool
>>     select MV64361
>>     select VT82C686
>> -    select IDE_VIA
>>     select SMBUS_EEPROM
>>     select VOF
>> # This should come with VT82C686
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index 61f4263953..8bc528a560 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -102,7 +102,7 @@ static void pegasos2_init(MachineState *machine)
>>     CPUPPCState *env;
>>     MemoryRegion *rom = g_new(MemoryRegion, 1);
>>     PCIBus *pci_bus;
>> -    PCIDevice *dev;
>> +    PCIDevice *dev, *via;
>>     I2CBus *i2c_bus;
>>     const char *fwname = machine->firmware ?: PROM_FILENAME;
>>     char *filename;
>> @@ -160,13 +160,12 @@ static void pegasos2_init(MachineState *machine)
>> 
>>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>>     /* VT8231 function 0: PCI-to-ISA Bridge */
>> -    dev = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>> +    via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>>                                           TYPE_VT8231_ISA);
>> -    qdev_connect_gpio_out(DEVICE(dev), 0,
>> +    qdev_connect_gpio_out(DEVICE(via), 0,
>>                           qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>> 
>> -    /* VT8231 function 1: IDE Controller */
>> -    dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 1), "via-ide");
>> +    dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>>     /* VT8231 function 2-3: USB Ports */
>> 

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BB]

Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>> The USB functions can be enabled/disabled through the ISA function. Also
>> its interrupt routing can be influenced there.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c   | 12 ++++++++++++
>> hw/mips/fuloong2e.c |  3 ---
>> hw/ppc/pegasos2.c   |  4 ----
>> 3 files changed, 12 insertions(+), 7 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 9d946cea54..66a4b9c230 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -23,6 +23,7 @@
>> #include "hw/intc/i8259.h"
>> #include "hw/irq.h"
>> #include "hw/dma/i8257.h"
>> +#include "hw/usb/hcd-uhci.h"
>> #include "hw/timer/i8254.h"
>> #include "hw/rtc/mc146818rtc.h"
>> #include "migration/vmstate.h"
>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>     qemu_irq *isa_irqs;
>>     ViaSuperIOState via_sio;
>>     PCIIDEState ide;
>> +    UHCIState uhci[2];
>> };
>> 
>> static const VMStateDescription vmstate_via = {
>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>     ViaISAState *s = VIA_ISA(obj);
>> 
>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>
>Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?

Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> }
>> 
>> static const TypeInfo via_isa_info = {
>> @@ -629,6 +633,14 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->ide), BUS(pci_bus), errp)) {
>>         return;
>>     }
>> +
>> +    /* Functions 2-3: USB Ports */
>> +    for (i = 0; i < ARRAY_SIZE(s->uhci); i++) {
>> +        qdev_prop_set_int32(DEVICE(&s->uhci[i]), "addr", d->devfn + 2 + i);
>> +        if (!qdev_realize(DEVICE(&s->uhci[i]), BUS(pci_bus), errp)) {
>> +            return;
>> +        }
>> +    }
>> }
>> 
>> /* TYPE_VT82C686B_ISA */
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 32605901e7..dc92223b76 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -208,9 +208,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
>> -
>>     dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 4), TYPE_VT82C686B_PM);
>>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>> 
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index 8bc528a560..85cca6f7a6 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -168,10 +168,6 @@ static void pegasos2_init(MachineState *machine)
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
>>     pci_ide_create_devs(dev);
>> 
>> -    /* VT8231 function 2-3: USB Ports */
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 2), "vt82c686b-usb-uhci");
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 3), "vt82c686b-usb-uhci");
>> -
>>     /* VT8231 function 4: Power Management Controller */
>>     dev = pci_create_simple(pci_bus, PCI_DEVFN(12, 4), TYPE_VT8231_PM);
>>     i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>> 

Re: [PATCH v3 08/10] hw/isa/vt82c686: Instantiate AC97 and MC97 functions in host device

[BB]

Am 31. August 2022 15:24:28 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>> The AC97 function's wakeup status is wired to the PM function and both
>> the AC97 and MC97 interrupt routing is determined by the ISA function.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/isa/vt82c686.c   | 16 ++++++++++++++++
>> hw/mips/fuloong2e.c |  4 ----
>> hw/ppc/pegasos2.c   |  5 -----
>> 3 files changed, 16 insertions(+), 9 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index fcc9894e8b..691a467b2c 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -553,6 +553,8 @@ struct ViaISAState {
>>     PCIIDEState ide;
>>     UHCIState uhci[2];
>>     ViaPMState pm;
>> +    PCIDevice ac97;
>> +    PCIDevice mc97;
>> };
>> 
>> static const VMStateDescription vmstate_via = {
>> @@ -572,6 +574,8 @@ static void via_isa_init(Object *obj)
>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>     object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>     object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>> +    object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
>> +    object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
>> }
>> 
>> static const TypeInfo via_isa_info = {
>> @@ -652,6 +656,18 @@ static void via_isa_realize(PCIDevice *d, Error **errp)
>>     if (!qdev_realize(DEVICE(&s->pm), BUS(pci_bus), errp)) {
>>         return;
>>     }
>> +
>> +    /* Function 5: AC97 Audio */
>> +    qdev_prop_set_int32(DEVICE(&s->ac97), "addr", d->devfn + 5);
>> +    if (!qdev_realize(DEVICE(&s->ac97), BUS(pci_bus), errp)) {
>> +        return;
>> +    }
>> +
>> +    /* Function 6: AC97 Modem */
>
>Is this MC97 Modem instead?

Yeah, MC97 Modem. I''ll send a v4.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> +    qdev_prop_set_int32(DEVICE(&s->mc97), "addr", d->devfn + 6);
>> +    if (!qdev_realize(DEVICE(&s->mc97), BUS(pci_bus), errp)) {
>> +        return;
>> +    }
>> }
>> 
>> /* TYPE_VT82C686B_ISA */
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 377108d313..2d8723ab74 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -210,10 +210,6 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
>> 
>>     dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
>>     *i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
>> -
>> -    /* Audio support */
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 5), TYPE_VIA_AC97);
>> -    pci_create_simple(pci_bus, PCI_DEVFN(slot, 6), TYPE_VIA_MC97);
>> }
>> 
>> /* Network support */
>> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
>> index e32944ee2b..09fdb7557f 100644
>> --- a/hw/ppc/pegasos2.c
>> +++ b/hw/ppc/pegasos2.c
>> @@ -159,7 +159,6 @@ static void pegasos2_init(MachineState *machine)
>>     pci_bus = mv64361_get_pci_bus(pm->mv, 1);
>> 
>>     /* VIA VT8231 South Bridge (multifunction PCI device) */
>> -    /* VT8231 function 0: PCI-to-ISA Bridge */
>>     via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
>>                                           TYPE_VT8231_ISA);
>>     qdev_connect_gpio_out(DEVICE(via), 0,
>> @@ -173,10 +172,6 @@ static void pegasos2_init(MachineState *machine)
>>     spd_data = spd_data_generate(DDR, machine->ram_size);
>>     smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
>> 
>> -    /* VT8231 function 5-6: AC97 Audio & Modem */
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 5), TYPE_VIA_AC97);
>> -    pci_create_simple(pci_bus, PCI_DEVFN(12, 6), TYPE_VIA_MC97);
>> -
>>     /* other PC hardware */
>>     pci_vga_init(pci_bus);
>> 
>> 

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, BB wrote:
> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>> The USB functions can be enabled/disabled through the ISA function. Also
>>> its interrupt routing can be influenced there.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>> hw/mips/fuloong2e.c |  3 ---
>>> hw/ppc/pegasos2.c   |  4 ----
>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 9d946cea54..66a4b9c230 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -23,6 +23,7 @@
>>> #include "hw/intc/i8259.h"
>>> #include "hw/irq.h"
>>> #include "hw/dma/i8257.h"
>>> +#include "hw/usb/hcd-uhci.h"
>>> #include "hw/timer/i8254.h"
>>> #include "hw/rtc/mc146818rtc.h"
>>> #include "migration/vmstate.h"
>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>     qemu_irq *isa_irqs;
>>>     ViaSuperIOState via_sio;
>>>     PCIIDEState ide;
>>> +    UHCIState uhci[2];
>>> };
>>>
>>> static const VMStateDescription vmstate_via = {
>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>     ViaISAState *s = VIA_ISA(obj);
>>>
>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>
>> Sorry for not saying this yesterday, this can also be done separately 
>> so no need for another version of this series if not needed for another 
>> reason but could we add a define for vt82c686b-usb-uhci in 
>> include/hw/isa/vt82c686.h and use that here and in 
>> hw/usb/vt82c686-uhci-pci.c ?
>
> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.

I don't think it needs a separate header just for this so I'd put it in 
vt82c686.h but I don't mind either way.

Regards,
BALATON Zoltan

Re: [PATCH v3 05/10] hw/isa/vt82c686: Instantiate IDE function in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, BB wrote:
> Am 31. August 2022 15:12:26 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>> The IDE function is closely tied to the ISA function (e.g. the IDE
>>> interrupt routing happens there), so it makes sense that the IDE
>>> function is instantiated within the south bridge itself.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>> ---
>>> configs/devices/mips64el-softmmu/default.mak |  1 -
>>> hw/isa/Kconfig                               |  1 +
>>> hw/isa/vt82c686.c                            | 17 +++++++++++++++++
>>> hw/mips/fuloong2e.c                          |  8 ++++----
>>> hw/ppc/Kconfig                               |  1 -
>>> hw/ppc/pegasos2.c                            |  9 ++++-----
>>> 6 files changed, 26 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
>>> index c610749ac1..d5188f7ea5 100644
>>> --- a/configs/devices/mips64el-softmmu/default.mak
>>> +++ b/configs/devices/mips64el-softmmu/default.mak
>>> @@ -1,7 +1,6 @@
>>> # Default configuration for mips64el-softmmu
>>>
>>> include ../mips-softmmu/common.mak
>>> -CONFIG_IDE_VIA=y
>>> CONFIG_FULOONG=y
>>> CONFIG_LOONGSON3V=y
>>> CONFIG_ATI_VGA=y
>>> diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>>> index d42143a991..20de7e9294 100644
>>> --- a/hw/isa/Kconfig
>>> +++ b/hw/isa/Kconfig
>>> @@ -53,6 +53,7 @@ config VT82C686
>>>     select I8254
>>>     select I8257
>>>     select I8259
>>> +    select IDE_VIA
>>>     select MC146818RTC
>>>     select PARALLEL
>>>
>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>> index 37e37b3855..9d946cea54 100644
>>> --- a/hw/isa/vt82c686.c
>>> +++ b/hw/isa/vt82c686.c
>>> @@ -17,6 +17,7 @@
>>> #include "hw/isa/vt82c686.h"
>>> #include "hw/pci/pci.h"
>>> #include "hw/qdev-properties.h"
>>> +#include "hw/ide/pci.h"
>>> #include "hw/isa/isa.h"
>>> #include "hw/isa/superio.h"
>>> #include "hw/intc/i8259.h"
>>> @@ -544,6 +545,7 @@ struct ViaISAState {
>>>     qemu_irq cpu_intr;
>>>     qemu_irq *isa_irqs;
>>>     ViaSuperIOState via_sio;
>>> +    PCIIDEState ide;
>>> };
>>>
>>> static const VMStateDescription vmstate_via = {
>>> @@ -556,10 +558,18 @@ static const VMStateDescription vmstate_via = {
>>>     }
>>> };
>>>
>>> +static void via_isa_init(Object *obj)
>>> +{
>>> +    ViaISAState *s = VIA_ISA(obj);
>>> +
>>> +    object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>> +}
>>> +
>>> static const TypeInfo via_isa_info = {
>>>     .name          = TYPE_VIA_ISA,
>>>     .parent        = TYPE_PCI_DEVICE,
>>>     .instance_size = sizeof(ViaISAState),
>>> +    .instance_init = via_isa_init,
>>
>> Did you verify this is actually called? I guess you could add a debug printf in the init method above or check the output of info qom-tree to see if ide apears below via-isa. I'm not sure because I think QOM does not call superclass methods if you override them, that's why the subclass realize methods called via_isa_realize before. In this case it may not cause a problem because ide-via does not have an init method so it will work with just realize called so the only effect may be that qom-tree is not like it should be. Or if this is called then I still don't get QOM.
>
> We discussed the semantics of init() vs. realize() when discussing patch 
> 1 in v1 which consolidates realize() methods. My understanding is that 
> init() behaves like C++ constructors which are called implicitly parent 
> first, child next. OTOH realize() methods behave like virtual methods 
> which get replaced by the most specific one, i.e. one needs to call the 
> parent implementation explicitly.

Indeed, it seems object_init inits all parents recursively. This is a bit 
confusing and maybe not well ecplained in docs/devel/qom.rst at least it 
does not mention instance init func or I missed it but not sure how this 
could be clarified.

Regards,
BALATON Zoltan

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BB]

Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 31 Aug 2022, BB wrote:
>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>> The USB functions can be enabled/disabled through the ISA function. Also
>>>> its interrupt routing can be influenced there.
>>>> 
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>> ---
>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>> hw/mips/fuloong2e.c |  3 ---
>>>> hw/ppc/pegasos2.c   |  4 ----
>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>> 
>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>> index 9d946cea54..66a4b9c230 100644
>>>> --- a/hw/isa/vt82c686.c
>>>> +++ b/hw/isa/vt82c686.c
>>>> @@ -23,6 +23,7 @@
>>>> #include "hw/intc/i8259.h"
>>>> #include "hw/irq.h"
>>>> #include "hw/dma/i8257.h"
>>>> +#include "hw/usb/hcd-uhci.h"
>>>> #include "hw/timer/i8254.h"
>>>> #include "hw/rtc/mc146818rtc.h"
>>>> #include "migration/vmstate.h"
>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>     qemu_irq *isa_irqs;
>>>>     ViaSuperIOState via_sio;
>>>>     PCIIDEState ide;
>>>> +    UHCIState uhci[2];
>>>> };
>>>> 
>>>> static const VMStateDescription vmstate_via = {
>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>     ViaISAState *s = VIA_ISA(obj);
>>>> 
>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>> 
>>> Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?
>> 
>> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.
>
>I don't think it needs a separate header just for this so I'd put it in vt82c686.h but I don't mind either way.

Alright, I'll take the easy route for now. Splitting in dedicated headers (also for the other devices) could be done in a separate series.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BALATON Zoltan]

On Wed, 31 Aug 2022, BB wrote:
> Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>> On Wed, 31 Aug 2022, BB wrote:
>>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>>> The USB functions can be enabled/disabled through the ISA function. Also
>>>>> its interrupt routing can be influenced there.
>>>>>
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>> ---
>>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>>> hw/mips/fuloong2e.c |  3 ---
>>>>> hw/ppc/pegasos2.c   |  4 ----
>>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>> index 9d946cea54..66a4b9c230 100644
>>>>> --- a/hw/isa/vt82c686.c
>>>>> +++ b/hw/isa/vt82c686.c
>>>>> @@ -23,6 +23,7 @@
>>>>> #include "hw/intc/i8259.h"
>>>>> #include "hw/irq.h"
>>>>> #include "hw/dma/i8257.h"
>>>>> +#include "hw/usb/hcd-uhci.h"
>>>>> #include "hw/timer/i8254.h"
>>>>> #include "hw/rtc/mc146818rtc.h"
>>>>> #include "migration/vmstate.h"
>>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>>     qemu_irq *isa_irqs;
>>>>>     ViaSuperIOState via_sio;
>>>>>     PCIIDEState ide;
>>>>> +    UHCIState uhci[2];
>>>>> };
>>>>>
>>>>> static const VMStateDescription vmstate_via = {
>>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>>     ViaISAState *s = VIA_ISA(obj);
>>>>>
>>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0], "vt82c686b-usb-uhci");
>>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1], "vt82c686b-usb-uhci");
>>>>
>>>> Sorry for not saying this yesterday, this can also be done separately so no need for another version of this series if not needed for another reason but could we add a define for vt82c686b-usb-uhci in include/hw/isa/vt82c686.h and use that here and in hw/usb/vt82c686-uhci-pci.c ?
>>>
>>> Would creating a dedicated header work, too? Board code doesn't need to see the define any longer.
>>
>> I don't think it needs a separate header just for this so I'd put it in vt82c686.h but I don't mind either way.
>
> Alright, I'll take the easy route for now. Splitting in dedicated headers (also for the other devices) could be done in a separate series.

I'll do this for via-ac97 when rabasing my WIP patch:

https://osdn.net/projects/qmiga/scm/git/qemu/commits

as I'll need to move ViaAC97State there too for embedding in ViaISAState. 
The other ones 
can stay in vt82c686.h I think.

(The reason this is still WIP is that it does not work and I'm not sure 
why, Maybe I need to test with a Linux guest to find out more but I 
haven't got to that yet.)

Regards,
BALATON Zoltan

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[Bernhard Beschow]

[-- Attachment #1: Type: text/plain, Size: 3446 bytes --]

On Wed, Aug 31, 2022 at 6:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Wed, 31 Aug 2022, BB wrote:
> > Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <
> balaton@eik.bme.hu>:
> >> On Wed, 31 Aug 2022, BB wrote:
> >>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <
> balaton@eik.bme.hu>:
> >>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
> >>>>> The USB functions can be enabled/disabled through the ISA function.
> Also
> >>>>> its interrupt routing can be influenced there.
> >>>>>
> >>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> >>>>> ---
> >>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
> >>>>> hw/mips/fuloong2e.c |  3 ---
> >>>>> hw/ppc/pegasos2.c   |  4 ----
> >>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> >>>>> index 9d946cea54..66a4b9c230 100644
> >>>>> --- a/hw/isa/vt82c686.c
> >>>>> +++ b/hw/isa/vt82c686.c
> >>>>> @@ -23,6 +23,7 @@
> >>>>> #include "hw/intc/i8259.h"
> >>>>> #include "hw/irq.h"
> >>>>> #include "hw/dma/i8257.h"
> >>>>> +#include "hw/usb/hcd-uhci.h"
> >>>>> #include "hw/timer/i8254.h"
> >>>>> #include "hw/rtc/mc146818rtc.h"
> >>>>> #include "migration/vmstate.h"
> >>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
> >>>>>     qemu_irq *isa_irqs;
> >>>>>     ViaSuperIOState via_sio;
> >>>>>     PCIIDEState ide;
> >>>>> +    UHCIState uhci[2];
> >>>>> };
> >>>>>
> >>>>> static const VMStateDescription vmstate_via = {
> >>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
> >>>>>     ViaISAState *s = VIA_ISA(obj);
> >>>>>
> >>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
> >>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0],
> "vt82c686b-usb-uhci");
> >>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1],
> "vt82c686b-usb-uhci");
> >>>>
> >>>> Sorry for not saying this yesterday, this can also be done separately
> so no need for another version of this series if not needed for another
> reason but could we add a define for vt82c686b-usb-uhci in
> include/hw/isa/vt82c686.h and use that here and in
> hw/usb/vt82c686-uhci-pci.c ?
> >>>
> >>> Would creating a dedicated header work, too? Board code doesn't need
> to see the define any longer.
> >>
> >> I don't think it needs a separate header just for this so I'd put it in
> vt82c686.h but I don't mind either way.
> >
> > Alright, I'll take the easy route for now. Splitting in dedicated
> headers (also for the other devices) could be done in a separate series.
>
> I'll do this for via-ac97 when rabasing my WIP patch:
>
> https://osdn.net/projects/qmiga/scm/git/qemu/commits
>
> as I'll need to move ViaAC97State there too for embedding in ViaISAState.
> The other ones
> can stay in vt82c686.h I think.
>
> (The reason this is still WIP is that it does not work and I'm not sure
> why, Maybe I need to test with a Linux guest to find out more but I
> haven't got to that yet.)
>

Hi Zoltan,

I've given your AC97 patches a spin on top of my WIP pc-via branch with a
Mandriva Linux live CD and *drumroll* `qemu-system-x86_64 -M pc -accel kvm
-cpu host`:

https://github.com/shentok/qemu/commits/pc-via

The good news is that the sound controls appeared in the UI but no sound
seemed to be played, though that could also be due to my setup (nested
virtualization).

Perhaps you find it convenient to test with Linux that way.

Best regards,
Bernhard

>
> Regards,
> BALATON Zoltan
>

[-- Attachment #2: Type: text/html, Size: 5359 bytes --]

Re: [PATCH v3 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BALATON Zoltan]

On Thu, 1 Sep 2022, Bernhard Beschow wrote:
> On Wed, Aug 31, 2022 at 6:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> On Wed, 31 Aug 2022, BB wrote:
>>> Am 31. August 2022 17:03:35 MESZ schrieb BALATON Zoltan <
>> balaton@eik.bme.hu>:
>>>> On Wed, 31 Aug 2022, BB wrote:
>>>>> Am 31. August 2022 15:23:37 MESZ schrieb BALATON Zoltan <
>> balaton@eik.bme.hu>:
>>>>>> On Wed, 31 Aug 2022, Bernhard Beschow wrote:
>>>>>>> The USB functions can be enabled/disabled through the ISA function.
>> Also
>>>>>>> its interrupt routing can be influenced there.
>>>>>>>
>>>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>>>> ---
>>>>>>> hw/isa/vt82c686.c   | 12 ++++++++++++
>>>>>>> hw/mips/fuloong2e.c |  3 ---
>>>>>>> hw/ppc/pegasos2.c   |  4 ----
>>>>>>> 3 files changed, 12 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>>>>>>> index 9d946cea54..66a4b9c230 100644
>>>>>>> --- a/hw/isa/vt82c686.c
>>>>>>> +++ b/hw/isa/vt82c686.c
>>>>>>> @@ -23,6 +23,7 @@
>>>>>>> #include "hw/intc/i8259.h"
>>>>>>> #include "hw/irq.h"
>>>>>>> #include "hw/dma/i8257.h"
>>>>>>> +#include "hw/usb/hcd-uhci.h"
>>>>>>> #include "hw/timer/i8254.h"
>>>>>>> #include "hw/rtc/mc146818rtc.h"
>>>>>>> #include "migration/vmstate.h"
>>>>>>> @@ -546,6 +547,7 @@ struct ViaISAState {
>>>>>>>     qemu_irq *isa_irqs;
>>>>>>>     ViaSuperIOState via_sio;
>>>>>>>     PCIIDEState ide;
>>>>>>> +    UHCIState uhci[2];
>>>>>>> };
>>>>>>>
>>>>>>> static const VMStateDescription vmstate_via = {
>>>>>>> @@ -563,6 +565,8 @@ static void via_isa_init(Object *obj)
>>>>>>>     ViaISAState *s = VIA_ISA(obj);
>>>>>>>
>>>>>>>     object_initialize_child(obj, "ide", &s->ide, "via-ide");
>>>>>>> +    object_initialize_child(obj, "uhci1", &s->uhci[0],
>> "vt82c686b-usb-uhci");
>>>>>>> +    object_initialize_child(obj, "uhci2", &s->uhci[1],
>> "vt82c686b-usb-uhci");
>>>>>>
>>>>>> Sorry for not saying this yesterday, this can also be done separately
>> so no need for another version of this series if not needed for another
>> reason but could we add a define for vt82c686b-usb-uhci in
>> include/hw/isa/vt82c686.h and use that here and in
>> hw/usb/vt82c686-uhci-pci.c ?
>>>>>
>>>>> Would creating a dedicated header work, too? Board code doesn't need
>> to see the define any longer.
>>>>
>>>> I don't think it needs a separate header just for this so I'd put it in
>> vt82c686.h but I don't mind either way.
>>>
>>> Alright, I'll take the easy route for now. Splitting in dedicated
>> headers (also for the other devices) could be done in a separate series.
>>
>> I'll do this for via-ac97 when rabasing my WIP patch:
>>
>> https://osdn.net/projects/qmiga/scm/git/qemu/commits
>>
>> as I'll need to move ViaAC97State there too for embedding in ViaISAState.
>> The other ones
>> can stay in vt82c686.h I think.
>>
>> (The reason this is still WIP is that it does not work and I'm not sure
>> why, Maybe I need to test with a Linux guest to find out more but I
>> haven't got to that yet.)
>>
>
> Hi Zoltan,
>
> I've given your AC97 patches a spin on top of my WIP pc-via branch with a
> Mandriva Linux live CD and *drumroll* `qemu-system-x86_64 -M pc -accel kvm
> -cpu host`:
>
> https://github.com/shentok/qemu/commits/pc-via

Interesting, now I see where this goes beyond just clean up.

> The good news is that the sound controls appeared in the UI but no sound
> seemed to be played, though that could also be due to my setup (nested
> virtualization).

Consodering that I get the same result with MorphOS on pegasos2 it's more 
likely some problem with the emulation than your setup but I could not yet 
find out what (I didn't try hard enough either). Probably I'm missing 
something in how sound emulation in QEMU should work or how the via sound 
function should work. The docs have detailed info in the regs but not much 
on what actually should happen, when irq should be raised and such.

> Perhaps you find it convenient to test with Linux that way.

Definitly, it's easier to find an x86 live CD with support for this chip 
than one for pegasos2. But I may wait a while before I get back to this.

Regards,
BALATON Zoltan