| Message ID | 20250308213640.13138-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/sd/sdhci: Set reset value of interrupt registers | expand |
On Sat, 8 Mar 2025, Philippe Mathieu-Daudé wrote: > TYPE_SYSBUS_SDHCI is a bit odd because it uses an union > to work with both SysBus / PCI parent. As this is not a > normal use, introduce SDHCIClass in its own commit. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/sd/sdhci.h | 9 +++++++++ > hw/sd/sdhci.c | 1 + > 2 files changed, 10 insertions(+) > > diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h > index 48247e9a20f..c4b20db3877 100644 > --- a/include/hw/sd/sdhci.h > +++ b/include/hw/sd/sdhci.h > @@ -107,6 +107,13 @@ struct SDHCIState { > }; > typedef struct SDHCIState SDHCIState; > > +typedef struct SDHCIClass { > + union { > + PCIDeviceClass pci_parent_class; > + SysBusDeviceClass sbd_parent_class; > + }; > +} SDHCIClass; > + > /* > * Controller does not provide transfer-complete interrupt when not > * busy. > @@ -123,6 +130,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, > #define TYPE_SYSBUS_SDHCI "generic-sdhci" > DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, > TYPE_SYSBUS_SDHCI) > +DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, > + TYPE_SYSBUS_SDHCI) Are these two together just OBJECT_DECLARE_TYPE? Then the above typedefs are also not needed just the struct definitions. Regards, BALATON Zoltan > #define TYPE_IMX_USDHC "imx-usdhc" > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index 149b748cbee..4917a9b3632 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -1960,6 +1960,7 @@ static const TypeInfo sdhci_types[] = { > .instance_size = sizeof(SDHCIState), > .instance_init = sdhci_sysbus_init, > .instance_finalize = sdhci_sysbus_finalize, > + .class_size = sizeof(SDHCIClass), > .class_init = sdhci_sysbus_class_init, > }, > { >
On 8/3/25 23:34, BALATON Zoltan wrote: > On Sat, 8 Mar 2025, Philippe Mathieu-Daudé wrote: >> TYPE_SYSBUS_SDHCI is a bit odd because it uses an union >> to work with both SysBus / PCI parent. As this is not a >> normal use, introduce SDHCIClass in its own commit. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/sd/sdhci.h | 9 +++++++++ >> hw/sd/sdhci.c | 1 + >> 2 files changed, 10 insertions(+) >> >> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >> index 48247e9a20f..c4b20db3877 100644 >> --- a/include/hw/sd/sdhci.h >> +++ b/include/hw/sd/sdhci.h >> @@ -107,6 +107,13 @@ struct SDHCIState { >> }; >> typedef struct SDHCIState SDHCIState; >> >> +typedef struct SDHCIClass { >> + union { >> + PCIDeviceClass pci_parent_class; >> + SysBusDeviceClass sbd_parent_class; >> + }; >> +} SDHCIClass; >> + >> /* >> * Controller does not provide transfer-complete interrupt when not >> * busy. >> @@ -123,6 +130,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, >> #define TYPE_SYSBUS_SDHCI "generic-sdhci" >> DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, >> TYPE_SYSBUS_SDHCI) >> +DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, >> + TYPE_SYSBUS_SDHCI) > > Are these two together just OBJECT_DECLARE_TYPE? Then the above typedefs > are also not needed just the struct definitions. I'd like to but it isn't possible because the same object state/class is used by distinct types (PCI & SysBus). The following (expected to be correct) change ...: -- >8 -- diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h index 966a1751f50..341b130995b 100644 --- a/include/hw/sd/sdhci.h +++ b/include/hw/sd/sdhci.h @@ -155,10 +155,6 @@ typedef struct SDHCIClass { #define TYPE_PCI_SDHCI "sdhci-pci" -DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, - TYPE_PCI_SDHCI) +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) #define TYPE_SYSBUS_SDHCI "generic-sdhci" -DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, - TYPE_SYSBUS_SDHCI) -DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, - TYPE_SYSBUS_SDHCI) +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) --- ... produces: FAILED: libqemu-aarch64-softmmu.a.p/hw_arm_mcimx6ul-evk.c.o In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_autoptr_clear_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1360:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1360 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1343:49: note: expanded from macro '_GLIB_AUTOPTR_CLEAR_FUNC_NAME' 1343 | #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) glib_autoptr_clear_##TypeName | ^ <scratch space>:77:1: note: expanded from here 77 | glib_autoptr_clear_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1360:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1360 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) (TypeName *_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1343:49: note: expanded from macro '_GLIB_AUTOPTR_CLEAR_FUNC_NAME' 1343 | #define _GLIB_AUTOPTR_CLEAR_FUNC_NAME(TypeName) glib_autoptr_clear_##TypeName | ^ <scratch space>:48:1: note: expanded from here 48 | glib_autoptr_clear_SDHCIState | ^ In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_autoptr_cleanup_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1362:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1362 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_FUNC_NAME(TypeName) (TypeName **_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1342:43: note: expanded from macro '_GLIB_AUTOPTR_FUNC_NAME' 1342 | #define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName | ^ <scratch space>:78:1: note: expanded from here 78 | glib_autoptr_cleanup_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1362:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1362 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_FUNC_NAME(TypeName) (TypeName **_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1342:43: note: expanded from macro '_GLIB_AUTOPTR_FUNC_NAME' 1342 | #define _GLIB_AUTOPTR_FUNC_NAME(TypeName) glib_autoptr_cleanup_##TypeName | ^ <scratch space>:49:1: note: expanded from here 49 | glib_autoptr_cleanup_SDHCIState | ^ In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_autoptr_destroy_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1364:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1364 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_DESTROY_FUNC_NAME(TypeName) (void *_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1344:51: note: expanded from macro '_GLIB_AUTOPTR_DESTROY_FUNC_NAME' 1344 | #define _GLIB_AUTOPTR_DESTROY_FUNC_NAME(TypeName) glib_autoptr_destroy_##TypeName | ^ <scratch space>:80:1: note: expanded from here 80 | glib_autoptr_destroy_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1364:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1364 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_DESTROY_FUNC_NAME(TypeName) (void *_ptr) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1344:51: note: expanded from macro '_GLIB_AUTOPTR_DESTROY_FUNC_NAME' 1344 | #define _GLIB_AUTOPTR_DESTROY_FUNC_NAME(TypeName) glib_autoptr_destroy_##TypeName | ^ <scratch space>:51:1: note: expanded from here 51 | glib_autoptr_destroy_SDHCIState | ^ In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_listautoptr_cleanup_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1366:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1366 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1346:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME' 1346 | #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName | ^ <scratch space>:81:1: note: expanded from here 81 | glib_listautoptr_cleanup_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1366:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1366 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1346:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME' 1346 | #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName | ^ <scratch space>:52:1: note: expanded from here 52 | glib_listautoptr_cleanup_SDHCIState | ^ In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_slistautoptr_cleanup_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1368:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1368 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) (GSList **_l) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1348:49: note: expanded from macro '_GLIB_AUTOPTR_SLIST_FUNC_NAME' 1348 | #define _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) glib_slistautoptr_cleanup_##TypeName | ^ <scratch space>:83:1: note: expanded from here 83 | glib_slistautoptr_cleanup_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1368:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1368 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) (GSList **_l) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1348:49: note: expanded from macro '_GLIB_AUTOPTR_SLIST_FUNC_NAME' 1348 | #define _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) glib_slistautoptr_cleanup_##TypeName | ^ <scratch space>:54:1: note: expanded from here 54 | glib_slistautoptr_cleanup_SDHCIState | ^ In file included from ../../hw/arm/mcimx6ul-evk.c:15: In file included from include/hw/arm/fsl-imx6ul.h:31: include/hw/sd/sdhci.h:165:1: error: redefinition of 'glib_queueautoptr_cleanup_SDHCIState' 165 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1370:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1370 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_QUEUE_FUNC_NAME(TypeName) (GQueue **_q) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1350:49: note: expanded from macro '_GLIB_AUTOPTR_QUEUE_FUNC_NAME' 1350 | #define _GLIB_AUTOPTR_QUEUE_FUNC_NAME(TypeName) glib_queueautoptr_cleanup_##TypeName | ^ <scratch space>:85:1: note: expanded from here 85 | glib_queueautoptr_cleanup_SDHCIState | ^ include/hw/sd/sdhci.h:158:1: note: previous definition is here 158 | OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) | ^ include/qom/object.h:238:5: note: expanded from macro 'OBJECT_DECLARE_TYPE' 238 | G_DEFINE_AUTOPTR_CLEANUP_FUNC(InstanceType, object_unref) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1379:3: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' 1379 | _GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS(TypeName, TypeName, func) | ^ /usr/include/glib-2.0/glib/gmacros.h:1370:36: note: expanded from macro '_GLIB_DEFINE_AUTOPTR_CLEANUP_FUNCS' 1370 | static G_GNUC_UNUSED inline void _GLIB_AUTOPTR_QUEUE_FUNC_NAME(TypeName) (GQueue **_q) \ | ^ /usr/include/glib-2.0/glib/gmacros.h:1350:49: note: expanded from macro '_GLIB_AUTOPTR_QUEUE_FUNC_NAME' 1350 | #define _GLIB_AUTOPTR_QUEUE_FUNC_NAME(TypeName) glib_queueautoptr_cleanup_##TypeName | ^ <scratch space>:56:1: note: expanded from here 56 | glib_queueautoptr_cleanup_SDHCIState | ^ 6 errors generated. ninja: build stopped: subcommand failed. Don't ask me why it is so verbose... Meanwhile the current legacy macros seems good enough for soft freeze ;)
On Sun, 9 Mar 2025, Philippe Mathieu-Daudé wrote: > On 8/3/25 23:34, BALATON Zoltan wrote: >> On Sat, 8 Mar 2025, Philippe Mathieu-Daudé wrote: >>> TYPE_SYSBUS_SDHCI is a bit odd because it uses an union >>> to work with both SysBus / PCI parent. As this is not a >>> normal use, introduce SDHCIClass in its own commit. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> include/hw/sd/sdhci.h | 9 +++++++++ >>> hw/sd/sdhci.c | 1 + >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >>> index 48247e9a20f..c4b20db3877 100644 >>> --- a/include/hw/sd/sdhci.h >>> +++ b/include/hw/sd/sdhci.h >>> @@ -107,6 +107,13 @@ struct SDHCIState { >>> }; >>> typedef struct SDHCIState SDHCIState; >>> >>> +typedef struct SDHCIClass { >>> + union { >>> + PCIDeviceClass pci_parent_class; >>> + SysBusDeviceClass sbd_parent_class; >>> + }; >>> +} SDHCIClass; >>> + >>> /* >>> * Controller does not provide transfer-complete interrupt when not >>> * busy. >>> @@ -123,6 +130,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, >>> #define TYPE_SYSBUS_SDHCI "generic-sdhci" >>> DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, >>> TYPE_SYSBUS_SDHCI) >>> +DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, >>> + TYPE_SYSBUS_SDHCI) >> >> Are these two together just OBJECT_DECLARE_TYPE? Then the above typedefs >> are also not needed just the struct definitions. > > I'd like to but it isn't possible because the same object state/class is > used by distinct types (PCI & SysBus). > > The following (expected to be correct) change ...: > -- >8 -- > diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h > index 966a1751f50..341b130995b 100644 > --- a/include/hw/sd/sdhci.h > +++ b/include/hw/sd/sdhci.h > @@ -155,10 +155,6 @@ typedef struct SDHCIClass { > #define TYPE_PCI_SDHCI "sdhci-pci" > -DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, > - TYPE_PCI_SDHCI) > +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) It would be same as the original patch if you omit this one and only use OBJECT_DECLARE_TYPE below. You didn't add CLASS_CHECKER to the PCI version in the original patch either. But I see now it's more complex and so maybe it's not so easy. > #define TYPE_SYSBUS_SDHCI "generic-sdhci" > -DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, > - TYPE_SYSBUS_SDHCI) > -DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, > - TYPE_SYSBUS_SDHCI) > +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) > --- [...] > Meanwhile the current legacy macros seems good enough for soft freeze ;) Good enough for me. I was also happy with my way simpler solution. This is much more clean up already. Regards, BALATON Zoltan
On 9/3/25 01:08, BALATON Zoltan wrote: > On Sun, 9 Mar 2025, Philippe Mathieu-Daudé wrote: >> On 8/3/25 23:34, BALATON Zoltan wrote: >>> On Sat, 8 Mar 2025, Philippe Mathieu-Daudé wrote: >>>> TYPE_SYSBUS_SDHCI is a bit odd because it uses an union >>>> to work with both SysBus / PCI parent. As this is not a >>>> normal use, introduce SDHCIClass in its own commit. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> --- >>>> include/hw/sd/sdhci.h | 9 +++++++++ >>>> hw/sd/sdhci.c | 1 + >>>> 2 files changed, 10 insertions(+) >>>> >>>> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >>>> index 48247e9a20f..c4b20db3877 100644 >>>> --- a/include/hw/sd/sdhci.h >>>> +++ b/include/hw/sd/sdhci.h >>>> @@ -107,6 +107,13 @@ struct SDHCIState { >>>> }; >>>> typedef struct SDHCIState SDHCIState; >>>> >>>> +typedef struct SDHCIClass { >>>> + union { >>>> + PCIDeviceClass pci_parent_class; >>>> + SysBusDeviceClass sbd_parent_class; >>>> + }; >>>> +} SDHCIClass; >>>> + >>>> /* >>>> * Controller does not provide transfer-complete interrupt when not >>>> * busy. >>>> @@ -123,6 +130,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, >>>> #define TYPE_SYSBUS_SDHCI "generic-sdhci" >>>> DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, >>>> TYPE_SYSBUS_SDHCI) >>>> +DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, >>>> + TYPE_SYSBUS_SDHCI) >>> >>> Are these two together just OBJECT_DECLARE_TYPE? Then the above >>> typedefs are also not needed just the struct definitions. >> >> I'd like to but it isn't possible because the same object state/class is >> used by distinct types (PCI & SysBus). >> >> The following (expected to be correct) change ...: >> -- >8 -- >> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h >> index 966a1751f50..341b130995b 100644 >> --- a/include/hw/sd/sdhci.h >> +++ b/include/hw/sd/sdhci.h >> @@ -155,10 +155,6 @@ typedef struct SDHCIClass { >> #define TYPE_PCI_SDHCI "sdhci-pci" >> -DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, >> - TYPE_PCI_SDHCI) >> +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, PCI_SDHCI) > > It would be same as the original patch if you omit this one and only use > OBJECT_DECLARE_TYPE below. Indeed that works. Kind of a kludge. Not worth than what we have, so I'll take it. > You didn't add CLASS_CHECKER to the PCI > version in the original patch either. But I see now it's more complex > and so maybe it's not so easy. Yeah, I'll postpone the QOM parentship cleanup for later. > >> #define TYPE_SYSBUS_SDHCI "generic-sdhci" >> -DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, >> - TYPE_SYSBUS_SDHCI) >> -DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, >> - TYPE_SYSBUS_SDHCI) >> +OBJECT_DECLARE_TYPE(SDHCIState, SDHCIClass, SYSBUS_SDHCI) >> --- > > [...] > >> Meanwhile the current legacy macros seems good enough for soft freeze ;) > > Good enough for me. I was also happy with my way simpler solution. This > is much more clean up already. > > Regards, > BALATON Zoltan
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h index 48247e9a20f..c4b20db3877 100644 --- a/include/hw/sd/sdhci.h +++ b/include/hw/sd/sdhci.h @@ -107,6 +107,13 @@ struct SDHCIState { }; typedef struct SDHCIState SDHCIState; +typedef struct SDHCIClass { + union { + PCIDeviceClass pci_parent_class; + SysBusDeviceClass sbd_parent_class; + }; +} SDHCIClass; + /* * Controller does not provide transfer-complete interrupt when not * busy. @@ -123,6 +130,8 @@ DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI, #define TYPE_SYSBUS_SDHCI "generic-sdhci" DECLARE_INSTANCE_CHECKER(SDHCIState, SYSBUS_SDHCI, TYPE_SYSBUS_SDHCI) +DECLARE_CLASS_CHECKERS(SDHCIClass, SYSBUS_SDHCI, + TYPE_SYSBUS_SDHCI) #define TYPE_IMX_USDHC "imx-usdhc" diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index 149b748cbee..4917a9b3632 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -1960,6 +1960,7 @@ static const TypeInfo sdhci_types[] = { .instance_size = sizeof(SDHCIState), .instance_init = sdhci_sysbus_init, .instance_finalize = sdhci_sysbus_finalize, + .class_size = sizeof(SDHCIClass), .class_init = sdhci_sysbus_class_init, }, {
TYPE_SYSBUS_SDHCI is a bit odd because it uses an union to work with both SysBus / PCI parent. As this is not a normal use, introduce SDHCIClass in its own commit. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/sd/sdhci.h | 9 +++++++++ hw/sd/sdhci.c | 1 + 2 files changed, 10 insertions(+)