On Mon, Aug 01, 2022 at 12:37:33PM +0100, Peter Maydell wrote: > On Wed, 27 Jul 2022 at 23:44, Ben Dooks <q...@ben.fluff.org> wrote: > > > > Change to using qemu_fdt_setprop_strings() instead of using > > \0 separated string arrays. > > > > Signed-off-by: Ben Dooks <q...@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, b...@fluff.org, http://www.fluff.org/ben/ Large Hadron Colada: A large Pina Colada that makes the universe disappear.