add qemu_fdt_setprop_strings() and use it in most places

add qemu_fdt_setprop_strings() and use it in most places

[Ben Dooks]

Add a helper for qemu_fdt_setprop_strings() to take a set of strings
to put into a device-tree, which removes several open-coded methods
such as setting an char arr[] = {..} or setting char val[] = "str\0str2";       

This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
is not fully tested over all of those, I may not have caught all places
this is to be replaced.                                                  

Changes:

v4:
 - fix checkpatch issues
 - remove error checking in hw/core/guest-loader.c
v3:
 - fix return value for the call                                                
 - add better help text                                                         
v2:                                                                             
 - fix node/path in comment                                                     

[PATCH v4 1/6] device_tree: add qemu_fdt_setprop_strings() helper

[Ben Dooks]

Add a helper to set a property from a set of strings
to reduce the following code:

    static const char * const clint_compat[2] = {
        "sifive,clint0", "riscv,clint0"
    };

    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
        (char **)&clint_compat, ARRAY_SIZE(clint_compat));

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
v4:
 - go back to the non-return call, no-one is using the result
v3;
 - fix return value for the call
 - add better help text
v2:
 - fix node/path in comment
---
 include/sysemu/device_tree.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index ef060a9759..d5c05b5ebb 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
 int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
                                   const char *prop, char **array, int len);
 
+/**
+ * qemu_fdt_setprop_strings: set a property from a set of strings
+ *
+ * @fdt: pointer to the dt blob
+ * @path: node name
+ * @prop: property array
+ *
+ * This is a helper for the qemu_fdt_setprop_string_array() function
+ * which takes a va-arg set of strings instead of having to setup a
+ * single use string array.
+ */
+#define qemu_fdt_setprop_strings(fdt, path, prop, ...)          \
+    do {                                                        \
+        static const char * const __strs[] = { __VA_ARGS__ };   \
+        qemu_fdt_setprop_string_array(fdt, path, prop,          \
+                (char **)&__strs, ARRAY_SIZE(__strs));          \
+     } while(0)
+
+
 int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
                              const char *property,
                              const char *target_node_path);
-- 
2.35.1

[PATCH v4 2/6] hw/core: don't check return on qemu_fdt_setprop_string_array()

[Ben Dooks]

The qemu_fdt_setprop_string_array() does not return error codes and
will call exit() if any of the fdt calls fails (and should print an
error with the node being altered). This is done to prepare for the
change for qemu_fdt_setprop_strings() helper which does not return
any error codes (hw/core/guest-loader.c is the only place where an
return is checked).

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
 hw/core/guest-loader.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index 391c875a29..c61ebc4144 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -57,25 +57,17 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
 
     if (s->kernel) {
         const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
-                                          (char **) &compat,
-                                          ARRAY_SIZE(compat)) < 0) {
-            error_setg(errp, "couldn't set %s/compatible", node);
-            return;
-        }
+        qemu_fdt_setprop_string_array(fdt, node, "compatible",
+                                      (char **) &compat,
+                                      ARRAY_SIZE(compat));
         if (s->args) {
-            if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) {
-                error_setg(errp, "couldn't set %s/bootargs", node);
-            }
+            qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
         }
     } else if (s->initrd) {
         const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
-                                          (char **) &compat,
-                                          ARRAY_SIZE(compat)) < 0) {
-            error_setg(errp, "couldn't set %s/compatible", node);
-            return;
-        }
+        qemu_fdt_setprop_string_array(fdt, node, "compatible",
+                                      (char **) &compat,
+                                      ARRAY_SIZE(compat));
     }
 }
 
-- 
2.35.1

[PATCH v4 3/6] hw/riscv: use qemu_fdt_setprop_strings() for string arrays

[Ben Dooks]

Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
 hw/riscv/sifive_u.c | 18 +++++-------------
 hw/riscv/spike.c    |  7 ++-----
 hw/riscv/virt.c     | 32 ++++++++------------------------
 3 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e4c814a3ea..dc112a253a 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
     char *nodename;
     uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
     uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
-    static const char * const ethclk_names[2] = { "pclk", "hclk" };
-    static const char * const clint_compat[2] = {
-        "sifive,clint0", "riscv,clint0"
-    };
-    static const char * const plic_compat[2] = {
-        "sifive,plic-1.0.0", "riscv,plic0"
-    };
 
     if (ms->dtb) {
         fdt = s->fdt = load_device_tree(ms->dtb, &s->fdt_size);
@@ -221,11 +214,11 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
     nodename = g_strdup_printf("/soc/clint@%lx",
         (long)memmap[SIFIVE_U_DEV_CLINT].base);
     qemu_fdt_add_subnode(fdt, nodename);
-    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-        (char **)&clint_compat, ARRAY_SIZE(clint_compat));
     qemu_fdt_setprop_cells(fdt, nodename, "reg",
         0x0, memmap[SIFIVE_U_DEV_CLINT].base,
         0x0, memmap[SIFIVE_U_DEV_CLINT].size);
+    qemu_fdt_setprop_strings(fdt, nodename, "compatible",
+                             "sifive,clint0", "riscv,clint0");
     qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
         cells, ms->smp.cpus * sizeof(uint32_t) * 4);
     g_free(cells);
@@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
         (long)memmap[SIFIVE_U_DEV_PLIC].base);
     qemu_fdt_add_subnode(fdt, nodename);
     qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
-    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
-        (char **)&plic_compat, ARRAY_SIZE(plic_compat));
+    qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
+                             "sifive,plic-1.0.0", "riscv,plic0");
     qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
         cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
@@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
     qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
     qemu_fdt_setprop_cells(fdt, nodename, "clocks",
         prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
-    qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
-        (char **)&ethclk_names, ARRAY_SIZE(ethclk_names));
+    qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
     qemu_fdt_setprop(fdt, nodename, "local-mac-address",
         s->soc.gem.conf.macaddr.a, ETH_ALEN);
     qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e41b6aa9f0..aa895779cd 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -59,9 +59,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
     uint32_t cpu_phandle, intc_phandle, phandle = 1;
     char *name, *mem_name, *clint_name, *clust_name;
     char *core_name, *cpu_name, *intc_name;
-    static const char * const clint_compat[2] = {
-        "sifive,clint0", "riscv,clint0"
-    };
 
     fdt = s->fdt = create_device_tree(&s->fdt_size);
     if (!fdt) {
@@ -159,8 +156,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
             (memmap[SPIKE_CLINT].size * socket);
         clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
         qemu_fdt_add_subnode(fdt, clint_name);
-        qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
-            (char **)&clint_compat, ARRAY_SIZE(clint_compat));
+        qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
+                                 "sifive,clint0", "riscv,clint0");
         qemu_fdt_setprop_cells(fdt, clint_name, "reg",
             0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
         qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index bc424dd2f5..c6aaa611a6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -261,11 +261,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
             intc_phandles[cpu]);
         if (riscv_feature(&s->soc[socket].harts[cpu].env,
                           RISCV_FEATURE_AIA)) {
-            static const char * const compat[2] = {
-                "riscv,cpu-intc-aia", "riscv,cpu-intc"
-            };
-            qemu_fdt_setprop_string_array(mc->fdt, intc_name, "compatible",
-                                      (char **)&compat, ARRAY_SIZE(compat));
+            qemu_fdt_setprop_strings(mc->fdt, intc_name, "compatible",
+                                     "riscv,cpu-intc-aia", "riscv,cpu-intc");
         } else {
             qemu_fdt_setprop_string(mc->fdt, intc_name, "compatible",
                 "riscv,cpu-intc");
@@ -310,9 +307,6 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     uint32_t *clint_cells;
     unsigned long clint_addr;
     MachineState *mc = MACHINE(s);
-    static const char * const clint_compat[2] = {
-        "sifive,clint0", "riscv,clint0"
-    };
 
     clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
 
@@ -326,9 +320,8 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
     clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
     clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
     qemu_fdt_add_subnode(mc->fdt, clint_name);
-    qemu_fdt_setprop_string_array(mc->fdt, clint_name, "compatible",
-                                  (char **)&clint_compat,
-                                  ARRAY_SIZE(clint_compat));
+    qemu_fdt_setprop_strings(mc->fdt, clint_name, "compatible",
+                             "sifive,clint0", "riscv,clint0");
     qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
         0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
     qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
@@ -437,9 +430,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     uint32_t *plic_cells;
     unsigned long plic_addr;
     MachineState *mc = MACHINE(s);
-    static const char * const plic_compat[2] = {
-        "sifive,plic-1.0.0", "riscv,plic0"
-    };
 
     if (kvm_enabled()) {
         plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
@@ -465,9 +455,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
     qemu_fdt_add_subnode(mc->fdt, plic_name);
     qemu_fdt_setprop_cell(mc->fdt, plic_name,
         "#interrupt-cells", FDT_PLIC_INT_CELLS);
-    qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
-                                  (char **)&plic_compat,
-                                  ARRAY_SIZE(plic_compat));
+    qemu_fdt_setprop_strings(mc->fdt, plic_name, "compatible",
+                             "sifive,plic-1.0.0", "riscv,plic0");
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
         plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
@@ -881,13 +870,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
     name = g_strdup_printf("/soc/test@%lx",
         (long)memmap[VIRT_TEST].base);
     qemu_fdt_add_subnode(mc->fdt, name);
-    {
-        static const char * const compat[3] = {
-            "sifive,test1", "sifive,test0", "syscon"
-        };
-        qemu_fdt_setprop_string_array(mc->fdt, name, "compatible",
-                                      (char **)&compat, ARRAY_SIZE(compat));
-    }
+    qemu_fdt_setprop_strings(mc->fdt, name, "compatible",
+                             "sifive,test1", "sifive,test0", "syscon");
     qemu_fdt_setprop_cells(mc->fdt, name, "reg",
         0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
     qemu_fdt_setprop_cell(mc->fdt, name, "phandle", test_phandle);
-- 
2.35.1

[PATCH v4 4/6] hw/core: use qemu_fdt_setprop_strings()

[Ben Dooks]

Change to using the qemu_fdt_setprop_strings() helper in
hw/core code.

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
 hw/core/guest-loader.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index c61ebc4144..7b8e32e06f 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -56,18 +56,15 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
     qemu_fdt_setprop(fdt, node, "reg", &reg_attr, sizeof(reg_attr));
 
     if (s->kernel) {
-        const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
-        qemu_fdt_setprop_string_array(fdt, node, "compatible",
-                                      (char **) &compat,
-                                      ARRAY_SIZE(compat));
+        qemu_fdt_setprop_strings(fdt, node, "compatible",
+                                 "multiboot,module", "multiboot,kernel");
+
         if (s->args) {
             qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
         }
     } else if (s->initrd) {
-        const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
-        qemu_fdt_setprop_string_array(fdt, node, "compatible",
-                                      (char **) &compat,
-                                      ARRAY_SIZE(compat));
+        qemu_fdt_setprop_strings(fdt, node, "compatible",
+                                 "multiboot,module", "multiboot,ramdisk");
     }
 }
 
-- 
2.35.1

[PATCH v4 5/6] hw/mips: use qemu_fdt_setprop_strings()

[Ben Dooks]

Change to using qemu_fdt_setprop_strings() helper in hw/mips.

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/mips/boston.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index d2ab9da1a0..759f6daafe 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
     MachineState *mc = s->mach;
     uint32_t platreg_ph, gic_ph, clk_ph;
     char *name, *gic_name, *platreg_name, *stdout_name;
-    static const char * const syscon_compat[2] = {
-        "img,boston-platform-regs", "syscon"
-    };
 
     fdt = create_device_tree(dt_size);
     if (!fdt) {
@@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
     platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
                                    memmap[BOSTON_PLATREG].base);
     qemu_fdt_add_subnode(fdt, platreg_name);
-    qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
-                                 (char **)&syscon_compat,
-                                 ARRAY_SIZE(syscon_compat));
+    qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
+                             "img,boston-platform-regs", "syscon");
     qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
                            memmap[BOSTON_PLATREG].base,
                            memmap[BOSTON_PLATREG].size);
-- 
2.35.1

[PATCH v4 6/6] hw/arm: change to use qemu_fdt_setprop_strings()

[Ben Dooks]

Change to using qemu_fdt_setprop_strings() instead of using
\0 separated string arrays. Note, also there were a few places
where qemu_fdt_setprop_string() can be used in the same areas.

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
v4:
 - fixed checkpatch errors with string
 - fixed patch subject
---
 hw/arm/boot.c             |  8 +++---
 hw/arm/virt.c             | 28 +++++++++------------
 hw/arm/xlnx-versal-virt.c | 51 ++++++++++++++++-----------------------
 3 files changed, 37 insertions(+), 50 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ada2717f76..766257cbfb 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -490,11 +490,11 @@ static void fdt_add_psci_node(void *fdt)
     qemu_fdt_add_subnode(fdt, "/psci");
     if (armcpu->psci_version >= QEMU_PSCI_VERSION_0_2) {
         if (armcpu->psci_version < QEMU_PSCI_VERSION_1_0) {
-            const char comp[] = "arm,psci-0.2\0arm,psci";
-            qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+            qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+                                     "arm,psci-0.2", "arm,psci");
         } else {
-            const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci";
-            qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp));
+            qemu_fdt_setprop_strings(fdt, "/psci", "compatible",
+                                     "arm,psci-1.0", "arm,psci-0.2", "arm,psci");
         }
 
         cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9633f822f3..2670c13ef1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -343,9 +343,8 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
 
     armcpu = ARM_CPU(qemu_get_cpu(0));
     if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
