On Wed, Jul 16, 2025 at 11:53:54AM +0200, Luc Michel wrote:
> Refactor the OSPI controller creation using the VersalMap structure.
>
> Note that the connection to the PMC IOU SLCR 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 | 12 +--
> hw/arm/xlnx-versal-virt.c | 41 ++++------
> hw/arm/xlnx-versal.c | 142 ++++++++++++++++++++---------------
> 3 files changed, 98 insertions(+), 97 deletions(-)
>
> diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h
> index 79ca9b13321..b7ef255d6fd 100644
> --- a/include/hw/arm/xlnx-versal.h
> +++ b/include/hw/arm/xlnx-versal.h
> @@ -18,12 +18,10 @@
> #include "hw/or-irq.h"
> #include "hw/intc/arm_gicv3.h"
> #include "hw/rtc/xlnx-zynqmp-rtc.h"
> #include "qom/object.h"
> #include "hw/nvram/xlnx-bbram.h"
> -#include "hw/ssi/xlnx-versal-ospi.h"
> -#include "hw/dma/xlnx_csu_dma.h"
> #include "hw/misc/xlnx-versal-crl.h"
> #include "hw/misc/xlnx-versal-pmc-iou-slcr.h"
> #include "hw/misc/xlnx-versal-trng.h"
> #include "net/can_emu.h"
> #include "hw/misc/xlnx-versal-cfu.h"
> @@ -86,18 +84,10 @@ struct Versal {
>
> /* The Platform Management Controller subsystem. */
> struct {
> struct {
> XlnxVersalPmcIouSlcr slcr;
> -
> - struct {
> - XlnxVersalOspi ospi;
> - XlnxCSUDMA dma_src;
> - XlnxCSUDMA dma_dst;
> - MemoryRegion linear_mr;
> - OrIRQState irq_orgate;
> - } ospi;
> } iou;
>
> XlnxZynqMPRTC rtc;
> XlnxVersalTRng trng;
> XlnxBBRam bbram;
> @@ -134,10 +124,12 @@ static inline void versal_set_fdt(Versal *s, void *fdt)
> s->cfg.fdt = fdt;
> }
>
> void versal_sdhci_plug_card(Versal *s, int sd_idx, BlockBackend *blk);
> void versal_efuse_attach_drive(Versal *s, BlockBackend *blk);
> +void versal_ospi_create_flash(Versal *s, int flash_idx, const char
> *flash_mdl,
> + 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
> diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
> index d3c84d1955a..43f3c3d0be9 100644
> --- a/hw/arm/xlnx-versal-virt.c
> +++ b/hw/arm/xlnx-versal-virt.c
> @@ -46,12 +46,12 @@ struct VersalVirt {
>
> CanBusState **canbus;
>
> struct {
> bool secure;
> + char *ospi_model;
> } cfg;
> - char *ospi_model;
> };
>
> static void fdt_create(VersalVirt *s)
> {
> MachineClass *mc = MACHINE_GET_CLASS(s);
> @@ -379,19 +379,19 @@ static void sd_plug_card(VersalVirt *s, int idx,
> DriveInfo *di)
>
> static char *versal_get_ospi_model(Object *obj, Error **errp)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
>
> - return g_strdup(s->ospi_model);
> + return g_strdup(s->cfg.ospi_model);
> }
>
> static void versal_set_ospi_model(Object *obj, const char *value, Error
> **errp)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
>
> - g_free(s->ospi_model);
> - s->ospi_model = g_strdup(value);
> + g_free(s->cfg.ospi_model);
> + s->cfg.ospi_model = g_strdup(value);
> }
>
>
> static void versal_virt_init(MachineState *machine)
> {
> @@ -480,42 +480,31 @@ static void versal_virt_init(MachineState *machine)
> s->binfo.dtb_limit = 0x1000000;
> }
> arm_load_kernel(&s->soc.fpd.apu.cpu[0], machine, &s->binfo);
>
> for (i = 0; i < XLNX_VERSAL_NUM_OSPI_FLASH; i++) {
> - BusState *spi_bus;
> - DeviceState *flash_dev;
> ObjectClass *flash_klass;
> - qemu_irq cs_line;
> DriveInfo *dinfo = drive_get(IF_MTD, 0, i);
> + BlockBackend *blk;
> + const char *mdl;
>
> - spi_bus = qdev_get_child_bus(DEVICE(&s->soc.pmc.iou.ospi), "spi0");
> -
> - if (s->ospi_model) {
> - flash_klass = object_class_by_name(s->ospi_model);
> + if (s->cfg.ospi_model) {
> + flash_klass = object_class_by_name(s->cfg.ospi_model);
> if (!flash_klass ||
> object_class_is_abstract(flash_klass) ||
> !object_class_dynamic_cast(flash_klass, TYPE_M25P80)) {
> error_report("'%s' is either abstract or"
> - " not a subtype of m25p80", s->ospi_model);
> + " not a subtype of m25p80", s->cfg.ospi_model);
> exit(1);
> }
> + mdl = s->cfg.ospi_model;
> + } else {
> + mdl = "mt35xu01g";
> }
>
> - flash_dev = qdev_new(s->ospi_model ? s->ospi_model : "mt35xu01g");
> -
> - if (dinfo) {
> - qdev_prop_set_drive_err(flash_dev, "drive",
> - blk_by_legacy_dinfo(dinfo),
> &error_fatal);
> - }
> - qdev_prop_set_uint8(flash_dev, "cs", i);
> - qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
> -
> - cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
> -
> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.pmc.iou.ospi),
> - i + 1, cs_line);
> + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> + versal_ospi_create_flash(&s->soc, i, mdl, blk);
> }
> }
>
> static void versal_virt_machine_instance_init(Object *obj)
> {
> @@ -540,11 +529,11 @@ static void versal_virt_machine_instance_init(Object
> *obj)
>
> static void versal_virt_machine_finalize(Object *obj)
> {
> VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(obj);
>
> - g_free(s->ospi_model);
> + g_free(s->cfg.ospi_model);
> g_free(s->canbus);
> }
>
> static void versal_virt_machine_class_init(ObjectClass *oc, const void *data)
> {
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index ed202b0fcda..58176fa11e5 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -32,10 +32,11 @@
> #include "hw/net/cadence_gem.h"
> #include "hw/dma/xlnx-zdma.h"
> #include "hw/misc/xlnx-versal-xramc.h"
> #include "hw/usb/xlnx-usb-subsystem.h"
> #include "hw/nvram/xlnx-versal-efuse.h"
> +#include "hw/ssi/xlnx-versal-ospi.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
>
> @@ -93,10 +94,19 @@ typedef struct VersalMap {
> struct VersalEfuseMap {
> uint64_t ctrl;
> uint64_t cache;
> int irq;
> } efuse;
> +
> + struct VersalOspiMap {
> + uint64_t ctrl;
> + uint64_t dac;
> + uint64_t dac_sz;
> + uint64_t dma_src;
> + uint64_t dma_dst;
> + int irq;
> + } ospi;
> } VersalMap;
>
> static const VersalMap VERSAL_MAP = {
> .uart[0] = { 0xff000000, 18 },
> .uart[1] = { 0xff010000, 19 },
> @@ -126,10 +136,17 @@ static const VersalMap VERSAL_MAP = {
>
> .usb[0] = { .xhci = 0xfe200000, .ctrl = 0xff9d0000, .irq = 22 },
> .num_usb = 1,
>
> .efuse = { .ctrl = 0xf1240000, .cache = 0xf1250000, .irq = 139 },
> +
> + .ospi = {
> + .ctrl = 0xf1010000,
> + .dac = 0xc0000000, .dac_sz = 0x20000000,
> + .dma_src = 0xf1011000, .dma_dst = 0xf1011800,
> + .irq = 124,
> + },
> };
>
> static const VersalMap *VERSION_TO_MAP[] = {
> [VERSAL_VER_VERSAL] = &VERSAL_MAP,
> };
> @@ -807,99 +824,78 @@ static void versal_create_pmc_iou_slcr(Versal *s,
> qemu_irq *pic)
>
> sysbus_connect_irq(sbd, 0,
> qdev_get_gpio_in(DEVICE(&s->pmc.apb_irq_orgate), 2));
> }
>
> -static void versal_create_ospi(Versal *s, qemu_irq *pic)
> +static DeviceState *versal_create_ospi(Versal *s,
> + const struct VersalOspiMap *map)
> {
> SysBusDevice *sbd;
> MemoryRegion *mr_dac;
> - qemu_irq ospi_mux_sel;
> - DeviceState *orgate;
> + DeviceState *dev, *dma_dst, *dma_src, *orgate;
> + MemoryRegion *linear_mr = g_new(MemoryRegion, 1);
>
> - memory_region_init(&s->pmc.iou.ospi.linear_mr, OBJECT(s),
> - "versal-ospi-linear-mr" , MM_PMC_OSPI_DAC_SIZE);
> + dev = qdev_new(TYPE_XILINX_VERSAL_OSPI);
> + object_property_add_child(OBJECT(s), "ospi", OBJECT(dev));
>
> - object_initialize_child(OBJECT(s), "versal-ospi", &s->pmc.iou.ospi.ospi,
> - TYPE_XILINX_VERSAL_OSPI);
> + memory_region_init(linear_mr, OBJECT(dev), "linear-mr", map->dac_sz);
>
> - mr_dac = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->pmc.iou.ospi.ospi),
> 1);
> - memory_region_add_subregion(&s->pmc.iou.ospi.linear_mr, 0x0, mr_dac);
> + mr_dac = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> + memory_region_add_subregion(linear_mr, 0x0, mr_dac);
>
> /* Create the OSPI destination DMA */
> - object_initialize_child(OBJECT(s), "versal-ospi-dma-dst",
> - &s->pmc.iou.ospi.dma_dst,
> - TYPE_XLNX_CSU_DMA);
> + dma_dst = qdev_new(TYPE_XLNX_CSU_DMA);
> + object_property_add_child(OBJECT(dev), "dma-dst-dev", OBJECT(dma_dst));
> + object_property_set_link(OBJECT(dma_dst), "dma",
> + OBJECT(get_system_memory()), &error_abort);
>
> - object_property_set_link(OBJECT(&s->pmc.iou.ospi.dma_dst),
> - "dma", OBJECT(get_system_memory()),
> - &error_abort);
> + sbd = SYS_BUS_DEVICE(dma_dst);
> + sysbus_realize_and_unref(sbd, &error_fatal);
>
> - sbd = SYS_BUS_DEVICE(&s->pmc.iou.ospi.dma_dst);
> - sysbus_realize(sbd, &error_fatal);
> -
> - memory_region_add_subregion(&s->mr_ps, MM_PMC_OSPI_DMA_DST,
> + memory_region_add_subregion(&s->mr_ps, map->dma_dst,
> sysbus_mmio_get_region(sbd, 0));
>
> /* Create the OSPI source DMA */
> - object_initialize_child(OBJECT(s), "versal-ospi-dma-src",
> - &s->pmc.iou.ospi.dma_src,
> - TYPE_XLNX_CSU_DMA);
> + dma_src = qdev_new(TYPE_XLNX_CSU_DMA);
> + object_property_add_child(OBJECT(dev), "dma-src-dev", OBJECT(dma_src));
>
> - object_property_set_bool(OBJECT(&s->pmc.iou.ospi.dma_src), "is-dst",
> - false, &error_abort);
> + object_property_set_bool(OBJECT(dma_src), "is-dst", false, &error_abort);
>
> - object_property_set_link(OBJECT(&s->pmc.iou.ospi.dma_src),
> - "dma", OBJECT(mr_dac), &error_abort);
> -
> - object_property_set_link(OBJECT(&s->pmc.iou.ospi.dma_src),
> - "stream-connected-dma",
> - OBJECT(&s->pmc.iou.ospi.dma_dst),
> + object_property_set_link(OBJECT(dma_src), "dma", OBJECT(mr_dac),
> &error_abort);
>
> - sbd = SYS_BUS_DEVICE(&s->pmc.iou.ospi.dma_src);
> - sysbus_realize(sbd, &error_fatal);
> + object_property_set_link(OBJECT(dma_src), "stream-connected-dma",
> + OBJECT(dma_dst), &error_abort);
>
> - memory_region_add_subregion(&s->mr_ps, MM_PMC_OSPI_DMA_SRC,
> + sbd = SYS_BUS_DEVICE(dma_src);
> + sysbus_realize_and_unref(sbd, &error_fatal);
> +
> + memory_region_add_subregion(&s->mr_ps, map->dma_src,
> sysbus_mmio_get_region(sbd, 0));
>
> /* Realize the OSPI */
> - object_property_set_link(OBJECT(&s->pmc.iou.ospi.ospi), "dma-src",
> - OBJECT(&s->pmc.iou.ospi.dma_src), &error_abort);
> + object_property_set_link(OBJECT(dev), "dma-src",
> + OBJECT(dma_src), &error_abort);
>
> - sbd = SYS_BUS_DEVICE(&s->pmc.iou.ospi.ospi);
> - sysbus_realize(sbd, &error_fatal);
> + sbd = SYS_BUS_DEVICE(dev);
> + sysbus_realize_and_unref(sbd, &error_fatal);
>
> - memory_region_add_subregion(&s->mr_ps, MM_PMC_OSPI,
> + memory_region_add_subregion(&s->mr_ps, map->ctrl,
> sysbus_mmio_get_region(sbd, 0));
>
> - memory_region_add_subregion(&s->mr_ps, MM_PMC_OSPI_DAC,
> - &s->pmc.iou.ospi.linear_mr);
> -
> - /* ospi_mux_sel */
> - ospi_mux_sel = qdev_get_gpio_in_named(DEVICE(&s->pmc.iou.ospi.ospi),
> - "ospi-mux-sel", 0);
> - qdev_connect_gpio_out_named(DEVICE(&s->pmc.iou.slcr), "ospi-mux-sel", 0,
> - ospi_mux_sel);
> + memory_region_add_subregion(&s->mr_ps, map->dac,
> + linear_mr);
>
> /* OSPI irq */
> - object_initialize_child(OBJECT(s), "ospi-irq-orgate",
> - &s->pmc.iou.ospi.irq_orgate, TYPE_OR_IRQ);
> - object_property_set_int(OBJECT(&s->pmc.iou.ospi.irq_orgate),
> - "num-lines", NUM_OSPI_IRQ_LINES, &error_fatal);
> + orgate = create_or_gate(s, OBJECT(dev), "irq-orgate", NUM_OSPI_IRQ_LINES,
> + map->irq);
>
> - orgate = DEVICE(&s->pmc.iou.ospi.irq_orgate);
> - qdev_realize(orgate, NULL, &error_fatal);
> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(orgate, 0));
> + sysbus_connect_irq(SYS_BUS_DEVICE(dma_src), 0, qdev_get_gpio_in(orgate,
> 1));
> + sysbus_connect_irq(SYS_BUS_DEVICE(dma_dst), 0, qdev_get_gpio_in(orgate,
> 2));
>
> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->pmc.iou.ospi.ospi), 0,
> - qdev_get_gpio_in(orgate, 0));
> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->pmc.iou.ospi.dma_src), 0,
> - qdev_get_gpio_in(orgate, 1));
> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->pmc.iou.ospi.dma_dst), 0,
> - qdev_get_gpio_in(orgate, 2));
> -
> - qdev_connect_gpio_out(orgate, 0, pic[VERSAL_OSPI_IRQ]);
> + return dev;
> }
>
> static void versal_create_cfu(Versal *s, qemu_irq *pic)
> {
> SysBusDevice *sbd;
> @@ -1256,17 +1252,17 @@ static void versal_realize(DeviceState *dev, Error
> **errp)
> for (i = 0; i < map->num_usb; i++) {
> versal_create_usb(s, &map->usb[i]);
> }
>
> versal_create_efuse(s, &map->efuse);
> + versal_create_ospi(s, &map->ospi);
>
> versal_create_pmc_apb_irq_orgate(s, pic);
> versal_create_rtc(s, pic);
> versal_create_trng(s, pic);
> versal_create_bbram(s, pic);
> versal_create_pmc_iou_slcr(s, pic);
> - versal_create_ospi(s, pic);
> versal_create_crl(s, pic);
> versal_create_cfu(s, pic);
> versal_map_ddr(s);
> versal_unimp(s);
>
> @@ -1308,10 +1304,34 @@ void versal_efuse_attach_drive(Versal *s,
> BlockBackend *blk)
> }
>
> qdev_prop_set_drive(efuse, "drive", blk);
> }
>
> +void versal_ospi_create_flash(Versal *s, int flash_idx, const char
> *flash_mdl,
> + BlockBackend *blk)
> +{
> + BusState *spi_bus;
> + DeviceState *flash, *ospi;
> + qemu_irq cs_line;
> +
> + ospi = DEVICE(versal_get_child(s, "ospi"));
> + spi_bus = qdev_get_child_bus(ospi, "spi0");
> +
> + flash = qdev_new(flash_mdl);
> +
> + if (blk) {
> + qdev_prop_set_drive_err(flash, "drive", blk, &error_fatal);
> + }
> + qdev_prop_set_uint8(flash, "cs", flash_idx);
> + qdev_realize_and_unref(flash, spi_bus, &error_fatal);
> +
> + cs_line = qdev_get_gpio_in_named(flash, SSI_GPIO_CS, 0);
> +
> + sysbus_connect_irq(SYS_BUS_DEVICE(ospi),
> + flash_idx + 1, cs_line);
> +}
> +
> int versal_get_num_can(VersalVersion version)
> {
> const VersalMap *map = VERSION_TO_MAP[version];
>
> return map->num_canfd;
> --
> 2.50.0
>
