[PATCH 0/4] mips: do not list individual devices from configs/

[PATCH 0/4] mips: do not list individual devices from configs/

[Paolo Bonzini]

Back when Kconfig was introduced, the individual dependencies for MIPS
boards were never added to hw/mips/Kconfig.  Do it now.

To simplify the task, include a couple cleanups to the SuperIO chip
configuration symbols, as well as a change that makes USB device
creation available even when building without default devices.

Tested by comparing old and new kconfigs; and also by building each of
the boards one by one, with default devices disabled, and checking that
the board can be started.

Paolo

Paolo Bonzini (4):
  isa: clean up Kconfig selections for ISA_SUPERIO
  isa: extract FDC37M81X to a separate file
  usb: inline device creation functions
  mips: do not list individual devices from configs/

 configs/devices/mips-softmmu/common.mak      | 28 +++------------
 configs/devices/mips64el-softmmu/default.mak |  3 --
 include/hw/usb.h                             | 26 ++++++++++++--
 hw/isa/fdc37m81x-superio.c                   | 37 ++++++++++++++++++++
 hw/isa/isa-superio.c                         | 19 ----------
 hw/mips/loongson3_virt.c                     |  5 +--
 hw/usb/bus.c                                 | 23 ------------
 hw/display/Kconfig                           |  2 +-
 hw/isa/Kconfig                               | 18 +++++-----
 hw/isa/meson.build                           |  1 +
 hw/mips/Kconfig                              | 25 ++++++++++---
 11 files changed, 99 insertions(+), 88 deletions(-)
 create mode 100644 hw/isa/fdc37m81x-superio.c

-- 
2.43.0

[PATCH 1/4] isa: clean up Kconfig selections for ISA_SUPERIO

[Paolo Bonzini]

All users of ISA_SUPERIO include a floppy disk controller, serial port
and parallel port via the automatic creation mechanism of isa-superio.c.

Select the symbol and remove it from the dependents.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/Kconfig | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 040a18c0709..7884179d08b 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -17,7 +17,11 @@ config ISA_SUPERIO
     bool
     select ISA_BUS
     select PCKBD
+    select PARALLEL
+    select SERIAL_ISA
     select FDC_ISA
+    # Some users of ISA_SUPERIO do not use it
+    #select IDE_ISA
 
 config PC87312
     bool
@@ -26,9 +30,6 @@ config PC87312
     select I8254
     select I8257
     select MC146818RTC
-    select SERIAL_ISA
-    select PARALLEL
-    select FDC_ISA
     select IDE_ISA
 
 config PIIX
@@ -49,8 +50,6 @@ config VT82C686
     select ISA_SUPERIO
     select ACPI
     select ACPI_SMBUS
-    select SERIAL_ISA
-    select FDC_ISA
     select USB_UHCI
     select APM
     select I8254
@@ -58,14 +57,10 @@ config VT82C686
     select I8259
     select IDE_VIA
     select MC146818RTC
-    select PARALLEL
 
 config SMC37C669
     bool
     select ISA_SUPERIO
-    select SERIAL_ISA
-    select PARALLEL
-    select FDC_ISA
 
 config LPC_ICH9
     bool
-- 
2.43.0

[PATCH 2/4] isa: extract FDC37M81X to a separate file

[Paolo Bonzini]

isa-superio.c currently defines a SuperIO chip that depends on
CONFIG_IDE_ISA, but not all users of isa-superio.c depend on that
symbol.  Extract the chip to a separate file so that there is an
obvious place to select IDE_ISA.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa/fdc37m81x-superio.c | 37 +++++++++++++++++++++++++++++++++++++
 hw/isa/isa-superio.c       | 19 -------------------
 hw/isa/Kconfig             |  5 +++++
 hw/isa/meson.build         |  1 +
 hw/mips/Kconfig            |  2 +-
 5 files changed, 44 insertions(+), 20 deletions(-)
 create mode 100644 hw/isa/fdc37m81x-superio.c

diff --git a/hw/isa/fdc37m81x-superio.c b/hw/isa/fdc37m81x-superio.c
new file mode 100644
index 00000000000..5ad769b69e8
--- /dev/null
+++ b/hw/isa/fdc37m81x-superio.c
@@ -0,0 +1,37 @@
+/*
+ * Generic ISA Super I/O
+ *
+ * Copyright (c) 2018 Philippe Mathieu-Daudé
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "hw/isa/superio.h"
+
+/* SMS FDC37M817 Super I/O */
+static void fdc37m81x_class_init(ObjectClass *klass, void *data)
+{
+    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
+
+    sc->serial.count = 2; /* NS16C550A */
+    sc->parallel.count = 1;
+    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
+    sc->ide.count = 0;
+}
+
+static const TypeInfo fdc37m81x_type_info = {
+    .name          = TYPE_FDC37M81X_SUPERIO,
+    .parent        = TYPE_ISA_SUPERIO,
+    .instance_size = sizeof(ISASuperIODevice),
+    .class_init    = fdc37m81x_class_init,
+};
+
+static void fdc37m81x_register_types(void)
+{
+    type_register_static(&fdc37m81x_type_info);
+}
+
+type_init(fdc37m81x_register_types)
diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
index 7dbfc374da3..80e0c34652e 100644
--- a/hw/isa/isa-superio.c
+++ b/hw/isa/isa-superio.c
@@ -187,28 +187,9 @@ static const TypeInfo isa_superio_type_info = {
     .class_init = isa_superio_class_init,
 };
 
-/* SMS FDC37M817 Super I/O */
-static void fdc37m81x_class_init(ObjectClass *klass, void *data)
-{
-    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
-
-    sc->serial.count = 2; /* NS16C550A */
-    sc->parallel.count = 1;
-    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
-    sc->ide.count = 0;
-}
-
-static const TypeInfo fdc37m81x_type_info = {
-    .name          = TYPE_FDC37M81X_SUPERIO,
-    .parent        = TYPE_ISA_SUPERIO,
-    .instance_size = sizeof(ISASuperIODevice),
-    .class_init    = fdc37m81x_class_init,
-};
-
 static void isa_superio_register_types(void)
 {
     type_register_static(&isa_superio_type_info);
-    type_register_static(&fdc37m81x_type_info);
 }
 
 type_init(isa_superio_register_types)
diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index 7884179d08b..af856af052d 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -23,6 +23,11 @@ config ISA_SUPERIO
     # Some users of ISA_SUPERIO do not use it
     #select IDE_ISA
 
+config FDC37M81X
+    bool
+    select ISA_BUS
+    select IDE_ISA
+
 config PC87312
     bool
     select ISA_SUPERIO
diff --git a/hw/isa/meson.build b/hw/isa/meson.build
index 2ab99ce0c6b..f650b395071 100644
--- a/hw/isa/meson.build
+++ b/hw/isa/meson.build
@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
 system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
 system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
 system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
 system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
 system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index ab61af209a0..94b969c21db 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,7 +1,7 @@
 config MALTA
     bool
+    select FDC37M81X
     select GT64120
-    select ISA_SUPERIO
     select PIIX
 
 config MIPSSIM
-- 
2.43.0

[PATCH 3/4] usb: inline device creation functions

[Paolo Bonzini]

Allow boards to use the device creation functions even if USB itself
is not available; of course the functions will fail inexorably, but
this can be okay if the calls are conditional on the existence of
some USB host controller device.  This is for example the case for
hw/mips/loongson3_virt.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/usb.h | 26 +++++++++++++++++++++++---
 hw/usb/bus.c     | 23 -----------------------
 2 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index 32c23a5ca2a..bd76c514d17 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -500,9 +500,6 @@ void usb_bus_release(USBBus *bus);
 USBBus *usb_bus_find(int busnr);
 void usb_legacy_register(const char *typename, const char *usbdevice_name,
                          USBDevice *(*usbdevice_init)(void));
-USBDevice *usb_new(const char *name);
-bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp);
-USBDevice *usb_create_simple(USBBus *bus, const char *name);
 USBDevice *usbdevice_create(const char *cmdline);
 void usb_register_port(USBBus *bus, USBPort *port, void *opaque, int index,
                        USBPortOps *ops, int speedmask);
@@ -582,4 +579,27 @@ void usb_pcap_init(FILE *fp);
 void usb_pcap_ctrl(USBPacket *p, bool setup);
 void usb_pcap_data(USBPacket *p, bool setup);
 
+static inline USBDevice *usb_new(const char *name)
+{
+    return USB_DEVICE(qdev_new(name));
+}
+
+static inline USBDevice *usb_try_new(const char *name)
+{
+    return USB_DEVICE(qdev_try_new(name));
+}
+
+static inline bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
+{
+    return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
+}
+
+static inline USBDevice *usb_create_simple(USBBus *bus, const char *name)
+{
+    USBDevice *dev = usb_new(name);
+
+    usb_realize_and_unref(dev, bus, &error_abort);
+    return dev;
+}
+
 #endif
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 59c39945ddd..76fda41b7ec 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -329,29 +329,6 @@ void usb_legacy_register(const char *typename, const char *usbdevice_name,
     }
 }
 
-USBDevice *usb_new(const char *name)
-{
-    return USB_DEVICE(qdev_new(name));
-}
-
-static USBDevice *usb_try_new(const char *name)
-{
-    return USB_DEVICE(qdev_try_new(name));
-}
-
-bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp)
-{
-    return qdev_realize_and_unref(&dev->qdev, &bus->qbus, errp);
-}
-
-USBDevice *usb_create_simple(USBBus *bus, const char *name)
-{
-    USBDevice *dev = usb_new(name);
-
-    usb_realize_and_unref(dev, bus, &error_abort);
-    return dev;
-}
-
 static void usb_fill_port(USBPort *port, void *opaque, int index,
                           USBPortOps *ops, int speedmask)
 {
-- 
2.43.0

[PATCH 4/4] mips: do not list individual devices from configs/

[Paolo Bonzini]

Add new "select" and "imply" directives if needed.  The resulting
config-devices.mak files are the same as before.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configs/devices/mips-softmmu/common.mak      | 28 +++-----------------
 configs/devices/mips64el-softmmu/default.mak |  3 ---
 hw/mips/loongson3_virt.c                     |  5 ++--
 hw/display/Kconfig                           |  2 +-
 hw/mips/Kconfig                              | 23 +++++++++++++---
 5 files changed, 28 insertions(+), 33 deletions(-)

diff --git a/configs/devices/mips-softmmu/common.mak b/configs/devices/mips-softmmu/common.mak
index 1a853841b27..416a5d353e8 100644
--- a/configs/devices/mips-softmmu/common.mak
+++ b/configs/devices/mips-softmmu/common.mak
@@ -1,28 +1,8 @@
 # Common mips*-softmmu CONFIG defines
 
-CONFIG_ISA_BUS=y
-CONFIG_PCI=y
-CONFIG_PCI_DEVICES=y
-CONFIG_VGA_ISA=y
-CONFIG_VGA_MMIO=y
-CONFIG_VGA_CIRRUS=y
-CONFIG_VMWARE_VGA=y
-CONFIG_SERIAL=y
-CONFIG_SERIAL_ISA=y
-CONFIG_PARALLEL=y
-CONFIG_I8254=y
-CONFIG_PCSPK=y
-CONFIG_PCKBD=y
-CONFIG_FDC=y
-CONFIG_I8257=y
-CONFIG_IDE_ISA=y
-CONFIG_PFLASH_CFI01=y
-CONFIG_I8259=y
-CONFIG_MC146818RTC=y
-CONFIG_MIPS_CPS=y
-CONFIG_MIPS_ITU=y
+# Uncomment the following lines to disable these optional devices:
+# CONFIG_PCI_DEVICES=n
+# CONFIG_TEST_DEVICES=n
+
 CONFIG_MALTA=y
-CONFIG_PCNET_PCI=y
 CONFIG_MIPSSIM=y
-CONFIG_SMBUS_EEPROM=y
-CONFIG_TEST_DEVICES=y
diff --git a/configs/devices/mips64el-softmmu/default.mak b/configs/devices/mips64el-softmmu/default.mak
index d5188f7ea58..88a37cf27f1 100644
--- a/configs/devices/mips64el-softmmu/default.mak
+++ b/configs/devices/mips64el-softmmu/default.mak
@@ -3,8 +3,5 @@
 include ../mips-softmmu/common.mak
 CONFIG_FULOONG=y
 CONFIG_LOONGSON3V=y
-CONFIG_ATI_VGA=y
-CONFIG_RTL8139_PCI=y
 CONFIG_JAZZ=y
-CONFIG_VT82C686=y
 CONFIG_MIPS_BOSTON=y
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 33eae01eca2..da47af2fa71 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -447,8 +447,9 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
 
     if (defaults_enabled() && object_class_by_name("pci-ohci")) {
         pci_create_simple(pci_bus, -1, "pci-ohci");
-        usb_create_simple(usb_bus_find(-1), "usb-kbd");
-        usb_create_simple(usb_bus_find(-1), "usb-tablet");
+        Object *usb_bus = object_resolve_path_type("", TYPE_USB_BUS, NULL);
+        usb_create_simple(USB_BUS(usb_bus), "usb-kbd");
+        usb_create_simple(USB_BUS(usb_bus), "usb-tablet");
     }
 
     for (i = 0; i < nb_nics; i++) {
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1aafe1923d2..5b2b3840f7a 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -55,7 +55,7 @@ config VGA_MMIO
 
 config VMWARE_VGA
     bool
-    default y if PCI_DEVICES && PC_PCI
+    default y if PCI_DEVICES && (PC_PCI || MIPS)
     depends on PCI
     select VGA
 
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index 94b969c21db..5c83ef49cf6 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -1,13 +1,19 @@
 config MALTA
     bool
+    imply PCNET_PCI
+    imply PCI_DEVICES
+    imply TEST_DEVICES
     select FDC37M81X
     select GT64120
+    select MIPS_CPS
     select PIIX
+    select PFLASH_CFI01
+    select SERIAL
+    select SMBUS_EEPROM
 
 config MIPSSIM
     bool
-    select ISA_BUS
-    select SERIAL_ISA
+    select SERIAL
     select MIPSNET
 
 config JAZZ
@@ -32,17 +38,26 @@ config JAZZ
 
 config FULOONG
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
+    imply ATI_VGA
+    imply RTL8139_PCI
     select PCI_BONITO
+    select SMBUS_EEPROM
     select VT82C686
 
 config LOONGSON3V
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
+    imply VIRTIO_PCI
+    imply VIRTIO_NET
     imply VIRTIO_VGA
     imply QXL if SPICE
+    imply USB_OHCI_PCI
     select SERIAL
     select GOLDFISH_RTC
     select LOONGSON_LIOINTC
-    select PCI_DEVICES
     select PCI_EXPRESS_GENERIC_BRIDGE
     select MSI_NONBROKEN
     select FW_CFG_MIPS
@@ -54,6 +69,8 @@ config MIPS_CPS
 
 config MIPS_BOSTON
     bool
+    imply PCI_DEVICES
+    imply TEST_DEVICES
     select FITLOADER
     select MIPS_CPS
     select PCI_EXPRESS_XILINX
-- 
2.43.0

Re: [PATCH 0/4] mips: do not list individual devices from configs/

[Bernhard Beschow]

Am 29. Januar 2024 13:37:44 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>Back when Kconfig was introduced, the individual dependencies for MIPS
>boards were never added to hw/mips/Kconfig.  Do it now.

There is also: https://patchew.org/QEMU/20230109204124.102592-1-shentey@gmail.com/

Best regards,
Bernhard

>
>To simplify the task, include a couple cleanups to the SuperIO chip
>configuration symbols, as well as a change that makes USB device
>creation available even when building without default devices.
>
>Tested by comparing old and new kconfigs; and also by building each of
>the boards one by one, with default devices disabled, and checking that
>the board can be started.
>
>Paolo
>
>Paolo Bonzini (4):
>  isa: clean up Kconfig selections for ISA_SUPERIO
>  isa: extract FDC37M81X to a separate file
>  usb: inline device creation functions
>  mips: do not list individual devices from configs/
>
> configs/devices/mips-softmmu/common.mak      | 28 +++------------
> configs/devices/mips64el-softmmu/default.mak |  3 --
> include/hw/usb.h                             | 26 ++++++++++++--
> hw/isa/fdc37m81x-superio.c                   | 37 ++++++++++++++++++++
> hw/isa/isa-superio.c                         | 19 ----------
> hw/mips/loongson3_virt.c                     |  5 +--
> hw/usb/bus.c                                 | 23 ------------
> hw/display/Kconfig                           |  2 +-
> hw/isa/Kconfig                               | 18 +++++-----
> hw/isa/meson.build                           |  1 +
> hw/mips/Kconfig                              | 25 ++++++++++---
> 11 files changed, 99 insertions(+), 88 deletions(-)
> create mode 100644 hw/isa/fdc37m81x-superio.c
>

Re: [PATCH 0/4] mips: do not list individual devices from configs/

[Paolo Bonzini]

On Mon, Jan 29, 2024 at 6:48 PM Bernhard Beschow <shentey@gmail.com> wrote:
> Am 29. Januar 2024 13:37:44 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
> >Back when Kconfig was introduced, the individual dependencies for MIPS
> >boards were never added to hw/mips/Kconfig.  Do it now.
>
> There is also: https://patchew.org/QEMU/20230109204124.102592-1-shentey@gmail.com/

Oh, that's a pity. Looks like your version doesn't apply anymore, but
we can salvage "[PATCH 2/4] hw/mips/Kconfig: Remove ISA dependencies
from MIPSsim board" which is slightly more complete than my version.

Paolo

Re: [PATCH 0/4] mips: do not list individual devices from configs/

[Bernhard Beschow]

Am 29. Januar 2024 18:16:08 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>On Mon, Jan 29, 2024 at 6:48 PM Bernhard Beschow <shentey@gmail.com> wrote:
>> Am 29. Januar 2024 13:37:44 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>> >Back when Kconfig was introduced, the individual dependencies for MIPS
>> >boards were never added to hw/mips/Kconfig.  Do it now.
>>
>> There is also: https://patchew.org/QEMU/20230109204124.102592-1-shentey@gmail.com/
>
>Oh, that's a pity. Looks like your version doesn't apply anymore, but
>we can salvage "[PATCH 2/4] hw/mips/Kconfig: Remove ISA dependencies
>from MIPSsim board" which is slightly more complete than my version.

Sounds good to me!

Bernhard
>
>Paolo
>

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Bernhard Beschow]

Am 29. Januar 2024 13:37:46 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>isa-superio.c currently defines a SuperIO chip that depends on
>CONFIG_IDE_ISA, but not all users of isa-superio.c depend on that
>symbol.  Extract the chip to a separate file so that there is an
>obvious place to select IDE_ISA.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> hw/isa/fdc37m81x-superio.c | 37 +++++++++++++++++++++++++++++++++++++
> hw/isa/isa-superio.c       | 19 -------------------
> hw/isa/Kconfig             |  5 +++++
> hw/isa/meson.build         |  1 +
> hw/mips/Kconfig            |  2 +-
> 5 files changed, 44 insertions(+), 20 deletions(-)
> create mode 100644 hw/isa/fdc37m81x-superio.c
>
>diff --git a/hw/isa/fdc37m81x-superio.c b/hw/isa/fdc37m81x-superio.c
>new file mode 100644
>index 00000000000..5ad769b69e8
>--- /dev/null
>+++ b/hw/isa/fdc37m81x-superio.c
>@@ -0,0 +1,37 @@
>+/*
>+ * Generic ISA Super I/O

Replace this description...

>+ *
>+ * Copyright (c) 2018 Philippe Mathieu-Daudé
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ * SPDX-License-Identifier: GPL-2.0-or-later
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "hw/isa/superio.h"
>+
>+/* SMS FDC37M817 Super I/O */

... with this one?

>+static void fdc37m81x_class_init(ObjectClass *klass, void *data)
>+{
>+    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
>+
>+    sc->serial.count = 2; /* NS16C550A */
>+    sc->parallel.count = 1;
>+    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
>+    sc->ide.count = 0;
>+}
>+

Don't we prefer a macro for below code? While touching the code we could use it. (Sorry I can't recall its name from the top of my head and I don't have access to the code right now).

Best regards,
Bernhard

>+static const TypeInfo fdc37m81x_type_info = {
>+    .name          = TYPE_FDC37M81X_SUPERIO,
>+    .parent        = TYPE_ISA_SUPERIO,
>+    .instance_size = sizeof(ISASuperIODevice),
>+    .class_init    = fdc37m81x_class_init,
>+};
>+
>+static void fdc37m81x_register_types(void)
>+{
>+    type_register_static(&fdc37m81x_type_info);
>+}
>+
>+type_init(fdc37m81x_register_types)
>diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
>index 7dbfc374da3..80e0c34652e 100644
>--- a/hw/isa/isa-superio.c
>+++ b/hw/isa/isa-superio.c
>@@ -187,28 +187,9 @@ static const TypeInfo isa_superio_type_info = {
>     .class_init = isa_superio_class_init,
> };
> 
>-/* SMS FDC37M817 Super I/O */
>-static void fdc37m81x_class_init(ObjectClass *klass, void *data)
>-{
>-    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
>-
>-    sc->serial.count = 2; /* NS16C550A */
>-    sc->parallel.count = 1;
>-    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
>-    sc->ide.count = 0;
>-}
>-
>-static const TypeInfo fdc37m81x_type_info = {
>-    .name          = TYPE_FDC37M81X_SUPERIO,
>-    .parent        = TYPE_ISA_SUPERIO,
>-    .instance_size = sizeof(ISASuperIODevice),
>-    .class_init    = fdc37m81x_class_init,
>-};
>-
> static void isa_superio_register_types(void)
> {
>     type_register_static(&isa_superio_type_info);
>-    type_register_static(&fdc37m81x_type_info);
> }
> 
> type_init(isa_superio_register_types)
>diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>index 7884179d08b..af856af052d 100644
>--- a/hw/isa/Kconfig
>+++ b/hw/isa/Kconfig
>@@ -23,6 +23,11 @@ config ISA_SUPERIO
>     # Some users of ISA_SUPERIO do not use it
>     #select IDE_ISA
> 
>+config FDC37M81X
>+    bool
>+    select ISA_BUS
>+    select IDE_ISA
>+
> config PC87312
>     bool
>     select ISA_SUPERIO
>diff --git a/hw/isa/meson.build b/hw/isa/meson.build
>index 2ab99ce0c6b..f650b395071 100644
>--- a/hw/isa/meson.build
>+++ b/hw/isa/meson.build
>@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
> system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
> system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
> system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
>+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
> system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
> system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
> 
>diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>index ab61af209a0..94b969c21db 100644
>--- a/hw/mips/Kconfig
>+++ b/hw/mips/Kconfig
>@@ -1,7 +1,7 @@
> config MALTA
>     bool
>+    select FDC37M81X
>     select GT64120
>-    select ISA_SUPERIO
>     select PIIX
> 
> config MIPSSIM

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Paolo Bonzini]

