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