Re: [Qemu-devel] [PATCH arm-devs v4 4/4] cpu/a9mpcore: Add Global Timer

[Andreas Färber]

Hi,

A general observation...

Am 02.12.2013 08:37, schrieb Peter Crosthwaite:
> diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> index a38464b..c09358c 100644
> --- a/hw/cpu/a9mpcore.c
> +++ b/hw/cpu/a9mpcore.c
> @@ -30,6 +30,9 @@ static void a9mp_priv_initfn(Object *obj)
>      object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
>      qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
>  
> +    object_initialize(&s->gtimer, sizeof(s->gtimer), TYPE_A9_GTIMER);
> +    qdev_set_parent_bus(DEVICE(&s->gtimer), sysbus_get_default());
> +
>      object_initialize(&s->mptimer, sizeof(s->mptimer), TYPE_ARM_MPTIMER);
>      qdev_set_parent_bus(DEVICE(&s->mptimer), sysbus_get_default());
>  

This code resulted from inlining qdev_create() and switching to in-place
instantiation.

Seeing this repetitive code again and again makes me wonder whether we
can just set a SysBusDevice's parent_bus in its instance_init function
to simplify it?

The only downsides I can think of is that SysBus can't instantiate
SysBusDevices in its instance_init then to avoid a circular dependency
and that the global SysBus might get a few refs more if set both in
instance_init and via qdev_set_parent_bus(), but we're not expecting it
to be destroyed anyway. CC'ing Paolo.

Outside the scope of this patch, obviously.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg