All of lore.kernel.org
 help / color / mirror / Atom feed
* add qemu_fdt_setprop_strings() and use it in most places
@ 2022-07-27 22:39 Ben Dooks
  2022-07-27 22:39 ` [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper Ben Dooks
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm

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.




^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
@ 2022-07-27 22:39 ` Ben Dooks
  2022-07-28  9:22   ` Andrew Jones
  2022-07-27 22:39 ` [PATCH v3 2/5] hw/riscv: use qemu_fdt_setprop_strings() for string arrays Ben Dooks
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm, 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>
---
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..83bdfe390e 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, ...)          \
+    ({ int __ret; do {                                          \
+        static const char * const __strs[] = { __VA_ARGS__ };   \
+        __ret = qemu_fdt_setprop_string_array(fdt, path, prop,  \
+                (char **)&__strs, ARRAY_SIZE(__strs));          \
+     } while(0); __ret; })
+
+
 int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
                              const char *property,
                              const char *target_node_path);
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 2/5] hw/riscv: use qemu_fdt_setprop_strings() for string arrays
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
  2022-07-27 22:39 ` [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper Ben Dooks
@ 2022-07-27 22:39 ` Ben Dooks
  2022-07-27 22:39 ` [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings() Ben Dooks
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm, 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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
  2022-07-27 22:39 ` [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper Ben Dooks
  2022-07-27 22:39 ` [PATCH v3 2/5] hw/riscv: use qemu_fdt_setprop_strings() for string arrays Ben Dooks
@ 2022-07-27 22:39 ` Ben Dooks
  2022-08-01 11:30   ` Peter Maydell
  2022-07-27 22:39 ` [PATCH v3 4/5] hw/mips: " Ben Dooks
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm, 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 | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/core/guest-loader.c b/hw/core/guest-loader.c
index 391c875a29..203090503e 100644
--- a/hw/core/guest-loader.c
+++ b/hw/core/guest-loader.c
@@ -56,10 +56,8 @@ 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" };
-        if (qemu_fdt_setprop_string_array(fdt, node, "compatible",
-                                          (char **) &compat,
-                                          ARRAY_SIZE(compat)) < 0) {
+        if (qemu_fdt_setprop_strings(fdt, node, "compatible",
+                                     "multiboot,module", "multiboot,kernel") < 0) {
             error_setg(errp, "couldn't set %s/compatible", node);
             return;
         }
@@ -69,10 +67,8 @@ static void loader_insert_platform_data(GuestLoaderState *s, int size,
             }
         }
     } 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) {
+        if (qemu_fdt_setprop_strings(fdt, node, "compatible",
+                                     "multiboot,module", "multiboot,ramdisk") < 0) {
             error_setg(errp, "couldn't set %s/compatible", node);
             return;
         }
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 4/5] hw/mips: use qemu_fdt_setprop_strings()
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
                   ` (2 preceding siblings ...)
  2022-07-27 22:39 ` [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings() Ben Dooks
@ 2022-07-27 22:39 ` Ben Dooks
  2022-08-01 11:30   ` Peter Maydell
  2022-07-27 22:39 ` [PATCH v3 5/5] hw/arm: change to " Ben Dooks
  2022-07-27 22:48 ` add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
  5 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm, Ben Dooks

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

Signed-off-by: Ben Dooks <qemu@ben.fluff.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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
                   ` (3 preceding siblings ...)
  2022-07-27 22:39 ` [PATCH v3 4/5] hw/mips: " Ben Dooks
@ 2022-07-27 22:39 ` Ben Dooks
  2022-08-01 11:37   ` Peter Maydell
  2022-07-27 22:48 ` add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
  5 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Alistair Francis, qemu-arm, Ben Dooks

Change to using qemu_fdt_setprop_strings() instead of using
\0 separated string arrays.

Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
---
 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..bf29b7ae60 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..2cfc11f0b2 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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: add qemu_fdt_setprop_strings() and use it in most places
  2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
                   ` (4 preceding siblings ...)
  2022-07-27 22:39 ` [PATCH v3 5/5] hw/arm: change to " Ben Dooks
@ 2022-07-27 22:48 ` Ben Dooks
  5 siblings, 0 replies; 18+ messages in thread
From: Ben Dooks @ 2022-07-27 22:48 UTC (permalink / raw)
  To: Ben Dooks; +Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Wed, Jul 27, 2022 at 11:39:00PM +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 added a branch at
     https://github.com/bendooks/qemu.git dev/fdt-string-array

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

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
  2022-07-27 22:39 ` [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper Ben Dooks
@ 2022-07-28  9:22   ` Andrew Jones
  2022-07-28 10:41     ` Ben Dooks
  2022-08-10 12:22     ` Andrew Jones
  0 siblings, 2 replies; 18+ messages in thread
From: Andrew Jones @ 2022-07-28  9:22 UTC (permalink / raw)
  To: Ben Dooks; +Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Wed, Jul 27, 2022 at 11:39:01PM +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>
> ---
> 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..83bdfe390e 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, ...)          \
> +    ({ int __ret; do {                                          \
> +        static const char * const __strs[] = { __VA_ARGS__ };   \
> +        __ret = qemu_fdt_setprop_string_array(fdt, path, prop,  \
> +                (char **)&__strs, ARRAY_SIZE(__strs));          \
> +     } while(0); __ret; })

The do { } while (0) shouldn't be necessary inside the ({}), but I
think we should change the places that are checking the return value
of qemu_fdt_setprop_string_array() to not check the return value,
because it will always be zero. qemu_fdt_setprop_string_array() calls
qemu_fdt_setprop() which exits QEMU on error. Then, after there are
no callers checking the return value we can change this macro to

#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)


Thanks,
drew


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
  2022-07-28  9:22   ` Andrew Jones
@ 2022-07-28 10:41     ` Ben Dooks
  2022-08-10 12:22     ` Andrew Jones
  1 sibling, 0 replies; 18+ messages in thread
From: Ben Dooks @ 2022-07-28 10:41 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Thu, Jul 28, 2022 at 11:22:27AM +0200, Andrew Jones wrote:
> On Wed, Jul 27, 2022 at 11:39:01PM +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>
> > ---
> > 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..83bdfe390e 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, ...)          \
> > +    ({ int __ret; do {                                          \
> > +        static const char * const __strs[] = { __VA_ARGS__ };   \
> > +        __ret = qemu_fdt_setprop_string_array(fdt, path, prop,  \
> > +                (char **)&__strs, ARRAY_SIZE(__strs));          \
> > +     } while(0); __ret; })
> 
> The do { } while (0) shouldn't be necessary inside the ({}), but I
> think we should change the places that are checking the return value
> of qemu_fdt_setprop_string_array() to not check the return value,
> because it will always be zero. qemu_fdt_setprop_string_array() calls
> qemu_fdt_setprop() which exits QEMU on error. Then, after there are
> no callers checking the return value we can change this macro to

I think I did this due to the hw/ppc changes that /do/ check the
return code but are different enough to not be trivially changable.

I'll go back and make this the original do {} while() tongith and
post a v4 for people to look. The hw/ppc stuff can stay as is for
now.

Thank you for the review.

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

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
  2022-07-27 22:39 ` [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings() Ben Dooks
@ 2022-08-01 11:30   ` Peter Maydell
  2022-08-09 18:50     ` Ben Dooks
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2022-08-01 11:30 UTC (permalink / raw)
  To: Ben Dooks; +Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Wed, 27 Jul 2022 at 23:39, 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: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 4/5] hw/mips: use qemu_fdt_setprop_strings()
  2022-07-27 22:39 ` [PATCH v3 4/5] hw/mips: " Ben Dooks
@ 2022-08-01 11:30   ` Peter Maydell
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2022-08-01 11:30 UTC (permalink / raw)
  To: Ben Dooks; +Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Wed, 27 Jul 2022 at 23:40, 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>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
  2022-07-27 22:39 ` [PATCH v3 5/5] hw/arm: change to " Ben Dooks
@ 2022-08-01 11:37   ` Peter Maydell
  2022-08-09 18:48     ` Ben Dooks
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2022-08-01 11:37 UTC (permalink / raw)
  To: Ben Dooks
  Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm, Edgar E. Iglesias

On Wed, 27 Jul 2022 at 23:44, Ben Dooks <qemu@ben.fluff.org> wrote:
>
> Change to using qemu_fdt_setprop_strings() instead of using
> \0 separated string arrays.
>
> Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> ---
>  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..bf29b7ae60 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");

I think you may have some stray trailing whitespace here.
checkpatch should be able to tell you.

> @@ -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 */

With this change, this comment becomes obsolete, and we should delete it too.

> -    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",



> @@ -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";

This looks suspiciously like a pre-existing bug to me.
Alaistair, Edgar -- shouldn't this be a NUL-separated
'compatible' string, rather than a comma-separated one?

>      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";

Ditto here...

>      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";

...and here.

> -    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);
>  }

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
  2022-08-01 11:37   ` Peter Maydell
@ 2022-08-09 18:48     ` Ben Dooks
  2022-08-09 19:22       ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-08-09 18:48 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm,
	Edgar E. Iglesias

On Mon, Aug 01, 2022 at 12:37:33PM +0100, Peter Maydell wrote:
> On Wed, 27 Jul 2022 at 23:44, Ben Dooks <qemu@ben.fluff.org> wrote:
> >
> > Change to using qemu_fdt_setprop_strings() instead of using
> > \0 separated string arrays.
> >
> > Signed-off-by: Ben Dooks <qemu@ben.fluff.org>
> > ---
> >  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..bf29b7ae60 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");
> 
> I think you may have some stray trailing whitespace here.
> checkpatch should be able to tell you.

ok, will check

> > @@ -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 */
> 
> With this change, this comment becomes obsolete, and we should delete it too.
> 
> > -    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",
> 
> 
> 
> > @@ -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";
> 
> This looks suspiciously like a pre-existing bug to me.
> Alaistair, Edgar -- shouldn't this be a NUL-separated
> 'compatible' string, rather than a comma-separated one?

I think the compat[] is fine, I should have probably added I also fixed
up to just call qemu_fdt_setprop_string()

> >      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";
> 
> Ditto here...
> 
> >      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";
> 
> ...and here.
> 
> > -    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);
> >  }
> 
> thanks
> -- PMM
> 

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

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
  2022-08-01 11:30   ` Peter Maydell
@ 2022-08-09 18:50     ` Ben Dooks
  2022-08-09 20:49       ` Peter Maydell
  0 siblings, 1 reply; 18+ messages in thread
From: Ben Dooks @ 2022-08-09 18:50 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Mon, Aug 01, 2022 at 12:30:22PM +0100, Peter Maydell wrote:
> On Wed, 27 Jul 2022 at 23:39, 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: Peter Maydell <peter.maydell@linaro.org>

I've had to make a second version, so assuming it'll need
reviewing again

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

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
  2022-08-09 18:48     ` Ben Dooks
@ 2022-08-09 19:22       ` Peter Maydell
  2022-08-09 19:32         ` Mark Cave-Ayland
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Maydell @ 2022-08-09 19:22 UTC (permalink / raw)
  To: Ben Dooks
  Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm,
	Edgar E. Iglesias

On Tue, 9 Aug 2022 at 19:48, Ben Dooks <ben@fluff.org> wrote:
>
> On Mon, Aug 01, 2022 at 12:37:33PM +0100, Peter Maydell wrote:
> > On Wed, 27 Jul 2022 at 23:44, Ben Dooks <qemu@ben.fluff.org> wrote:
> > > @@ -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";
> >
> > This looks suspiciously like a pre-existing bug to me.
> > Alaistair, Edgar -- shouldn't this be a NUL-separated
> > 'compatible' string, rather than a comma-separated one?
>
> I think the compat[] is fine, I should have probably added I also fixed
> up to just call qemu_fdt_setprop_string()

I guess if it's definitely supposed to be one string instead of two that's OK.

-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 5/5] hw/arm: change to use qemu_fdt_setprop_strings()
  2022-08-09 19:22       ` Peter Maydell
@ 2022-08-09 19:32         ` Mark Cave-Ayland
  0 siblings, 0 replies; 18+ messages in thread
From: Mark Cave-Ayland @ 2022-08-09 19:32 UTC (permalink / raw)
  To: Peter Maydell, Ben Dooks
  Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm,
	Edgar E. Iglesias

On 09/08/2022 20:22, Peter Maydell wrote:

> On Tue, 9 Aug 2022 at 19:48, Ben Dooks <ben@fluff.org> wrote:
>>
>> On Mon, Aug 01, 2022 at 12:37:33PM +0100, Peter Maydell wrote:
>>> On Wed, 27 Jul 2022 at 23:44, Ben Dooks <qemu@ben.fluff.org> wrote:
>>>> @@ -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";
>>>
>>> This looks suspiciously like a pre-existing bug to me.
>>> Alaistair, Edgar -- shouldn't this be a NUL-separated
>>> 'compatible' string, rather than a comma-separated one?
>>
>> I think the compat[] is fine, I should have probably added I also fixed
>> up to just call qemu_fdt_setprop_string()
> 
> I guess if it's definitely supposed to be one string instead of two that's OK.

FWIW the compat strings look like they are using the older <manufacturer>,<device> 
notation similar to SUNW,tcx and SUNW,cgthree so if Xilinx is the manufacturer I 
think they are correct.


ATB,

Mark.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings()
  2022-08-09 18:50     ` Ben Dooks
@ 2022-08-09 20:49       ` Peter Maydell
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Maydell @ 2022-08-09 20:49 UTC (permalink / raw)
  To: Ben Dooks; +Cc: Ben Dooks, qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Tue, 9 Aug 2022 at 19:50, Ben Dooks <ben@fluff.org> wrote:
>
> On Mon, Aug 01, 2022 at 12:30:22PM +0100, Peter Maydell wrote:
> > On Wed, 27 Jul 2022 at 23:39, 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: Peter Maydell <peter.maydell@linaro.org>
>
> I've had to make a second version, so assuming it'll need
> reviewing again

If you didn't change the specific patch that I gave a reviewed-by
tag for, then you should put that tag into the commit message
next to your signed-off-by line in the patch you send in the v2
series, to indicate it's already been reviewed and doesn't need
re-doing in v2.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper
  2022-07-28  9:22   ` Andrew Jones
  2022-07-28 10:41     ` Ben Dooks
@ 2022-08-10 12:22     ` Andrew Jones
  1 sibling, 0 replies; 18+ messages in thread
From: Andrew Jones @ 2022-08-10 12:22 UTC (permalink / raw)
  To: Ben Dooks; +Cc: qemu-devel, qemu-riscv, Alistair Francis, qemu-arm

On Thu, Jul 28, 2022 at 11:22:27AM +0200, Andrew Jones wrote:
> On Wed, Jul 27, 2022 at 11:39:01PM +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>
> > ---
> > 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..83bdfe390e 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, ...)          \
> > +    ({ int __ret; do {                                          \
> > +        static const char * const __strs[] = { __VA_ARGS__ };   \
> > +        __ret = qemu_fdt_setprop_string_array(fdt, path, prop,  \
> > +                (char **)&__strs, ARRAY_SIZE(__strs));          \
> > +     } while(0); __ret; })
> 
> The do { } while (0) shouldn't be necessary inside the ({}), but I
> think we should change the places that are checking the return value
> of qemu_fdt_setprop_string_array() to not check the return value,
> because it will always be zero. qemu_fdt_setprop_string_array() calls
> qemu_fdt_setprop() which exits QEMU on error. Then, after there are
> no callers checking the return value we can change this macro to
> 
> #define qemu_fdt_setprop_strings(fdt, path, prop, ...)          \
>     do {                                                       \
>        static const char * const __strs[] = { __VA_ARGS__ };   \

Eh, I just realized I didn't notice the static the first time I read this
and so I copy+pasted here in my suggestion. Sorry about that.

Thanks,
drew


>        qemu_fdt_setprop_string_array(fdt, path, prop,          \
>                (char **)&__strs, ARRAY_SIZE(__strs));          \
>      } while(0)
> 
> 
> Thanks,
> drew


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-08-10 12:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-27 22:39 add qemu_fdt_setprop_strings() and use it in most places Ben Dooks
2022-07-27 22:39 ` [PATCH v3 1/5] device_tree: add qemu_fdt_setprop_strings() helper Ben Dooks
2022-07-28  9:22   ` Andrew Jones
2022-07-28 10:41     ` Ben Dooks
2022-08-10 12:22     ` Andrew Jones
2022-07-27 22:39 ` [PATCH v3 2/5] hw/riscv: use qemu_fdt_setprop_strings() for string arrays Ben Dooks
2022-07-27 22:39 ` [PATCH v3 3/5] hw/core: use qemu_fdt_setprop_strings() Ben Dooks
2022-08-01 11:30   ` Peter Maydell
2022-08-09 18:50     ` Ben Dooks
2022-08-09 20:49       ` Peter Maydell
2022-07-27 22:39 ` [PATCH v3 4/5] hw/mips: " Ben Dooks
2022-08-01 11:30   ` Peter Maydell
2022-07-27 22:39 ` [PATCH v3 5/5] hw/arm: change to " Ben Dooks
2022-08-01 11:37   ` Peter Maydell
2022-08-09 18:48     ` Ben Dooks
2022-08-09 19:22       ` Peter Maydell
2022-08-09 19:32         ` Mark Cave-Ayland
2022-07-27 22:48 ` add qemu_fdt_setprop_strings() and use it in most places Ben Dooks

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.