On 02/18/19 13:56, Markus Armbruster wrote: > pflash_cfi01_register() creates a TYPE_CFI_PFLASH01 device, sets > properties, realizes, and wires up. > > We have three modified copies of it, because their users need to set > additional properties, or have the wiring done differently. > > Factor out their common part into pflash_cfi01_create(). > > Signed-off-by: Markus Armbruster <[email protected]> > --- > hw/arm/vexpress.c | 22 +++++----------------- > hw/arm/virt.c | 26 +++++++++----------------- > hw/block/pflash_cfi01.c | 39 +++++++++++++++++++++++++++------------ > hw/xtensa/xtfpga.c | 18 +++++++----------- > include/hw/block/flash.h | 8 ++++++++ > 5 files changed, 56 insertions(+), 57 deletions(-) > > diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c > index 00913f2655..b23c63ed24 100644 > --- a/hw/arm/vexpress.c > +++ b/hw/arm/vexpress.c > @@ -515,26 +515,14 @@ static void vexpress_modify_dtb(const struct > arm_boot_info *info, void *fdt) > static PFlashCFI01 *ve_pflash_cfi01_register(hwaddr base, const char *name, > DriveInfo *di) > { > - DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01); > + DeviceState *dev; > > - if (di) { > - qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di), > - &error_abort); > - } > - > - qdev_prop_set_uint32(dev, "num-blocks", > - VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE); > - qdev_prop_set_uint64(dev, "sector-length", VEXPRESS_FLASH_SECT_SIZE); > - qdev_prop_set_uint8(dev, "width", 4); > + dev = DEVICE(pflash_cfi01_create(name, VEXPRESS_FLASH_SIZE, > + di ? blk_by_legacy_dinfo(di) : NULL, > + VEXPRESS_FLASH_SECT_SIZE, > + 4, 0x89, 0x18, 0x00, 0x00, false)); > qdev_prop_set_uint8(dev, "device-width", 2); > - qdev_prop_set_bit(dev, "big-endian", false); > - qdev_prop_set_uint16(dev, "id0", 0x89); > - qdev_prop_set_uint16(dev, "id1", 0x18); > - qdev_prop_set_uint16(dev, "id2", 0x00); > - qdev_prop_set_uint16(dev, "id3", 0x00); > - qdev_prop_set_string(dev, "name", name); > qdev_init_nofail(dev); > - > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > return CFI_PFLASH01(dev); > } > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index b7d53b2b87..7787918483 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -48,6 +48,7 @@ > #include "exec/address-spaces.h" > #include "qemu/bitops.h" > #include "qemu/error-report.h" > +#include "qemu/units.h" > #include "hw/pci-host/gpex.h" > #include "hw/arm/sysbus-fdt.h" > #include "hw/platform-bus.h" > @@ -875,29 +876,20 @@ static void create_one_flash(const char *name, hwaddr > flashbase, > * parameters as the flash devices on the Versatile Express board. > */ > DriveInfo *dinfo = drive_get_next(IF_PFLASH); > - DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01); > - SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > - const uint64_t sectorlength = 256 * 1024; > + DeviceState *dev; > + SysBusDevice *sbd; > > - if (dinfo) { > - qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo), > - &error_abort); > - } > + dev = DEVICE(pflash_cfi01_create(name, flashsize, > + dinfo ? blk_by_legacy_dinfo(dinfo) : > NULL, > + 256 * KiB, > + 4, 0x89, 0x18, 0x00, 0x00, false)); > > - qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength); > - qdev_prop_set_uint64(dev, "sector-length", sectorlength); > - qdev_prop_set_uint8(dev, "width", 4); > qdev_prop_set_uint8(dev, "device-width", 2); > - qdev_prop_set_bit(dev, "big-endian", false); > - qdev_prop_set_uint16(dev, "id0", 0x89); > - qdev_prop_set_uint16(dev, "id1", 0x18); > - qdev_prop_set_uint16(dev, "id2", 0x00); > - qdev_prop_set_uint16(dev, "id3", 0x00); > - qdev_prop_set_string(dev, "name", name); > qdev_init_nofail(dev); > > + sbd = SYS_BUS_DEVICE(dev); > memory_region_add_subregion(sysmem, flashbase, > - sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), > 0)); > + sysbus_mmio_get_region(sbd, 0)); > > if (file) { > char *fn; > diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c > index 2e161f937f..00c2efd0d7 100644 > --- a/hw/block/pflash_cfi01.c > +++ b/hw/block/pflash_cfi01.c > @@ -920,15 +920,14 @@ static void pflash_cfi01_register_types(void) > > type_init(pflash_cfi01_register_types) > > -PFlashCFI01 *pflash_cfi01_register(hwaddr base, > - const char *name, > - hwaddr size, > - BlockBackend *blk, > - uint32_t sector_len, > - int bank_width, > - uint16_t id0, uint16_t id1, > - uint16_t id2, uint16_t id3, > - int be) > +PFlashCFI01 *pflash_cfi01_create(const char *name, > + hwaddr size, > + BlockBackend *blk, > + uint32_t sector_len, > + int bank_width, > + uint16_t id0, uint16_t id1, > + uint16_t id2, uint16_t id3, > + int be) > { > DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01); > > @@ -945,12 +944,28 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base, > qdev_prop_set_uint16(dev, "id2", id2); > qdev_prop_set_uint16(dev, "id3", id3); > qdev_prop_set_string(dev, "name", name); > - qdev_init_nofail(dev); > - > - sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > return CFI_PFLASH01(dev); > } > > +PFlashCFI01 *pflash_cfi01_register(hwaddr base, > + const char *name, > + hwaddr size, > + BlockBackend *blk, > + uint32_t sector_len, > + int bank_width, > + uint16_t id0, uint16_t id1, > + uint16_t id2, uint16_t id3, > + int be) > +{ > + PFlashCFI01 *dev = pflash_cfi01_create(name, size, blk, > + sector_len, bank_width, > + id0, id1, id2, id3, be); > + > + qdev_init_nofail(DEVICE(dev)); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > + return dev; > +} > + > MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl) > { > return &fl->mem; > diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c > index a726d5632a..0e96e73ee2 100644 > --- a/hw/xtensa/xtfpga.c > +++ b/hw/xtensa/xtfpga.c > @@ -167,21 +167,17 @@ static PFlashCFI01 *xtfpga_flash_init(MemoryRegion > *address_space, > DriveInfo *dinfo, int be) > { > SysBusDevice *s; > - DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01); > + PFlashCFI01 *dev = pflash_cfi01_create("xtfpga.io.flash", > + board->flash->size, > + blk_by_legacy_dinfo(dinfo), > + board->flash->sector_size, > + 2, 0, 0, 0, 0, be); > > - qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo), > - &error_abort); > - qdev_prop_set_uint32(dev, "num-blocks", > - board->flash->size / board->flash->sector_size); > - qdev_prop_set_uint64(dev, "sector-length", board->flash->sector_size); > - qdev_prop_set_uint8(dev, "width", 2); > - qdev_prop_set_bit(dev, "big-endian", be); > - qdev_prop_set_string(dev, "name", "xtfpga.io.flash"); > - qdev_init_nofail(dev); > + qdev_init_nofail(DEVICE(dev)); > s = SYS_BUS_DEVICE(dev); > memory_region_add_subregion(address_space, board->flash->base, > sysbus_mmio_get_region(s, 0)); > - return CFI_PFLASH01(dev); > + return dev; > } > > static uint64_t translate_phys_addr(void *opaque, uint64_t addr) > diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h > index 24b13eb525..dbb25ba382 100644 > --- a/include/hw/block/flash.h > +++ b/include/hw/block/flash.h > @@ -13,6 +13,14 @@ > > typedef struct PFlashCFI01 PFlashCFI01; > > +PFlashCFI01 *pflash_cfi01_create(const char *name, > + hwaddr size, > + BlockBackend *blk, > + uint32_t sector_len, > + int width, > + uint16_t id0, uint16_t id1, > + uint16_t id2, uint16_t id3, > + int be); > PFlashCFI01 *pflash_cfi01_register(hwaddr base, > const char *name, > hwaddr size, >
Writing this patch must have been "fun"; my eyes are bleeding from cross-referencing the new parameters with the removed properties. :/ I think the patch is correct. Reviewed-by: Laszlo Ersek <[email protected]> Thanks Laszlo