On Mon, Jan 29, 2024 at 8:49 PM Bernhard Beschow <shentey@gmail.com> wrote:
> Don't we prefer a macro for below code? While touching the code we could use it. (Sorry I can't recall its name from the top of my head and I don't have access to the code right now).

Ah yeah, OBJECT_DEFINE_TYPE. Not sure it's much of a win because
neither finalize or instance_init are required here.

Paolo

Re: [PATCH 1/4] isa: clean up Kconfig selections for ISA_SUPERIO

[Richard Henderson]

On 1/29/24 23:37, Paolo Bonzini wrote:
> All users of ISA_SUPERIO include a floppy disk controller, serial port
> and parallel port via the automatic creation mechanism of isa-superio.c.
> 
> Select the symbol and remove it from the dependents.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   hw/isa/Kconfig | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 3/4] usb: inline device creation functions

[Richard Henderson]

On 1/29/24 23:37, Paolo Bonzini wrote:
> Allow boards to use the device creation functions even if USB itself
> is not available; of course the functions will fail inexorably, but
> this can be okay if the calls are conditional on the existence of
> some USB host controller device.  This is for example the case for
> hw/mips/loongson3_virt.c.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   include/hw/usb.h | 26 +++++++++++++++++++++++---
>   hw/usb/bus.c     | 23 -----------------------
>   2 files changed, 23 insertions(+), 26 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Philippe Mathieu-Daudé]

On 29/1/24 21:26, Paolo Bonzini wrote:
> On Mon, Jan 29, 2024 at 8:49 PM Bernhard Beschow <shentey@gmail.com> wrote:
>> Don't we prefer a macro for below code? While touching the code we could use it. (Sorry I can't recall its name from the top of my head and I don't have access to the code right now).
> 
> Ah yeah, OBJECT_DEFINE_TYPE. Not sure it's much of a win because
> neither finalize or instance_init are required here.

Bernhard likely meant type_init() -> DEFINE_TYPES().

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Bernhard Beschow]

Am 30. Januar 2024 07:48:50 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 29/1/24 21:26, Paolo Bonzini wrote:
>> On Mon, Jan 29, 2024 at 8:49 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>> Don't we prefer a macro for below code? While touching the code we could use it. (Sorry I can't recall its name from the top of my head and I don't have access to the code right now).
>> 
>> Ah yeah, OBJECT_DEFINE_TYPE. Not sure it's much of a win because
>> neither finalize or instance_init are required here.
>
>Bernhard likely meant type_init() -> DEFINE_TYPES().

Indeed. And it doesn't cover the TypeInfo structure which for some reason I thought it did...

Anyway, I pointed this out because the maintainer of the device model gave similar comments in the past. So, speaking of maintainers: The MAINTAINERS file might need an update, too.

