From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55292) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvSXs-0003NT-RG for qemu-devel@nongnu.org; Sat, 06 Jul 2013 09:29:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UvSXr-0000nz-MN for qemu-devel@nongnu.org; Sat, 06 Jul 2013 09:29:00 -0400 Received: from cantor2.suse.de ([195.135.220.15]:55181 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UvSXr-0000nm-Cf for qemu-devel@nongnu.org; Sat, 06 Jul 2013 09:28:59 -0400 Message-ID: <51D81B94.6060002@suse.de> Date: Sat, 06 Jul 2013 15:28:52 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1372626065-6043-1-git-send-email-afaerber@suse.de> <1372626065-6043-8-git-send-email-afaerber@suse.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 07/15] timer/arm_mptimer: Convert to QOM realize List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , qemu-devel@nongnu.org, Paolo Bonzini Am 01.07.2013 11:33, schrieb Peter Crosthwaite: > Hi Andreas, >=20 > On Mon, Jul 1, 2013 at 7:00 AM, Andreas F=E4rber wro= te: >> From: Andreas F=E4rber >> >> Split the SysBusDevice initfn into instance_init and realizefn. >> >> Signed-off-by: Andreas F=E4rber >> --- >> hw/timer/arm_mptimer.c | 25 +++++++++++++++---------- >> 1 file changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c >> index 588e34b..a19ffa3 100644 >> --- a/hw/timer/arm_mptimer.c >> +++ b/hw/timer/arm_mptimer.c >> @@ -226,8 +226,18 @@ static void arm_mptimer_reset(DeviceState *dev) >> } >> } >> >> -static int arm_mptimer_init(SysBusDevice *dev) >> +static void arm_mptimer_init(Object *obj) >> { >> + ARMMPTimerState *s =3D ARM_MP_TIMER(obj); >> + >> + memory_region_init_io(&s->iomem, &arm_thistimer_ops, s, >> + "arm_mptimer_timer", 0x20); >> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); >> +} >=20 > Splitting off only one of the memory region setups to init seems a bit > awkward. Is it really worth it given that we need to wait for > realization for the rest to occur anyway? Well, my main interest wrt MemoryRegions here is to have the *mpcore_priv container MemoryRegion mappable before realize and to avoid having to implement unnecessary cleanups in unrealize. An alternative would be for PMM to use his array properties to dynamically allocate the MemoryRegions before realize, but so far he has failed to come up with a solution... Another solution, since this is a fixed-length array, would be to always initialize all MemoryRegions and just keep adding all 3(?) in realize. FWIU adding subregions is desired in instance_init as long as it affects only containing devices and not a global address space. Regards, Andreas >> + >> +static void arm_mptimer_realize(DeviceState *dev, Error **errp) >> +{ >> + SysBusDevice *sbd =3D SYS_BUS_DEVICE(dev); >> ARMMPTimerState *s =3D ARM_MP_TIMER(dev); >> int i; >> >> @@ -244,19 +254,14 @@ static int arm_mptimer_init(SysBusDevice *dev) >> * * timer for core 1 >> * and so on. >> */ >> - memory_region_init_io(&s->iomem, &arm_thistimer_ops, s, >> - "arm_mptimer_timer", 0x20); >> - sysbus_init_mmio(dev, &s->iomem); >> for (i =3D 0; i < s->num_cpu; i++) { >> TimerBlock *tb =3D &s->timerblock[i]; >> tb->timer =3D qemu_new_timer_ns(vm_clock, timerblock_tick, tb= ); >> - sysbus_init_irq(dev, &tb->irq); >> + sysbus_init_irq(sbd, &tb->irq); >> memory_region_init_io(&tb->iomem, &timerblock_ops, tb, >> "arm_mptimer_timerblock", 0x20); >> - sysbus_init_mmio(dev, &tb->iomem); >> + sysbus_init_mmio(sbd, &tb->iomem); >> } >> - >> - return 0; >> } >> >> static const VMStateDescription vmstate_timerblock =3D { >> @@ -293,9 +298,8 @@ static Property arm_mptimer_properties[] =3D { >> static void arm_mptimer_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *dc =3D DEVICE_CLASS(klass); >> - SysBusDeviceClass *sbc =3D SYS_BUS_DEVICE_CLASS(klass); >> >> - sbc->init =3D arm_mptimer_init; >> + dc->realize =3D arm_mptimer_realize; >> dc->vmsd =3D &vmstate_arm_mptimer; >> dc->reset =3D arm_mptimer_reset; >> dc->no_user =3D 1; >> @@ -306,6 +310,7 @@ static const TypeInfo arm_mptimer_info =3D { >> .name =3D TYPE_ARM_MP_TIMER, >> .parent =3D TYPE_SYS_BUS_DEVICE, >> .instance_size =3D sizeof(ARMMPTimerState), >> + .instance_init =3D arm_mptimer_init, >> .class_init =3D arm_mptimer_class_init, >> }; >> >> -- >> 1.8.1.4 >> >> --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg