diff mbox series

[16/20] hw/block: Factor pflash_cfi02_create() out of pflash_cfi02_register()

Message ID 20230104220449.41337-17-philmd@linaro.org
State New
Headers show
Series hw: Remove implicit sysbus_mmio_map() from pflash APIs | expand

Commit Message

Philippe Mathieu-Daudé Jan. 4, 2023, 10:04 p.m. UTC 
Currently pflash_cfi02_register():

 1/ creates a TYPE_PFLASH_CFI02 qdev instance
 2/ maps the first MMIO region to the system bus

The first minor issue is the implicit sysbus mapping is not
obvious (the function name could mention it), and the function
is not documented.

Another issue is we are forced to map on sysbus, thus code
wanting to simply instantiate this device are forced to open
code the qdev creation.

This is a problem in a heterogeneous system where not all cores
has access to the sysbus, or if we want to map the pflash on
different address spaces.

To clarify this API, extract the qdev creation in a new helper
named pflash_cfi02_create().

We don't document pflash_cfi02_register() because we are going
to remove it in a pair of commits.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/block/pflash_cfi02.c  | 55 ++++++++++++++++++++++++++--------------
 include/hw/block/flash.h | 14 +++++++++-
 2 files changed, 49 insertions(+), 20 deletions(-)

Comments

Bin Meng Jan. 8, 2023, 12:34 p.m. UTC | #1 
On Thu, Jan 5, 2023 at 6:51 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Currently pflash_cfi02_register():
>
>  1/ creates a TYPE_PFLASH_CFI02 qdev instance
>  2/ maps the first MMIO region to the system bus
>
> The first minor issue is the implicit sysbus mapping is not
> obvious (the function name could mention it), and the function
> is not documented.
>
> Another issue is we are forced to map on sysbus, thus code
> wanting to simply instantiate this device are forced to open
> code the qdev creation.
>
> This is a problem in a heterogeneous system where not all cores
> has access to the sysbus, or if we want to map the pflash on
> different address spaces.
>
> To clarify this API, extract the qdev creation in a new helper
> named pflash_cfi02_create().
>
> We don't document pflash_cfi02_register() because we are going
> to remove it in a pair of commits.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/block/pflash_cfi02.c  | 55 ++++++++++++++++++++++++++--------------
>  include/hw/block/flash.h | 14 +++++++++-
>  2 files changed, 49 insertions(+), 20 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
diff mbox series

Patch

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2a99b286b0..176f93b512 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -994,6 +994,37 @@  static void pflash_cfi02_register_types(void)
 
 type_init(pflash_cfi02_register_types)
 
+DeviceState *pflash_cfi02_create(const char *name, hwaddr size,
+                                 BlockBackend *blk, uint32_t sector_len,
+                                 int nb_mappings, int bank_width,
+                                 uint16_t id0, uint16_t id1,
+                                 uint16_t id2, uint16_t id3,
+                                 uint16_t unlock_addr0, uint16_t unlock_addr1,
+                                 int be)
+{
+    DeviceState *dev = qdev_new(TYPE_PFLASH_CFI02);
+
+    if (blk) {
+        qdev_prop_set_drive(dev, "drive", blk);
+    }
+    assert(QEMU_IS_ALIGNED(size, sector_len));
+    qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
+    qdev_prop_set_uint32(dev, "sector-length", sector_len);
+    qdev_prop_set_uint8(dev, "width", bank_width);
+    qdev_prop_set_uint8(dev, "mappings", nb_mappings);
+    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_uint16(dev, "unlock-addr0", unlock_addr0);
+    qdev_prop_set_uint16(dev, "unlock-addr1", unlock_addr1);
+    qdev_prop_set_string(dev, "name", name);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+
+    return dev;
+}
+
 PFlashCFI02 *pflash_cfi02_register(hwaddr base,
                                    const char *name,
                                    hwaddr size,
@@ -1006,26 +1037,12 @@  PFlashCFI02 *pflash_cfi02_register(hwaddr base,
                                    uint16_t unlock_addr1,
                                    int be)
 {
-    DeviceState *dev = qdev_new(TYPE_PFLASH_CFI02);
-
-    if (blk) {
-        qdev_prop_set_drive(dev, "drive", blk);
-    }
-    assert(QEMU_IS_ALIGNED(size, sector_len));
-    qdev_prop_set_uint32(dev, "num-blocks", size / sector_len);
-    qdev_prop_set_uint32(dev, "sector-length", sector_len);
-    qdev_prop_set_uint8(dev, "width", width);
-    qdev_prop_set_uint8(dev, "mappings", nb_mappings);
-    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_uint16(dev, "unlock-addr0", unlock_addr0);
-    qdev_prop_set_uint16(dev, "unlock-addr1", unlock_addr1);
-    qdev_prop_set_string(dev, "name", name);
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    DeviceState *dev;
 
+    dev = pflash_cfi02_create(name, size, blk, sector_len,
+                              nb_mappings, width, id0, id1, id2, id3,
+                              unlock_addr0, unlock_addr1, be);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
+
     return PFLASH_CFI02(dev);
 }
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 321aede8ef..78b078955e 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -32,7 +32,19 @@  void pflash_cfi01_legacy_drive(DeviceState *dev, DriveInfo *dinfo);
 #define TYPE_PFLASH_CFI02 "cfi.pflash02"
 OBJECT_DECLARE_SIMPLE_TYPE(PFlashCFI02, PFLASH_CFI02)
 
-
+/**
+ * Create and realize a parallel NOR flash (CFI type 2) on the heap.
+ *
+ * Create the device state structure, initialize it, and drop the
+ * reference to it (the device is realized).
+ */
+DeviceState *pflash_cfi02_create(const char *name, hwaddr size,
+                                 BlockBackend *blk, uint32_t sector_len,
+                                 int nb_mappings, int bank_width,
+                                 uint16_t id0, uint16_t id1,
+                                 uint16_t id2, uint16_t id3,
+                                 uint16_t unlock_addr0, uint16_t unlock_addr1,
+                                 int be);
 PFlashCFI02 *pflash_cfi02_register(hwaddr base,
                                    const char *name,
                                    hwaddr size,