Best regards,
Bernhard

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Bernhard Beschow]

Am 29. Januar 2024 13:37:46 UTC schrieb Paolo Bonzini <pbonzini@redhat.com>:
>isa-superio.c currently defines a SuperIO chip that depends on
>CONFIG_IDE_ISA, but not all users of isa-superio.c depend on that
>symbol.  Extract the chip to a separate file so that there is an
>obvious place to select IDE_ISA.
>
>Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>---
> hw/isa/fdc37m81x-superio.c | 37 +++++++++++++++++++++++++++++++++++++
> hw/isa/isa-superio.c       | 19 -------------------
> hw/isa/Kconfig             |  5 +++++
> hw/isa/meson.build         |  1 +
> hw/mips/Kconfig            |  2 +-
> 5 files changed, 44 insertions(+), 20 deletions(-)
> create mode 100644 hw/isa/fdc37m81x-superio.c
>
>diff --git a/hw/isa/fdc37m81x-superio.c b/hw/isa/fdc37m81x-superio.c
>new file mode 100644
>index 00000000000..5ad769b69e8
>--- /dev/null
>+++ b/hw/isa/fdc37m81x-superio.c
>@@ -0,0 +1,37 @@
>+/*
>+ * Generic ISA Super I/O
>+ *
>+ * Copyright (c) 2018 Philippe Mathieu-Daudé
>+ *
>+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
>+ * See the COPYING file in the top-level directory.
>+ * SPDX-License-Identifier: GPL-2.0-or-later
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "hw/isa/superio.h"
>+
>+/* SMS FDC37M817 Super I/O */
>+static void fdc37m81x_class_init(ObjectClass *klass, void *data)
>+{
>+    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
>+
>+    sc->serial.count = 2; /* NS16C550A */
>+    sc->parallel.count = 1;
>+    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
>+    sc->ide.count = 0;
>+}
>+
>+static const TypeInfo fdc37m81x_type_info = {
>+    .name          = TYPE_FDC37M81X_SUPERIO,
>+    .parent        = TYPE_ISA_SUPERIO,
>+    .instance_size = sizeof(ISASuperIODevice),
>+    .class_init    = fdc37m81x_class_init,
>+};
>+
>+static void fdc37m81x_register_types(void)
>+{
>+    type_register_static(&fdc37m81x_type_info);
>+}
>+
>+type_init(fdc37m81x_register_types)
>diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
>index 7dbfc374da3..80e0c34652e 100644
>--- a/hw/isa/isa-superio.c
>+++ b/hw/isa/isa-superio.c
>@@ -187,28 +187,9 @@ static const TypeInfo isa_superio_type_info = {
>     .class_init = isa_superio_class_init,
> };
> 
>-/* SMS FDC37M817 Super I/O */
>-static void fdc37m81x_class_init(ObjectClass *klass, void *data)
>-{
>-    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
>-
>-    sc->serial.count = 2; /* NS16C550A */
>-    sc->parallel.count = 1;
>-    sc->floppy.count = 1; /* SMSC 82077AA Compatible */

>-    sc->ide.count = 0;

Isn't this assignment redundant?

>-}
>-
>-static const TypeInfo fdc37m81x_type_info = {
>-    .name          = TYPE_FDC37M81X_SUPERIO,
>-    .parent        = TYPE_ISA_SUPERIO,
>-    .instance_size = sizeof(ISASuperIODevice),
>-    .class_init    = fdc37m81x_class_init,
>-};
>-
> static void isa_superio_register_types(void)
> {
>     type_register_static(&isa_superio_type_info);
>-    type_register_static(&fdc37m81x_type_info);
> }
> 
> type_init(isa_superio_register_types)
>diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
>index 7884179d08b..af856af052d 100644
>--- a/hw/isa/Kconfig
>+++ b/hw/isa/Kconfig
>@@ -23,6 +23,11 @@ config ISA_SUPERIO
>     # Some users of ISA_SUPERIO do not use it
>     #select IDE_ISA
> 
>+config FDC37M81X
>+    bool

