On Thu, 7 Jan 2021, Philippe Mathieu-Daudé wrote:
Hi Zoltan,

On 1/6/21 10:13 PM, BALATON Zoltan wrote:
The vt82c686b-pm model can be shared between VT82C686B and VT8231. The
only difference between the two is the device id in what we emulate so
make an abstract via-pm model by renaming appropriately and add types
for vt82c686b-pm and vt8231-pm based on it.

Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
 hw/isa/vt82c686.c         | 87 ++++++++++++++++++++++++++-------------
 include/hw/isa/vt82c686.h |  1 +
 2 files changed, 59 insertions(+), 29 deletions(-)
...

+typedef struct via_pm_init_info {
+    uint16_t device_id;
+} ViaPMInitInfo;
+
 static void via_pm_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    ViaPMInitInfo *info = data;

-    k->realize = vt82c686b_pm_realize;
+    k->realize = via_pm_realize;
     k->config_write = pm_write_config;
     k->vendor_id = PCI_VENDOR_ID_VIA;
-    k->device_id = PCI_DEVICE_ID_VIA_ACPI;
+    k->device_id = info->device_id;
     k->class_id = PCI_CLASS_BRIDGE_OTHER;
     k->revision = 0x40;
-    dc->reset = vt82c686b_pm_reset;
-    dc->desc = "PM";
+    dc->reset = via_pm_reset;

+    /* Reason: part of VIA south bridge, does not exist stand alone */
+    dc->user_creatable = false;
     dc->vmsd = &vmstate_acpi;
-    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);

Please do this change in a previous patch.

OK, done.

 }

 static const TypeInfo via_pm_info = {
-    .name          = TYPE_VT82C686B_PM,
+    .name          = TYPE_VIA_PM,
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(VT686PMState),
-    .class_init    = via_pm_class_init,
+    .instance_size = sizeof(ViaPMState),
+    .abstract      = true,
     .interfaces = (InterfaceInfo[]) {
         { INTERFACE_CONVENTIONAL_PCI_DEVICE },
         { },
     },
 };

+static const ViaPMInitInfo vt82c686b_pm_init_info = {
+    .device_id = PCI_DEVICE_ID_VIA_ACPI,
+};
+
+static const TypeInfo vt82c686b_pm_info = {
+    .name          = TYPE_VT82C686B_PM,
+    .parent        = TYPE_VIA_PM,
+    .class_init    = via_pm_class_init,
+    .class_data    = (void *)&vt82c686b_pm_init_info,

Igor said new code should avoid using .class_data:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg678305.html
Can you convert to "leaf class"? Then this patch is good to go.

That says for machines it is not advised (and Igor generally prefers init funcs everywhere) but this is a device model. Is it still not allowed to use class_data here? I think this is shorter this way than with an init function but I may try to convert if absolutely necessary.

Regards,
BALATON Zoltan

A trivial example of conversion is commit f0eeb4b6154
("hw/arm/raspi: Avoid using TypeInfo::class_data pointer").

+};
+
+static const ViaPMInitInfo vt8231_pm_init_info = {
+    .device_id = 0x8235,
+};
+
+static const TypeInfo vt8231_pm_info = {
+    .name          = TYPE_VT8231_PM,
+    .parent        = TYPE_VIA_PM,
+    .class_init    = via_pm_class_init,
+    .class_data    = (void *)&vt8231_pm_init_info,
+};



Reply via email to