[PATCH v2 00/23] E500 Cleanup

[PATCH v2 00/23] E500 Cleanup

[Bernhard Beschow]

This series is part of a bigger series exploring data-driven machine creation
using device tree blobs on top of the e500 machines [1]. It contains patches to
make this exploration easier which are also expected to provide value in
themselves.

The cleanup starts with the e500 machine class itself, then proceeds with
machine-specific device models and concludes with more or less loosely related
devices. Device cleanup mostly consists of using the DEFINE_TYPES() macro.

v2:
* Add R-b tags (Cedric, Zoltan -- thanks!)
* Add missing S-o-b tag to ds1338 patch (Cedric)
* Populate POR PLL ratio status register with real-world values (Zoltan)
* Rephrase one commit message (Zoltan)
* Also rename header of ppce500_ccsr to match struct name
* Don't mention ppc440_bamboo in license since unrelated (Zoltan)
* Various style changes (Zoltan)

[1] https://github.com/shentok/qemu/tree/e500-fdt

Bernhard Beschow (23):
  hw/ppc/e500: Do not leak struct boot_info
  hw/ppc/e500: Remove firstenv variable
  hw/ppc/e500: Prefer QOM cast
  hw/ppc/e500: Remove unused "irqs" parameter
  hw/ppc/e500: Add missing device tree properties to i2c controller node
  hw/ppc/e500: Use SysBusDevice API to access TYPE_CCSR's internal
    resources
  hw/ppc/e500: Extract ppce500_ccsr.c
  hw/ppc/ppce500_ccsr: Log access to unimplemented registers
  hw/ppc/mpc8544_guts: Populate POR PLL ratio status register
  hw/i2c/mpc_i2c: Convert DPRINTF to trace events for register access
  hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
  hw/pci-host/ppce500: Reuse TYPE_PPC_E500_PCI_BRIDGE define
  hw/pci-host/ppce500: Prefer DEFINE_TYPES() macro
  hw/gpio/mpc8xxx: Prefer DEFINE_TYPES() macro
  hw/ppc/mpc8544_guts: Prefer DEFINE_TYPES() macro
  hw/net/fsl_etsec/etsec: Prefer DEFINE_TYPES() macro
  hw/intc: Guard openpic_kvm.c by dedicated OPENPIC_KVM Kconfig switch
  hw/sd/sdhci: Prefer DEFINE_TYPES() macro
  hw/block/pflash_cfi01: Prefer DEFINE_TYPES() macro
  hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
  hw/rtc/ds1338: Prefer DEFINE_TYPES() macro
  hw/usb/hcd-ehci-sysbus: Prefer DEFINE_TYPES() macro
  hw/vfio/platform: Let vfio_start_eventfd_injection() take
    VFIOPlatformDevice pointer

 MAINTAINERS                            |   2 +-
 hw/ppc/e500.h                          |   9 +-
 hw/ppc/{e500-ccsr.h => ppce500_ccsr.h} |   8 +-
 hw/block/pflash_cfi01.c                |  21 ++---
 hw/gpio/mpc8xxx.c                      |  22 ++---
 hw/i2c/mpc_i2c.c                       |  29 +++---
 hw/i2c/smbus_eeprom.c                  |  19 ++--
 hw/net/fsl_etsec/etsec.c               |  22 ++---
 hw/pci-host/ppce500.c                  |  54 +++++------
 hw/ppc/e500.c                          |  65 ++++----------
 hw/ppc/mpc8544_guts.c                  |  32 ++++---
 hw/ppc/ppce500_ccsr.c                  |  64 ++++++++++++++
 hw/rtc/ds1338.c                        |  20 ++---
 hw/sd/sdhci.c                          |  62 ++++++-------
 hw/usb/hcd-ehci-sysbus.c               | 118 +++++++++++--------------
 hw/vfio/platform.c                     |   7 +-
 hw/i2c/trace-events                    |   5 ++
 hw/intc/Kconfig                        |   4 +
 hw/intc/meson.build                    |   3 +-
 hw/ppc/meson.build                     |   1 +
 hw/ppc/trace-events                    |   3 +
 21 files changed, 285 insertions(+), 285 deletions(-)
 rename hw/ppc/{e500-ccsr.h => ppce500_ccsr.h} (71%)
 create mode 100644 hw/ppc/ppce500_ccsr.c

-- 
2.46.2

[PATCH v2 01/23] hw/ppc/e500: Do not leak struct boot_info

[Bernhard Beschow]

The struct is allocated once with g_new0() but never free()'d. Fix the leakage
by adding an attribute to struct PPCE500MachineState which avoids the
allocation.

While at it remove the obsolete /*< private >*/ markers.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.h |  9 +++++++--
 hw/ppc/e500.c | 17 ++++-------------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
index 8c09ef92e4..5654bb7907 100644
--- a/hw/ppc/e500.h
+++ b/hw/ppc/e500.h
@@ -5,18 +5,23 @@
 #include "hw/platform-bus.h"
 #include "qom/object.h"
 
+typedef struct boot_info {
+    uint32_t dt_base;
+    uint32_t dt_size;
+    uint32_t entry;
+} boot_info;
+
 struct PPCE500MachineState {
-    /*< private >*/
     MachineState parent_obj;
 
     /* points to instance of TYPE_PLATFORM_BUS_DEVICE if
      * board supports dynamic sysbus devices
      */
     PlatformBusDevice *pbus_dev;
+    boot_info boot_info;
 };
 
 struct PPCE500MachineClass {
-    /*< private >*/
     MachineClass parent_class;
 
     /* required -- must at least add toplevel board compatible */
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 3bd12b54ab..75b051009f 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -80,13 +80,6 @@
 
 #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
 
-struct boot_info
-{
-    uint32_t dt_base;
-    uint32_t dt_size;
-    uint32_t entry;
-};
-
 static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
                                 int nr_slots, int *len)
 {
@@ -919,7 +912,6 @@ void ppce500_init(MachineState *machine)
     bool kernel_as_payload;
     hwaddr bios_entry = 0;
     target_long payload_size;
-    struct boot_info *boot_info = NULL;
     int dt_size;
     int i;
     unsigned int smp_cpus = machine->smp.cpus;
@@ -974,9 +966,8 @@ void ppce500_init(MachineState *machine)
         /* Register reset handler */
         if (!i) {
             /* Primary CPU */
-            boot_info = g_new0(struct boot_info, 1);
             qemu_register_reset(ppce500_cpu_reset, cpu);
-            env->load_info = boot_info;
+            env->load_info = &pms->boot_info;
         } else {
             /* Secondary CPUs */
             qemu_register_reset(ppce500_cpu_reset_sec, cpu);
@@ -1274,9 +1265,9 @@ void ppce500_init(MachineState *machine)
     }
     assert(dt_size < DTB_MAX_SIZE);
 
-    boot_info->entry = bios_entry;
-    boot_info->dt_base = dt_base;
-    boot_info->dt_size = dt_size;
+    pms->boot_info.entry = bios_entry;
+    pms->boot_info.dt_base = dt_base;
+    pms->boot_info.dt_size = dt_size;
 }
 
 static void e500_ccsr_initfn(Object *obj)
-- 
2.46.2

Re: [PATCH v2 01/23] hw/ppc/e500: Do not leak struct boot_info

[BALATON Zoltan]

On Sat, 5 Oct 2024, Bernhard Beschow wrote:
> The struct is allocated once with g_new0() but never free()'d. Fix the leakage
> by adding an attribute to struct PPCE500MachineState which avoids the
> allocation.
>
> While at it remove the obsolete /*< private >*/ markers.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/ppc/e500.h |  9 +++++++--
> hw/ppc/e500.c | 17 ++++-------------
> 2 files changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
> index 8c09ef92e4..5654bb7907 100644
> --- a/hw/ppc/e500.h
> +++ b/hw/ppc/e500.h
> @@ -5,18 +5,23 @@
> #include "hw/platform-bus.h"
> #include "qom/object.h"
>
> +typedef struct boot_info {
> +    uint32_t dt_base;
> +    uint32_t dt_size;
> +    uint32_t entry;
> +} boot_info;
> +

You either don't need a typedef or don't need a struct name. Since coding 
style says typedefs should be camel case but other machines don't use a 
typedef it's probably simplest to drop the typedef and define the machine 
state field as struct boot_info below for consistency with other similar 
structs. Otherwise you'd have to come up with some camel case type name 
here but that would be less consistent with other machines. So I'd go 
without the typedef.

Regards,
BALATON Zoltan

