On Mon, 22 Apr 2013 15:22:30 +0200 Andreas Färber <afaer...@suse.de> wrote:
> Am 16.04.2013 00:12, schrieb Igor Mammedov: > > ... to provide hotplug-able bus. > > > > * icc-bridge will serve as a parent for icc-bus and provide > > mmio mapping services to child icc-devices. > > * icc-device will replace SysBusDevice as a parent of APIC > > and IOAPIC devices. > > > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > > --- > > v2: > > * Rebase on top the last HW reorganization series. > > * Move hw/icc_bus.c into hw/cpu/ and hw/icc_bus.h include/hw/i386/ > > http://wiki.qemu.org/QOMConventions > > Comments below... > [...] > > + > > +static void icc_device_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *k = DEVICE_CLASS(klass); > > + > > + k->init = icc_device_init; > > Please use k->realize for any new base device type or type derived from > SysBusDevice. > > If there's nothing to do at this level, you can just provide a dummy > realize function here to override Device's and override it (without > saving the parent's dummy function) in IOAPIC or wherever you need it. > I'll try it, and if changes isn't big, I'll merge into this patch. > > + k->bus_type = TYPE_ICC_BUS; > > +} > > + > > +static const TypeInfo icc_device_info = { > > + .name = TYPE_ICC_DEVICE, > > + .parent = TYPE_DEVICE, > > + .abstract = true, > > + .instance_size = sizeof(ICCDevice), > > + .class_size = sizeof(ICCDeviceClass), > > + .class_init = icc_device_class_init, > > +}; > > + > > +typedef struct ICCBridgeState { > > + SysBusDevice busdev; > > parent_obj please - makes it clear that it is a QOM struct and flags > accidental uses of obsolete SysBus macros. done > > > +} ICCBridgeState; > > +#define ICC_BRIGDE(obj) OBJECT_CHECK(ICCBridgeState, (obj), > > TYPE_ICC_BRIDGE) + > > + > > +static void icc_bridge_initfn(Object *obj) > > +{ > > + qbus_create(TYPE_ICC_BUS, DEVICE(obj), "icc-bus"); > > qbus_create_inplace()? done [...] > > BTW Anthony recently suggested to drop the "fn" where there's no init > vs. initfn name conflicts; I don't mind either way. changed to init [...] > > /*< private >*/ > > > + BusState qbus; > > parent_obj done > > /*< public >*/ > > ...since it is in a header. > > > +} ICCBus; > > +#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) > > + > > +typedef struct ICCDevice { > > + DeviceState qdev; > > dito s/qdev/parent_obj/ will affect APIC and IOAPIC patches make-ing them even bigger and more difficult to review distracting from real changes. it would be better to send follow-up patch, that does trivial QOM conversions afterwards. I'm not sure that making ICCDevice.qdev private would be good performance wise since it would require using DEVICE() dynamic cast in apic_check_pic(). I'd like to keep this as is for now and put cleanup patch on TODO list for 1.6 to get rid of DO_UPCAST() macro in apic/ioapic files. > > Who becomes the MAINTAINER of ICC? Perhaps me?? > > Regards, > Andreas >
