Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <address@hidden> wrote:
> pflash_cfi01_register always registers FLASH to the default
> system_memory region, which may not always be the right thing.
>
> Provide pflash_cfi01_init function that only creates FLASH device and
> sets its properties, leaving MMIO region registration to its caller.
>

You should just use QOM to create your device inline without need for
a construction helper. See Vexpress's ve_pflash_cfi01_register for an
example.

Regards,
Peter

> Cc: Kevin Wolf <address@hidden>
> Signed-off-by: Max Filippov <address@hidden>
> ---
>  hw/block/pflash_cfi01.c  | 29 +++++++++++++++++++++--------
>  include/hw/block/flash.h |  7 +++++++
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 2ba6c77..dfa7085 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -909,13 +909,12 @@ static void pflash_cfi01_register_types(void)
>
>  type_init(pflash_cfi01_register_types)
>
> -pflash_t *pflash_cfi01_register(hwaddr base,
> -                                DeviceState *qdev, const char *name,
> -                                hwaddr size,
> -                                BlockBackend *blk,
> -                                uint32_t sector_len, int nb_blocs,
> -                                int bank_width, uint16_t id0, uint16_t id1,
> -                                uint16_t id2, uint16_t id3, int be)
> +pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
> +                            hwaddr size,
> +                            BlockBackend *blk,
> +                            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, TYPE_CFI_PFLASH01);
>
> @@ -932,9 +931,23 @@ pflash_t *pflash_cfi01_register(hwaddr base,
>      qdev_prop_set_uint16(dev, "id3", id3);
>      qdev_prop_set_string(dev, "name", name);
>      qdev_init_nofail(dev);
> +    return CFI_PFLASH01(dev);
> +}
> +
> +pflash_t *pflash_cfi01_register(hwaddr base,
> +                                DeviceState *qdev, const char *name,
> +                                hwaddr size,
> +                                BlockBackend *blk,
> +                                uint32_t sector_len, int nb_blocs,
> +                                int bank_width, uint16_t id0, uint16_t id1,
> +                                uint16_t id2, uint16_t id3, int be)
> +{
> +    pflash_t *dev = pflash_cfi01_init(qdev, name, size, blk,
> +                                      sector_len, nb_blocs, bank_width,
> +                                      id0, id1, id2, id3, be);
>
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> -    return CFI_PFLASH01(dev);
> +    return dev;
>  }
>
>  MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
> index 50ccbbc..a57d6e9 100644
> --- a/include/hw/block/flash.h
> +++ b/include/hw/block/flash.h
> @@ -16,6 +16,13 @@ pflash_t *pflash_cfi01_register(hwaddr base,
>                                  uint16_t id0, uint16_t id1,
>                                  uint16_t id2, uint16_t id3, int be);
>
> +pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
> +                            hwaddr size,
> +                            BlockBackend *blk,
> +                            uint32_t sector_len, int nb_blocs, int width,
> +                            uint16_t id0, uint16_t id1,
> +                            uint16_t id2, uint16_t id3, int be);
> +
>  /* pflash_cfi02.c */
>  pflash_t *pflash_cfi02_register(hwaddr base,
>                                  DeviceState *qdev, const char *name,
> --
> 1.8.1.4
>
>