On Thu, 7 Jan 2021 11:38:21 +0100 (CET)
BALATON Zoltan <bala...@eik.bme.hu> wrote:
> 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.
For this simple case class_init would be cleaner as it doesn't need casting
(void*).
But I'm fine with either approaches here.
> 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,
Is it possible to replace magic number with a human readable macro?
> >> +};
> >> +
> >> +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,
> >> +};
> >>
> >>
> >
> >
