| Message ID | 1497099616-2615-2-git-send-email-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mark, On 06/10/2017 10:00 AM, Mark Cave-Ayland wrote: > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/sparc64/sun4u.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c > index 69f565d..98ee6f5 100644 > --- a/hw/sparc64/sun4u.c > +++ b/hw/sparc64/sun4u.c > @@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem, > graphic_width, graphic_height, graphic_depth, > (uint8_t *)&nd_table[0].macaddr); > > - fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT); > + dev = qdev_create(NULL, TYPE_FW_CFG_IO); > + qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT); > + qdev_prop_set_bit(dev, "dma_enabled", false); > + qdev_init_nofail(dev); > + s = SYS_BUS_DEVICE(dev); > + memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT, > + sysbus_mmio_get_region(s, 0)); Now that you exported TYPE_FW_CFG_IO I think this might be useful/cleaner to have that code in an static inlined function in fw_cfg.h: DeviceState *fw_cfg_create(uint32_t iobase[, bool dma_enabled]); What do you think? Anyway: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > + > + fw_cfg = FW_CFG(dev); > fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); > fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus); > fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size); >
On 10/06/17 18:55, Philippe Mathieu-Daudé wrote: > Hi Mark, > > On 06/10/2017 10:00 AM, Mark Cave-Ayland wrote: >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/sparc64/sun4u.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c >> index 69f565d..98ee6f5 100644 >> --- a/hw/sparc64/sun4u.c >> +++ b/hw/sparc64/sun4u.c >> @@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion >> *address_space_mem, >> graphic_width, graphic_height, graphic_depth, >> (uint8_t *)&nd_table[0].macaddr); >> >> - fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT); >> + dev = qdev_create(NULL, TYPE_FW_CFG_IO); >> + qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT); >> + qdev_prop_set_bit(dev, "dma_enabled", false); >> + qdev_init_nofail(dev); >> + s = SYS_BUS_DEVICE(dev); >> + memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT, >> + sysbus_mmio_get_region(s, 0)); > > Now that you exported TYPE_FW_CFG_IO I think this might be > useful/cleaner to have that code in an static inlined function in fw_cfg.h: > > DeviceState *fw_cfg_create(uint32_t iobase[, bool dma_enabled]); > > What do you think? > > Anyway: > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> I'd be inclined to leave the patch in its current form unless anyone strongly needs more wrapper functions. The important part of the patchset from my perspective is that it brings the fw_cfg MMIO and ioport implementations a lot closer together. ATB, Mark.
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 69f565d..98ee6f5 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -512,7 +512,15 @@ static void sun4uv_init(MemoryRegion *address_space_mem, graphic_width, graphic_height, graphic_depth, (uint8_t *)&nd_table[0].macaddr); - fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT); + dev = qdev_create(NULL, TYPE_FW_CFG_IO); + qdev_prop_set_uint32(dev, "iobase", BIOS_CFG_IOPORT); + qdev_prop_set_bit(dev, "dma_enabled", false); + qdev_init_nofail(dev); + s = SYS_BUS_DEVICE(dev); + memory_region_add_subregion(get_system_io(), BIOS_CFG_IOPORT, + sysbus_mmio_get_region(s, 0)); + + fw_cfg = FW_CFG(dev); fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus); fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus); fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/sparc64/sun4u.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)