[PATCH 0/9] hw: Convert simple devices from SysBus to QDev

[PATCH 0/9] hw: Convert simple devices from SysBus to QDev

[Philippe Mathieu-Daudé]

Various simple devices (usually GPIO related) don't access
the 'system bus': no IRQ lines / memory regions exposed via
the SysBus API, no memory region mapped.

To reduce complexity, remove the sysbus onion layer from
these devices.

Introduce the qdev_create_simple() helper similar to the
pattern used for other device creation helpers:

 $ git grep _create_simple include/
 include/hw/i2c/i2c.h:175:I2CSlave *i2c_slave_create_simple(I2CBus *bus, const char *name, uint8_t addr);
 include/hw/isa/isa.h:86:ISADevice *isa_create_simple(ISABus *bus, const char *name);
 include/hw/misc/led.h:93:LEDState *led_create_simple(Object *parentobj,
 include/hw/pci/pci.h:735:PCIDevice *pci_create_simple_multifunction(PCIBus *bus, int devfn,
 include/hw/pci/pci.h:738:PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 include/hw/sysbus.h:100:static inline DeviceState *sysbus_create_simple(const char *name,
 include/hw/usb.h:505:USBDevice *usb_create_simple(USBBus *bus, const char *name);

Based-on: <20230109084121.72138-1-philmd@linaro.org>
          "hw/i2c/bitbang_i2c: Housekeeping"

Philippe Mathieu-Daudé (9):
  hw/arm/musicpal: Remove unused dummy MemoryRegion
  qdev: Add qdev_create_simple() helper
  hw/i2c/bitbang_i2c: Convert TYPE_GPIO_I2C from SysBus to QDev
  hw/arm/tosa: Convert TYPE_TOSA_MISC_GPIO from SysBus to QDev
  hw/arm/palm: Convert TYPE_PALM_MISC_GPIO from SysBus to QDev
  hw/arm/musicpal: Convert TYPE_MUSICPAL_KEY from SysBus to QDev
  hw/arm/spitz: Convert TYPE_SPITZ_KEYBOARD from SysBus to QDev
  hw/arm/tosa: Convert TYPE_SPITZ_MISC_GPIO from SysBus to QDev
  hw/i386/kvm: Convert TYPE_KVM_CLOCK from SysBus to QDev

 hw/arm/musicpal.c      | 12 ++++--------
 hw/arm/palm.c          |  9 ++-------
 hw/arm/spitz.c         | 18 ++++++++++++------
 hw/arm/tosa.c          |  9 ++-------
 hw/core/qdev.c         |  9 +++++++++
 hw/i2c/bitbang_i2c.c   |  5 ++---
 hw/i386/kvm/clock.c    |  6 +++---
 include/hw/qdev-core.h | 11 +++++++++++
 8 files changed, 45 insertions(+), 34 deletions(-)

-- 
2.38.1

[PATCH 1/9] hw/arm/musicpal: Remove unused dummy MemoryRegion

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/musicpal.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 813232682f..23359a6ac3 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1071,7 +1071,6 @@ struct musicpal_key_state {
     SysBusDevice parent_obj;
     /*< public >*/
 
-    MemoryRegion iomem;
     uint32_t kbd_extended;
     uint32_t pressed_keys;
     qemu_irq out[8];
@@ -1160,9 +1159,6 @@ static void musicpal_key_init(Object *obj)
     DeviceState *dev = DEVICE(sbd);
     musicpal_key_state *s = MUSICPAL_KEY(dev);
 
-    memory_region_init(&s->iomem, obj, "dummy", 0);
-    sysbus_init_mmio(sbd, &s->iomem);
-
     s->kbd_extended = 0;
     s->pressed_keys = 0;
 
-- 
2.38.1

Re: [PATCH 1/9] hw/arm/musicpal: Remove unused dummy MemoryRegion

[Richard Henderson]

On 1/9/23 01:26, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/musicpal.c | 4 ----
>   1 file changed, 4 deletions(-)

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

r~

[PATCH 2/9] qdev: Add qdev_create_simple() helper

[Philippe Mathieu-Daudé]

Add a helper to easily create a qdev with no (or default) property.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/core/qdev.c         |  9 +++++++++
 include/hw/qdev-core.h | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index d759c4602c..4afed2a24c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -174,6 +174,15 @@ DeviceState *qdev_try_new(const char *name)
     return DEVICE(object_new(name));
 }
 
+DeviceState *qdev_create_simple(const char *name, Error **errp)
+{
+    DeviceState *dev = qdev_new(name);
+
+    qdev_realize_and_unref(dev, NULL, errp);
+
+    return dev;
+}
+
 static QTAILQ_HEAD(, DeviceListener) device_listeners
     = QTAILQ_HEAD_INITIALIZER(device_listeners);
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 35fddb19a6..ca6b2b8458 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -397,6 +397,17 @@ bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp);
  * the life of the simulation and should not be unrealized and freed.
  */
 void qdev_unrealize(DeviceState *dev);
+
+/**
+ * Create and realize a device on the heap.
+ * @name: device type to create (we assert() that this type exists)
+ * @errp: pointer to error object
+ *
+ * Create the device state structure, initialize it, and drop the
+ * reference to it (the device is realized).
+ */
+DeviceState *qdev_create_simple(const char *name, Error **errp);
+
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version);
 HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev);