> struct PPCE500MachineState {
> -    /*< private >*/
>     MachineState parent_obj;
>
>     /* points to instance of TYPE_PLATFORM_BUS_DEVICE if
>      * board supports dynamic sysbus devices
>      */
>     PlatformBusDevice *pbus_dev;
> +    boot_info boot_info;
> };
>
> struct PPCE500MachineClass {
> -    /*< private >*/
>     MachineClass parent_class;
>
>     /* required -- must at least add toplevel board compatible */
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 3bd12b54ab..75b051009f 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -80,13 +80,6 @@
>
> #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
>
> -struct boot_info
> -{
> -    uint32_t dt_base;
> -    uint32_t dt_size;
> -    uint32_t entry;
> -};
> -
> static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
>                                 int nr_slots, int *len)
> {
> @@ -919,7 +912,6 @@ void ppce500_init(MachineState *machine)
>     bool kernel_as_payload;
>     hwaddr bios_entry = 0;
>     target_long payload_size;
> -    struct boot_info *boot_info = NULL;
>     int dt_size;
>     int i;
>     unsigned int smp_cpus = machine->smp.cpus;
> @@ -974,9 +966,8 @@ void ppce500_init(MachineState *machine)
>         /* Register reset handler */
>         if (!i) {
>             /* Primary CPU */
> -            boot_info = g_new0(struct boot_info, 1);
>             qemu_register_reset(ppce500_cpu_reset, cpu);
> -            env->load_info = boot_info;
> +            env->load_info = &pms->boot_info;
>         } else {
>             /* Secondary CPUs */
>             qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> @@ -1274,9 +1265,9 @@ void ppce500_init(MachineState *machine)
>     }
>     assert(dt_size < DTB_MAX_SIZE);
>
> -    boot_info->entry = bios_entry;
> -    boot_info->dt_base = dt_base;
> -    boot_info->dt_size = dt_size;
> +    pms->boot_info.entry = bios_entry;
> +    pms->boot_info.dt_base = dt_base;
> +    pms->boot_info.dt_size = dt_size;
> }
>
> static void e500_ccsr_initfn(Object *obj)
>

Re: [PATCH v2 01/23] hw/ppc/e500: Do not leak struct boot_info

[Bernhard Beschow]

Am 6. Oktober 2024 16:56:52 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sat, 5 Oct 2024, Bernhard Beschow wrote:
>> The struct is allocated once with g_new0() but never free()'d. Fix the leakage
>> by adding an attribute to struct PPCE500MachineState which avoids the
>> allocation.
>> 
>> While at it remove the obsolete /*< private >*/ markers.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/ppc/e500.h |  9 +++++++--
>> hw/ppc/e500.c | 17 ++++-------------
>> 2 files changed, 11 insertions(+), 15 deletions(-)
>> 
>> diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
>> index 8c09ef92e4..5654bb7907 100644
>> --- a/hw/ppc/e500.h
>> +++ b/hw/ppc/e500.h
>> @@ -5,18 +5,23 @@
>> #include "hw/platform-bus.h"
>> #include "qom/object.h"
>> 
>> +typedef struct boot_info {
>> +    uint32_t dt_base;
>> +    uint32_t dt_size;
>> +    uint32_t entry;
>> +} boot_info;
>> +
>
>You either don't need a typedef or don't need a struct name. Since coding style says typedefs should be camel case but other machines don't use a typedef it's probably simplest to drop the typedef and define the machine state field as struct boot_info below for consistency with other similar structs. Otherwise you'd have to come up with some camel case type name here but that would be less consistent with other machines. So I'd go without the typedef.

Indeed, not sure why I added the typedef. I'll omit the typedef then.

This code hasn't changed since v1, so it would have been nice to have caught that in the first round to avoid another spin.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> struct PPCE500MachineState {
>> -    /*< private >*/
>>     MachineState parent_obj;
>> 
>>     /* points to instance of TYPE_PLATFORM_BUS_DEVICE if
>>      * board supports dynamic sysbus devices
>>      */
>>     PlatformBusDevice *pbus_dev;
>> +    boot_info boot_info;
>> };
>> 
>> struct PPCE500MachineClass {
>> -    /*< private >*/
>>     MachineClass parent_class;
>> 
>>     /* required -- must at least add toplevel board compatible */
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 3bd12b54ab..75b051009f 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -80,13 +80,6 @@
>> 
>> #define PLATFORM_CLK_FREQ_HZ       (400 * 1000 * 1000)
>> 
>> -struct boot_info
>> -{
>> -    uint32_t dt_base;
>> -    uint32_t dt_size;
>> -    uint32_t entry;
>> -};
>> -
>> static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
>>                                 int nr_slots, int *len)
>> {
>> @@ -919,7 +912,6 @@ void ppce500_init(MachineState *machine)
>>     bool kernel_as_payload;
>>     hwaddr bios_entry = 0;
>>     target_long payload_size;
>> -    struct boot_info *boot_info = NULL;
>>     int dt_size;
>>     int i;
>>     unsigned int smp_cpus = machine->smp.cpus;
>> @@ -974,9 +966,8 @@ void ppce500_init(MachineState *machine)
>>         /* Register reset handler */
>>         if (!i) {
>>             /* Primary CPU */
>> -            boot_info = g_new0(struct boot_info, 1);
>>             qemu_register_reset(ppce500_cpu_reset, cpu);
>> -            env->load_info = boot_info;
>> +            env->load_info = &pms->boot_info;
>>         } else {
>>             /* Secondary CPUs */
>>             qemu_register_reset(ppce500_cpu_reset_sec, cpu);
>> @@ -1274,9 +1265,9 @@ void ppce500_init(MachineState *machine)
>>     }
>>     assert(dt_size < DTB_MAX_SIZE);
>> 
>> -    boot_info->entry = bios_entry;
>> -    boot_info->dt_base = dt_base;
>> -    boot_info->dt_size = dt_size;
>> +    pms->boot_info.entry = bios_entry;
>> +    pms->boot_info.dt_base = dt_base;
>> +    pms->boot_info.dt_size = dt_size;
>> }
>> 
>> static void e500_ccsr_initfn(Object *obj)
>> 

[PATCH v2 02/23] hw/ppc/e500: Remove firstenv variable

[Bernhard Beschow]

The firstenv variable is never read, so remove it. The env variable is then only
used inside the loop, so move it there to restrict its scope.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 75b051009f..f68779a1ea 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -899,7 +899,6 @@ void ppce500_init(MachineState *machine)
     const PPCE500MachineClass *pmc = PPCE500_MACHINE_GET_CLASS(machine);
     MachineClass *mc = MACHINE_CLASS(pmc);
     PCIBus *pci_bus;
-    CPUPPCState *env = NULL;
     uint64_t loadaddr;
     hwaddr kernel_base = -1LL;
     int kernel_size = 0;
@@ -921,7 +920,6 @@ void ppce500_init(MachineState *machine)
     IrqLines *irqs;
     DeviceState *dev, *mpicdev;
     DriveInfo *dinfo;
-    CPUPPCState *firstenv = NULL;
     MemoryRegion *ccsr_addr_space;
     SysBusDevice *s;
     PPCE500CCSRState *ccsr;
@@ -930,6 +928,7 @@ void ppce500_init(MachineState *machine)
     irqs = g_new0(IrqLines, smp_cpus);
     for (i = 0; i < smp_cpus; i++) {
         PowerPCCPU *cpu;
+        CPUPPCState *env;
         CPUState *cs;
 
         cpu = POWERPC_CPU(object_new(machine->cpu_type));
@@ -950,10 +949,6 @@ void ppce500_init(MachineState *machine)
                                  &error_abort);
         qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
 
-        if (!firstenv) {
-            firstenv = env;
-        }
-
         irqs[i].irq[OPENPIC_OUTPUT_INT] =
             qdev_get_gpio_in(DEVICE(cpu), PPCE500_INPUT_INT);
         irqs[i].irq[OPENPIC_OUTPUT_CINT] =
@@ -974,8 +969,6 @@ void ppce500_init(MachineState *machine)
         }
     }
 
-    env = firstenv;
-
     if (!QEMU_IS_ALIGNED(machine->ram_size, RAM_SIZES_ALIGN)) {
         error_report("RAM size must be multiple of %" PRIu64, RAM_SIZES_ALIGN);
         exit(EXIT_FAILURE);
-- 
2.46.2

[PATCH v2 03/23] hw/ppc/e500: Prefer QOM cast

[Bernhard Beschow]

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f68779a1ea..32996c188e 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1008,7 +1008,7 @@ void ppce500_init(MachineState *machine)
     sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC8544_I2C_IRQ));
     memory_region_add_subregion(ccsr_addr_space, MPC8544_I2C_REGS_OFFSET,
                                 sysbus_mmio_get_region(s, 0));
-    i2c = (I2CBus *)qdev_get_child_bus(dev, "i2c");
+    i2c = I2C_BUS(qdev_get_child_bus(dev, "i2c"));
     i2c_slave_create_simple(i2c, "ds1338", RTC_REGS_OFFSET);
 
     /* eSDHC */