+    select ISA_SUPERIO
(in alphabetical order)

>+    select ISA_BUS

Shouldn't it `depend on ISA_BUS`? If so, the previous patch should be adapted as well.

>+    select IDE_ISA

Not needed. This device doesn't have an IDE interface.

Best regards,
Bernhard

>+
> config PC87312
>     bool
>     select ISA_SUPERIO
>diff --git a/hw/isa/meson.build b/hw/isa/meson.build
>index 2ab99ce0c6b..f650b395071 100644
>--- a/hw/isa/meson.build
>+++ b/hw/isa/meson.build
>@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
> system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
> system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
> system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
>+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
> system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
> system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
> 
>diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
>index ab61af209a0..94b969c21db 100644
>--- a/hw/mips/Kconfig
>+++ b/hw/mips/Kconfig
>@@ -1,7 +1,7 @@
> config MALTA
>     bool
>+    select FDC37M81X
>     select GT64120
>-    select ISA_SUPERIO
>     select PIIX
> 
> config MIPSSIM

Re: [PATCH 2/4] isa: extract FDC37M81X to a separate file

[Paolo Bonzini]

On Sun, Feb 4, 2024 at 9:00 PM Bernhard Beschow <shentey@gmail.com> wrote:
> >-/* SMS FDC37M817 Super I/O */
> >-static void fdc37m81x_class_init(ObjectClass *klass, void *data)
> >-{
> >-    ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass);
> >-
> >-    sc->serial.count = 2; /* NS16C550A */
> >-    sc->parallel.count = 1;
> >-    sc->floppy.count = 1; /* SMSC 82077AA Compatible */
>
> >-    sc->ide.count = 0;
>
> Isn't this assignment redundant?