-- 
2.38.1

[PATCH 3/9] hw/i2c/bitbang_i2c: Convert TYPE_GPIO_I2C from SysBus to QDev

[Philippe Mathieu-Daudé]

Nothing in TYPE_GPIO_I2C access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/musicpal.c    | 2 +-
 hw/i2c/bitbang_i2c.c | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 23359a6ac3..13f4dbdbc3 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1297,7 +1297,7 @@ static void musicpal_init(MachineState *machine)
 
     dev = sysbus_create_simple(TYPE_MUSICPAL_GPIO, MP_GPIO_BASE,
                                qdev_get_gpio_in(pic, MP_GPIO_IRQ));
-    i2c_dev = sysbus_create_simple(TYPE_GPIO_I2C, -1, NULL);
+    i2c_dev = qdev_create_simple(TYPE_GPIO_I2C, &error_fatal);
     i2c = (I2CBus *)qdev_get_child_bus(i2c_dev, "i2c");
 
     lcd_dev = sysbus_create_simple(TYPE_MUSICPAL_LCD, MP_LCD_BASE, NULL);
diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c
index ffc5b95410..58503f2b27 100644
--- a/hw/i2c/bitbang_i2c.c
+++ b/hw/i2c/bitbang_i2c.c
@@ -13,7 +13,6 @@
 #include "qemu/osdep.h"
 #include "hw/irq.h"
 #include "hw/i2c/bitbang_i2c.h"
-#include "hw/sysbus.h"
 #include "qemu/module.h"
 #include "qom/object.h"
 #include "trace.h"
@@ -191,7 +190,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(GPIOI2CState, GPIO_I2C)
 
 struct GPIOI2CState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    DeviceState parent_obj;
     /*< public >*/
 
     bitbang_i2c_interface bitbang;