@@ -1057,7 +1057,7 @@ void ppce500_init(MachineState *machine)
     memory_region_add_subregion(ccsr_addr_space, MPC8544_PCI_REGS_OFFSET,
                                 sysbus_mmio_get_region(s, 0));
 
-    pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
+    pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
     if (!pci_bus)
         printf("couldn't create PCI controller!\n");
 
-- 
2.46.2

[PATCH v2 04/23] hw/ppc/e500: Remove unused "irqs" parameter

[Bernhard Beschow]

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 32996c188e..228287b457 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -825,7 +825,7 @@ static DeviceState *ppce500_init_mpic_qemu(PPCE500MachineState *pms,
 }
 
 static DeviceState *ppce500_init_mpic_kvm(const PPCE500MachineClass *pmc,
-                                          IrqLines *irqs, Error **errp)
+                                          Error **errp)
 {
 #ifdef CONFIG_KVM
     DeviceState *dev;
@@ -865,7 +865,7 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms,
         Error *err = NULL;
 
         if (kvm_kernel_irqchip_allowed()) {
-            dev = ppce500_init_mpic_kvm(pmc, irqs, &err);
+            dev = ppce500_init_mpic_kvm(pmc, &err);
         }
         if (kvm_kernel_irqchip_required() && !dev) {
             error_reportf_err(err,
-- 
2.46.2

[PATCH v2 05/23] hw/ppc/e500: Add missing device tree properties to i2c controller node

[Bernhard Beschow]

When compiling a decompiled device tree blob created with dumpdtb, dtc complains
with:

  /soc@e0000000/i2c@3000: incorrect #address-cells for I2C bus
  /soc@e0000000/i2c@3000: incorrect #size-cells for I2C bus

Fix this by adding the missing device tree properties.

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 228287b457..e2a4f265a5 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -196,6 +196,8 @@ static void dt_i2c_create(void *fdt, const char *soc, const char *mpic,
     qemu_fdt_setprop_cells(fdt, i2c, "cell-index", 0);
     qemu_fdt_setprop_cells(fdt, i2c, "interrupts", irq0, 0x2);
     qemu_fdt_setprop_phandle(fdt, i2c, "interrupt-parent", mpic);
+    qemu_fdt_setprop_cell(fdt, i2c, "#size-cells", 0);
+    qemu_fdt_setprop_cell(fdt, i2c, "#address-cells", 1);
     qemu_fdt_setprop_string(fdt, "/aliases", alias, i2c);
 
     g_free(i2c);
-- 
2.46.2

[PATCH v2 06/23] hw/ppc/e500: Use SysBusDevice API to access TYPE_CCSR's internal resources

[Bernhard Beschow]

Rather than accessing the attributes of TYPE_CCSR directly, use the SysBusDevice
API which exists exactly for that purpose. Furthermore, registering the memory
region with the SysBusDevice API makes it show up in QMP's `info qom-tree`
command.

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/ppce500.c | 10 +++++-----
 hw/ppc/e500.c         |  8 ++++----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 95b983b2b3..97e5d47cec 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -16,7 +16,6 @@
 
 #include "qemu/osdep.h"
 #include "hw/irq.h"
-#include "hw/ppc/e500-ccsr.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "hw/pci/pci_device.h"
@@ -419,11 +418,12 @@ static const VMStateDescription vmstate_ppce500_pci = {
 static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp)
 {
     PPCE500PCIBridgeState *b = PPC_E500_PCI_BRIDGE(d);
-    PPCE500CCSRState *ccsr = CCSR(container_get(qdev_get_machine(),
-                                  "/e500-ccsr"));
+    SysBusDevice *ccsr = SYS_BUS_DEVICE(container_get(qdev_get_machine(),
+                                                      "/e500-ccsr"));
+    MemoryRegion *ccsr_space = sysbus_mmio_get_region(ccsr, 0);
 
-    memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", &ccsr->ccsr_space,
-                             0, int128_get64(ccsr->ccsr_space.size));
+    memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0",
+                             ccsr_space, 0, int128_get64(ccsr_space->size));
     pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0);
 }
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index e2a4f265a5..2225533e33 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -924,7 +924,6 @@ void ppce500_init(MachineState *machine)
     DriveInfo *dinfo;
     MemoryRegion *ccsr_addr_space;
     SysBusDevice *s;
-    PPCE500CCSRState *ccsr;
     I2CBus *i2c;
 
     irqs = g_new0(IrqLines, smp_cpus);
@@ -980,10 +979,10 @@ void ppce500_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0, machine->ram);
 
     dev = qdev_new("e500-ccsr");
+    s = SYS_BUS_DEVICE(dev);
     object_property_add_child(OBJECT(machine), "e500-ccsr", OBJECT(dev));
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    ccsr = CCSR(dev);
-    ccsr_addr_space = &ccsr->ccsr_space;
+    sysbus_realize_and_unref(s, &error_fatal);
+    ccsr_addr_space = sysbus_mmio_get_region(s, 0);
     memory_region_add_subregion(address_space_mem, pmc->ccsrbar_base,
                                 ccsr_addr_space);
 
@@ -1270,6 +1269,7 @@ static void e500_ccsr_initfn(Object *obj)
     PPCE500CCSRState *ccsr = CCSR(obj);
     memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
                        MPC8544_CCSRBAR_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(ccsr), &ccsr->ccsr_space);
 }
 
 static const TypeInfo e500_ccsr_info = {
-- 
2.46.2

[PATCH v2 07/23] hw/ppc/e500: Extract ppce500_ccsr.c

[Bernhard Beschow]

The device model already has a header file, so extract its implementation into
an accompanying source file like other e500 devices. While at it rename the
header file to reflect the name of the structure defined there.

This commit is also a preparation for the next commit.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 MAINTAINERS                            |  2 +-
 hw/ppc/{e500-ccsr.h => ppce500_ccsr.h} |  8 +++---
 hw/ppc/e500.c                          | 21 ++--------------
 hw/ppc/ppce500_ccsr.c                  | 34 ++++++++++++++++++++++++++
 hw/ppc/meson.build                     |  1 +
 5 files changed, 42 insertions(+), 24 deletions(-)
 rename hw/ppc/{e500-ccsr.h => ppce500_ccsr.h} (71%)
 create mode 100644 hw/ppc/ppce500_ccsr.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 34fffcb5be..f1755c8ad8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1382,7 +1382,7 @@ M: Bernhard Beschow <shentey@gmail.com>
 L: qemu-ppc@nongnu.org
 S: Odd Fixes
 F: hw/ppc/e500*
-F: hw/ppc/ppce500_spin.c
+F: hw/ppc/ppce500_*
 F: hw/gpio/mpc8xxx.c
 F: hw/i2c/mpc_i2c.c
 F: hw/net/fsl_etsec/
diff --git a/hw/ppc/e500-ccsr.h b/hw/ppc/ppce500_ccsr.h
similarity index 71%
rename from hw/ppc/e500-ccsr.h
rename to hw/ppc/ppce500_ccsr.h
index 249c17be3b..36cbb811ca 100644
--- a/hw/ppc/e500-ccsr.h
+++ b/hw/ppc/ppce500_ccsr.h
@@ -1,13 +1,13 @@
-#ifndef E500_CCSR_H
-#define E500_CCSR_H
+#ifndef PPCE500_CCSR_H
+#define PPCE500_CCSR_H
 
 #include "hw/sysbus.h"
 #include "qom/object.h"
 
+#define MPC85XX_CCSRBAR_SIZE       0x00100000ULL
+
 struct PPCE500CCSRState {
-    /*< private >*/
     SysBusDevice parent;
-    /*< public >*/
 
     MemoryRegion ccsr_space;
 };
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 2225533e33..74836ebb64 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -20,7 +20,7 @@
 #include "qemu/guest-random.h"
 #include "qapi/error.h"
 #include "e500.h"
-#include "e500-ccsr.h"
+#include "ppce500_ccsr.h"
 #include "net/net.h"
 #include "qemu/config-file.h"
 #include "hw/block/flash.h"
@@ -61,7 +61,6 @@
 #define RAM_SIZES_ALIGN            (64 * MiB)
 
 /* TODO: parameterize */
-#define MPC8544_CCSRBAR_SIZE       0x00100000ULL
 #define MPC8544_MPIC_REGS_OFFSET   0x40000ULL
 #define MPC8544_MSI_REGS_OFFSET   0x41600ULL
 #define MPC8544_SERIAL0_REGS_OFFSET 0x4500ULL
@@ -532,7 +531,7 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
     qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
     qemu_fdt_setprop_cells(fdt, soc, "ranges", 0x0,
                            pmc->ccsrbar_base >> 32, pmc->ccsrbar_base,
-                           MPC8544_CCSRBAR_SIZE);
+                           MPC85XX_CCSRBAR_SIZE);
     /* XXX should contain a reasonable value */
     qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
 
