From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39194)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1WiPOx-0006hA-86
	for qemu-devel@nongnu.org; Thu, 08 May 2014 10:34:30 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1WiPOo-0002XD-V6
	for qemu-devel@nongnu.org; Thu, 08 May 2014 10:34:23 -0400
Received: from cantor2.suse.de ([195.135.220.15]:52910 helo=mx2.suse.de)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <afaerber@suse.de>) id 1WiPOo-0002X5-Pb
	for qemu-devel@nongnu.org; Thu, 08 May 2014 10:34:14 -0400
Message-ID: <536B95E4.5020009@suse.de>
Date: Thu, 08 May 2014 16:34:12 +0200
From: =?ISO-8859-1?Q?Andreas_F=E4rber?= <afaerber@suse.de>
MIME-Version: 1.0
References: <1392800720-2765-1-git-send-email-mark.cave-ayland@ilande.co.uk>	<1392800720-2765-2-git-send-email-mark.cave-ayland@ilande.co.uk>	<20140219133502.GA20305@dorilex>
	<5305247D.7060705@ilande.co.uk>
In-Reply-To: <5305247D.7060705@ilande.co.uk>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCHv3 1/2] sun4m: Add Sun CG3 framebuffer and
 corresponding OpenBIOS FCode ROM
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Leandro Dorileo <l@dorileo.org>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>, Bob Breuer <breuerr@mc.net>, Anthony Liguori <aliguori@amazon.com>, Artyom Tarasenko <atar4qemu@gmail.com>

Hi,

Am 19.02.2014 22:39, schrieb Mark Cave-Ayland:
> On 19/02/14 13:35, Leandro Dorileo wrote:
>=20
> Hi Leandro,
>=20
>>> +static void cg3_realizefn(DeviceState *dev, Error **errp)
>>> +{
>>> +    SysBusDevice *sbd =3D SYS_BUS_DEVICE(dev);
>>> +    CG3State *s =3D CG3(dev);
>>> +    int ret;
>>> +    char *fcode_filename;
>>> +
>>> +    /* FCode ROM */
>>> +    memory_region_init_ram(&s->rom, NULL, "cg3.prom",
>>> FCODE_MAX_ROM_SIZE);
>>> +    vmstate_register_ram_global(&s->rom);
>>> +    memory_region_set_readonly(&s->rom, true);
>>> +    sysbus_init_mmio(sbd,&s->rom);
>>> +
>>
>>
>> I think this initialization code could be done in a SysBusDeviceClass
>> init operation,
>> don't you think?
>=20
> I think it's possible since these MemoryRegions don't depend upon
> properties, but I leave that to Andres who seems reasonably happy with
> the patchset in its current form.

Just seeing this...

memory_region_init_ram() and sysbus_init_mmio() could indeed be moved to
an .instance_init function, given that FCODE_MAX_ROM_SIZE is constant.
The others no. It makes a difference when considering reentrancy of the
property code via qom-set (just posted a patchset that makes playing
with that easier), although there's probably more corner cases to
consider. Could either of you follow up with a cleanup?

Regards,
Andreas

>>> +    fcode_filename =3D qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_F=
ILE);
>>> +    if (fcode_filename) {
>>> +        ret =3D load_image_targphys(fcode_filename, s->prom_addr,
>>> +                                  FCODE_MAX_ROM_SIZE);
>>> +        if (ret<  0 || ret>  FCODE_MAX_ROM_SIZE) {
>>> +            error_report("cg3: could not load prom '%s'",
>>> CG3_ROM_FILE);

--=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