On Wed, Jul 16, 2025 at 11:53:48AM +0200, Luc Michel wrote:
> Refactor the SDHCI controllers creation using the VersalMap structure.
>
> Signed-off-by: Luc Michel <luc.mic...@amd.com>
Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com>
> ---
> include/hw/arm/xlnx-versal.h | 5 +-
> hw/arm/xlnx-versal-virt.c | 43 ++--------------
> hw/arm/xlnx-versal.c | 96 ++++++++++++++++++++++++++++--------
> 3 files changed, 83 insertions(+), 61 deletions(-)
>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 007c91b596e..4a7a2d85aac 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -14,11 +14,10 @@
> #define XLNX_VERSAL_H
>
> #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/dma/xlnx-zdma.h"
> #include "hw/net/cadence_gem.h"
> #include "hw/rtc/xlnx-zynqmp-rtc.h"
> #include "qom/object.h"
> @@ -103,11 +102,10 @@ struct Versal {
> } lpd;
>
> /* The Platform Management Controller subsystem. */
> struct {
> struct {
> - SDHCIState sd[XLNX_VERSAL_NR_SDS];
> XlnxVersalPmcIouSlcr slcr;
>
> struct {
> XlnxVersalOspi ospi;
> XlnxCSUDMA dma_src;
> @@ -154,11 +152,14 @@ static inline void versal_set_fdt(Versal *s, void *fdt)
> {
> g_assert(!qdev_is_realized(DEVICE(s)));
> s->cfg.fdt = fdt;
> }
>
> +void versal_sdhci_plug_card(Versal *s, int sd_idx, BlockBackend *blk);
> +
> int versal_get_num_can(VersalVersion version);
> +int versal_get_num_sdhci(VersalVersion version);
>
> /* Memory-map and IRQ definitions. Copied a subset from
> * auto-generated files. */
>
> #define VERSAL_GIC_MAINT_IRQ 9
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index 18a3ea169d0..99ce84d5a46 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -282,36 +282,10 @@ static void fdt_add_zdma_nodes(VersalVirt *s)
> qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat));
> 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--) {
> - uint64_t addr = MM_PMC_SD0 + MM_PMC_SD0_SIZE * i;
> - char *name = g_strdup_printf("/sdhci@%" PRIx64, addr);
> -
> - qemu_fdt_add_subnode(s->fdt, name);
> -
> - 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_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));
> - 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);
> @@ -562,20 +536,15 @@ static void efuse_attach_drive(XlnxEFuse *dev)
> if (blk) {
> qdev_prop_set_drive(DEVICE(dev), "drive", blk);
> }
> }
>
> -static void sd_plugin_card(SDHCIState *sd, DriveInfo *di)
> +static void sd_plug_card(VersalVirt *s, int idx, DriveInfo *di)
> {
> BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
> - DeviceState *card;
>
> - card = qdev_new(TYPE_SD_CARD);
> - object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card));
> - qdev_prop_set_drive_err(card, "drive", blk, &error_fatal);
> - qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sd), "sd-bus"),
> - &error_fatal);
> + versal_sdhci_plug_card(&s->soc, idx, blk);
> }
>
> static char *versal_get_ospi_model(Object *obj, Error **errp)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
> @@ -643,11 +612,10 @@ static void versal_virt_init(MachineState *machine)
> fdt_add_gem_nodes(s);
> fdt_add_gic_nodes(s);
> fdt_add_timer_nodes(s);
> fdt_add_zdma_nodes(s);
> fdt_add_usb_xhci_nodes(s);
> - fdt_add_sd_nodes(s);
> 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);
> @@ -666,14 +634,13 @@ static void versal_virt_init(MachineState *machine)
> bbram_attach_drive(&s->soc.pmc.bbram);
>
> /* Attach efuse backend, if given */
> efuse_attach_drive(&s->soc.pmc.efuse);
>
> - /* Plugin SD cards. */
> - for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) {
> - sd_plugin_card(&s->soc.pmc.iou.sd[i],
> - drive_get(IF_SD, 0, i));
> + /* Plug SD cards */
> + for (i = 0; i < versal_get_num_sdhci(VERSAL_VER_VERSAL); i++) {
> + sd_plug_card(s, i, drive_get(IF_SD, 0, i));
> }
>
> s->binfo.ram_size = machine->ram_size;
> s->binfo.loader_start = 0x0;
> s->binfo.get_dtb = versal_virt_get_dtb;
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 5dccf231942..b963a05935b 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -26,10 +26,11 @@
> #include "target/arm/gtimer.h"
> #include "system/device_tree.h"
> #include "hw/arm/fdt.h"
> #include "hw/char/pl011.h"
> #include "hw/net/xlnx-versal-canfd.h"
> +#include "hw/sd/sdhci.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
>
> @@ -45,20 +46,27 @@ typedef struct VersalMap {
> VersalSimplePeriphMap uart[2];
> size_t num_uart;
>
> VersalSimplePeriphMap canfd[4];
> size_t num_canfd;
> +
> + VersalSimplePeriphMap sdhci[2];
> + size_t num_sdhci;
> } VersalMap;
>
> static const VersalMap VERSAL_MAP = {
> .uart[0] = { 0xff000000, 18 },
> .uart[1] = { 0xff010000, 19 },
> .num_uart = 2,
>
> .canfd[0] = { 0xff060000, 20 },
> .canfd[1] = { 0xff070000, 21 },
> .num_canfd = 2,
> +
> + .sdhci[0] = { 0xf1040000, 126 },
> + .sdhci[1] = { 0xf1050000, 128 },
> + .num_sdhci = 2,
> };
>
> static const VersalMap *VERSION_TO_MAP[] = {
> [VERSAL_VER_VERSAL] = &VERSAL_MAP,
> };
> @@ -71,10 +79,22 @@ static inline VersalVersion versal_get_version(Versal *s)
> static inline const VersalMap *versal_get_map(Versal *s)
> {
> return VERSION_TO_MAP[versal_get_version(s)];
> }
>
> +static inline Object *versal_get_child(Versal *s, const char *child)
> +{
> + return object_resolve_path_at(OBJECT(s), child);
> +}
> +
> +static inline Object *versal_get_child_idx(Versal *s, const char *child,
> + size_t idx)
> +{
> + g_autofree char *n = g_strdup_printf("%s[%zu]", child, idx);
> +
> + return versal_get_child(s, n);
> +}
>
> static qemu_irq versal_get_irq(Versal *s, int irq_idx)
> {
> return qdev_get_gpio_in(DEVICE(&s->fpd.apu.gic), irq_idx);
> }
> @@ -422,36 +442,43 @@ static void versal_create_admas(Versal *s, qemu_irq
> *pic)
> g_free(name);
> }
> }
>
> #define SDHCI_CAPABILITIES 0x280737ec6481 /* Same as on ZynqMP. */
> -static void versal_create_sds(Versal *s, qemu_irq *pic)
> +static void versal_create_sdhci(Versal *s,
> + const VersalSimplePeriphMap *map)
> {
> - int i;
> + DeviceState *dev;
> + MemoryRegion *mr;
> + g_autofree char *node;
> + const char compatible[] = "arasan,sdhci-8.9a";
> + const char clocknames[] = "clk_xin\0clk_ahb";
>
> - for (i = 0; i < ARRAY_SIZE(s->pmc.iou.sd); i++) {
> - DeviceState *dev;
> - MemoryRegion *mr;
> + dev = qdev_new(TYPE_SYSBUS_SDHCI);
> + object_property_add_child(OBJECT(s), "sdhci[*]", OBJECT(dev));
>
> - object_initialize_child(OBJECT(s), "sd[*]", &s->pmc.iou.sd[i],
> - TYPE_SYSBUS_SDHCI);
> - dev = DEVICE(&s->pmc.iou.sd[i]);
> + object_property_set_uint(OBJECT(dev), "sd-spec-version", 3,
> + &error_fatal);
> + object_property_set_uint(OBJECT(dev), "capareg", SDHCI_CAPABILITIES,
> + &error_fatal);
> + object_property_set_uint(OBJECT(dev), "uhs", UHS_I, &error_fatal);
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>
> - object_property_set_uint(OBJECT(dev), "sd-spec-version", 3,
> - &error_fatal);
> - object_property_set_uint(OBJECT(dev), "capareg", SDHCI_CAPABILITIES,
> - &error_fatal);
> - object_property_set_uint(OBJECT(dev), "uhs", UHS_I, &error_fatal);
> - 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,
> - MM_PMC_SD0 + i * MM_PMC_SD0_SIZE, mr);
> + versal_sysbus_connect_irq(s, SYS_BUS_DEVICE(dev), 0, map->irq);
>
> - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> - pic[VERSAL_SD0_IRQ_0 + i * 2]);
> - }
> + node = versal_fdt_add_simple_subnode(s, "/sdhci", map->addr, 0x10000,
> + compatible, sizeof(compatible));
> + qemu_fdt_setprop_cells(s->cfg.fdt, node, "clocks",
> + s->phandle.clk_25mhz, s->phandle.clk_25mhz);
> + 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);
> }
>
> static void versal_create_pmc_apb_irq_orgate(Versal *s, qemu_irq *pic)
> {
> DeviceState *orgate;
> @@ -1064,14 +1091,17 @@ static void versal_realize(DeviceState *dev, Error
> **errp)
>
> for (i = 0; i < map->num_canfd; i++) {
> versal_create_canfd(s, &map->canfd[i], s->cfg.canbus[i]);
> }
>
> + for (i = 0; i < map->num_sdhci; i++) {
> + versal_create_sdhci(s, &map->sdhci[i]);
> + }
> +
> versal_create_usbs(s, pic);
> versal_create_gems(s, pic);
> versal_create_admas(s, pic);
> - versal_create_sds(s, pic);
> versal_create_pmc_apb_irq_orgate(s, pic);
> versal_create_rtc(s, pic);
> versal_create_trng(s, pic);
> versal_create_xrams(s, pic);
> versal_create_bbram(s, pic);
> @@ -1091,17 +1121,41 @@ static void versal_realize(DeviceState *dev, Error
> **errp)
> memory_region_add_subregion_overlap(&s->fpd.apu.mr, 0, &s->mr_ps, 0);
> memory_region_add_subregion_overlap(&s->lpd.rpu.mr, 0,
> &s->lpd.rpu.mr_ps_alias, 0);
> }
>
> +void versal_sdhci_plug_card(Versal *s, int sd_idx, BlockBackend *blk)
> +{
> + DeviceState *sdhci, *card;
> +
> + sdhci = DEVICE(versal_get_child_idx(s, "sdhci", sd_idx));
> +
> + if (sdhci == NULL) {
> + return;
> + }
> +
> + card = qdev_new(TYPE_SD_CARD);
> + object_property_add_child(OBJECT(sdhci), "card[*]", OBJECT(card));
> + qdev_prop_set_drive_err(card, "drive", blk, &error_fatal);
> + qdev_realize_and_unref(card, qdev_get_child_bus(DEVICE(sdhci), "sd-bus"),
> + &error_fatal);
> +}
> +
> int versal_get_num_can(VersalVersion version)
> {
> const VersalMap *map = VERSION_TO_MAP[version];
>
> return map->num_canfd;
> }
>
> +int versal_get_num_sdhci(VersalVersion version)
> +{
> + const VersalMap *map = VERSION_TO_MAP[version];
> +
> + return map->num_sdhci;
> +}
> +
> static void versal_base_init(Object *obj)
> {
> Versal *s = XLNX_VERSAL_BASE(obj);
> size_t i, num_can;
>
> --
> 2.50.0
>