@@ -1264,21 +1263,6 @@ void ppce500_init(MachineState *machine)
     pms->boot_info.dt_size = dt_size;
 }
 
-static void e500_ccsr_initfn(Object *obj)
-{
-    PPCE500CCSRState *ccsr = CCSR(obj);
-    memory_region_init(&ccsr->ccsr_space, obj, "e500-ccsr",
-                       MPC8544_CCSRBAR_SIZE);
-    sysbus_init_mmio(SYS_BUS_DEVICE(ccsr), &ccsr->ccsr_space);
-}
-
-static const TypeInfo e500_ccsr_info = {
-    .name          = TYPE_CCSR,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(PPCE500CCSRState),
-    .instance_init = e500_ccsr_initfn,
-};
-
 static const TypeInfo ppce500_info = {
     .name          = TYPE_PPCE500_MACHINE,
     .parent        = TYPE_MACHINE,
@@ -1289,7 +1273,6 @@ static const TypeInfo ppce500_info = {
 
 static void e500_register_types(void)
 {
-    type_register_static(&e500_ccsr_info);
     type_register_static(&ppce500_info);
 }
 
diff --git a/hw/ppc/ppce500_ccsr.c b/hw/ppc/ppce500_ccsr.c
new file mode 100644
index 0000000000..5d0e1e0e89
--- /dev/null
+++ b/hw/ppc/ppce500_ccsr.c
@@ -0,0 +1,34 @@
+/*
+ * QEMU PowerPC E500 embedded processors CCSR space emulation
+ *
+ * Copyright (C) 2009 Freescale Semiconductor, Inc. All rights reserved.
+ *
+ * Author: Yu Liu,     <yu.liu@freescale.com>
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of  the GNU General  Public License as published by
+ * the Free Software Foundation;  either version 2 of the  License, or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "ppce500_ccsr.h"
+
+static void ppce500_ccsr_init(Object *obj)
+{
+    PPCE500CCSRState *s = CCSR(obj);
+
+    memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->ccsr_space);
+}
+
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_CCSR,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(PPCE500CCSRState),
+        .instance_init = ppce500_ccsr_init,
+    },
+};
+
+DEFINE_TYPES(types)
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index 7cd9189869..43c746795a 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -81,6 +81,7 @@ ppc_ss.add(when: 'CONFIG_MPC8544DS', if_true: files('mpc8544ds.c'))
 ppc_ss.add(when: 'CONFIG_E500', if_true: files(
   'e500.c',
   'mpc8544_guts.c',
+  'ppce500_ccsr.c',
   'ppce500_spin.c'
 ))
 # PowerPC 440 Xilinx ML507 reference board.
-- 
2.46.2

[PATCH v2 08/23] hw/ppc/ppce500_ccsr: Log access to unimplemented registers

[Bernhard Beschow]

The CCSR space is just a container which is meant to be covered by platform
device memory regions. However, QEMU only implements a subset of these devices.
Add some logging to see which devices a guest attempts to access.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/ppce500_ccsr.c | 32 +++++++++++++++++++++++++++++++-
 hw/ppc/trace-events   |  3 +++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/ppce500_ccsr.c b/hw/ppc/ppce500_ccsr.c
index 5d0e1e0e89..6659560674 100644
--- a/hw/ppc/ppce500_ccsr.c
+++ b/hw/ppc/ppce500_ccsr.c
@@ -13,12 +13,42 @@
 
 #include "qemu/osdep.h"
 #include "ppce500_ccsr.h"
+#include "qemu/log.h"
+#include "trace.h"
+
+static uint64_t ppce500_ccsr_io_read(void *opaque, hwaddr addr, unsigned size)
+{
+    uint64_t value = 0;
+
+    trace_ppce500_ccsr_io_read(addr, value, size);
+    qemu_log_mask(LOG_UNIMP,
+                  "%s: unimplemented [0x%" HWADDR_PRIx "] -> 0\n",
+                  __func__, addr);
+
+    return value;
+}
+
+static void ppce500_ccsr_io_write(void *opaque, hwaddr addr, uint64_t value,
+                                  unsigned size)
+{
+    trace_ppce500_ccsr_io_write(addr, value, size);
+    qemu_log_mask(LOG_UNIMP,
+                  "%s: unimplemented [0x%" HWADDR_PRIx "] <- 0x%" PRIx32 "\n",
+                  __func__, addr, (uint32_t)value);
+}
+
+static const MemoryRegionOps ppce500_ccsr_ops = {
+    .read = ppce500_ccsr_io_read,
+    .write = ppce500_ccsr_io_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
 
 static void ppce500_ccsr_init(Object *obj)
 {
     PPCE500CCSRState *s = CCSR(obj);
 
-    memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
+    memory_region_init_io(&s->ccsr_space, obj, &ppce500_ccsr_ops, obj,
+                          "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->ccsr_space);
 }
 
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 1f125ce841..ca4c231c9f 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -143,6 +143,9 @@ ppc_irq_cpu(const char *action) "%s"
 ppc_dcr_read(uint32_t addr, uint32_t val) "DRCN[0x%x] -> 0x%x"
 ppc_dcr_write(uint32_t addr, uint32_t val) "DRCN[0x%x] <- 0x%x"
 
+ppce500_ccsr_io_read(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] -> 0x%08x (size: 0x%" PRIu8 ")"
+ppce500_ccsr_io_write(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] <- 0x%08x (size: 0x%" PRIu8 ")"
+
 # prep_systemio.c
 prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
 prep_systemio_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
-- 
2.46.2

Re: [PATCH v2 08/23] hw/ppc/ppce500_ccsr: Log access to unimplemented registers

[BALATON Zoltan]

On Sat, 5 Oct 2024, Bernhard Beschow wrote:
> The CCSR space is just a container which is meant to be covered by platform
> device memory regions. However, QEMU only implements a subset of these devices.
> Add some logging to see which devices a guest attempts to access.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
> hw/ppc/ppce500_ccsr.c | 32 +++++++++++++++++++++++++++++++-
> hw/ppc/trace-events   |  3 +++
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/ppce500_ccsr.c b/hw/ppc/ppce500_ccsr.c
> index 5d0e1e0e89..6659560674 100644
> --- a/hw/ppc/ppce500_ccsr.c
> +++ b/hw/ppc/ppce500_ccsr.c
> @@ -13,12 +13,42 @@
>
> #include "qemu/osdep.h"
> #include "ppce500_ccsr.h"
> +#include "qemu/log.h"
> +#include "trace.h"
> +
> +static uint64_t ppce500_ccsr_io_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t value = 0;
> +
> +    trace_ppce500_ccsr_io_read(addr, value, size);
> +    qemu_log_mask(LOG_UNIMP,
> +                  "%s: unimplemented [0x%" HWADDR_PRIx "] -> 0\n",
> +                  __func__, addr);

I'm not sure having both unimp log and traces is the best way. I thought 
unimp log with an unimplemented device area would be the simplest and 
least intrusive for the code but if you prefer traces then maybe we don't 
need unimp logs. But adding these otherwise empty functions (which won't 
get populated as subdevices have their own regions) still bothers me a bit 
but not enough to block this if others have no opinion on it.

I also had this patch:
https://patchew.org/QEMU/cover.1728232526.git.balaton@eik.bme.hu/
which I first thought might help but that's about guest_errors not unimp 
logs so does not apply here. What other unimp logs get in the way here 
that makes traces a better choice?

Regards,
BALATON Zoltan

> +
> +    return value;
> +}
> +
> +static void ppce500_ccsr_io_write(void *opaque, hwaddr addr, uint64_t value,
> +                                  unsigned size)
> +{
> +    trace_ppce500_ccsr_io_write(addr, value, size);
> +    qemu_log_mask(LOG_UNIMP,
> +                  "%s: unimplemented [0x%" HWADDR_PRIx "] <- 0x%" PRIx32 "\n",
> +                  __func__, addr, (uint32_t)value);
> +}
> +
> +static const MemoryRegionOps ppce500_ccsr_ops = {
> +    .read = ppce500_ccsr_io_read,
> +    .write = ppce500_ccsr_io_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
>
> static void ppce500_ccsr_init(Object *obj)
> {
>     PPCE500CCSRState *s = CCSR(obj);
>
> -    memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
> +    memory_region_init_io(&s->ccsr_space, obj, &ppce500_ccsr_ops, obj,
> +                          "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
>     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->ccsr_space);
> }
>
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index 1f125ce841..ca4c231c9f 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -143,6 +143,9 @@ ppc_irq_cpu(const char *action) "%s"
> ppc_dcr_read(uint32_t addr, uint32_t val) "DRCN[0x%x] -> 0x%x"
> ppc_dcr_write(uint32_t addr, uint32_t val) "DRCN[0x%x] <- 0x%x"
>
> +ppce500_ccsr_io_read(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] -> 0x%08x (size: 0x%" PRIu8 ")"
> +ppce500_ccsr_io_write(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] <- 0x%08x (size: 0x%" PRIu8 ")"
> +
> # prep_systemio.c
> prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
> prep_systemio_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
>

Re: [PATCH v2 08/23] hw/ppc/ppce500_ccsr: Log access to unimplemented registers

[Bernhard Beschow]

Am 6. Oktober 2024 17:12:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sat, 5 Oct 2024, Bernhard Beschow wrote:
>> The CCSR space is just a container which is meant to be covered by platform
>> device memory regions. However, QEMU only implements a subset of these devices.
>> Add some logging to see which devices a guest attempts to access.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> ---
>> hw/ppc/ppce500_ccsr.c | 32 +++++++++++++++++++++++++++++++-
>> hw/ppc/trace-events   |  3 +++
>> 2 files changed, 34 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/ppc/ppce500_ccsr.c b/hw/ppc/ppce500_ccsr.c
>> index 5d0e1e0e89..6659560674 100644
>> --- a/hw/ppc/ppce500_ccsr.c
>> +++ b/hw/ppc/ppce500_ccsr.c
>> @@ -13,12 +13,42 @@
>> 
>> #include "qemu/osdep.h"
>> #include "ppce500_ccsr.h"
>> +#include "qemu/log.h"
>> +#include "trace.h"
>> +
>> +static uint64_t ppce500_ccsr_io_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    uint64_t value = 0;
>> +
>> +    trace_ppce500_ccsr_io_read(addr, value, size);
>> +    qemu_log_mask(LOG_UNIMP,
>> +                  "%s: unimplemented [0x%" HWADDR_PRIx "] -> 0\n",
>> +                  __func__, addr);
>
>I'm not sure having both unimp log and traces is the best way. I thought unimp log with an unimplemented device area would be the simplest and least intrusive for the code but if you prefer traces then maybe we don't need unimp logs. But adding these otherwise empty functions (which won't get populated as subdevices have their own regions) still bothers me a bit but not enough to block this if others have no opinion on it.
>
>I also had this patch:
>https://patchew.org/QEMU/cover.1728232526.git.balaton@eik.bme.hu/
>which I first thought might help but that's about guest_errors not unimp logs so does not apply here. What other unimp logs get in the way here that makes traces a better choice?

I have some rough implementations of a few other device models with unimp logging. Having a dedicated trace here allows to see which unoccupied CCSR regions a guest pokes. The unimp logging here actually disturbs the above, so I'd remove it here.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> +
>> +    return value;
>> +}
>> +
>> +static void ppce500_ccsr_io_write(void *opaque, hwaddr addr, uint64_t value,
>> +                                  unsigned size)
>> +{
>> +    trace_ppce500_ccsr_io_write(addr, value, size);
>> +    qemu_log_mask(LOG_UNIMP,
>> +                  "%s: unimplemented [0x%" HWADDR_PRIx "] <- 0x%" PRIx32 "\n",
>> +                  __func__, addr, (uint32_t)value);
>> +}
>> +
>> +static const MemoryRegionOps ppce500_ccsr_ops = {
>> +    .read = ppce500_ccsr_io_read,
>> +    .write = ppce500_ccsr_io_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> 
>> static void ppce500_ccsr_init(Object *obj)
>> {
>>     PPCE500CCSRState *s = CCSR(obj);
>> 
>> -    memory_region_init(&s->ccsr_space, obj, "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
>> +    memory_region_init_io(&s->ccsr_space, obj, &ppce500_ccsr_ops, obj,
>> +                          "e500-ccsr", MPC85XX_CCSRBAR_SIZE);
>>     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->ccsr_space);
>> }
>> 
>> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
>> index 1f125ce841..ca4c231c9f 100644
>> --- a/hw/ppc/trace-events
>> +++ b/hw/ppc/trace-events
>> @@ -143,6 +143,9 @@ ppc_irq_cpu(const char *action) "%s"
>> ppc_dcr_read(uint32_t addr, uint32_t val) "DRCN[0x%x] -> 0x%x"
>> ppc_dcr_write(uint32_t addr, uint32_t val) "DRCN[0x%x] <- 0x%x"
>> 
>> +ppce500_ccsr_io_read(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] -> 0x%08x (size: 0x%" PRIu8 ")"
>> +ppce500_ccsr_io_write(uint32_t index, uint32_t val, uint8_t size) "[0x%" PRIx32 "] <- 0x%08x (size: 0x%" PRIu8 ")"
>> +
>> # prep_systemio.c
>> prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
>> prep_systemio_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
>> 

[PATCH v2 09/23] hw/ppc/mpc8544_guts: Populate POR PLL ratio status register

[Bernhard Beschow]

Populate this read-only register with some arbitrary values which avoids
U-Boot's get_clocks() to hang().

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

diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
index e3540b0281..c02b34ccde 100644
--- a/hw/ppc/mpc8544_guts.c
+++ b/hw/ppc/mpc8544_guts.c
@@ -29,6 +29,12 @@
 #define MPC8544_GUTS_RSTCR_RESET      0x02
 
 #define MPC8544_GUTS_ADDR_PORPLLSR    0x00
+REG32(GUTS_PORPLLSR, 0x00)
+    FIELD(GUTS_PORPLLSR, E500_1_RATIO, 24, 6)
+    FIELD(GUTS_PORPLLSR, E500_0_RATIO, 16, 6)
+    FIELD(GUTS_PORPLLSR, DDR_RATIO, 9, 5)
+    FIELD(GUTS_PORPLLSR, PLAT_RATIO, 1, 5)
+
 #define MPC8544_GUTS_ADDR_PORBMSR     0x04
 #define MPC8544_GUTS_ADDR_PORIMPSCR   0x08
 #define MPC8544_GUTS_ADDR_PORDEVSR    0x0C
@@ -75,6 +81,12 @@ static uint64_t mpc8544_guts_read(void *opaque, hwaddr addr,
 
     addr &= MPC8544_GUTS_MMIO_SIZE - 1;
     switch (addr) {
+    case MPC8544_GUTS_ADDR_PORPLLSR:
+        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_1_RATIO, 6); /* 3:1 */
+        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_0_RATIO, 6); /* 3:1 */
+        value = FIELD_DP32(value, GUTS_PORPLLSR, DDR_RATIO, 12); /* 12:1 */
+        value = FIELD_DP32(value, GUTS_PORPLLSR, PLAT_RATIO, 6); /* 6:1 */
+        break;
     case MPC8544_GUTS_ADDR_PVR:
         value = env->spr[SPR_PVR];
         break;
-- 
2.46.2

Re: [PATCH v2 09/23] hw/ppc/mpc8544_guts: Populate POR PLL ratio status register

[BALATON Zoltan]

On Sat, 5 Oct 2024, Bernhard Beschow wrote:
> Populate this read-only register with some arbitrary values which avoids
> U-Boot's get_clocks() to hang().
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

I think the register fields are overkill, a value with a comment or some 
or'ed shift'ed values with a comment would be simpler but the result is 
the same so

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Regards,
BALATON Zoltan

> ---
> hw/ppc/mpc8544_guts.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
> index e3540b0281..c02b34ccde 100644
> --- a/hw/ppc/mpc8544_guts.c
> +++ b/hw/ppc/mpc8544_guts.c
> @@ -29,6 +29,12 @@
> #define MPC8544_GUTS_RSTCR_RESET      0x02
>
> #define MPC8544_GUTS_ADDR_PORPLLSR    0x00
> +REG32(GUTS_PORPLLSR, 0x00)
> +    FIELD(GUTS_PORPLLSR, E500_1_RATIO, 24, 6)
> +    FIELD(GUTS_PORPLLSR, E500_0_RATIO, 16, 6)
> +    FIELD(GUTS_PORPLLSR, DDR_RATIO, 9, 5)
> +    FIELD(GUTS_PORPLLSR, PLAT_RATIO, 1, 5)
> +
> #define MPC8544_GUTS_ADDR_PORBMSR     0x04
> #define MPC8544_GUTS_ADDR_PORIMPSCR   0x08
> #define MPC8544_GUTS_ADDR_PORDEVSR    0x0C
> @@ -75,6 +81,12 @@ static uint64_t mpc8544_guts_read(void *opaque, hwaddr addr,
>
>     addr &= MPC8544_GUTS_MMIO_SIZE - 1;
>     switch (addr) {
> +    case MPC8544_GUTS_ADDR_PORPLLSR:
> +        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_1_RATIO, 6); /* 3:1 */
> +        value = FIELD_DP32(value, GUTS_PORPLLSR, E500_0_RATIO, 6); /* 3:1 */
> +        value = FIELD_DP32(value, GUTS_PORPLLSR, DDR_RATIO, 12); /* 12:1 */
> +        value = FIELD_DP32(value, GUTS_PORPLLSR, PLAT_RATIO, 6); /* 6:1 */
> +        break;
>     case MPC8544_GUTS_ADDR_PVR:
>         value = env->spr[SPR_PVR];
>         break;
>

[PATCH v2 10/23] hw/i2c/mpc_i2c: Convert DPRINTF to trace events for register access

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/mpc_i2c.c    | 9 +++++----
 hw/i2c/trace-events | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
index 2467d1a9aa..3d79c15653 100644
--- a/hw/i2c/mpc_i2c.c
+++ b/hw/i2c/mpc_i2c.c
@@ -24,6 +24,7 @@
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qom/object.h"
+#include "trace.h"
 
 /* #define DEBUG_I2C */
 
@@ -224,8 +225,8 @@ static uint64_t mpc_i2c_read(void *opaque, hwaddr addr, unsigned size)
         break;
     }
 
-    DPRINTF("%s: addr " HWADDR_FMT_plx " %02" PRIx32 "\n", __func__,
-                                         addr, value);
+    trace_mpc_i2c_read(addr, value);
+
     return (uint64_t)value;
 }
 
