| Message ID | 20230109120833.3330-12-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw: Remove implicit sysbus_mmio_map() from pflash APIs | expand |
On Mon, 9 Jan 2023 at 12:31, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > pflash_cfi02_register() hides an implicit sysbus mapping of > MMIO region #0. This is not practical in a heterogeneous world > where multiple cores use different address spaces. In order to > remove pflash_cfi02_register() from the pflash API, open-code it > as a qdev creation call followed by an explicit sysbus mapping. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/arm/digic_boards.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c > index 4093af09cb..3700f05ecc 100644 > --- a/hw/arm/digic_boards.c > +++ b/hw/arm/digic_boards.c > @@ -30,6 +30,7 @@ > #include "qemu/error-report.h" > #include "hw/arm/digic.h" > #include "hw/block/flash.h" > +#include "hw/qdev-properties.h" > #include "hw/loader.h" > #include "sysemu/qtest.h" > #include "qemu/units.h" > @@ -115,13 +116,26 @@ static void digic4_add_k8p3215uqb_rom(DigicState *s, hwaddr addr, > { > #define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024) > #define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024) > + DeviceState *dev; > > - pflash_cfi02_register(addr, "pflash", FLASH_K8P3215UQB_SIZE, > - NULL, FLASH_K8P3215UQB_SECTOR_SIZE, > - DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE, > - 4, > - 0x00EC, 0x007E, 0x0003, 0x0001, > - 0x0555, 0x2aa, 0); > + dev = qdev_new(TYPE_PFLASH_CFI02); > + qdev_prop_set_string(dev, "name", "pflash"); > + qdev_prop_set_drive(dev, "drive", NULL); > + qdev_prop_set_uint32(dev, "num-blocks", > + FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE); > + qdev_prop_set_uint32(dev, "sector-length", FLASH_K8P3215UQB_SECTOR_SIZE); > + qdev_prop_set_uint8(dev, "device-width", > + DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE); > + qdev_prop_set_uint8(dev, "mappings", 4); > + qdev_prop_set_uint8(dev, "big-endian", false); > + qdev_prop_set_uint16(dev, "id0", 0x00ec); > + qdev_prop_set_uint16(dev, "id1", 0x007e); > + qdev_prop_set_uint16(dev, "id2", 0x0003); > + qdev_prop_set_uint16(dev, "id3", 0x0001); > + qdev_prop_set_uint16(dev, "unlock-addr0", 0x555); > + qdev_prop_set_uint16(dev, "unlock-addr1", 0x2aa); > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); Reviewed-by: Peter Maydell <peter.maydell@linaro.org> (give or take whether we choose to rename the 'width' property.) Not for this patch, but I'm a bit unsure about the use of DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE to calculate width. This gives us a value of 2, which is correct given the comment that says it's a 4Mx16 flash, but maybe it would be clearer just to set the value of 'width' to 2 directly? thanks -- PMM
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c index 4093af09cb..3700f05ecc 100644 --- a/hw/arm/digic_boards.c +++ b/hw/arm/digic_boards.c @@ -30,6 +30,7 @@ #include "qemu/error-report.h" #include "hw/arm/digic.h" #include "hw/block/flash.h" +#include "hw/qdev-properties.h" #include "hw/loader.h" #include "sysemu/qtest.h" #include "qemu/units.h" @@ -115,13 +116,26 @@ static void digic4_add_k8p3215uqb_rom(DigicState *s, hwaddr addr, { #define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024) #define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024) + DeviceState *dev; - pflash_cfi02_register(addr, "pflash", FLASH_K8P3215UQB_SIZE, - NULL, FLASH_K8P3215UQB_SECTOR_SIZE, - DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE, - 4, - 0x00EC, 0x007E, 0x0003, 0x0001, - 0x0555, 0x2aa, 0); + dev = qdev_new(TYPE_PFLASH_CFI02); + qdev_prop_set_string(dev, "name", "pflash"); + qdev_prop_set_drive(dev, "drive", NULL); + qdev_prop_set_uint32(dev, "num-blocks", + FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE); + qdev_prop_set_uint32(dev, "sector-length", FLASH_K8P3215UQB_SECTOR_SIZE); + qdev_prop_set_uint8(dev, "device-width", + DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE); + qdev_prop_set_uint8(dev, "mappings", 4); + qdev_prop_set_uint8(dev, "big-endian", false); + qdev_prop_set_uint16(dev, "id0", 0x00ec); + qdev_prop_set_uint16(dev, "id1", 0x007e); + qdev_prop_set_uint16(dev, "id2", 0x0003); + qdev_prop_set_uint16(dev, "id3", 0x0001); + qdev_prop_set_uint16(dev, "unlock-addr0", 0x555); + qdev_prop_set_uint16(dev, "unlock-addr1", 0x2aa); + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr); digic_load_rom(s, addr, FLASH_K8P3215UQB_SIZE, filename); }
pflash_cfi02_register() hides an implicit sysbus mapping of MMIO region #0. This is not practical in a heterogeneous world where multiple cores use different address spaces. In order to remove pflash_cfi02_register() from the pflash API, open-code it as a qdev creation call followed by an explicit sysbus mapping. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/arm/digic_boards.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)