On Wed, Jul 16, 2025 at 11:53:46AM +0200, Luc Michel wrote:
> Refactor the UARTs creations. The VersalMap struct is now used to
> describe the SoC and its peripherals. For now it contains the two UARTs
> mapping information. The creation function now embeds the FDT creation
> logic as well. The devices are now created dynamically using qdev_new
> and (qdev|sysbus)_realize_and_unref.
>
> This will allow to rely entirely on the VersalMap structure to create
> the SoC and allow easy addition of new SoCs of the same family (like
> versal2 coming with next commits).
>
> Note that the connection to the CRL is removed for now and will be
> re-added by next commits.
>
> Signed-off-by: Luc Michel <luc.mic...@amd.com>
Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com>
> ---
> include/hw/arm/xlnx-versal.h | 2 -
> hw/arm/xlnx-versal-virt.c | 36 +--------
> hw/arm/xlnx-versal.c | 142 ++++++++++++++++++++++++++++-------
> 3 files changed, 117 insertions(+), 63 deletions(-)
>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index f2a62b43552..b01ddeb1423 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -16,11 +16,10 @@
> #include "hw/sysbus.h"
> #include "hw/cpu/cluster.h"
> #include "hw/or-irq.h"
> #include "hw/sd/sdhci.h"
> #include "hw/intc/arm_gicv3.h"
> -#include "hw/char/pl011.h"
> #include "hw/dma/xlnx-zdma.h"
> #include "hw/net/cadence_gem.h"
> #include "hw/rtc/xlnx-zynqmp-rtc.h"
> #include "qom/object.h"
> #include "hw/usb/xlnx-usb-subsystem.h"
> @@ -78,11 +77,10 @@ struct Versal {
>
> struct {
> MemoryRegion mr_ocm;
>
> struct {
> - PL011State uart[XLNX_VERSAL_NR_UARTS];
> CadenceGEMState gem[XLNX_VERSAL_NR_GEMS];
> OrIRQState gem_irq_orgate[XLNX_VERSAL_NR_GEMS];
> XlnxZDMA adma[XLNX_VERSAL_NR_ADMAS];
> VersalUsb2 usb;
> CanBusState *canbus[XLNX_VERSAL_NR_CANFD];
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index c30dcca424c..7213ead466d 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -75,10 +75,11 @@ static void fdt_create(VersalVirt *s)
>
> s->phandle.usb = qemu_fdt_alloc_phandle(s->fdt);
> s->phandle.dwc = qemu_fdt_alloc_phandle(s->fdt);
> /* Create /chosen node for load_dtb. */
> qemu_fdt_add_subnode(s->fdt, "/chosen");
> + qemu_fdt_add_subnode(s->fdt, "/aliases");
>
> /* Header */
> qemu_fdt_setprop_cell(s->fdt, "/", "interrupt-parent", s->phandle.gic);
> qemu_fdt_setprop_cell(s->fdt, "/", "#size-cells", 0x2);
> qemu_fdt_setprop_cell(s->fdt, "/", "#address-cells", 0x2);
> @@ -206,44 +207,10 @@ static void fdt_add_usb_xhci_nodes(VersalVirt *s)
> qemu_fdt_setprop_cell(s->fdt, name, "phandle", s->phandle.dwc);
> qemu_fdt_setprop_string(s->fdt, name, "maximum-speed", "high-speed");
> g_free(name);
> }
>
> -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++) {
> - char *name = g_strdup_printf("/uart@%" PRIx64, addrs[i]);
> - qemu_fdt_add_subnode(s->fdt, name);
> - 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_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(s->fdt, name, "u-boot,dm-pre-reloc", NULL, 0);
> -
> - if (addrs[i] == MM_UART0) {
> - /* Select UART0. */
> - qemu_fdt_setprop_string(s->fdt, "/chosen", "stdout-path", name);
> - }
> - g_free(name);
> - }
> -}
> -
> static void fdt_add_canfd_nodes(VersalVirt *s)
> {
> uint64_t addrs[] = { MM_CANFD1, MM_CANFD0 };
> uint32_t size[] = { MM_CANFD1_SIZE, MM_CANFD0_SIZE };
> unsigned int irqs[] = { VERSAL_CANFD1_IRQ_0, VERSAL_CANFD0_IRQ_0 };
> @@ -700,11 +667,10 @@ static void versal_virt_init(MachineState *machine)
> &error_abort);
>
> fdt_create(s);
> versal_set_fdt(&s->soc, s->fdt);
> 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);
> fdt_add_zdma_nodes(s);
> fdt_add_usb_xhci_nodes(s);
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 7bb55751e5c..87468cbc291 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -24,18 +24,96 @@
> #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"
> +#include "hw/char/pl011.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
>
> #define VERSAL_NUM_PMC_APB_IRQS 18
> #define NUM_OSPI_IRQ_LINES 3
>
> +typedef struct VersalSimplePeriphMap {
> + uint64_t addr;
> + int irq;
> +} VersalSimplePeriphMap;
> +
> +typedef struct VersalMap {
> + VersalSimplePeriphMap uart[2];
> + size_t num_uart;
> +} VersalMap;
> +
> +static const VersalMap VERSAL_MAP = {
> + .uart[0] = { 0xff000000, 18 },
> + .uart[1] = { 0xff010000, 19 },
> + .num_uart = 2,
> +};
> +
> +static const VersalMap *VERSION_TO_MAP[] = {
> + [VERSAL_VER_VERSAL] = &VERSAL_MAP,
> +};
> +
> +static inline VersalVersion versal_get_version(Versal *s)
> +{
> + return XLNX_VERSAL_BASE_GET_CLASS(s)->version;
> +}
> +
> +static inline const VersalMap *versal_get_map(Versal *s)
> +{
> + return VERSION_TO_MAP[versal_get_version(s)];
> +}
> +
> +
> +static qemu_irq versal_get_irq(Versal *s, int irq_idx)
> +{
> + return qdev_get_gpio_in(DEVICE(&s->fpd.apu.gic), irq_idx);
> +}
> +
> +static void versal_sysbus_connect_irq(Versal *s, SysBusDevice *sbd,
> + int sbd_idx, int irq_idx)
> +{
> + qemu_irq irq = versal_get_irq(s, irq_idx);
> +
> + if (irq == NULL) {
> + return;
> + }
> +
> + sysbus_connect_irq(sbd, sbd_idx, irq);
> +}
> +
> +static inline char *versal_fdt_add_subnode(Versal *s, const char *path,
> + uint64_t at, const char *compat,
> + size_t compat_sz)
> +{
> + char *p;
> +
> + p = g_strdup_printf("%s@%" PRIx64, path, at);
> + qemu_fdt_add_subnode(s->cfg.fdt, p);
> +
> + if (!strncmp(compat, "memory", compat_sz)) {
> + qemu_fdt_setprop(s->cfg.fdt, p, "device_type", compat, compat_sz);
> + } else {
> + qemu_fdt_setprop(s->cfg.fdt, p, "compatible", compat, compat_sz);
> + }
> +
> + return p;
> +}
> +
> +static inline char *versal_fdt_add_simple_subnode(Versal *s, const char
> *path,
> + uint64_t addr, uint64_t
> len,
> + const char *compat,
> + size_t compat_sz)
> +{
> + char *p = versal_fdt_add_subnode(s, path, addr, compat, compat_sz);
> +
> + qemu_fdt_setprop_sized_cells(s->cfg.fdt, p, "reg", 2, addr, 2, len);
> + return p;
> +}
> +
> static void versal_create_apu_cpus(Versal *s)
> {
> int i;
>
> object_initialize_child(OBJECT(s), "apu-cluster", &s->fpd.apu.cluster,
> @@ -165,32 +243,48 @@ static void versal_create_rpu_cpus(Versal *s)
> }
>
> qdev_realize(DEVICE(&s->lpd.rpu.cluster), NULL, &error_fatal);
> }
>
> -static void versal_create_uarts(Versal *s, qemu_irq *pic)
> +static void versal_create_uart(Versal *s,
> + const VersalSimplePeriphMap *map,
> + int chardev_idx)
> {
> - int i;
> + DeviceState *dev;
> + MemoryRegion *mr;
> + g_autofree char *node;
> + g_autofree char *alias;
> + const char compatible[] = "arm,pl011\0arm,sbsa-uart";
> + const char clocknames[] = "uartclk\0apb_pclk";
>
> - for (i = 0; i < ARRAY_SIZE(s->lpd.iou.uart); i++) {
> - static const int irqs[] = { VERSAL_UART0_IRQ_0, VERSAL_UART1_IRQ_0};
> - static const uint64_t addrs[] = { MM_UART0, MM_UART1 };
> - char *name = g_strdup_printf("uart%d", i);
> - DeviceState *dev;
> - MemoryRegion *mr;
> + dev = qdev_new(TYPE_PL011);
> + object_property_add_child(OBJECT(s), "uart[*]", OBJECT(dev));
> + qdev_prop_set_chr(dev, "chardev", serial_hd(chardev_idx));
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> - object_initialize_child(OBJECT(s), name, &s->lpd.iou.uart[i],
> - TYPE_PL011);
> - dev = DEVICE(&s->lpd.iou.uart[i]);
> - qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> - sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> + memory_region_add_subregion(&s->mr_ps, map->addr, mr);
>
> - mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> - memory_region_add_subregion(&s->mr_ps, addrs[i], mr);
> + versal_sysbus_connect_irq(s, SYS_BUS_DEVICE(dev), 0, map->irq);
>
> - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irqs[i]]);
> - g_free(name);
> + node = versal_fdt_add_simple_subnode(s, "/uart", map->addr, 0x1000,
> + compatible, sizeof(compatible));
> + qemu_fdt_setprop_cell(s->cfg.fdt, node, "current-speed", 115200);
> + qemu_fdt_setprop_cells(s->cfg.fdt, node, "clocks",
> + s->phandle.clk_125mhz, s->phandle.clk_125mhz);
> + qemu_fdt_setprop(s->cfg.fdt, node, "clock-names", clocknames,
> + sizeof(clocknames));
> + qemu_fdt_setprop_cells(s->cfg.fdt, node, "interrupts",
> + GIC_FDT_IRQ_TYPE_SPI, map->irq,
> + GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> + qemu_fdt_setprop(s->cfg.fdt, node, "u-boot,dm-pre-reloc", NULL, 0);
> +
> + alias = g_strdup_printf("serial%d", chardev_idx);
> + qemu_fdt_setprop_string(s->cfg.fdt, "/aliases", alias, node);
> +
> + if (chardev_idx == 0) {
> + qemu_fdt_setprop_string(s->cfg.fdt, "/chosen", "stdout-path", node);
> }
> }
>
> static void versal_create_canfds(Versal *s, qemu_irq *pic)
> {
> @@ -781,18 +875,10 @@ static void versal_create_crl(Versal *s, qemu_irq *pic)
> object_property_set_link(OBJECT(&s->lpd.crl),
> name, OBJECT(&s->lpd.iou.adma[i]),
> &error_abort);
> }
>
> - for (i = 0; i < ARRAY_SIZE(s->lpd.iou.uart); i++) {
> - g_autofree gchar *name = g_strdup_printf("uart[%d]", i);
> -
> - object_property_set_link(OBJECT(&s->lpd.crl),
> - name, OBJECT(&s->lpd.iou.uart[i]),
> - &error_abort);
> - }
> -
> object_property_set_link(OBJECT(&s->lpd.crl),
> "usb", OBJECT(&s->lpd.iou.usb),
> &error_abort);
>
> sysbus_realize(sbd, &error_fatal);
> @@ -955,11 +1041,15 @@ static void versal_realize(DeviceState *dev, Error
> **errp)
> s->phandle.clk_125mhz = fdt_add_clk_node(s, "/clk125", 125 * 1000 *
> 1000);
>
> versal_create_apu_cpus(s);
> versal_create_apu_gic(s, pic);
> versal_create_rpu_cpus(s);
> - versal_create_uarts(s, pic);
> +
> + for (i = 0; i < map->num_uart; i++) {
> + versal_create_uart(s, &map->uart[i], i);
> + }
> +
> versal_create_canfds(s, pic);
> versal_create_usbs(s, pic);
> versal_create_gems(s, pic);
> versal_create_admas(s, pic);
> versal_create_sds(s, pic);
> --
> 2.50.0
>