@@ -234,8 +235,8 @@ static void mpc_i2c_write(void *opaque, hwaddr addr,
 {
     MPCI2CState *s = opaque;
 
-    DPRINTF("%s: addr " HWADDR_FMT_plx " val %08" PRIx64 "\n", __func__,
-                                             addr, value);
+    trace_mpc_i2c_write(addr, value);
+
     switch (addr) {
     case MPC_I2C_ADR:
         s->adr = value & CADR_MASK;
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index 6900e06eda..f708a7ace1 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -35,6 +35,11 @@ aspeed_i2c_bus_write(uint32_t busid, uint64_t offset, unsigned size, uint64_t va
 aspeed_i2c_bus_send(const char *mode, int i, int count, uint8_t byte) "%s send %d/%d 0x%02x"
 aspeed_i2c_bus_recv(const char *mode, int i, int count, uint8_t byte) "%s recv %d/%d 0x%02x"
 
+# mpc_i2c.c
+
+mpc_i2c_read(uint64_t addr, uint32_t value) "[0x%" PRIx64 "] -> 0x%02" PRIx32
+mpc_i2c_write(uint64_t addr, uint32_t value) "[0x%" PRIx64 "] <- 0x%02" PRIx32
+
 # npcm7xx_smbus.c
 
 npcm7xx_smbus_read(const char *id, uint64_t offset, uint64_t value, unsigned size) "%s offset: 0x%04" PRIx64 " value: 0x%02" PRIx64 " size: %u"
-- 
2.46.2

[PATCH v2 11/23] hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/mpc_i2c.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/i2c/mpc_i2c.c b/hw/i2c/mpc_i2c.c
index 3d79c15653..16f4309ea9 100644
--- a/hw/i2c/mpc_i2c.c
+++ b/hw/i2c/mpc_i2c.c
@@ -20,7 +20,6 @@
 #include "qemu/osdep.h"
 #include "hw/i2c/i2c.h"
 #include "hw/irq.h"
-#include "qemu/module.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
 #include "qom/object.h"
@@ -345,16 +344,13 @@ static void mpc_i2c_class_init(ObjectClass *klass, void *data)
     dc->desc = "MPC I2C Controller";
 }
 
-static const TypeInfo mpc_i2c_type_info = {
-    .name          = TYPE_MPC_I2C,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(MPCI2CState),
-    .class_init    = mpc_i2c_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_MPC_I2C,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(MPCI2CState),
+        .class_init    = mpc_i2c_class_init,
+    },
 };
 
-static void mpc_i2c_register_types(void)
-{
-    type_register_static(&mpc_i2c_type_info);
-}
-
-type_init(mpc_i2c_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 12/23] hw/pci-host/ppce500: Reuse TYPE_PPC_E500_PCI_BRIDGE define

[Bernhard Beschow]

Prefer a macro rather than a string literal when instantiaging device models.

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/ppce500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 97e5d47cec..d7ff2ba778 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -475,7 +475,7 @@ static void e500_pcihost_realize(DeviceState *dev, Error **errp)
     address_space_init(&s->bm_as, &s->bm, "pci-bm");
     pci_setup_iommu(b, &ppce500_iommu_ops, s);
 
-    pci_create_simple(b, 0, "e500-host-bridge");
+    pci_create_simple(b, 0, TYPE_PPC_E500_PCI_BRIDGE);
 
     memory_region_init(&s->container, OBJECT(h), "pci-container", PCIE500_ALL_SIZE);
     memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, h,
-- 
2.46.2

[PATCH v2 13/23] hw/pci-host/ppce500: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/pci-host/ppce500.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index d7ff2ba778..1ce79ea20c 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -21,7 +21,6 @@
 #include "hw/pci/pci_device.h"
 #include "hw/pci/pci_host.h"
 #include "qemu/bswap.h"
-#include "qemu/module.h"
 #include "hw/pci-host/ppce500.h"
 #include "qom/object.h"
 
@@ -508,17 +507,6 @@ static void e500_host_bridge_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static const TypeInfo e500_host_bridge_info = {
-    .name          = TYPE_PPC_E500_PCI_BRIDGE,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(PPCE500PCIBridgeState),
-    .class_init    = e500_host_bridge_class_init,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { },
-    },
-};
-
 static Property pcihost_properties[] = {
     DEFINE_PROP_UINT32("first_slot", PPCE500PCIState, first_slot, 0x11),
     DEFINE_PROP_UINT32("first_pin_irq", PPCE500PCIState, first_pin_irq, 0x1),
@@ -535,17 +523,23 @@ static void e500_pcihost_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_ppce500_pci;
 }
 
-static const TypeInfo e500_pcihost_info = {
-    .name          = TYPE_PPC_E500_PCI_HOST_BRIDGE,
-    .parent        = TYPE_PCI_HOST_BRIDGE,
-    .instance_size = sizeof(PPCE500PCIState),
-    .class_init    = e500_pcihost_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_PPC_E500_PCI_BRIDGE,
+        .parent        = TYPE_PCI_DEVICE,
+        .instance_size = sizeof(PPCE500PCIBridgeState),
+        .class_init    = e500_host_bridge_class_init,
+        .interfaces    = (InterfaceInfo[]) {
+            { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+            { },
+        },
+    },
+    {
+        .name          = TYPE_PPC_E500_PCI_HOST_BRIDGE,
+        .parent        = TYPE_PCI_HOST_BRIDGE,
+        .instance_size = sizeof(PPCE500PCIState),
+        .class_init    = e500_pcihost_class_init,
+    },
 };
 
-static void e500_pci_register_types(void)
-{
-    type_register_static(&e500_pcihost_info);
-    type_register_static(&e500_host_bridge_info);
-}
-
-type_init(e500_pci_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 14/23] hw/gpio/mpc8xxx: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/mpc8xxx.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/gpio/mpc8xxx.c b/hw/gpio/mpc8xxx.c
index 63b7a5c881..de183c3be5 100644
--- a/hw/gpio/mpc8xxx.c
+++ b/hw/gpio/mpc8xxx.c
@@ -23,7 +23,6 @@
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
-#include "qemu/module.h"
 #include "qom/object.h"
 
 #define TYPE_MPC8XXX_GPIO "mpc8xxx_gpio"
@@ -208,17 +207,14 @@ static void mpc8xxx_gpio_class_init(ObjectClass *klass, void *data)
     device_class_set_legacy_reset(dc, mpc8xxx_gpio_reset);
 }
 
