diff mbox

[2/2,v2] Set proper device-width for vexpress flash

Message ID 1382122225-2985-3-git-send-email-roy.franz@linaro.org
State New
Headers show

Commit Message

Roy Franz Oct. 18, 2013, 6:50 p.m. UTC
Create vexpress specific pflash registration
function which properly configures the device-width
of 16 bits (2 bytes) for the NOR flash on the
vexpress platform.  This change is required for
buffered flash writes to work properly.

Signed-off-by: Roy Franz <roy.franz@linaro.org>
---
 hw/arm/vexpress.c |   38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

Comments

Peter Maydell Oct. 19, 2013, 9:47 a.m. UTC | #1
On 18 October 2013 19:50, Roy Franz <roy.franz@linaro.org> wrote:
> Create vexpress specific pflash registration
> function which properly configures the device-width
> of 16 bits (2 bytes) for the NOR flash on the
> vexpress platform.  This change is required for
> buffered flash writes to work properly.

> +/* Open code a private version of plfash registration since we

"pflash"

> + * need to set non-default device width for Vexpress platform.
> + */
> +static pflash_t *ve_pflash_cfi01_register(hwaddr base,
> +                                DeviceState *qdev, const char *name,
> +                                hwaddr size,
> +                                BlockDriverState *bs,
> +                                uint32_t sector_len, int nb_blocs,
> +                                int bank_width, uint16_t id0, uint16_t id1,
> +                                uint16_t id2, uint16_t id3, int be)
> +{
> +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
> +
> +    if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
> +        abort();
> +    }
> +    qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
> +    qdev_prop_set_uint64(dev, "sector-length", sector_len);
> +    qdev_prop_set_uint8(dev, "width", bank_width);
> +    qdev_prop_set_uint8(dev, "device-width", 2);
> +    qdev_prop_set_uint8(dev, "big-endian", !!be);
> +    qdev_prop_set_uint16(dev, "id0", id0);
> +    qdev_prop_set_uint16(dev, "id1", id1);
> +    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 OBJECT_CHECK(pflash_t, (dev), "cfi.pflash01");
> +}
> +
>  static void vexpress_common_init(VEDBoardInfo *daughterboard,
>                                   QEMUMachineInitArgs *args)
>  {
> @@ -561,7 +594,8 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
>      sysbus_create_simple("pl111", map[VE_CLCD], pic[14]);
>
>      dinfo = drive_get_next(IF_PFLASH);
> -    pflash0 = pflash_cfi01_register(map[VE_NORFLASH0], NULL, "vexpress.flash0",
> +    pflash0 = ve_pflash_cfi01_register(map[VE_NORFLASH0], NULL,
> +            "vexpress.flash0",
>              VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
>              VEXPRESS_FLASH_SECT_SIZE,
>              VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
> @@ -580,7 +614,7 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
>      }
>
>      dinfo = drive_get_next(IF_PFLASH);
> -    if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, "vexpress.flash1",
> +    if (!ve_pflash_cfi01_register(map[VE_NORFLASH1], NULL, "vexpress.flash1",
>              VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
>              VEXPRESS_FLASH_SECT_SIZE,
>              VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,

Almost all the parameters you're passing here are the same for both calls,
so it would be better to hard code them in the utility function.

-- PMM
Roy Franz Oct. 19, 2013, 5:04 p.m. UTC | #2
Here is my updated patch to fix buffered flash writes on the VExpress
platform.  Buffered writes should now work properly on platforms whose
flash interface width is different from the device width.  The default
is for the device-width to be set to the interface width, so platforms
that can benefit from this change will need to be updated.  This patchset
updates the configuration for the VExpress platform which requires it.
UEFI firmware uses buffered writes for persistent variable storage,
and this patchset enables this usage on QEMU.

Changes from v2:
(All changes in patch 2/2, 1/1 unchanged.)
* Set flash invariant properties directly in VExpress specific flash init
  routine rather than passing long argument list.
* Fix typo in comment.

Changes from v1:
* Add device-width property and use this to mask write length instead
  of devices mas write length
* Update vexpress init code to set device-width to proper value.

Roy Franz (2):
  Add device-width property to pflash_cfi01
  Set proper device-width for vexpress flash

 hw/arm/vexpress.c       |   43 +++++++++++++++++++++++++++++++++----------
 hw/block/pflash_cfi01.c |   21 +++++++++++++--------
 2 files changed, 46 insertions(+), 18 deletions(-)
diff mbox

Patch

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index f48de00..2087245 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -480,6 +480,39 @@  static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
     }
 }
 
+
+/* Open code a private version of plfash registration since we
+ * need to set non-default device width for Vexpress platform.
+ */
+static pflash_t *ve_pflash_cfi01_register(hwaddr base,
+                                DeviceState *qdev, const char *name,
+                                hwaddr size,
+                                BlockDriverState *bs,
+                                uint32_t sector_len, int nb_blocs,
+                                int bank_width, uint16_t id0, uint16_t id1,
+                                uint16_t id2, uint16_t id3, int be)
+{
+    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+
+    if (bs && qdev_prop_set_drive(dev, "drive", bs)) {
+        abort();
+    }
+    qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
+    qdev_prop_set_uint64(dev, "sector-length", sector_len);
+    qdev_prop_set_uint8(dev, "width", bank_width);
+    qdev_prop_set_uint8(dev, "device-width", 2);
+    qdev_prop_set_uint8(dev, "big-endian", !!be);
+    qdev_prop_set_uint16(dev, "id0", id0);
+    qdev_prop_set_uint16(dev, "id1", id1);
+    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 OBJECT_CHECK(pflash_t, (dev), "cfi.pflash01");
+}
+
 static void vexpress_common_init(VEDBoardInfo *daughterboard,
                                  QEMUMachineInitArgs *args)
 {
@@ -561,7 +594,8 @@  static void vexpress_common_init(VEDBoardInfo *daughterboard,
     sysbus_create_simple("pl111", map[VE_CLCD], pic[14]);
 
     dinfo = drive_get_next(IF_PFLASH);
-    pflash0 = pflash_cfi01_register(map[VE_NORFLASH0], NULL, "vexpress.flash0",
+    pflash0 = ve_pflash_cfi01_register(map[VE_NORFLASH0], NULL,
+            "vexpress.flash0",
             VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
             VEXPRESS_FLASH_SECT_SIZE,
             VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
@@ -580,7 +614,7 @@  static void vexpress_common_init(VEDBoardInfo *daughterboard,
     }
 
     dinfo = drive_get_next(IF_PFLASH);
-    if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, "vexpress.flash1",
+    if (!ve_pflash_cfi01_register(map[VE_NORFLASH1], NULL, "vexpress.flash1",
             VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
             VEXPRESS_FLASH_SECT_SIZE,
             VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,