On Fri, Sep 12, 2025 at 12:00:11PM +0200, Luc Michel wrote: > The following commits will move FDT creation logic from the > xlnx-versal-virt machine to the xlnx-versal SoC itself. Prepare this by > passing the FDT handle to the SoC before it is realized. If no FDT is > passed, a dummy one is created internally as a stub to the fdt function > calls. > > For now the SoC only creates the two clock nodes. The ones from the > xlnx-versal virt machine are renamed with a `old-' prefix and will be > removed once they are not referenced anymore.
Hi Luc, > > Signed-off-by: Luc Michel <[email protected]> > Reviewed-by: Francisco Iglesias <[email protected]> > --- > include/hw/arm/xlnx-versal.h | 12 ++++++++++++ > hw/arm/xlnx-versal-virt.c | 9 ++++++--- > hw/arm/xlnx-versal.c | 28 ++++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+), 3 deletions(-) > > diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h > index 1f92e314d6c..f2a62b43552 100644 > --- a/include/hw/arm/xlnx-versal.h > +++ b/include/hw/arm/xlnx-versal.h > @@ -134,21 +134,33 @@ struct Versal { > XlnxVersalCFrameBcastReg cframe_bcast; > > OrIRQState apb_irq_orgate; > } pmc; > > + struct { > + uint32_t clk_25mhz; > + uint32_t clk_125mhz; > + } phandle; > + > struct { > MemoryRegion *mr_ddr; > + void *fdt; > } cfg; > }; > > struct VersalClass { > SysBusDeviceClass parent; > > VersalVersion version; > }; > > +static inline void versal_set_fdt(Versal *s, void *fdt) > +{ > + g_assert(!qdev_is_realized(DEVICE(s))); > + s->cfg.fdt = fdt; > +} > + Should this be a property of some sort? it looks a little odd to bypass QOM.. > /* Memory-map and IRQ definitions. Copied a subset from > * auto-generated files. */ > > #define VERSAL_GIC_MAINT_IRQ 9 > #define VERSAL_TIMER_VIRT_IRQ 11 > diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c > index adadbb72902..d1c65afa2ac 100644 > --- a/hw/arm/xlnx-versal-virt.c > +++ b/hw/arm/xlnx-versal-virt.c > @@ -1,9 +1,10 @@ > /* > * Xilinx Versal Virtual board. > * > * Copyright (c) 2018 Xilinx Inc. > + * Copyright (c) 2025 Advanced Micro Devices, Inc. > * Written by Edgar E. Iglesias > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 or > * (at your option) any later version. > @@ -695,14 +696,16 @@ static void versal_virt_init(MachineState *machine) > &error_abort); > object_property_set_link(OBJECT(&s->soc), "canbus0", > OBJECT(s->canbus[0]), > &error_abort); > object_property_set_link(OBJECT(&s->soc), "canbus1", > OBJECT(s->canbus[1]), > &error_abort); > - sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal); > > fdt_create(s); > + versal_set_fdt(&s->soc, s->fdt); > + sysbus_realize(SYS_BUS_DEVICE(&s->soc), &error_fatal); > create_virtio_regions(s); > + > fdt_add_gem_nodes(s); > fdt_add_uart_nodes(s); > fdt_add_canfd_nodes(s); > fdt_add_gic_nodes(s); > fdt_add_timer_nodes(s); > @@ -712,12 +715,12 @@ static void versal_virt_init(MachineState *machine) > fdt_add_rtc_node(s); > fdt_add_bbram_node(s); > fdt_add_efuse_ctrl_node(s); > fdt_add_efuse_cache_node(s); > fdt_add_cpu_nodes(s, psci_conduit); > - fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz); > - fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz); > + fdt_add_clk_node(s, "/old-clk125", 125000000, s->phandle.clk_125Mhz); > + fdt_add_clk_node(s, "/old-clk25", 25000000, s->phandle.clk_25Mhz); > > /* Make the APU cpu address space visible to virtio and other > * modules unaware of multiple address-spaces. */ > memory_region_add_subregion_overlap(get_system_memory(), > 0, &s->soc.fpd.apu.mr, 0); > diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c > index 4da656318f6..fda8fdf786a 100644 > --- a/hw/arm/xlnx-versal.c > +++ b/hw/arm/xlnx-versal.c > @@ -22,10 +22,12 @@ > #include "hw/misc/unimp.h" > #include "hw/arm/xlnx-versal.h" > #include "qemu/log.h" > #include "target/arm/cpu-qom.h" > #include "target/arm/gtimer.h" > +#include "system/device_tree.h" > +#include "hw/arm/fdt.h" > > #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72") > #define XLNX_VERSAL_RCPU_TYPE ARM_CPU_TYPE_NAME("cortex-r5f") > #define GEM_REVISION 0x40070106 > > @@ -917,15 +919,41 @@ static void versal_unimp(Versal *s) > qdev_connect_gpio_out_named(DEVICE(&s->pmc.iou.slcr), > SYSBUS_DEVICE_GPIO_IRQ, 0, > gpio_in); > } > > +static uint32_t fdt_add_clk_node(Versal *s, const char *name, > + unsigned int freq_hz) > +{ > + uint32_t phandle; > + > + phandle = qemu_fdt_alloc_phandle(s->cfg.fdt); > + > + qemu_fdt_add_subnode(s->cfg.fdt, name); > + qemu_fdt_setprop_cell(s->cfg.fdt, name, "phandle", phandle); > + qemu_fdt_setprop_cell(s->cfg.fdt, name, "clock-frequency", freq_hz); > + qemu_fdt_setprop_cell(s->cfg.fdt, name, "#clock-cells", 0x0); > + qemu_fdt_setprop_string(s->cfg.fdt, name, "compatible", "fixed-clock"); > + qemu_fdt_setprop(s->cfg.fdt, name, "u-boot,dm-pre-reloc", NULL, 0); > + > + return phandle; > +} > + > static void versal_realize(DeviceState *dev, Error **errp) > { > Versal *s = XLNX_VERSAL_BASE(dev); > qemu_irq pic[XLNX_VERSAL_NR_IRQS]; > > + if (s->cfg.fdt == NULL) { > + int fdt_size; > + > + s->cfg.fdt = create_device_tree(&fdt_size); > + } > + > + s->phandle.clk_25mhz = fdt_add_clk_node(s, "/clk25", 25 * 1000 * 1000); > + s->phandle.clk_125mhz = fdt_add_clk_node(s, "/clk125", 125 * 1000 * > 1000); > + Should we be adding nodes if s->cfg.fdt wasn't created by us? If the user passes a dtb, I wonder if we should just assume the user knows what they are doing and use it as is... Or do you have use-cases where it makes sense? > versal_create_apu_cpus(s); > versal_create_apu_gic(s, pic); > versal_create_rpu_cpus(s); > versal_create_uarts(s, pic); > versal_create_canfds(s, pic); > -- > 2.50.1 >
