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

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

[Bernhard Beschow]

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

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                            | 119 +++++++++++++++----
 hw/mips/fuloong2e.c                          |  21 ++--
 hw/ppc/Kconfig                               |   1 -
 hw/ppc/pegasos2.c                            |  25 ++--
 include/hw/isa/vt82c686.h                    |   2 -
 7 files changed, 115 insertions(+), 55 deletions(-)

-- 
2.37.2

[PATCH v2 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.2

[PATCH v2 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.2

[PATCH v2 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.2

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

[BALATON Zoltan]

On Tue, 30 Aug 2022, Bernhard Beschow wrote:
> Unlike get_system_memory(), pci_address_space() respects the memory tree
> available to the parent device.

I don't know if this is correct but likely nothing will use it so probably 
does not break anything.

Regards,
BALATON Zoltan

> 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);
>

[PATCH v2 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.2

[PATCH v2 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.2

[PATCH v2 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..6aba7f29de 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.2

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

[BALATON Zoltan]

On Tue, 30 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..6aba7f29de 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) {

It does not really matter but Usually i++ is used in for loops so seeing 
++i here is a bit odd but this alone isn't worth a new version.

Regards,
BALATON Zoltan

> +        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 v2 06/10] hw/isa/vt82c686: Instantiate USB functions in host device

[BB]

Am 31. August 2022 00:33:33 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Tue, 30 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..6aba7f29de 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) {
>
>It does not really matter but Usually i++ is used in for loops so seeing ++i here is a bit odd but this alone isn't worth a new version.

Fixed in v3.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> +        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"));
>> 

[PATCH v2 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         | 12 ++++++++++++
 hw/mips/fuloong2e.c       |  2 +-
 hw/ppc/pegasos2.c         |  3 +--
 include/hw/isa/vt82c686.h |  2 --
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 6aba7f29de..4e66570655 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -40,6 +40,9 @@
 #define TYPE_VIA_PM "via-pm"
 OBJECT_DECLARE_SIMPLE_TYPE(ViaPMState, VIA_PM)
 
+#define TYPE_VT82C686B_PM "vt82c686b-pm"
+#define TYPE_VT8231_PM "vt8231-pm"
+
 struct ViaPMState {
     PCIDevice dev;
     MemoryRegion io;
@@ -548,6 +551,7 @@ struct ViaISAState {
     ViaSuperIOState via_sio;
     PCIIDEState ide;
     UHCIState uhci[2];
+    ViaPMState pm;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -641,6 +645,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 +693,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 +757,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.2

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

[BALATON Zoltan]

On Tue, 30 Aug 2022, Bernhard Beschow wrote:
> 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         | 12 ++++++++++++
> hw/mips/fuloong2e.c       |  2 +-
> hw/ppc/pegasos2.c         |  3 +--
> include/hw/isa/vt82c686.h |  2 --
> 4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 6aba7f29de..4e66570655 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -40,6 +40,9 @@
> #define TYPE_VIA_PM "via-pm"
> OBJECT_DECLARE_SIMPLE_TYPE(ViaPMState, VIA_PM)
>
> +#define TYPE_VT82C686B_PM "vt82c686b-pm"
> +#define TYPE_VT8231_PM "vt8231-pm"

These defines should be further down before vt82c686b_pm_init_info and 
vt8231_pm_init_info respectively to keep object class definitions 
together. Here the generic abstract superclass is defined, followed be the 
specific chips so it's too early to define these at this point.

Regards,
BALATON Zoltan

> +
> struct ViaPMState {
>     PCIDevice dev;
>     MemoryRegion io;
> @@ -548,6 +551,7 @@ struct ViaISAState {
>     ViaSuperIOState via_sio;
>     PCIIDEState ide;
>     UHCIState uhci[2];
> +    ViaPMState pm;
> };
>
> static const VMStateDescription vmstate_via = {
> @@ -641,6 +645,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 +693,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 +757,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"
>
>

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

[BB]

Am 31. August 2022 00:39:22 MESZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Tue, 30 Aug 2022, Bernhard Beschow wrote:
>> 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         | 12 ++++++++++++
>> hw/mips/fuloong2e.c       |  2 +-
>> hw/ppc/pegasos2.c         |  3 +--
>> include/hw/isa/vt82c686.h |  2 --
>> 4 files changed, 14 insertions(+), 5 deletions(-)
>> 
>> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
>> index 6aba7f29de..4e66570655 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -40,6 +40,9 @@
>> #define TYPE_VIA_PM "via-pm"
>> OBJECT_DECLARE_SIMPLE_TYPE(ViaPMState, VIA_PM)
>> 
>> +#define TYPE_VT82C686B_PM "vt82c686b-pm"
>> +#define TYPE_VT8231_PM "vt8231-pm"
>
>These defines should be further down before vt82c686b_pm_init_info and vt8231_pm_init_info respectively to keep object class definitions together. Here the generic abstract superclass is defined, followed be the specific chips so it's too early to define these at this point.

Right. Fixed in v3.

Regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> +
>> struct ViaPMState {
>>     PCIDevice dev;
>>     MemoryRegion io;
>> @@ -548,6 +551,7 @@ struct ViaISAState {
>>     ViaSuperIOState via_sio;
>>     PCIIDEState ide;
>>     UHCIState uhci[2];
>> +    ViaPMState pm;
>> };
>> 
>> static const VMStateDescription vmstate_via = {
>> @@ -641,6 +645,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 +693,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 +757,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"
>> 
>> 

[PATCH v2 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 4e66570655..150e9401d0 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -552,6 +552,8 @@ struct ViaISAState {
     PCIIDEState ide;
     UHCIState uhci[2];
     ViaPMState pm;
+    PCIDevice ac97;
+    PCIDevice mc97;
 };
 
 static const VMStateDescription vmstate_via = {
@@ -571,6 +573,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 = {
@@ -651,6 +655,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.2

[PATCH v2 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 150e9401d0..0ef9446374 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -549,6 +549,7 @@ struct ViaISAState {
     qemu_irq cpu_intr;
     qemu_irq *isa_irqs;
     ViaSuperIOState via_sio;
+    RTCState rtc;
     PCIIDEState ide;
     UHCIState uhci[2];
     ViaPMState pm;
@@ -570,6 +571,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");
@@ -623,7 +625,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.2

[PATCH v2 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 0ef9446374..a23ffbb3ff 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -631,8 +631,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.2

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

[Philippe Mathieu-Daudé via]

On 30/8/22 21:00, Bernhard Beschow wrote:
> 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.

IIUC, this is only true for Pegasos II, not (yet) for the Fuloong 2E.

> 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 0ef9446374..a23ffbb3ff 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -631,8 +631,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",

We already have a 'machine' pointer.

> +                              object_resolve_path_component(OBJECT(via),
> +                                                            "rtc"),
> +                              "date");
>       qdev_connect_gpio_out(DEVICE(via), 0,
>                             qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>   

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

[Bernhard Beschow]

Am 30. August 2022 21:46:57 UTC schrieb "Philippe Mathieu-Daudé" <f4bug@amsat.org>:
>On 30/8/22 21:00, Bernhard Beschow wrote:
>> 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.
>
>IIUC, this is only true for Pegasos II, not (yet) for the Fuloong 2E.
>
>> 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 0ef9446374..a23ffbb3ff 100644
>> --- a/hw/isa/vt82c686.c
>> +++ b/hw/isa/vt82c686.c
>> @@ -631,8 +631,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",
>
>We already have a 'machine' pointer.

Fixed in v5.

>
>> +                              object_resolve_path_component(OBJECT(via),
>> +                                                            "rtc"),
>> +                              "date");
>>       qdev_connect_gpio_out(DEVICE(via), 0,
>>                             qdev_get_gpio_in_named(pm->mv, "gpp", 31));
>>   
>

Re: [PATCH v2 00/10] Instantiate VT82xx functions in host device

[Philippe Mathieu-Daudé via]

On 30/8/22 21:00, Bernhard Beschow wrote:
> 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
> 
> 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.

Excellent!

Series:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 00/10] Instantiate VT82xx functions in host device

[Philippe Mathieu-Daudé via]

On 30/8/22 21:00, Bernhard Beschow wrote:
> 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
> 
> 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.

Excellent!

Series:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>