-static const TypeInfo mpc8xxx_gpio_info = {
-    .name          = TYPE_MPC8XXX_GPIO,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(MPC8XXXGPIOState),
-    .instance_init = mpc8xxx_gpio_initfn,
-    .class_init    = mpc8xxx_gpio_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_MPC8XXX_GPIO,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(MPC8XXXGPIOState),
+        .instance_init = mpc8xxx_gpio_initfn,
+        .class_init    = mpc8xxx_gpio_class_init,
+    },
 };
 
-static void mpc8xxx_gpio_register_types(void)
-{
-    type_register_static(&mpc8xxx_gpio_info);
-}
-
-type_init(mpc8xxx_gpio_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 15/23] hw/ppc/mpc8544_guts: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/mpc8544_guts.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/mpc8544_guts.c b/hw/ppc/mpc8544_guts.c
index c02b34ccde..cf2317b3ab 100644
--- a/hw/ppc/mpc8544_guts.c
+++ b/hw/ppc/mpc8544_guts.c
@@ -18,7 +18,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/module.h"
 #include "qemu/log.h"
 #include "sysemu/runstate.h"
 #include "cpu.h"
@@ -141,16 +140,13 @@ static void mpc8544_guts_initfn(Object *obj)
     sysbus_init_mmio(d, &s->iomem);
 }
 