I think it's there for clarity.

> >+config FDC37M81X
> >+    bool
>
> +    select ISA_SUPERIO
> (in alphabetical order)
>
> >+    select ISA_BUS
>
> Shouldn't it `depend on ISA_BUS`?

It should be there in "config ISA_SUPERIO". That was a mistake in the
original patch that introduced hw/isa/Kconfig. And VT82C686 should
also select ISA_BUS because it's a combined PCI-ISA bridge and SuperIO
chip.

Paolo

> >+    select IDE_ISA
>
> Not needed. This device doesn't have an IDE interface.
>
> Best regards,
> Bernhard
>
> >+
> > config PC87312
> >     bool
> >     select ISA_SUPERIO
> >diff --git a/hw/isa/meson.build b/hw/isa/meson.build
> >index 2ab99ce0c6b..f650b395071 100644
> >--- a/hw/isa/meson.build
> >+++ b/hw/isa/meson.build
> >@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('isa-bus.c'))
> > system_ss.add(when: 'CONFIG_ISA_SUPERIO', if_true: files('isa-superio.c'))
> > system_ss.add(when: 'CONFIG_PC87312', if_true: files('pc87312.c'))
> > system_ss.add(when: 'CONFIG_PIIX', if_true: files('piix.c'))
> >+system_ss.add(when: 'CONFIG_FDC37M81X', if_true: files('fdc37m81x-superio.c'))
> > system_ss.add(when: 'CONFIG_SMC37C669', if_true: files('smc37c669-superio.c'))
> > system_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686.c'))
> >
> >diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
> >index ab61af209a0..94b969c21db 100644
> >--- a/hw/mips/Kconfig
> >+++ b/hw/mips/Kconfig
> >@@ -1,7 +1,7 @@
> > config MALTA
> >     bool
> >+    select FDC37M81X
> >     select GT64120
> >-    select ISA_SUPERIO
> >     select PIIX
> >
> > config MIPSSIM
>