Re: [Qemu-devel] [PATCH 2/2 v2] Set proper device-width for vexpress flash

[Peter Maydell]

On 18 October 2013 19:50, Roy Franz <address@hidden> 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