Re: [PATCH v7 02/22] hw/i3c/aspeed_i3c: Switch to DEFINE_TYPES() and align parent_obj naming

Cédric Le Goater
2026-02-25T10:48:20Z

On 2/25/26 03:12, Jamin Lin wrote:
> Following review feedback, update the Aspeed I3C device to use the
> DEFINE_TYPES() macro instead of an explicit type registration function.
>
> DEFINE_TYPES() is the currently recommended approach in QEMU for
> registering multiple TypeInfo entries and avoids boilerplate
> type_init() code.
>
> Additionally, rename embedded SysBusDevice fields from "parent" to
> "parent_obj" to comply with the QEMU coding style guidelines for QOM
> objects.
>
> No functional change.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
> include/hw/i3c/aspeed_i3c.h | 16 ++++++----------
> hw/i3c/aspeed_i3c.c | 35 +++++++++++++++--------------------
> 2 files changed, 21 insertions(+), 30 deletions(-)
>
> diff --git a/include/hw/i3c/aspeed_i3c.h b/include/hw/i3c/aspeed_i3c.h
> index 7a984e1f01..bd0ffc84ea 100644
> --- a/include/hw/i3c/aspeed_i3c.h
> +++ b/include/hw/i3c/aspeed_i3c.h
> @@ -21,28 +21,24 @@ OBJECT_DECLARE_TYPE(AspeedI3CState, AspeedI3CClass, ASPEED_I3C)
> #define ASPEED_I3C_NR_DEVICES 6
>
> OBJECT_DECLARE_SIMPLE_TYPE(AspeedI3CDevice, ASPEED_I3C_DEVICE)
> -typedef struct AspeedI3CDevice {
> - /* <private> */
> - SysBusDevice parent;
> +struct AspeedI3CDevice {
> + SysBusDevice parent_obj;
>
> - /* <public> */
> MemoryRegion mr;
> qemu_irq irq;
>
> uint8_t id;
> uint32_t regs[ASPEED_I3C_DEVICE_NR_REGS];
> -} AspeedI3CDevice;
> +};
>
> -typedef struct AspeedI3CState {
> - /* <private> */
> - SysBusDevice parent;
> +struct AspeedI3CState {
> + SysBusDevice parent_obj;
>
> - /* <public> */
> MemoryRegion iomem;
> MemoryRegion iomem_container;
> qemu_irq irq;
>
> uint32_t regs[ASPEED_I3C_NR_REGS];
> AspeedI3CDevice devices[ASPEED_I3C_NR_DEVICES];
> -} AspeedI3CState;
> +};
> #endif /* ASPEED_I3C_H */
> diff --git a/hw/i3c/aspeed_i3c.c b/hw/i3c/aspeed_i3c.c
> index fff259ff66..e7cdfbfdbd 100644
> --- a/hw/i3c/aspeed_i3c.c
> +++ b/hw/i3c/aspeed_i3c.c
> @@ -337,13 +337,6 @@ static void aspeed_i3c_device_class_init(ObjectClass *klass, const void *data)
> device_class_set_props(dc, aspeed_i3c_device_properties);
> }
>
> -static const TypeInfo aspeed_i3c_device_info = {
> - .name = TYPE_ASPEED_I3C_DEVICE,
> - .parent = TYPE_SYS_BUS_DEVICE,
> - .instance_size = sizeof(AspeedI3CDevice),
> - .class_init = aspeed_i3c_device_class_init,
> -};
> -
> static const VMStateDescription vmstate_aspeed_i3c = {
> .name = TYPE_ASPEED_I3C,
> .version_id = 1,
> @@ -366,18 +359,20 @@ static void aspeed_i3c_class_init(ObjectClass *klass, const void *data)
> dc->vmsd = &vmstate_aspeed_i3c;
> }
>
> -static const TypeInfo aspeed_i3c_info = {
> - .name = TYPE_ASPEED_I3C,
> - .parent = TYPE_SYS_BUS_DEVICE,
> - .instance_init = aspeed_i3c_instance_init,
> - .instance_size = sizeof(AspeedI3CState),
> - .class_init = aspeed_i3c_class_init,
> +static const TypeInfo aspeed_i3c_types[] = {
> + {
> + .name = TYPE_ASPEED_I3C,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_init = aspeed_i3c_instance_init,
> + .instance_size = sizeof(AspeedI3CState),
> + .class_init = aspeed_i3c_class_init,
> + },
> + {
> + .name = TYPE_ASPEED_I3C_DEVICE,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedI3CDevice),
> + .class_init = aspeed_i3c_device_class_init,
> + },
> };
>
> -static void aspeed_i3c_register_types(void)
> -{
> - type_register_static(&aspeed_i3c_device_info);
> - type_register_static(&aspeed_i3c_info);
> -}
> -
> -type_init(aspeed_i3c_register_types);
> +DEFINE_TYPES(aspeed_i3c_types)

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.