From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47592)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WGSfd-00072t-Mp
	for qemu-devel@nongnu.org; Thu, 20 Feb 2014 07:24:11 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WGSfX-0001Hc-Mh
	for qemu-devel@nongnu.org; Thu, 20 Feb 2014 07:24:05 -0500
Received: from mail-ob0-f170.google.com ([209.85.214.170]:43660)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <l@dorileo.org>) id 1WGSfX-0001HU-Hb
	for qemu-devel@nongnu.org; Thu, 20 Feb 2014 07:23:59 -0500
Received: by mail-ob0-f170.google.com with SMTP id va2so1972261obc.1
	for <qemu-devel@nongnu.org>; Thu, 20 Feb 2014 04:23:58 -0800 (PST)
Date: Thu, 20 Feb 2014 12:23:48 +0000
From: Leandro Dorileo <l@dorileo.org>
Message-ID: <20140220122348.GA22837@dorilex>
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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5305247D.7060705@ilande.co.uk>
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>
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>

On Wed, Feb 19, 2014 at 09:39:09PM +0000, Mark Cave-Ayland wrote:
> On 19/02/14 13:35, Leandro Dorileo wrote:
> 
> Hi Leandro,
> 
> >>+static void cg3_realizefn(DeviceState *dev, Error **errp)
> >>+{
> >>+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >>+    CG3State *s = 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?
> 
> 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.


Yes, I just saw his comment in the patch 02...

> 
> >
> >>+    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
> >>+    if (fcode_filename) {
> >>+        ret = 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);
> >
> >
> >What happens if we fail to load the rom file? is the framebuffer supposed to work?
> 
> I guess the framebuffer would still "work" although nothing would be able to
> find its address because the node wouldn't exist in the device tree?
> 

Ok, when we move this code to an instance init op we handle it a bit better.

> 
> ATB,
> 
> Mark.

-- 
Leandro Dorileo