-static const TypeInfo mpc8544_guts_info = {
-    .name          = TYPE_MPC8544_GUTS,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(GutsState),
-    .instance_init = mpc8544_guts_initfn,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_MPC8544_GUTS,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(GutsState),
+        .instance_init = mpc8544_guts_initfn,
+    },
 };
 
-static void mpc8544_guts_register_types(void)
-{
-    type_register_static(&mpc8544_guts_info);
-}
-
-type_init(mpc8544_guts_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 16/23] hw/net/fsl_etsec/etsec: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/net/fsl_etsec/etsec.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index 3fdd16ef2e..9bd886b996 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -36,7 +36,6 @@
 #include "registers.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
-#include "qemu/module.h"
 
 /* #define HEX_DUMP */
 /* #define DEBUG_REGISTER */
@@ -431,17 +430,14 @@ static void etsec_class_init(ObjectClass *klass, void *data)
     dc->user_creatable = true;
 }
 
-static const TypeInfo etsec_info = {
-    .name                  = TYPE_ETSEC_COMMON,
-    .parent                = TYPE_SYS_BUS_DEVICE,
-    .instance_size         = sizeof(eTSEC),
-    .class_init            = etsec_class_init,
-    .instance_init         = etsec_instance_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_ETSEC_COMMON,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(eTSEC),
+        .class_init    = etsec_class_init,
+        .instance_init = etsec_instance_init,
+    },
 };
 
-static void etsec_register_types(void)
-{
-    type_register_static(&etsec_info);
-}
-
-type_init(etsec_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 17/23] hw/intc: Guard openpic_kvm.c by dedicated OPENPIC_KVM Kconfig switch

[Bernhard Beschow]

Allows to clearly mark code sections relying on this device type.

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/ppc/e500.c       | 2 +-
 hw/intc/Kconfig     | 4 ++++
 hw/intc/meson.build | 3 +--
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 74836ebb64..566c3b6f8e 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -828,7 +828,7 @@ static DeviceState *ppce500_init_mpic_qemu(PPCE500MachineState *pms,
 static DeviceState *ppce500_init_mpic_kvm(const PPCE500MachineClass *pmc,
                                           Error **errp)
 {
-#ifdef CONFIG_KVM
+#ifdef CONFIG_OPENPIC_KVM
     DeviceState *dev;
     CPUState *cs;
 
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index dd405bdb5d..a3df98ae59 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -16,6 +16,10 @@ config OPENPIC
     bool
     select MSI_NONBROKEN
 
+config OPENPIC_KVM
+    bool
+    depends on OPENPIC && KVM
+
 config APIC
     bool
     select MSI_NONBROKEN
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 6bfdc4eb33..b9de6bf5c6 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -48,8 +48,7 @@ specific_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic.c'))
 specific_ss.add(when: 'CONFIG_LOONGSON_LIOINTC', if_true: files('loongson_liointc.c'))
 specific_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('mips_gic.c'))
 specific_ss.add(when: 'CONFIG_OMPIC', if_true: files('ompic.c'))
-specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_OPENPIC'],
-		if_true: files('openpic_kvm.c'))
+specific_ss.add(when: 'CONFIG_OPENPIC_KVM', if_true: files('openpic_kvm.c'))
 specific_ss.add(when: 'CONFIG_POWERNV', if_true: files('xics_pnv.c', 'pnv_xive.c', 'pnv_xive2.c'))
 specific_ss.add(when: 'CONFIG_PPC_UIC', if_true: files('ppc-uic.c'))
 specific_ss.add(when: 'CONFIG_RX_ICU', if_true: files('rx_icu.c'))
-- 
2.46.2

[PATCH v2 18/23] hw/sd/sdhci: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/sd/sdhci.c | 62 +++++++++++++++++++++------------------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ed01499391..aa89513796 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -37,7 +37,6 @@
 #include "migration/vmstate.h"
 #include "sdhci-internal.h"
 #include "qemu/log.h"
-#include "qemu/module.h"
 #include "trace.h"
 #include "qom/object.h"
 
@@ -1598,15 +1597,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
     sdhci_common_class_init(klass, data);
 }
 
-static const TypeInfo sdhci_sysbus_info = {
-    .name = TYPE_SYSBUS_SDHCI,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(SDHCIState),
-    .instance_init = sdhci_sysbus_init,
-    .instance_finalize = sdhci_sysbus_finalize,
-    .class_init = sdhci_sysbus_class_init,
-};
-
 /* --- qdev bus master --- */
 
 static void sdhci_bus_class_init(ObjectClass *klass, void *data)
@@ -1617,13 +1607,6 @@ static void sdhci_bus_class_init(ObjectClass *klass, void *data)
     sbc->set_readonly = sdhci_set_readonly;
 }
 
-static const TypeInfo sdhci_bus_info = {
-    .name = TYPE_SDHCI_BUS,
-    .parent = TYPE_SD_BUS,
-    .instance_size = sizeof(SDBus),
-    .class_init = sdhci_bus_class_init,
-};
-
 /* --- qdev i.MX eSDHC --- */
 
 #define USDHC_MIX_CTRL                  0x48
@@ -1882,12 +1865,6 @@ static void imx_usdhc_init(Object *obj)
     s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ;
 }
 
-static const TypeInfo imx_usdhc_info = {
-    .name = TYPE_IMX_USDHC,
-    .parent = TYPE_SYSBUS_SDHCI,
-    .instance_init = imx_usdhc_init,
-};
-
 /* --- qdev Samsung s3c --- */
 
 #define S3C_SDHCI_CONTROL2      0x80
@@ -1946,18 +1923,31 @@ static void sdhci_s3c_init(Object *obj)
     s->io_ops = &sdhci_s3c_mmio_ops;
 }
 