-        const char compat[] = "arm,armv8-timer\0arm,armv7-timer";
-        qemu_fdt_setprop(ms->fdt, "/timer", "compatible",
-                         compat, sizeof(compat));
+        qemu_fdt_setprop_strings(ms->fdt, "/timer", "compatible",
+                                 "arm,armv8-timer", "arm,armv7-timer");
     } else {
         qemu_fdt_setprop_string(ms->fdt, "/timer", "compatible",
                                 "arm,armv7-timer");
@@ -843,8 +842,6 @@ static void create_uart(const VirtMachineState *vms, int uart,
     hwaddr base = vms->memmap[uart].base;
     hwaddr size = vms->memmap[uart].size;
     int irq = vms->irqmap[uart];
-    const char compat[] = "arm,pl011\0arm,primecell";
-    const char clocknames[] = "uartclk\0apb_pclk";
     DeviceState *dev = qdev_new(TYPE_PL011);
     SysBusDevice *s = SYS_BUS_DEVICE(dev);
     MachineState *ms = MACHINE(vms);
@@ -858,8 +855,8 @@ static void create_uart(const VirtMachineState *vms, int uart,
     nodename = g_strdup_printf("/pl011@%" PRIx64, base);
     qemu_fdt_add_subnode(ms->fdt, nodename);
     /* Note that we can't use setprop_string because of the embedded NUL */
-    qemu_fdt_setprop(ms->fdt, nodename, "compatible",
-                         compat, sizeof(compat));
+    qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+                             "arm,pl011", "arm,primecell");
     qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
                                      2, base, 2, size);
     qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -867,8 +864,8 @@ static void create_uart(const VirtMachineState *vms, int uart,
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI);
     qemu_fdt_setprop_cells(ms->fdt, nodename, "clocks",
                                vms->clock_phandle, vms->clock_phandle);
-    qemu_fdt_setprop(ms->fdt, nodename, "clock-names",
-                         clocknames, sizeof(clocknames));
+    qemu_fdt_setprop_strings(ms->fdt, nodename, "clock-names",
+                             "uartclk", "apb_pclk");
 
     if (uart == VIRT_UART) {
         qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
@@ -890,14 +887,14 @@ static void create_rtc(const VirtMachineState *vms)
     hwaddr base = vms->memmap[VIRT_RTC].base;
     hwaddr size = vms->memmap[VIRT_RTC].size;
     int irq = vms->irqmap[VIRT_RTC];
-    const char compat[] = "arm,pl031\0arm,primecell";
     MachineState *ms = MACHINE(vms);
 
     sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq));
 
     nodename = g_strdup_printf("/pl031@%" PRIx64, base);
     qemu_fdt_add_subnode(ms->fdt, nodename);
-    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+                             "arm,pl031", "arm,primecell");
     qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
                                  2, base, 2, size);
     qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -983,7 +980,6 @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio,
     hwaddr base = vms->memmap[gpio].base;
     hwaddr size = vms->memmap[gpio].size;
     int irq = vms->irqmap[gpio];
-    const char compat[] = "arm,pl061\0arm,primecell";
     SysBusDevice *s;
     MachineState *ms = MACHINE(vms);
 
@@ -1001,7 +997,8 @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio,
     qemu_fdt_add_subnode(ms->fdt, nodename);
     qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg",
                                  2, base, 2, size);
-    qemu_fdt_setprop(ms->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_strings(ms->fdt, nodename, "compatible",
+                             "arm,pl061", "arm,primecell");
     qemu_fdt_setprop_cell(ms->fdt, nodename, "#gpio-cells", 2);
     qemu_fdt_setprop(ms->fdt, nodename, "gpio-controller", NULL, 0);
     qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
@@ -1325,7 +1322,6 @@ static void create_smmu(const VirtMachineState *vms,
     int i;
     hwaddr base = vms->memmap[VIRT_SMMU].base;
     hwaddr size = vms->memmap[VIRT_SMMU].size;
-    const char irq_names[] = "eventq\0priq\0cmdq-sync\0gerror";
     DeviceState *dev;
     MachineState *ms = MACHINE(vms);
 
@@ -1355,8 +1351,8 @@ static void create_smmu(const VirtMachineState *vms,
             GIC_FDT_IRQ_TYPE_SPI, irq + 2, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI,
             GIC_FDT_IRQ_TYPE_SPI, irq + 3, GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
 
-    qemu_fdt_setprop(ms->fdt, node, "interrupt-names", irq_names,
-                     sizeof(irq_names));
+    qemu_fdt_setprop_strings(ms->fdt, node, "interrupt-names",
+                             "eventq", "priq", "cmdq-sync", "gerror");
 
     qemu_fdt_setprop_cell(ms->fdt, node, "clocks", vms->clock_phandle);
     qemu_fdt_setprop_string(ms->fdt, node, "clock-names", "apb_pclk");
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 37fc9b919c..20ae5b0eb1 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -153,7 +153,6 @@ static void fdt_add_timer_nodes(VersalVirt *s)
 
 static void fdt_add_usb_xhci_nodes(VersalVirt *s)
 {
-    const char clocknames[] = "bus_clk\0ref_clk";
     const char irq_name[] = "dwc_usb3";
     const char compatVersalDWC3[] = "xlnx,versal-dwc3";
     const char compatDWC3[] = "snps,dwc3";
@@ -165,8 +164,8 @@ static void fdt_add_usb_xhci_nodes(VersalVirt *s)
     qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                  2, MM_USB2_CTRL_REGS,
                                  2, MM_USB2_CTRL_REGS_SIZE);
-    qemu_fdt_setprop(s->fdt, name, "clock-names",
-                         clocknames, sizeof(clocknames));
+    qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
+                             "bus_clk", "ref_clk");
     qemu_fdt_setprop_cells(s->fdt, name, "clocks",
                                s->phandle.clk_25Mhz, s->phandle.clk_125Mhz);
     qemu_fdt_setprop(s->fdt, name, "ranges", NULL, 0);
@@ -205,8 +204,6 @@ static void fdt_add_uart_nodes(VersalVirt *s)
 {
     uint64_t addrs[] = { MM_UART1, MM_UART0 };
     unsigned int irqs[] = { VERSAL_UART1_IRQ_0, VERSAL_UART0_IRQ_0 };
-    const char compat[] = "arm,pl011\0arm,sbsa-uart";
-    const char clocknames[] = "uartclk\0apb_pclk";
     int i;
 
     for (i = 0; i < ARRAY_SIZE(addrs); i++) {
@@ -215,16 +212,16 @@ static void fdt_add_uart_nodes(VersalVirt *s)
         qemu_fdt_setprop_cell(s->fdt, name, "current-speed", 115200);
         qemu_fdt_setprop_cells(s->fdt, name, "clocks",
                                s->phandle.clk_125Mhz, s->phandle.clk_125Mhz);
-        qemu_fdt_setprop(s->fdt, name, "clock-names",
-                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
+                                 "uartclk", "apb_pclk");
 
         qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
                                GIC_FDT_IRQ_TYPE_SPI, irqs[i],
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI);
         qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                      2, addrs[i], 2, 0x1000);
-        qemu_fdt_setprop(s->fdt, name, "compatible",
-                         compat, sizeof(compat));
+        qemu_fdt_setprop_strings(s->fdt, name, "compatible",
+                                 "arm,pl011", "arm,sbsa-uart");
         qemu_fdt_setprop(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
 
         if (addrs[i] == MM_UART0) {
@@ -251,8 +248,6 @@ static void fdt_add_gem_nodes(VersalVirt *s)
 {
     uint64_t addrs[] = { MM_GEM1, MM_GEM0 };
     unsigned int irqs[] = { VERSAL_GEM1_IRQ_0, VERSAL_GEM0_IRQ_0 };
-    const char clocknames[] = "pclk\0hclk\0tx_clk\0rx_clk";
-    const char compat_gem[] = "cdns,zynqmp-gem\0cdns,gem";
     int i;
 
     for (i = 0; i < ARRAY_SIZE(addrs); i++) {
@@ -266,8 +261,8 @@ static void fdt_add_gem_nodes(VersalVirt *s)
         qemu_fdt_setprop_cells(s->fdt, name, "clocks",
                                s->phandle.clk_25Mhz, s->phandle.clk_25Mhz,
                                s->phandle.clk_125Mhz, s->phandle.clk_125Mhz);
-        qemu_fdt_setprop(s->fdt, name, "clock-names",
-                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
+                                 "pclk", "hclk", "tx_clk", "rx_clk");
         qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
                                GIC_FDT_IRQ_TYPE_SPI, irqs[i],
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI,
@@ -275,8 +270,8 @@ static void fdt_add_gem_nodes(VersalVirt *s)
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI);
         qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                      2, addrs[i], 2, 0x1000);
-        qemu_fdt_setprop(s->fdt, name, "compatible",
-                         compat_gem, sizeof(compat_gem));
+        qemu_fdt_setprop_strings(s->fdt, name, "compatible",
+                                 "cdns,zynqmp-gem", "cdns,gem");
         qemu_fdt_setprop_cell(s->fdt, name, "#address-cells", 1);
         qemu_fdt_setprop_cell(s->fdt, name, "#size-cells", 0);
         g_free(name);
@@ -285,8 +280,6 @@ static void fdt_add_gem_nodes(VersalVirt *s)
 
 static void fdt_add_zdma_nodes(VersalVirt *s)
 {
-    const char clocknames[] = "clk_main\0clk_apb";
-    const char compat[] = "xlnx,zynqmp-dma-1.0";
     int i;
 
     for (i = XLNX_VERSAL_NR_ADMAS - 1; i >= 0; i--) {
@@ -298,22 +291,21 @@ static void fdt_add_zdma_nodes(VersalVirt *s)
         qemu_fdt_setprop_cell(s->fdt, name, "xlnx,bus-width", 64);
         qemu_fdt_setprop_cells(s->fdt, name, "clocks",
                                s->phandle.clk_25Mhz, s->phandle.clk_25Mhz);
-        qemu_fdt_setprop(s->fdt, name, "clock-names",
-                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
+                                 "clk_main", "clk_apb");
         qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
                                GIC_FDT_IRQ_TYPE_SPI, VERSAL_ADMA_IRQ_0 + i,
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI);
         qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                      2, addr, 2, 0x1000);
-        qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
+        qemu_fdt_setprop_string(s->fdt, name, "compatible",
+                                "xlnx,zynqmp-dma-1.0");
         g_free(name);
     }
 }
 
 static void fdt_add_sd_nodes(VersalVirt *s)
 {
-    const char clocknames[] = "clk_xin\0clk_ahb";
-    const char compat[] = "arasan,sdhci-8.9a";
     int i;
 
     for (i = ARRAY_SIZE(s->soc.pmc.iou.sd) - 1; i >= 0; i--) {
@@ -324,22 +316,21 @@ static void fdt_add_sd_nodes(VersalVirt *s)
 
         qemu_fdt_setprop_cells(s->fdt, name, "clocks",
                                s->phandle.clk_25Mhz, s->phandle.clk_25Mhz);
-        qemu_fdt_setprop(s->fdt, name, "clock-names",
-                         clocknames, sizeof(clocknames));
+        qemu_fdt_setprop_strings(s->fdt, name, "clock-names",
+                                 "clk_xin", "clk_ahb");
         qemu_fdt_setprop_cells(s->fdt, name, "interrupts",
                                GIC_FDT_IRQ_TYPE_SPI, VERSAL_SD0_IRQ_0 + i * 2,
                                GIC_FDT_IRQ_FLAGS_LEVEL_HI);
         qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                      2, addr, 2, MM_PMC_SD0_SIZE);
-        qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
+        qemu_fdt_setprop_string(s->fdt, name, "compatible",
+                                "arasan,sdhci-8.9a");
         g_free(name);
     }
 }
 
 static void fdt_add_rtc_node(VersalVirt *s)
 {
-    const char compat[] = "xlnx,zynqmp-rtc";
-    const char interrupt_names[] = "alarm\0sec";
     char *name = g_strdup_printf("/rtc@%x", MM_PMC_RTC);
 
     qemu_fdt_add_subnode(s->fdt, name);
@@ -349,11 +340,11 @@ static void fdt_add_rtc_node(VersalVirt *s)
                            GIC_FDT_IRQ_FLAGS_LEVEL_HI,
                            GIC_FDT_IRQ_TYPE_SPI, VERSAL_RTC_SECONDS_IRQ,
                            GIC_FDT_IRQ_FLAGS_LEVEL_HI);
-    qemu_fdt_setprop(s->fdt, name, "interrupt-names",
-                     interrupt_names, sizeof(interrupt_names));
+    qemu_fdt_setprop_strings(s->fdt, name, "interrupt-names",
+                             "alarm", "sec");
     qemu_fdt_setprop_sized_cells(s->fdt, name, "reg",
                                  2, MM_PMC_RTC, 2, MM_PMC_RTC_SIZE);
-    qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_string(s->fdt, name, "compatible", "xlnx,zynqmp-rtc");
     g_free(name);
 }
 
-- 
2.35.1

Re: add qemu_fdt_setprop_strings() and use it in most places

[Ben Dooks]

On Tue, Aug 09, 2022 at 07:56:34PM +0100, Ben Dooks wrote:
> Add a helper for qemu_fdt_setprop_strings() to take a set of strings
> to put into a device-tree, which removes several open-coded methods
> such as setting an char arr[] = {..} or setting char val[] = "str\0str2";       
> 
> This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
> is not fully tested over all of those, I may not have caught all places
> this is to be replaced.                                                  

I've put a github repo up with this and possibly some other bits I am
working on:

https://github.com/bendooks/qemu/tree/dev/fdt-string-array-v4

> Changes:
> 
> v4:
>  - fix checkpatch issues
>  - remove error checking in hw/core/guest-loader.c
> v3:
>  - fix return value for the call                                                
>  - add better help text                                                         
> v2:                                                                             
>  - fix node/path in comment                                                     
> 
> 
> 

-- 
Ben Dooks, ben@fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.

Re: [PATCH v4 1/6] device_tree: add qemu_fdt_setprop_strings() helper

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:35PM +0100, Ben Dooks wrote:
> Add a helper to set a property from a set of strings
> to reduce the following code:
> 
>     static const char * const clint_compat[2] = {
>         "sifive,clint0", "riscv,clint0"
>     };
> 
>     qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
>         (char **)&clint_compat, ARRAY_SIZE(clint_compat));
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
> v4:
>  - go back to the non-return call, no-one is using the result
> v3;
>  - fix return value for the call
>  - add better help text
> v2:
>  - fix node/path in comment
> ---
>  include/sysemu/device_tree.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index ef060a9759..d5c05b5ebb 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -87,6 +87,25 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path,
>  int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
>                                    const char *prop, char **array, int len);
>  
> +/**
> + * qemu_fdt_setprop_strings: set a property from a set of strings
> + *
> + * @fdt: pointer to the dt blob
> + * @path: node name
> + * @prop: property array
> + *
> + * This is a helper for the qemu_fdt_setprop_string_array() function
> + * which takes a va-arg set of strings instead of having to setup a
> + * single use string array.
> + */
> +#define qemu_fdt_setprop_strings(fdt, path, prop, ...)          \
> +    do {                                                        \
> +        static const char * const __strs[] = { __VA_ARGS__ };   \

           ^^ We don't want that static there. That'll keep the storage
	   for each invocation and require that __VA_ARGS__ are always
	   constants only.

	   I'd drop the const's too since __strs is only used for a
	   single call where it immediately gets the const's cast'ed
	   away.

> +        qemu_fdt_setprop_string_array(fdt, path, prop,          \
> +                (char **)&__strs, ARRAY_SIZE(__strs));          \

                   ^^ And then this can just be __strs

> +     } while(0)
> +
> +
>  int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
>                               const char *property,
>                               const char *target_node_path);
> -- 
> 2.35.1
> 
> 

Thanks,
drew

Re: [PATCH v4 2/6] hw/core: don't check return on qemu_fdt_setprop_string_array()

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:36PM +0100, Ben Dooks wrote:
> The qemu_fdt_setprop_string_array() does not return error codes and
> will call exit() if any of the fdt calls fails (and should print an
> error with the node being altered). This is done to prepare for the
> change for qemu_fdt_setprop_strings() helper which does not return
> any error codes (hw/core/guest-loader.c is the only place where an
> return is checked).
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
>  hw/core/guest-loader.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> index 391c875a29..c61ebc4144 100644
> --- a/hw/core/guest-loader.c
> +++ b/hw/core/guest-loader.c
> @@ -57,25 +57,17 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
>  
>      if (s->kernel) {
>          const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> -        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                          (char **) &compat,
> -                                          ARRAY_SIZE(compat)) < 0) {
> -            error_setg(errp, "couldn't set %s/compatible", node);
> -            return;
> -        }
> +        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                      (char **) &compat,
> +                                      ARRAY_SIZE(compat));
>          if (s->args) {
> -            if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) {
> -                error_setg(errp, "couldn't set %s/bootargs", node);
> -            }
> +            qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
>          }
>      } else if (s->initrd) {
>          const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
> -        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                          (char **) &compat,
> -                                          ARRAY_SIZE(compat)) < 0) {
> -            error_setg(errp, "couldn't set %s/compatible", node);
> -            return;
> -        }
> +        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                      (char **) &compat,
> +                                      ARRAY_SIZE(compat));
>      }
>  }
>  
> -- 
> 2.35.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v4 3/6] hw/riscv: use qemu_fdt_setprop_strings() for string arrays

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:37PM +0100, Ben Dooks wrote:
> Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
>  hw/riscv/sifive_u.c | 18 +++++-------------
>  hw/riscv/spike.c    |  7 ++-----
>  hw/riscv/virt.c     | 32 ++++++++------------------------
>  3 files changed, 15 insertions(+), 42 deletions(-)
> 
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index e4c814a3ea..dc112a253a 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      char *nodename;
>      uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
>      uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
> -    static const char * const ethclk_names[2] = { "pclk", "hclk" };
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
> -    static const char * const plic_compat[2] = {
> -        "sifive,plic-1.0.0", "riscv,plic0"
> -    };
>  
>      if (ms->dtb) {
>          fdt = s->fdt = load_device_tree(ms->dtb, &s->fdt_size);
> @@ -221,11 +214,11 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      nodename = g_strdup_printf("/soc/clint@%lx",
>          (long)memmap[SIFIVE_U_DEV_CLINT].base);
>      qemu_fdt_add_subnode(fdt, nodename);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -        (char **)&clint_compat, ARRAY_SIZE(clint_compat));
>      qemu_fdt_setprop_cells(fdt, nodename, "reg",
>          0x0, memmap[SIFIVE_U_DEV_CLINT].base,
>          0x0, memmap[SIFIVE_U_DEV_CLINT].size);
> +    qemu_fdt_setprop_strings(fdt, nodename, "compatible",
> +                             "sifive,clint0", "riscv,clint0");

Why move this down?

>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
>          cells, ms->smp.cpus * sizeof(uint32_t) * 4);
>      g_free(cells);
> @@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>          (long)memmap[SIFIVE_U_DEV_PLIC].base);
>      qemu_fdt_add_subnode(fdt, nodename);
>      qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -        (char **)&plic_compat, ARRAY_SIZE(plic_compat));
> +    qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
> +                             "sifive,plic-1.0.0", "riscv,plic0");
>      qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
>          cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
> @@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
>      qemu_fdt_setprop_cells(fdt, nodename, "clocks",
>          prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
> -        (char **)&ethclk_names, ARRAY_SIZE(ethclk_names));
> +    qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
>      qemu_fdt_setprop(fdt, nodename, "local-mac-address",
>          s->soc.gem.conf.macaddr.a, ETH_ALEN);
>      qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index e41b6aa9f0..aa895779cd 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -59,9 +59,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>      uint32_t cpu_phandle, intc_phandle, phandle = 1;
>      char *name, *mem_name, *clint_name, *clust_name;
>      char *core_name, *cpu_name, *intc_name;
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
>  
>      fdt = s->fdt = create_device_tree(&s->fdt_size);
>      if (!fdt) {
> @@ -159,8 +156,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>              (memmap[SPIKE_CLINT].size * socket);
>          clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
>          qemu_fdt_add_subnode(fdt, clint_name);
> -        qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
> -            (char **)&clint_compat, ARRAY_SIZE(clint_compat));
> +        qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
> +                                 "sifive,clint0", "riscv,clint0");
>          qemu_fdt_setprop_cells(fdt, clint_name, "reg",
>              0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
>          qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..c6aaa611a6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -261,11 +261,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
>              intc_phandles[cpu]);
>          if (riscv_feature(&s->soc[socket].harts[cpu].env,
>                            RISCV_FEATURE_AIA)) {
> -            static const char * const compat[2] = {
> -                "riscv,cpu-intc-aia", "riscv,cpu-intc"
> -            };
> -            qemu_fdt_setprop_string_array(mc->fdt, intc_name, "compatible",
> -                                      (char **)&compat, ARRAY_SIZE(compat));
> +            qemu_fdt_setprop_strings(mc->fdt, intc_name, "compatible",
> +                                     "riscv,cpu-intc-aia", "riscv,cpu-intc");
>          } else {
>              qemu_fdt_setprop_string(mc->fdt, intc_name, "compatible",
>                  "riscv,cpu-intc");
> @@ -310,9 +307,6 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
>      uint32_t *clint_cells;
>      unsigned long clint_addr;
>      MachineState *mc = MACHINE(s);
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
>  
>      clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
>  
> @@ -326,9 +320,8 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
>      clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
>      clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
>      qemu_fdt_add_subnode(mc->fdt, clint_name);
> -    qemu_fdt_setprop_string_array(mc->fdt, clint_name, "compatible",
> -                                  (char **)&clint_compat,
> -                                  ARRAY_SIZE(clint_compat));
> +    qemu_fdt_setprop_strings(mc->fdt, clint_name, "compatible",
> +                             "sifive,clint0", "riscv,clint0");
>      qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
>          0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
>      qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
> @@ -437,9 +430,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      uint32_t *plic_cells;
>      unsigned long plic_addr;
>      MachineState *mc = MACHINE(s);
> -    static const char * const plic_compat[2] = {
> -        "sifive,plic-1.0.0", "riscv,plic0"
> -    };
>  
>      if (kvm_enabled()) {
>          plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> @@ -465,9 +455,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      qemu_fdt_add_subnode(mc->fdt, plic_name);
>      qemu_fdt_setprop_cell(mc->fdt, plic_name,
>          "#interrupt-cells", FDT_PLIC_INT_CELLS);
> -    qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
> -                                  (char **)&plic_compat,
> -                                  ARRAY_SIZE(plic_compat));
> +    qemu_fdt_setprop_strings(mc->fdt, plic_name, "compatible",
> +                             "sifive,plic-1.0.0", "riscv,plic0");
>      qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
>          plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> @@ -881,13 +870,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>      name = g_strdup_printf("/soc/test@%lx",
>          (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(mc->fdt, name);
> -    {
> -        static const char * const compat[3] = {
> -            "sifive,test1", "sifive,test0", "syscon"
> -        };
> -        qemu_fdt_setprop_string_array(mc->fdt, name, "compatible",
> -                                      (char **)&compat, ARRAY_SIZE(compat));
> -    }
> +    qemu_fdt_setprop_strings(mc->fdt, name, "compatible",
> +                             "sifive,test1", "sifive,test0", "syscon");
>      qemu_fdt_setprop_cells(mc->fdt, name, "reg",
>          0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
>      qemu_fdt_setprop_cell(mc->fdt, name, "phandle", test_phandle);
> -- 
> 2.35.1
> 
>

Other than the unnecessary line move

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v4 4/6] hw/core: use qemu_fdt_setprop_strings()

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:38PM +0100, Ben Dooks wrote:
> Change to using the qemu_fdt_setprop_strings() helper in
> hw/core code.
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
>  hw/core/guest-loader.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> index c61ebc4144..7b8e32e06f 100644
> --- a/hw/core/guest-loader.c
> +++ b/hw/core/guest-loader.c
> @@ -56,18 +56,15 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
>      qemu_fdt_setprop(fdt, node, "reg", &reg_attr, sizeof(reg_attr));
>  
>      if (s->kernel) {
> -        const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> -        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                      (char **) &compat,
> -                                      ARRAY_SIZE(compat));
> +        qemu_fdt_setprop_strings(fdt, node, "compatible",
> +                                 "multiboot,module", "multiboot,kernel");
> +
>          if (s->args) {
>              qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
>          }
>      } else if (s->initrd) {
> -        const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
> -        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                      (char **) &compat,
> -                                      ARRAY_SIZE(compat));
> +        qemu_fdt_setprop_strings(fdt, node, "compatible",
> +                                 "multiboot,module", "multiboot,ramdisk");
>      }
>  }
>  
> -- 
> 2.35.1
> 
>


Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v4 5/6] hw/mips: use qemu_fdt_setprop_strings()

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:39PM +0100, Ben Dooks wrote:
> Change to using qemu_fdt_setprop_strings() helper in hw/mips.
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/mips/boston.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index d2ab9da1a0..759f6daafe 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
>      MachineState *mc = s->mach;
>      uint32_t platreg_ph, gic_ph, clk_ph;
>      char *name, *gic_name, *platreg_name, *stdout_name;
> -    static const char * const syscon_compat[2] = {
> -        "img,boston-platform-regs", "syscon"
> -    };
>  
>      fdt = create_device_tree(dt_size);
>      if (!fdt) {
> @@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
>      platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
>                                     memmap[BOSTON_PLATREG].base);
>      qemu_fdt_add_subnode(fdt, platreg_name);
> -    qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
> -                                 (char **)&syscon_compat,
> -                                 ARRAY_SIZE(syscon_compat));
> +    qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
> +                             "img,boston-platform-regs", "syscon");
>      qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
>                             memmap[BOSTON_PLATREG].base,
>                             memmap[BOSTON_PLATREG].size);
> -- 
> 2.35.1
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v4 6/6] hw/arm: change to use qemu_fdt_setprop_strings()

[Andrew Jones]

On Tue, Aug 09, 2022 at 07:56:40PM +0100, Ben Dooks wrote:
> Change to using qemu_fdt_setprop_strings() instead of using
> \0 separated string arrays. Note, also there were a few places
> where qemu_fdt_setprop_string() can be used in the same areas.
> 
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
> v4:
>  - fixed checkpatch errors with string
>  - fixed patch subject
> ---
>  hw/arm/boot.c             |  8 +++---
>  hw/arm/virt.c             | 28 +++++++++------------
>  hw/arm/xlnx-versal-virt.c | 51 ++++++++++++++++-----------------------
>  3 files changed, 37 insertions(+), 50 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH v4 2/6] hw/core: don't check return on qemu_fdt_setprop_string_array()

[Alistair Francis]

On Wed, Aug 10, 2022 at 5:08 AM Ben Dooks <qemu@ben.fluff.org> wrote:
>
> The qemu_fdt_setprop_string_array() does not return error codes and
> will call exit() if any of the fdt calls fails (and should print an
> error with the node being altered). This is done to prepare for the
> change for qemu_fdt_setprop_strings() helper which does not return
> any error codes (hw/core/guest-loader.c is the only place where an
> return is checked).
>
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/guest-loader.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> index 391c875a29..c61ebc4144 100644
> --- a/hw/core/guest-loader.c
> +++ b/hw/core/guest-loader.c
> @@ -57,25 +57,17 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
>
>      if (s->kernel) {
>          const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> -        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                          (char **) &compat,
> -                                          ARRAY_SIZE(compat)) < 0) {
> -            error_setg(errp, "couldn't set %s/compatible", node);
> -            return;
> -        }
> +        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                      (char **) &compat,
> +                                      ARRAY_SIZE(compat));
>          if (s->args) {
> -            if (qemu_fdt_setprop_string(fdt, node, "bootargs", s->args) < 0) {
> -                error_setg(errp, "couldn't set %s/bootargs", node);
> -            }
> +            qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
>          }
>      } else if (s->initrd) {
>          const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
> -        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                          (char **) &compat,
> -                                          ARRAY_SIZE(compat)) < 0) {
> -            error_setg(errp, "couldn't set %s/compatible", node);
> -            return;
> -        }
> +        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> +                                      (char **) &compat,
> +                                      ARRAY_SIZE(compat));
>      }
>  }
>
> --
> 2.35.1
>
>

Re: [PATCH v4 4/6] hw/core: use qemu_fdt_setprop_strings()

[Alistair Francis]

On Wed, Aug 10, 2022 at 5:13 AM Ben Dooks <qemu@ben.fluff.org> wrote:
>
> Change to using the qemu_fdt_setprop_strings() helper in
> hw/core code.
>
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/guest-loader.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
> index c61ebc4144..7b8e32e06f 100644
> --- a/hw/core/guest-loader.c
> +++ b/hw/core/guest-loader.c
> @@ -56,18 +56,15 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
>      qemu_fdt_setprop(fdt, node, "reg", &reg_attr, sizeof(reg_attr));
>
>      if (s->kernel) {
> -        const char *compat[2] = { "multiboot,module", "multiboot,kernel" };
> -        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                      (char **) &compat,
> -                                      ARRAY_SIZE(compat));
> +        qemu_fdt_setprop_strings(fdt, node, "compatible",
> +                                 "multiboot,module", "multiboot,kernel");
> +
>          if (s->args) {
>              qemu_fdt_setprop_string(fdt, node, "bootargs", s->args);
>          }
>      } else if (s->initrd) {
> -        const char *compat[2] = { "multiboot,module", "multiboot,ramdisk" };
> -        qemu_fdt_setprop_string_array(fdt, node, "compatible",
> -                                      (char **) &compat,
> -                                      ARRAY_SIZE(compat));
> +        qemu_fdt_setprop_strings(fdt, node, "compatible",
> +                                 "multiboot,module", "multiboot,ramdisk");
>      }
>  }
>
> --
> 2.35.1
>
>

Re: [PATCH v4 3/6] hw/riscv: use qemu_fdt_setprop_strings() for string arrays

[Alistair Francis]

On Wed, Aug 10, 2022 at 4:58 AM Ben Dooks <qemu@ben.fluff.org> wrote:
>
> Use the qemu_fdt_setprop_strings() in sifve_u.c to simplify the code.
>
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/sifive_u.c | 18 +++++-------------
>  hw/riscv/spike.c    |  7 ++-----
>  hw/riscv/virt.c     | 32 ++++++++------------------------
>  3 files changed, 15 insertions(+), 42 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index e4c814a3ea..dc112a253a 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -103,13 +103,6 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      char *nodename;
>      uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
>      uint32_t hfclk_phandle, rtcclk_phandle, phy_phandle;
> -    static const char * const ethclk_names[2] = { "pclk", "hclk" };
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
> -    static const char * const plic_compat[2] = {
> -        "sifive,plic-1.0.0", "riscv,plic0"
> -    };
>
>      if (ms->dtb) {
>          fdt = s->fdt = load_device_tree(ms->dtb, &s->fdt_size);
> @@ -221,11 +214,11 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      nodename = g_strdup_printf("/soc/clint@%lx",
>          (long)memmap[SIFIVE_U_DEV_CLINT].base);
>      qemu_fdt_add_subnode(fdt, nodename);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -        (char **)&clint_compat, ARRAY_SIZE(clint_compat));
>      qemu_fdt_setprop_cells(fdt, nodename, "reg",
>          0x0, memmap[SIFIVE_U_DEV_CLINT].base,
>          0x0, memmap[SIFIVE_U_DEV_CLINT].size);
> +    qemu_fdt_setprop_strings(fdt, nodename, "compatible",
> +                             "sifive,clint0", "riscv,clint0");
>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
>          cells, ms->smp.cpus * sizeof(uint32_t) * 4);
>      g_free(cells);
> @@ -279,8 +272,8 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>          (long)memmap[SIFIVE_U_DEV_PLIC].base);
>      qemu_fdt_add_subnode(fdt, nodename);
>      qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 1);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "compatible",
> -        (char **)&plic_compat, ARRAY_SIZE(plic_compat));
> +    qemu_fdt_setprop_strings(fdt, nodename, "compatbile",
> +                             "sifive,plic-1.0.0", "riscv,plic0");
>      qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(fdt, nodename, "interrupts-extended",
>          cells, (ms->smp.cpus * 4 - 2) * sizeof(uint32_t));
> @@ -426,8 +419,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry *memmap,
>      qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_GEM_IRQ);
>      qemu_fdt_setprop_cells(fdt, nodename, "clocks",
>          prci_phandle, PRCI_CLK_GEMGXLPLL, prci_phandle, PRCI_CLK_GEMGXLPLL);
> -    qemu_fdt_setprop_string_array(fdt, nodename, "clock-names",
> -        (char **)&ethclk_names, ARRAY_SIZE(ethclk_names));
> +    qemu_fdt_setprop_strings(fdt, nodename, "clock-names", "pclk", "hclk");
>      qemu_fdt_setprop(fdt, nodename, "local-mac-address",
>          s->soc.gem.conf.macaddr.a, ETH_ALEN);
>      qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index e41b6aa9f0..aa895779cd 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -59,9 +59,6 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>      uint32_t cpu_phandle, intc_phandle, phandle = 1;
>      char *name, *mem_name, *clint_name, *clust_name;
>      char *core_name, *cpu_name, *intc_name;
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
>
>      fdt = s->fdt = create_device_tree(&s->fdt_size);
>      if (!fdt) {
> @@ -159,8 +156,8 @@ static void create_fdt(SpikeState *s, const MemMapEntry *memmap,
>              (memmap[SPIKE_CLINT].size * socket);
>          clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
>          qemu_fdt_add_subnode(fdt, clint_name);
> -        qemu_fdt_setprop_string_array(fdt, clint_name, "compatible",
> -            (char **)&clint_compat, ARRAY_SIZE(clint_compat));
> +        qemu_fdt_setprop_strings(fdt, clint_name, "compatible",
> +                                 "sifive,clint0", "riscv,clint0");
>          qemu_fdt_setprop_cells(fdt, clint_name, "reg",
>              0x0, clint_addr, 0x0, memmap[SPIKE_CLINT].size);
>          qemu_fdt_setprop(fdt, clint_name, "interrupts-extended",
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..c6aaa611a6 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -261,11 +261,8 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket,
>              intc_phandles[cpu]);
>          if (riscv_feature(&s->soc[socket].harts[cpu].env,
>                            RISCV_FEATURE_AIA)) {
> -            static const char * const compat[2] = {
> -                "riscv,cpu-intc-aia", "riscv,cpu-intc"
> -            };
> -            qemu_fdt_setprop_string_array(mc->fdt, intc_name, "compatible",
> -                                      (char **)&compat, ARRAY_SIZE(compat));
> +            qemu_fdt_setprop_strings(mc->fdt, intc_name, "compatible",
> +                                     "riscv,cpu-intc-aia", "riscv,cpu-intc");
>          } else {
>              qemu_fdt_setprop_string(mc->fdt, intc_name, "compatible",
>                  "riscv,cpu-intc");
> @@ -310,9 +307,6 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
>      uint32_t *clint_cells;
>      unsigned long clint_addr;
>      MachineState *mc = MACHINE(s);
> -    static const char * const clint_compat[2] = {
> -        "sifive,clint0", "riscv,clint0"
> -    };
>
>      clint_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
>
> @@ -326,9 +320,8 @@ static void create_fdt_socket_clint(RISCVVirtState *s,
>      clint_addr = memmap[VIRT_CLINT].base + (memmap[VIRT_CLINT].size * socket);
>      clint_name = g_strdup_printf("/soc/clint@%lx", clint_addr);
>      qemu_fdt_add_subnode(mc->fdt, clint_name);
> -    qemu_fdt_setprop_string_array(mc->fdt, clint_name, "compatible",
> -                                  (char **)&clint_compat,
> -                                  ARRAY_SIZE(clint_compat));
> +    qemu_fdt_setprop_strings(mc->fdt, clint_name, "compatible",
> +                             "sifive,clint0", "riscv,clint0");
>      qemu_fdt_setprop_cells(mc->fdt, clint_name, "reg",
>          0x0, clint_addr, 0x0, memmap[VIRT_CLINT].size);
>      qemu_fdt_setprop(mc->fdt, clint_name, "interrupts-extended",
> @@ -437,9 +430,6 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      uint32_t *plic_cells;
>      unsigned long plic_addr;
>      MachineState *mc = MACHINE(s);
> -    static const char * const plic_compat[2] = {
> -        "sifive,plic-1.0.0", "riscv,plic0"
> -    };
>
>      if (kvm_enabled()) {
>          plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 2);
> @@ -465,9 +455,8 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
>      qemu_fdt_add_subnode(mc->fdt, plic_name);
>      qemu_fdt_setprop_cell(mc->fdt, plic_name,
>          "#interrupt-cells", FDT_PLIC_INT_CELLS);
> -    qemu_fdt_setprop_string_array(mc->fdt, plic_name, "compatible",
> -                                  (char **)&plic_compat,
> -                                  ARRAY_SIZE(plic_compat));
> +    qemu_fdt_setprop_strings(mc->fdt, plic_name, "compatible",
> +                             "sifive,plic-1.0.0", "riscv,plic0");
>      qemu_fdt_setprop(mc->fdt, plic_name, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(mc->fdt, plic_name, "interrupts-extended",
>          plic_cells, s->soc[socket].num_harts * sizeof(uint32_t) * 4);
> @@ -881,13 +870,8 @@ static void create_fdt_reset(RISCVVirtState *s, const MemMapEntry *memmap,
>      name = g_strdup_printf("/soc/test@%lx",
>          (long)memmap[VIRT_TEST].base);
>      qemu_fdt_add_subnode(mc->fdt, name);
> -    {
> -        static const char * const compat[3] = {
> -            "sifive,test1", "sifive,test0", "syscon"
> -        };
> -        qemu_fdt_setprop_string_array(mc->fdt, name, "compatible",
> -                                      (char **)&compat, ARRAY_SIZE(compat));
> -    }
> +    qemu_fdt_setprop_strings(mc->fdt, name, "compatible",
> +                             "sifive,test1", "sifive,test0", "syscon");
>      qemu_fdt_setprop_cells(mc->fdt, name, "reg",
>          0x0, memmap[VIRT_TEST].base, 0x0, memmap[VIRT_TEST].size);
>      qemu_fdt_setprop_cell(mc->fdt, name, "phandle", test_phandle);
> --
> 2.35.1
>
>

Re: [PATCH v4 5/6] hw/mips: use qemu_fdt_setprop_strings()

[Alistair Francis]

On Wed, Aug 10, 2022 at 4:58 AM Ben Dooks <qemu@ben.fluff.org> wrote:
>
> Change to using qemu_fdt_setprop_strings() helper in hw/mips.
>
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/mips/boston.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index d2ab9da1a0..759f6daafe 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -515,9 +515,6 @@ static const void *create_fdt(BostonState *s,
>      MachineState *mc = s->mach;
>      uint32_t platreg_ph, gic_ph, clk_ph;
>      char *name, *gic_name, *platreg_name, *stdout_name;
> -    static const char * const syscon_compat[2] = {
> -        "img,boston-platform-regs", "syscon"
> -    };
>
>      fdt = create_device_tree(dt_size);
>      if (!fdt) {
> @@ -608,9 +605,8 @@ static const void *create_fdt(BostonState *s,
>      platreg_name = g_strdup_printf("/soc/system-controller@%" HWADDR_PRIx,
>                                     memmap[BOSTON_PLATREG].base);
>      qemu_fdt_add_subnode(fdt, platreg_name);
> -    qemu_fdt_setprop_string_array(fdt, platreg_name, "compatible",
> -                                 (char **)&syscon_compat,
> -                                 ARRAY_SIZE(syscon_compat));
> +    qemu_fdt_setprop_strings(fdt, platreg_name, "compatible",
> +                             "img,boston-platform-regs", "syscon");
>      qemu_fdt_setprop_cells(fdt, platreg_name, "reg",
>                             memmap[BOSTON_PLATREG].base,
>                             memmap[BOSTON_PLATREG].size);
> --
> 2.35.1
>
>

Re: add qemu_fdt_setprop_strings() and use it in most places

[Peter Maydell]

On Tue, 9 Aug 2022 at 19:57, Ben Dooks <qemu@ben.fluff.org> wrote:
>
> Add a helper for qemu_fdt_setprop_strings() to take a set of strings
> to put into a device-tree, which removes several open-coded methods
> such as setting an char arr[] = {..} or setting char val[] = "str\0str2";
>
> This is for hw/arm, hw/mips and hw/riscv as well as a couple of cores. It
> is not fully tested over all of those, I may not have caught all places
> this is to be replaced.

Hi; just as a minor process thing for next time you send a
patchset: the usual convention is that the 'cover letter'
email has a subject starting with a tag like "[PATCH v4 0/6]".
This makes it easier to pick the cover letter out when eyeballing
a set of subject lines, and also helps some of the automated
tooling (eg I think this is why https://patchew.org/QEMU/ didn't
notice this series). Usually 'git format-patch --cover-letter'
will get this right automatically for you.

thanks
-- PMM