On Sat, 24 Mar 2018 12:35:22 -0300 Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> Hi, > > On 02/20/2018 10:23 AM, Igor Mammedov wrote: > > On Tue, 20 Feb 2018 13:13:49 +0100 > > Paolo Bonzini <pbonz...@redhat.com> wrote: > > > >> On 16/02/2018 18:40, Philippe Mathieu-Daudé wrote: > >>> we can keep object_initialize() when no parent, > >>> and add object_initialize_child(, const char *childname, Object *parent) > >>> 'parent' last because all previous args are child-related. > >>> > >>>> > >>>>> + qdev_set_parent_bus(DEVICE(child), sysbus_get_default()); > >>>> and then assuming we don't create sysbus devices, nor should be able to, > >>>> which are not attached to sysbus_get_default() this one can go sysbus' > >>>> instance_init() > >>> good idea. > >>> > > [...] > > > >> I'm not sure about moving qdev_set_parent_bus to instance_init(). It > >> would be a bit different from other buses and possibly confusing. > > That's what this series sort of does, i.e. creating a type/class > > specific helper(s). Which becomes confusing as number of helpers > > increases (frankly it's just 2 different approaches to code i.e. > > functional vs OOM). > > > > It could be better to keep single QOM API and let SYSBUS base class > > instance_init() to do all implicit initialization that must be done > > for inherited classes. > > What is the consensus on this series once 2.12 gets released? At least we should add and make current code use it object_initialize_child() It should save quite a bit of code in ARM target As for qdev_set_parent_bus(DEVICE, sysbus_get_default()) it is a separate issue, but I'd still go with TYPE_SYS_BUS_DEVICE setting bus in its instance_init so that inherited types nor their users would have to deal with it. > > Yes, it will be different from other devices with buses but > > users won't really care (there is no other buss to assign to), > > for them it will look like a typical bus-less device and it > > would be less error-prone to code. > > > > > >> Potentially there could be a "hierarchy" of *_initialize_child > >> functions, adding or removing arguments as needed for the specific kind > >> of bus: > >> > >> /* adds qdev_set_parent_bus */ > >> device_initialize_child(Object *parent, const char *childname, > >> BusState *bus, void *data, size_t size, > >> const char *typename); > >> /* uses sysbus_get_default() */ > >> sysbus_initialize_child(Object *parent, const char *childname, > >> void *data, size_t size, > >> const char *typename); > > > > > >> /* initializes "addr" property too */ > >> pci_initialize_child(Object *parent, const char *childname, > >> BusState *bus, int addr, > >> void *data, size_t size, > >> const char *typename); > > PCI could also incorporate PCI specific bus setting/ > > verification logic inside of base class. > > > > It could allow us to drop bus assigning magic in qdev_device_add(), > > bringing it closer to simple QOM object handling, with specifics > > abstracted by respective TYPE implementations. > > Maybe we would be able to unify -device with -object in the end. > > Thanks, > > Phil.