On 08/05/14 15:34, Andreas Färber wrote:
Hi,
Am 19.02.2014 22:39, schrieb Mark Cave-Ayland:
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.
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?
Is something like this correct? It also seems that the register I/O
region initialisation can get moved into the initfn since that doesn't
depend on any properties either.
If it looks good, I'll spin up a proper patchset that does the same to
TCX too.
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -274,19 +274,30 @@ static const GraphicHwOps cg3_ops = {
.gfx_update = cg3_update_display,
};
-static void cg3_realizefn(DeviceState *dev, Error **errp)
+static void cg3_initfn(Object *obj)
{
- SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
- CG3State *s = CG3(dev);
- int ret;
- char *fcode_filename;
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ CG3State *s = CG3(obj);
/* 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);
+
+ memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
+ CG3_REG_SIZE);
+ sysbus_init_mmio(sbd, &s->reg);
+}
+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 */
fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, CG3_ROM_FILE);
if (fcode_filename) {
ret = load_image_targphys(fcode_filename, s->prom_addr,
@@ -296,10 +307,6 @@ static void cg3_realizefn(DeviceState *dev, Error
**errp)
}
}
- memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
- CG3_REG_SIZE);
- sysbus_init_mmio(sbd, &s->reg);
-
memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size);
vmstate_register_ram_global(&s->vram_mem);
sysbus_init_mmio(sbd, &s->vram_mem);
@@ -374,6 +381,7 @@ static const TypeInfo cg3_info = {
.name = TYPE_CG3,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(CG3State),
+ .instance_init = cg3_initfn,
.class_init = cg3_class_init,
};
ATB,
Mark.