@@ -233,7 +232,7 @@ static void gpio_i2c_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo gpio_i2c_info = {
     .name          = TYPE_GPIO_I2C,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DEVICE,
     .instance_size = sizeof(GPIOI2CState),
     .instance_init = gpio_i2c_init,
     .class_init    = gpio_i2c_class_init,
-- 
2.38.1

[PATCH 4/9] hw/arm/tosa: Convert TYPE_TOSA_MISC_GPIO from SysBus to QDev

[Philippe Mathieu-Daudé]

Drop the pointless TosaMiscGPIOState structure definition,
since it defaults to the size of the parent's type.

Nothing in TYPE_TOSA_MISC_GPIO access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/tosa.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index d5a6763cf9..ec0db24e8f 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -78,10 +78,6 @@ static void tosa_microdrive_attach(PXA2xxState *cpu)
 #define TYPE_TOSA_MISC_GPIO "tosa-misc-gpio"
 OBJECT_DECLARE_SIMPLE_TYPE(TosaMiscGPIOState, TOSA_MISC_GPIO)
 
-struct TosaMiscGPIOState {
-    SysBusDevice parent_obj;
-};
-
 static void tosa_reset(void *opaque, int line, int level)
 {
     if (level) {
@@ -104,7 +100,7 @@ static void tosa_gpio_setup(PXA2xxState *cpu,
     DeviceState *misc_gpio;
     LEDState *led[4];
 
-    misc_gpio = sysbus_create_simple(TYPE_TOSA_MISC_GPIO, -1, NULL);
+    misc_gpio = qdev_create_simple(TYPE_TOSA_MISC_GPIO, &error_fatal);
 
     /* MMC/SD host */
     pxa2xx_mmci_handlers(cpu->mmc,
@@ -307,8 +303,7 @@ static const TypeInfo tosa_ssp_info = {
 
 static const TypeInfo tosa_misc_gpio_info = {
     .name          = TYPE_TOSA_MISC_GPIO,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(TosaMiscGPIOState),
+    .parent        = TYPE_DEVICE,
     .instance_init = tosa_misc_gpio_init,
     /*
      * No class init required: device has no internal state so does not
-- 
2.38.1

[PATCH 5/9] hw/arm/palm: Convert TYPE_PALM_MISC_GPIO from SysBus to QDev

[Philippe Mathieu-Daudé]

Drop the pointless PalmMiscGPIOState structure definition,
since it defaults to the size of the parent's type.

Nothing in TYPE_PALM_MISC_GPIO access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/palm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 68e11dd1ec..912164e93f 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -134,10 +134,6 @@ static void palmte_button_event(void *opaque, int keycode)
 #define TYPE_PALM_MISC_GPIO "palm-misc-gpio"
 OBJECT_DECLARE_SIMPLE_TYPE(PalmMiscGPIOState, PALM_MISC_GPIO)
 
-struct PalmMiscGPIOState {
-    SysBusDevice parent_obj;
-};
-
 static void palmte_onoff_gpios(void *opaque, int line, int level)
 {
     switch (line) {
@@ -174,8 +170,7 @@ static void palm_misc_gpio_init(Object *obj)
 
 static const TypeInfo palm_misc_gpio_info = {
     .name = TYPE_PALM_MISC_GPIO,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(PalmMiscGPIOState),
+    .parent = TYPE_DEVICE,
     .instance_init = palm_misc_gpio_init,
     /*
      * No class init required: device has no internal state so does not
@@ -187,7 +182,7 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu)
 {
     DeviceState *misc_gpio;
 
-    misc_gpio = sysbus_create_simple(TYPE_PALM_MISC_GPIO, -1, NULL);
+    misc_gpio = qdev_create_simple(TYPE_PALM_MISC_GPIO, &error_fatal);
 
     omap_mmc_handlers(cpu->mmc,
                     qdev_get_gpio_in(cpu->gpio, PALMTE_MMC_WP_GPIO),
-- 
2.38.1

[PATCH 6/9] hw/arm/musicpal: Convert TYPE_MUSICPAL_KEY from SysBus to QDev

[Philippe Mathieu-Daudé]

Nothing in TYPE_MUSICPAL_KEY access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/musicpal.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 13f4dbdbc3..b496ff8a22 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1068,7 +1068,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(musicpal_key_state, MUSICPAL_KEY)
 
 struct musicpal_key_state {
     /*< private >*/
-    SysBusDevice parent_obj;
+    DeviceState parent_obj;
     /*< public >*/
 
     uint32_t kbd_extended;
@@ -1187,7 +1187,7 @@ static void musicpal_key_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo musicpal_key_info = {
     .name          = TYPE_MUSICPAL_KEY,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DEVICE,
     .instance_size = sizeof(musicpal_key_state),
     .instance_init = musicpal_key_init,
     .class_init    = musicpal_key_class_init,
@@ -1301,7 +1301,7 @@ static void musicpal_init(MachineState *machine)
     i2c = (I2CBus *)qdev_get_child_bus(i2c_dev, "i2c");
 
     lcd_dev = sysbus_create_simple(TYPE_MUSICPAL_LCD, MP_LCD_BASE, NULL);
-    key_dev = sysbus_create_simple(TYPE_MUSICPAL_KEY, -1, NULL);
+    key_dev = qdev_create_simple(TYPE_MUSICPAL_KEY, &error_fatal);
 
     /* I2C read data */
     qdev_connect_gpio_out(i2c_dev, 0,
-- 
2.38.1

[PATCH 7/9] hw/arm/spitz: Convert TYPE_SPITZ_KEYBOARD from SysBus to QDev

[Philippe Mathieu-Daudé]

Nothing in TYPE_SPITZ_KEYBOARD access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/spitz.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 5aab0b8565..6605d658ed 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -260,8 +260,10 @@ static const int spitz_gpiomap[5] = {
 OBJECT_DECLARE_SIMPLE_TYPE(SpitzKeyboardState, SPITZ_KEYBOARD)
 
 struct SpitzKeyboardState {
-    SysBusDevice parent_obj;
+    /*< private >*/
+    DeviceState parent_obj;
 
+    /*< public >*/
     qemu_irq sense[SPITZ_KEY_SENSE_NUM];
     qemu_irq gpiomap[5];
     int keymap[0x80];
@@ -514,7 +516,7 @@ static void spitz_keyboard_register(PXA2xxState *cpu)
     DeviceState *dev;
     SpitzKeyboardState *s;
 
-    dev = sysbus_create_simple(TYPE_SPITZ_KEYBOARD, -1, NULL);
+    dev = qdev_create_simple(TYPE_SPITZ_KEYBOARD, &error_fatal);
     s = SPITZ_KEYBOARD(dev);
 
     for (i = 0; i < SPITZ_KEY_SENSE_NUM; i ++)
@@ -1193,7 +1195,7 @@ static void spitz_keyboard_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo spitz_keyboard_info = {
     .name          = TYPE_SPITZ_KEYBOARD,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DEVICE,
     .instance_size = sizeof(SpitzKeyboardState),
     .instance_init = spitz_keyboard_init,
     .class_init    = spitz_keyboard_class_init,
-- 
2.38.1

[PATCH 8/9] hw/arm/tosa: Convert TYPE_SPITZ_MISC_GPIO from SysBus to QDev

[Philippe Mathieu-Daudé]

Nothing in TYPE_SPITZ_MISC_GPIO access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/spitz.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 6605d658ed..31445329ff 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -819,7 +819,9 @@ static void spitz_akita_i2c_setup(PXA2xxState *cpu)
 OBJECT_DECLARE_SIMPLE_TYPE(SpitzMiscGPIOState, SPITZ_MISC_GPIO)
 
 struct SpitzMiscGPIOState {
-    SysBusDevice parent_obj;
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
 
     qemu_irq adc_value;
 };
@@ -883,7 +885,9 @@ static void spitz_misc_gpio_init(Object *obj)
 
 static void spitz_scoop_gpio_setup(SpitzMachineState *sms)
 {
-    DeviceState *miscdev = sysbus_create_simple(TYPE_SPITZ_MISC_GPIO, -1, NULL);
+    DeviceState *miscdev;
+
+    miscdev = qdev_create_simple(TYPE_SPITZ_MISC_GPIO, &error_fatal);
 
     sms->misc_gpio = miscdev;
 
@@ -1260,7 +1264,7 @@ static const TypeInfo spitz_lcdtg_info = {
 
 static const TypeInfo spitz_misc_gpio_info = {
     .name = TYPE_SPITZ_MISC_GPIO,
-    .parent = TYPE_SYS_BUS_DEVICE,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(SpitzMiscGPIOState),
     .instance_init = spitz_misc_gpio_init,
     /*
-- 
2.38.1

[PATCH 9/9] hw/i386/kvm: Convert TYPE_KVM_CLOCK from SysBus to QDev

[Philippe Mathieu-Daudé]

Nothing in TYPE_KVM_CLOCK access the SysBus API,
convert it to a plain QDev.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/kvm/clock.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index df70b4a033..da0984b5a0 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -35,7 +35,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(KVMClockState, KVM_CLOCK)
 
 struct KVMClockState {
     /*< private >*/
-    SysBusDevice busdev;
+    DeviceState busdev;
     /*< public >*/
 
     uint64_t clock;
@@ -322,7 +322,7 @@ static void kvmclock_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo kvmclock_info = {
     .name          = TYPE_KVM_CLOCK,
-    .parent        = TYPE_SYS_BUS_DEVICE,
+    .parent        = TYPE_DEVICE,
     .instance_size = sizeof(KVMClockState),
     .class_init    = kvmclock_class_init,
 };
@@ -338,7 +338,7 @@ void kvmclock_create(bool create_always)
     if (create_always ||
         cpu->env.features[FEAT_KVM] & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
                                        (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
-        sysbus_create_simple(TYPE_KVM_CLOCK, -1, NULL);
+        qdev_realize(qdev_new(TYPE_KVM_CLOCK), NULL, &error_fatal));
     }
 }
 
-- 
2.38.1

Re: [PATCH 0/9] hw: Convert simple devices from SysBus to QDev

[Peter Maydell]

On Mon, 9 Jan 2023 at 09:26, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Various simple devices (usually GPIO related) don't access
> the 'system bus': no IRQ lines / memory regions exposed via
> the SysBus API, no memory region mapped.
>
> To reduce complexity, remove the sysbus onion layer from
> these devices.

This will break reset for all these devices. We can't
do this until we've sorted out reset first, I'm afraid.

thanks
-- PMM