On Tue, Aug 23, 2022 at 2:35 AM BALATON Zoltan <bala...@eik.bme.hu> wrote:
> On Tue, 23 Aug 2022, Bernhard Beschow wrote: > > The object creation now happens in chip-specific init methods which > > allows the realize methods to be consolidated into one method. Shifting > > the logic into the init methods has the addidional advantage that the > > parent object's init methods are called implicitly. > > This and later patches titled "QOM'ify" don't do what I understand on > QOMifying which is turining an old device model into a QOM object. These > devices are already QOMified, what's really done here is that they are > moved to the ViaISAState or embedded into it and created as part of the > south bridge rather then individually by the boards. Either my > understanding of what QOMify means is wrong or these patches are misnamed. > I think your understanding is correct. Peter refers to it as the embed-the-device-struct style [1] which I can take as inspiration for renaming my patches in v2. Technically via_isa_realize() is the realize method of the abstract > TYPE_VIA_ISA class which were overriden by the subclasses but since QOM > does not call overridden methods implicitly this had to be explicitly > called in the overriding realize methods of the subclasses. Now that we > don't have to ovverride the method maybe it could be set once on the > VIA_ISA class then it may work that way but as it's done here is also OK > maybe as a reminder that this super class method should be called by any > overriding method if one's added in the future for some reason. > Right. This would involve moving some code around and creating a class struct. Do you think it's worth it? Best regards, Bernhard [1] http://patchwork.ozlabs.org/project/qemu-devel/patch/20220205175913.31738-2-shen...@gmail.com/ Regards, > BALATON Zoltan > > > Signed-off-by: Bernhard Beschow <shen...@gmail.com> > > --- > > hw/isa/vt82c686.c | 33 ++++++++++++++++++--------------- > > 1 file changed, 18 insertions(+), 15 deletions(-) > > > > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > > index 8f656251b8..0217c98fe4 100644 > > --- a/hw/isa/vt82c686.c > > +++ b/hw/isa/vt82c686.c > > @@ -544,7 +544,7 @@ struct ViaISAState { > > qemu_irq cpu_intr; > > qemu_irq *isa_irqs; > > ISABus *isa_bus; > > - ViaSuperIOState *via_sio; > > + ViaSuperIOState via_sio; > > }; > > > > static const VMStateDescription vmstate_via = { > > @@ -602,6 +602,11 @@ static void via_isa_realize(PCIDevice *d, Error > **errp) > > d->wmask[i] = 0; > > } > > } > > + > > + /* Super I/O */ > > + if (!qdev_realize(DEVICE(&s->via_sio), BUS(s->isa_bus), errp)) { > > + return; > > + } > > } > > > > /* TYPE_VT82C686B_ISA */ > > @@ -615,7 +620,7 @@ static void vt82c686b_write_config(PCIDevice *d, > uint32_t addr, > > pci_default_write_config(d, addr, val, len); > > if (addr == 0x85) { > > /* BIT(1): enable or disable superio config io ports */ > > - via_superio_io_enable(s->via_sio, val & BIT(1)); > > + via_superio_io_enable(&s->via_sio, val & BIT(1)); > > } > > } > > > > @@ -639,13 +644,11 @@ static void vt82c686b_isa_reset(DeviceState *dev) > > pci_conf[0x77] = 0x10; /* GPIO Control 1/2/3/4 */ > > } > > > > -static void vt82c686b_realize(PCIDevice *d, Error **errp) > > +static void vt82c686b_init(Object *obj) > > { > > - ViaISAState *s = VIA_ISA(d); > > + ViaISAState *s = VIA_ISA(obj); > > > > - via_isa_realize(d, errp); > > - s->via_sio = VIA_SUPERIO(isa_create_simple(s->isa_bus, > > - TYPE_VT82C686B_SUPERIO)); > > + object_initialize_child(obj, "sio", &s->via_sio, > TYPE_VT82C686B_SUPERIO); > > } > > > > static void vt82c686b_class_init(ObjectClass *klass, void *data) > > @@ -653,7 +656,7 @@ static void vt82c686b_class_init(ObjectClass *klass, > void *data) > > DeviceClass *dc = DEVICE_CLASS(klass); > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > - k->realize = vt82c686b_realize; > > + k->realize = via_isa_realize; > > k->config_write = vt82c686b_write_config; > > k->vendor_id = PCI_VENDOR_ID_VIA; > > k->device_id = PCI_DEVICE_ID_VIA_82C686B_ISA; > > @@ -670,6 +673,7 @@ static const TypeInfo vt82c686b_isa_info = { > > .name = TYPE_VT82C686B_ISA, > > .parent = TYPE_VIA_ISA, > > .instance_size = sizeof(ViaISAState), > > + .instance_init = vt82c686b_init, > > .class_init = vt82c686b_class_init, > > }; > > > > @@ -684,7 +688,7 @@ static void vt8231_write_config(PCIDevice *d, > uint32_t addr, > > pci_default_write_config(d, addr, val, len); > > if (addr == 0x50) { > > /* BIT(2): enable or disable superio config io ports */ > > - via_superio_io_enable(s->via_sio, val & BIT(2)); > > + via_superio_io_enable(&s->via_sio, val & BIT(2)); > > } > > } > > > > @@ -703,13 +707,11 @@ static void vt8231_isa_reset(DeviceState *dev) > > pci_conf[0x6b] = 0x01; /* Fast IR I/O Base */ > > } > > > > -static void vt8231_realize(PCIDevice *d, Error **errp) > > +static void vt8231_init(Object *obj) > > { > > - ViaISAState *s = VIA_ISA(d); > > + ViaISAState *s = VIA_ISA(obj); > > > > - via_isa_realize(d, errp); > > - s->via_sio = VIA_SUPERIO(isa_create_simple(s->isa_bus, > > - TYPE_VT8231_SUPERIO)); > > + object_initialize_child(obj, "sio", &s->via_sio, > TYPE_VT8231_SUPERIO); > > } > > > > static void vt8231_class_init(ObjectClass *klass, void *data) > > @@ -717,7 +719,7 @@ static void vt8231_class_init(ObjectClass *klass, > void *data) > > DeviceClass *dc = DEVICE_CLASS(klass); > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > - k->realize = vt8231_realize; > > + k->realize = via_isa_realize; > > k->config_write = vt8231_write_config; > > k->vendor_id = PCI_VENDOR_ID_VIA; > > k->device_id = PCI_DEVICE_ID_VIA_8231_ISA; > > @@ -734,6 +736,7 @@ static const TypeInfo vt8231_isa_info = { > > .name = TYPE_VT8231_ISA, > > .parent = TYPE_VIA_ISA, > > .instance_size = sizeof(ViaISAState), > > + .instance_init = vt8231_init, > > .class_init = vt8231_class_init, > > }; > > > > >