-static const TypeInfo sdhci_s3c_info = {
-    .name = TYPE_S3C_SDHCI  ,
-    .parent = TYPE_SYSBUS_SDHCI,
-    .instance_init = sdhci_s3c_init,
+static const TypeInfo types[] = {
+    {
+        .name = TYPE_SDHCI_BUS,
+        .parent = TYPE_SD_BUS,
+        .instance_size = sizeof(SDBus),
+        .class_init = sdhci_bus_class_init,
+    },
+    {
+        .name = TYPE_SYSBUS_SDHCI,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(SDHCIState),
+        .instance_init = sdhci_sysbus_init,
+        .instance_finalize = sdhci_sysbus_finalize,
+        .class_init = sdhci_sysbus_class_init,
+    },
+    {
+        .name = TYPE_IMX_USDHC,
+        .parent = TYPE_SYSBUS_SDHCI,
+        .instance_init = imx_usdhc_init,
+    },
+    {
+        .name = TYPE_S3C_SDHCI,
+        .parent = TYPE_SYSBUS_SDHCI,
+        .instance_init = sdhci_s3c_init,
+    },
 };
 
-static void sdhci_register_types(void)
-{
-    type_register_static(&sdhci_sysbus_info);
-    type_register_static(&sdhci_bus_info);
-    type_register_static(&imx_usdhc_info);
-    type_register_static(&sdhci_s3c_info);
-}
-
-type_init(sdhci_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 19/23] hw/block/pflash_cfi01: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/block/pflash_cfi01.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 7b6ec64442..cf11dada29 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -47,7 +47,6 @@
 #include "qemu/bitops.h"
 #include "qemu/host-utils.h"
 #include "qemu/log.h"
-#include "qemu/module.h"
 #include "qemu/option.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
@@ -947,20 +946,16 @@ static void pflash_cfi01_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
-
-static const TypeInfo pflash_cfi01_info = {
-    .name           = TYPE_PFLASH_CFI01,
-    .parent         = TYPE_SYS_BUS_DEVICE,
-    .instance_size  = sizeof(PFlashCFI01),
-    .class_init     = pflash_cfi01_class_init,
+static const TypeInfo types[] = {
+    {
+        .name           = TYPE_PFLASH_CFI01,
+        .parent         = TYPE_SYS_BUS_DEVICE,
+        .instance_size  = sizeof(PFlashCFI01),
+        .class_init     = pflash_cfi01_class_init,
+    },
 };
 
-static void pflash_cfi01_register_types(void)
-{
-    type_register_static(&pflash_cfi01_info);
-}
-
-type_init(pflash_cfi01_register_types)
+DEFINE_TYPES(types)
 
 PFlashCFI01 *pflash_cfi01_register(hwaddr base,
                                    const char *name,
-- 
2.46.2

Re: [PATCH v2 19/23] hw/block/pflash_cfi01: Prefer DEFINE_TYPES() macro

[Kevin Wolf]

Am 05.10.2024 um 21:45 hat Bernhard Beschow geschrieben:
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

[PATCH v2 20/23] hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i2c/smbus_eeprom.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
index 9e62c27a1a..1d4d9704bf 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -151,19 +151,16 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static const TypeInfo smbus_eeprom_info = {
-    .name          = TYPE_SMBUS_EEPROM,
-    .parent        = TYPE_SMBUS_DEVICE,
-    .instance_size = sizeof(SMBusEEPROMDevice),
-    .class_init    = smbus_eeprom_class_initfn,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_SMBUS_EEPROM,
+        .parent        = TYPE_SMBUS_DEVICE,
+        .instance_size = sizeof(SMBusEEPROMDevice),
+        .class_init    = smbus_eeprom_class_initfn,
+    },
 };
 
-static void smbus_eeprom_register_types(void)
-{
-    type_register_static(&smbus_eeprom_info);
-}
-
-type_init(smbus_eeprom_register_types)
+DEFINE_TYPES(types)
 
 void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
 {
-- 
2.46.2

[PATCH v2 21/23] hw/rtc/ds1338: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

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

diff --git a/hw/rtc/ds1338.c b/hw/rtc/ds1338.c
index a5fe221418..6de13caf99 100644
--- a/hw/rtc/ds1338.c
+++ b/hw/rtc/ds1338.c
@@ -14,7 +14,6 @@
 #include "hw/i2c/i2c.h"
 #include "migration/vmstate.h"
 #include "qemu/bcd.h"
-#include "qemu/module.h"
 #include "qom/object.h"
 #include "sysemu/rtc.h"
 
@@ -227,16 +226,13 @@ static void ds1338_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_ds1338;
 }
 
-static const TypeInfo ds1338_info = {
-    .name          = TYPE_DS1338,
-    .parent        = TYPE_I2C_SLAVE,
-    .instance_size = sizeof(DS1338State),
-    .class_init    = ds1338_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_DS1338,
+        .parent        = TYPE_I2C_SLAVE,
+        .instance_size = sizeof(DS1338State),
+        .class_init    = ds1338_class_init,
+    },
 };
 
-static void ds1338_register_types(void)
-{
-    type_register_static(&ds1338_info);
-}
-
-type_init(ds1338_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 22/23] hw/usb/hcd-ehci-sysbus: Prefer DEFINE_TYPES() macro

[Bernhard Beschow]

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/usb/hcd-ehci-sysbus.c | 118 +++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 68 deletions(-)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 2b1652f7a8..87a3bebe3e 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -19,7 +19,6 @@
 #include "hw/qdev-properties.h"
 #include "hw/usb/hcd-ehci.h"
 #include "migration/vmstate.h"
-#include "qemu/module.h"
 
 static const VMStateDescription vmstate_ehci_sysbus = {
     .name        = "ehci-sysbus",
@@ -97,17 +96,6 @@ static void ehci_sysbus_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_type_info = {
-    .name          = TYPE_SYS_BUS_EHCI,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(EHCISysBusState),
-    .instance_init = ehci_sysbus_init,
-    .instance_finalize = ehci_sysbus_finalize,
-    .abstract      = true,
-    .class_init    = ehci_sysbus_class_init,
-    .class_size    = sizeof(SysBusEHCIClass),
-};
-
 static void ehci_platform_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -118,12 +106,6 @@ static void ehci_platform_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_platform_type_info = {
-    .name          = TYPE_PLATFORM_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_platform_class_init,
-};
-
 static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -134,12 +116,6 @@ static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_exynos4210_type_info = {
-    .name          = TYPE_EXYNOS4210_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_exynos4210_class_init,
-};
-
 static void ehci_aw_h3_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -150,12 +126,6 @@ static void ehci_aw_h3_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_aw_h3_type_info = {
-    .name          = TYPE_AW_H3_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_aw_h3_class_init,
-};
-
 static void ehci_npcm7xx_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -168,12 +138,6 @@ static void ehci_npcm7xx_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_npcm7xx_type_info = {
-    .name          = TYPE_NPCM7XX_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_npcm7xx_class_init,
-};
-
 static void ehci_tegra2_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -184,12 +148,6 @@ static void ehci_tegra2_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_tegra2_type_info = {
-    .name          = TYPE_TEGRA2_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_tegra2_class_init,
-};
-
 static void ehci_ppc4xx_init(Object *o)
 {
     EHCISysBusState *s = SYS_BUS_EHCI(o);
@@ -207,13 +165,6 @@ static void ehci_ppc4xx_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_ppc4xx_type_info = {
-    .name          = TYPE_PPC4xx_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .class_init    = ehci_ppc4xx_class_init,
-    .instance_init = ehci_ppc4xx_init,
-};
-
 /*
  * Faraday FUSBH200 USB 2.0 EHCI
  */
@@ -282,24 +233,55 @@ static void fusbh200_ehci_class_init(ObjectClass *oc, void *data)
     set_bit(DEVICE_CATEGORY_USB, dc->categories);
 }
 
-static const TypeInfo ehci_fusbh200_type_info = {
-    .name          = TYPE_FUSBH200_EHCI,
-    .parent        = TYPE_SYS_BUS_EHCI,
-    .instance_size = sizeof(FUSBH200EHCIState),
-    .instance_init = fusbh200_ehci_init,
-    .class_init    = fusbh200_ehci_class_init,
+static const TypeInfo types[] = {
+    {
+        .name          = TYPE_SYS_BUS_EHCI,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(EHCISysBusState),
+        .instance_init = ehci_sysbus_init,
+        .instance_finalize = ehci_sysbus_finalize,
+        .abstract      = true,
+        .class_init    = ehci_sysbus_class_init,
+        .class_size    = sizeof(SysBusEHCIClass),
+    },
+    {
+        .name          = TYPE_PLATFORM_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_platform_class_init,
+    },
+    {
+        .name          = TYPE_EXYNOS4210_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_exynos4210_class_init,
+    },
+    {
+        .name          = TYPE_AW_H3_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_aw_h3_class_init,
+    },
+    {
+        .name          = TYPE_NPCM7XX_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_npcm7xx_class_init,
+    },
+    {
+        .name          = TYPE_TEGRA2_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_tegra2_class_init,
+    },
+    {
+        .name          = TYPE_PPC4xx_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .class_init    = ehci_ppc4xx_class_init,
+        .instance_init = ehci_ppc4xx_init,
+    },
+    {
+        .name          = TYPE_FUSBH200_EHCI,
+        .parent        = TYPE_SYS_BUS_EHCI,
+        .instance_size = sizeof(FUSBH200EHCIState),
+        .instance_init = fusbh200_ehci_init,
+        .class_init    = fusbh200_ehci_class_init,
+    },
 };
 
-static void ehci_sysbus_register_types(void)
-{
-    type_register_static(&ehci_type_info);
-    type_register_static(&ehci_platform_type_info);
-    type_register_static(&ehci_exynos4210_type_info);
-    type_register_static(&ehci_aw_h3_type_info);
-    type_register_static(&ehci_npcm7xx_type_info);
-    type_register_static(&ehci_tegra2_type_info);
-    type_register_static(&ehci_ppc4xx_type_info);
-    type_register_static(&ehci_fusbh200_type_info);
-}
-
-type_init(ehci_sysbus_register_types)
+DEFINE_TYPES(types)
-- 
2.46.2

[PATCH v2 23/23] hw/vfio/platform: Let vfio_start_eventfd_injection() take VFIOPlatformDevice pointer

[Bernhard Beschow]

Avoids one downcast, making the code more type-safe.

Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/vfio/platform.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index a85c199c76..77bbfbf62c 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -318,13 +318,12 @@ static void vfio_platform_eoi(VFIODevice *vbasedev)
 /**
  * vfio_start_eventfd_injection - starts the virtual IRQ injection using
  * user-side handled eventfds
- * @sbdev: the sysbus device handle
+ * @vdev: the VFIO platform device handle
  * @irq: the qemu irq handle
  */
 
-static void vfio_start_eventfd_injection(SysBusDevice *sbdev, qemu_irq irq)
+static void vfio_start_eventfd_injection(VFIOPlatformDevice *vdev, qemu_irq irq)
 {
-    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
     VFIOINTp *intp;
 
     QLIST_FOREACH(intp, &vdev->intp_list, next) {
@@ -417,7 +416,7 @@ fail_vfio:
     kvm_irqchip_remove_irqfd_notifier(kvm_state, intp->interrupt, irq);
     abort();
 fail_irqfd:
-    vfio_start_eventfd_injection(sbdev, irq);
+    vfio_start_eventfd_injection(vdev, irq);
     return;
 }
 
-- 
2.46.2