All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify
@ 2021-08-19 16:34 Philippe Mathieu-Daudé
  2021-08-19 16:34 ` [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-19 16:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, qemu-arm, Philippe Mathieu-Daudé,
	Alistair Francis, Peter Maydell

Fix a qdev realization ordering, then simplify a pair of DMA
devices by always passing a MemoryRegion property to the device.
Doing so we can move the AddressSpace field to the device struct,
removing need for heap allocation.

Philippe Mathieu-Daudé (4):
  hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma
  hw/dma/xlnx_csu_dma: Run trivial checks early in realize()
  hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set
  hw/dma/xlnx-zdma Always expect 'dma' link property to be set

 include/hw/dma/xlnx-zdma.h    |  2 +-
 include/hw/dma/xlnx_csu_dma.h |  2 +-
 hw/arm/xlnx-versal.c          |  2 ++
 hw/arm/xlnx-zynqmp.c          | 54 +++++++++++++++++++++--------------
 hw/dma/xlnx-zdma.c            | 24 ++++++++--------
 hw/dma/xlnx_csu_dma.c         | 31 ++++++++++----------
 6 files changed, 63 insertions(+), 52 deletions(-)

-- 
2.31.1




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma
  2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
@ 2021-08-19 16:34 ` Philippe Mathieu-Daudé
  2021-08-19 22:50   ` Alistair Francis
  2021-08-19 16:34 ` [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize() Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-19 16:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, qemu-arm, Philippe Mathieu-Daudé,
	Alistair Francis, Peter Maydell

If we link QOM object (a) as a property of QOM object (b),
we must set the property *before* (b) is realized.

Move QSPI realization *after* QSPI DMA.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/xlnx-zynqmp.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 3597e8db4de..9724978761b 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -570,26 +570,6 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
         g_free(bus_name);
     }
 
-    if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
-        return;
-    }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
-
-    for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
-        gchar *bus_name;
-        gchar *target_bus;
-
-        /* Alias controller SPI bus to the SoC itself */
-        bus_name = g_strdup_printf("qspi%d", i);
-        target_bus = g_strdup_printf("spi%d", i);
-        object_property_add_alias(OBJECT(s), bus_name,
-                                  OBJECT(&s->qspi), target_bus);
-        g_free(bus_name);
-        g_free(target_bus);
-    }
-
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->dp), errp)) {
         return;
     }
@@ -646,8 +626,26 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
 
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi_dma), 0, QSPI_DMA_ADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0, gic_spi[QSPI_IRQ]);
-    object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
-                             OBJECT(&s->qspi_dma), errp);
+
+    if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
+                                  OBJECT(&s->qspi_dma), errp)) {
+         return;
+    }
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
+        g_autofree gchar *bus_name = g_strdup_printf("qspi%d", i);
+        g_autofree gchar *target_bus = g_strdup_printf("spi%d", i);
+
+        /* Alias controller SPI bus to the SoC itself */
+        object_property_add_alias(OBJECT(s), bus_name,
+                                  OBJECT(&s->qspi), target_bus);
+    }
 }
 
 static Property xlnx_zynqmp_props[] = {
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize()
  2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
  2021-08-19 16:34 ` [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma Philippe Mathieu-Daudé
@ 2021-08-19 16:34 ` Philippe Mathieu-Daudé
  2021-08-19 22:51   ` Alistair Francis
  2021-08-19 16:34 ` [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-19 16:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, qemu-arm, Philippe Mathieu-Daudé,
	Alistair Francis, Peter Maydell

If some property are not set, we'll return indicating a failure,
so it is pointless to allocate / initialize some fields too early.
Move the trivial checks earlier in realize().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/dma/xlnx_csu_dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c
index 797b4fed8f5..2d19f415ef3 100644
--- a/hw/dma/xlnx_csu_dma.c
+++ b/hw/dma/xlnx_csu_dma.c
@@ -626,6 +626,11 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
     XlnxCSUDMA *s = XLNX_CSU_DMA(dev);
     RegisterInfoArray *reg_array;
 
+    if (!s->is_dst && !s->tx_dev) {
+        error_setg(errp, "zynqmp.csu-dma: Stream not connected");
+        return;
+    }
+
     reg_array =
         register_init_block32(dev, xlnx_csu_dma_regs_info[!!s->is_dst],
                               XLNX_CSU_DMA_R_MAX,
@@ -640,11 +645,6 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
     sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
 
-    if (!s->is_dst && !s->tx_dev) {
-        error_setg(errp, "zynqmp.csu-dma: Stream not connected");
-        return;
-    }
-
     s->src_timer = ptimer_init(xlnx_csu_dma_src_timeout_hit,
                                s, PTIMER_POLICY_DEFAULT);
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set
  2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
  2021-08-19 16:34 ` [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma Philippe Mathieu-Daudé
  2021-08-19 16:34 ` [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize() Philippe Mathieu-Daudé
@ 2021-08-19 16:34 ` Philippe Mathieu-Daudé
  2021-08-20  9:01   ` Peter Maydell
  2021-08-19 16:34 ` [PATCH v2 4/4] hw/dma/xlnx-zdma " Philippe Mathieu-Daudé
  2021-08-26 13:19 ` [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Peter Maydell
  4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-19 16:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, qemu-arm, Philippe Mathieu-Daudé,
	Alistair Francis, Peter Maydell

Simplify by always passing a MemoryRegion property to the device.
Doing so we can move the AddressSpace field to the device struct,
removing need for heap allocation.

Update the Xilinx ZynqMP SoC model to pass the default system
memory instead of a NULL value.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/dma/xlnx_csu_dma.h |  2 +-
 hw/arm/xlnx-zynqmp.c          |  4 ++++
 hw/dma/xlnx_csu_dma.c         | 21 ++++++++++-----------
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/hw/dma/xlnx_csu_dma.h b/include/hw/dma/xlnx_csu_dma.h
index 204d94c6737..9e9dc551e99 100644
--- a/include/hw/dma/xlnx_csu_dma.h
+++ b/include/hw/dma/xlnx_csu_dma.h
@@ -30,7 +30,7 @@ typedef struct XlnxCSUDMA {
     MemoryRegion iomem;
     MemTxAttrs attr;
     MemoryRegion *dma_mr;
-    AddressSpace *dma_as;
+    AddressSpace dma_as;
     qemu_irq irq;
     StreamSink *tx_dev; /* Used as generic StreamSink */
     ptimer_state *src_timer;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 9724978761b..4344e223f2d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -620,6 +620,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                            gic_spi[adma_ch_intr[i]]);
     }
 
+    if (!object_property_set_link(OBJECT(&s->qspi_dma), "dma",
+                                  OBJECT(system_memory), errp)) {
+        return;
+    }
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi_dma), errp)) {
         return;
     }
diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c
index 2d19f415ef3..896bb3574dd 100644
--- a/hw/dma/xlnx_csu_dma.c
+++ b/hw/dma/xlnx_csu_dma.c
@@ -201,11 +201,11 @@ static uint32_t xlnx_csu_dma_read(XlnxCSUDMA *s, uint8_t *buf, uint32_t len)
         for (i = 0; i < len && (result == MEMTX_OK); i += s->width) {
             uint32_t mlen = MIN(len - i, s->width);
 
-            result = address_space_rw(s->dma_as, addr, s->attr,
+            result = address_space_rw(&s->dma_as, addr, s->attr,
                                       buf + i, mlen, false);
         }
     } else {
-        result = address_space_rw(s->dma_as, addr, s->attr, buf, len, false);
+        result = address_space_rw(&s->dma_as, addr, s->attr, buf, len, false);
     }
 
     if (result == MEMTX_OK) {
@@ -232,12 +232,12 @@ static uint32_t xlnx_csu_dma_write(XlnxCSUDMA *s, uint8_t *buf, uint32_t len)
         for (i = 0; i < len && (result == MEMTX_OK); i += s->width) {
             uint32_t mlen = MIN(len - i, s->width);
 
-            result = address_space_rw(s->dma_as, addr, s->attr,
+            result = address_space_rw(&s->dma_as, addr, s->attr,
                                       buf, mlen, true);
             buf += mlen;
         }
     } else {
-        result = address_space_rw(s->dma_as, addr, s->attr, buf, len, true);
+        result = address_space_rw(&s->dma_as, addr, s->attr, buf, len, true);
     }
 
     if (result != MEMTX_OK) {
@@ -631,6 +631,12 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!s->dma_mr) {
+        error_setg(errp, TYPE_XLNX_CSU_DMA " 'dma' link not set");
+        return;
+    }
+    address_space_init(&s->dma_as, s->dma_mr, "csu-dma");
+
     reg_array =
         register_init_block32(dev, xlnx_csu_dma_regs_info[!!s->is_dst],
                               XLNX_CSU_DMA_R_MAX,
@@ -648,13 +654,6 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
     s->src_timer = ptimer_init(xlnx_csu_dma_src_timeout_hit,
                                s, PTIMER_POLICY_DEFAULT);
 
-    if (s->dma_mr) {
-        s->dma_as = g_malloc0(sizeof(AddressSpace));
-        address_space_init(s->dma_as, s->dma_mr, NULL);
-    } else {
-        s->dma_as = &address_space_memory;
-    }
-
     s->attr = MEMTXATTRS_UNSPECIFIED;
 
     s->r_size_last_word = 0;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] hw/dma/xlnx-zdma Always expect 'dma' link property to be set
  2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2021-08-19 16:34 ` [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set Philippe Mathieu-Daudé
@ 2021-08-19 16:34 ` Philippe Mathieu-Daudé
  2021-08-20  9:02   ` Peter Maydell
  2021-08-26 13:19 ` [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Peter Maydell
  4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-19 16:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, qemu-arm, Philippe Mathieu-Daudé,
	Alistair Francis, Peter Maydell

Simplify by always passing a MemoryRegion property to the device.
Doing so we can move the AddressSpace field to the device struct,
removing need for heap allocation.

Update the Xilinx ZynqMP / Versal SoC models to pass the default
system memory instead of a NULL value.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/dma/xlnx-zdma.h |  2 +-
 hw/arm/xlnx-versal.c       |  2 ++
 hw/arm/xlnx-zynqmp.c       |  8 ++++++++
 hw/dma/xlnx-zdma.c         | 24 ++++++++++++------------
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/include/hw/dma/xlnx-zdma.h b/include/hw/dma/xlnx-zdma.h
index 6602e7ffa72..efc75217d59 100644
--- a/include/hw/dma/xlnx-zdma.h
+++ b/include/hw/dma/xlnx-zdma.h
@@ -56,7 +56,7 @@ struct XlnxZDMA {
     MemoryRegion iomem;
     MemTxAttrs attr;
     MemoryRegion *dma_mr;
-    AddressSpace *dma_as;
+    AddressSpace dma_as;
     qemu_irq irq_zdma_ch_imr;
 
     struct {
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index fb776834f7e..d60eb4fb184 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -218,6 +218,8 @@ static void versal_create_admas(Versal *s, qemu_irq *pic)
                                 TYPE_XLNX_ZDMA);
         dev = DEVICE(&s->lpd.iou.adma[i]);
         object_property_set_int(OBJECT(dev), "bus-width", 128, &error_abort);
+        object_property_set_link(OBJECT(dev), "dma",
+                                 OBJECT(get_system_memory()), &error_fatal);
         sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
 
         mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4344e223f2d..6cfce26210d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -601,6 +601,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                                       errp)) {
             return;
         }
+        if (!object_property_set_link(OBJECT(&s->gdma[i]), "dma",
+                                      OBJECT(system_memory), errp)) {
+            return;
+        }
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->gdma[i]), errp)) {
             return;
         }
@@ -611,6 +615,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_ADMA_CH; i++) {
+        if (!object_property_set_link(OBJECT(&s->adma[i]), "dma",
+                                      OBJECT(system_memory), errp)) {
+            return;
+        }
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->adma[i]), errp)) {
             return;
         }
diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c
index fa38a556341..a5a92b4ff8c 100644
--- a/hw/dma/xlnx-zdma.c
+++ b/hw/dma/xlnx-zdma.c
@@ -320,9 +320,9 @@ static bool zdma_load_descriptor(XlnxZDMA *s, uint64_t addr,
         return false;
     }
 
-    descr->addr = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
-    descr->size = address_space_ldl_le(s->dma_as, addr + 8, s->attr, NULL);
-    descr->attr = address_space_ldl_le(s->dma_as, addr + 12, s->attr, NULL);
+    descr->addr = address_space_ldq_le(&s->dma_as, addr, s->attr, NULL);
+    descr->size = address_space_ldl_le(&s->dma_as, addr + 8, s->attr, NULL);
+    descr->attr = address_space_ldl_le(&s->dma_as, addr + 12, s->attr, NULL);
     return true;
 }
 
@@ -354,7 +354,7 @@ static void zdma_update_descr_addr(XlnxZDMA *s, bool type,
     } else {
         addr = zdma_get_regaddr64(s, basereg);
         addr += sizeof(s->dsc_dst);
-        next = address_space_ldq_le(s->dma_as, addr, s->attr, NULL);
+        next = address_space_ldq_le(&s->dma_as, addr, s->attr, NULL);
     }
 
     zdma_put_regaddr64(s, basereg, next);
@@ -421,7 +421,7 @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len)
             }
         }
 
-        address_space_write(s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen);
+        address_space_write(&s->dma_as, s->dsc_dst.addr, s->attr, buf, dlen);
         if (burst_type == AXI_BURST_INCR) {
             s->dsc_dst.addr += dlen;
         }
@@ -497,7 +497,7 @@ static void zdma_process_descr(XlnxZDMA *s)
                 len = s->cfg.bus_width / 8;
             }
         } else {
-            address_space_read(s->dma_as, src_addr, s->attr, s->buf, len);
+            address_space_read(&s->dma_as, src_addr, s->attr, s->buf, len);
             if (burst_type == AXI_BURST_INCR) {
                 src_addr += len;
             }
@@ -765,6 +765,12 @@ static void zdma_realize(DeviceState *dev, Error **errp)
     XlnxZDMA *s = XLNX_ZDMA(dev);
     unsigned int i;
 
+    if (!s->dma_mr) {
+        error_setg(errp, TYPE_XLNX_ZDMA " 'dma' link not set");
+        return;
+    }
+    address_space_init(&s->dma_as, s->dma_mr, "zdma-dma");
+
     for (i = 0; i < ARRAY_SIZE(zdma_regs_info); ++i) {
         RegisterInfo *r = &s->regs_info[zdma_regs_info[i].addr / 4];
 
@@ -777,12 +783,6 @@ static void zdma_realize(DeviceState *dev, Error **errp)
         };
     }
 
-    if (s->dma_mr) {
-        s->dma_as = g_malloc0(sizeof(AddressSpace));
-        address_space_init(s->dma_as, s->dma_mr, NULL);
-    } else {
-        s->dma_as = &address_space_memory;
-    }
     s->attr = MEMTXATTRS_UNSPECIFIED;
 }
 
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma
  2021-08-19 16:34 ` [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma Philippe Mathieu-Daudé
@ 2021-08-19 22:50   ` Alistair Francis
  0 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2021-08-19 22:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, qemu-arm, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Maydell

On Fri, Aug 20, 2021 at 2:35 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> If we link QOM object (a) as a property of QOM object (b),
> we must set the property *before* (b) is realized.
>
> Move QSPI realization *after* QSPI DMA.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/xlnx-zynqmp.c | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 3597e8db4de..9724978761b 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -570,26 +570,6 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>          g_free(bus_name);
>      }
>
> -    if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
> -        return;
> -    }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
> -
> -    for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
> -        gchar *bus_name;
> -        gchar *target_bus;
> -
> -        /* Alias controller SPI bus to the SoC itself */
> -        bus_name = g_strdup_printf("qspi%d", i);
> -        target_bus = g_strdup_printf("spi%d", i);
> -        object_property_add_alias(OBJECT(s), bus_name,
> -                                  OBJECT(&s->qspi), target_bus);
> -        g_free(bus_name);
> -        g_free(target_bus);
> -    }
> -
>      if (!sysbus_realize(SYS_BUS_DEVICE(&s->dp), errp)) {
>          return;
>      }
> @@ -646,8 +626,26 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi_dma), 0, QSPI_DMA_ADDR);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi_dma), 0, gic_spi[QSPI_IRQ]);
> -    object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
> -                             OBJECT(&s->qspi_dma), errp);
> +
> +    if (!object_property_set_link(OBJECT(&s->qspi), "stream-connected-dma",
> +                                  OBJECT(&s->qspi_dma), errp)) {
> +         return;
> +    }
> +    if (!sysbus_realize(SYS_BUS_DEVICE(&s->qspi), errp)) {
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 0, QSPI_ADDR);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->qspi), 1, LQSPI_ADDR);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->qspi), 0, gic_spi[QSPI_IRQ]);
> +
> +    for (i = 0; i < XLNX_ZYNQMP_NUM_QSPI_BUS; i++) {
> +        g_autofree gchar *bus_name = g_strdup_printf("qspi%d", i);
> +        g_autofree gchar *target_bus = g_strdup_printf("spi%d", i);
> +
> +        /* Alias controller SPI bus to the SoC itself */
> +        object_property_add_alias(OBJECT(s), bus_name,
> +                                  OBJECT(&s->qspi), target_bus);
> +    }
>  }
>
>  static Property xlnx_zynqmp_props[] = {
> --
> 2.31.1
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize()
  2021-08-19 16:34 ` [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize() Philippe Mathieu-Daudé
@ 2021-08-19 22:51   ` Alistair Francis
  0 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2021-08-19 22:51 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, qemu-arm, qemu-devel@nongnu.org Developers,
	Alistair Francis, Peter Maydell

On Fri, Aug 20, 2021 at 2:35 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> If some property are not set, we'll return indicating a failure,
> so it is pointless to allocate / initialize some fields too early.
> Move the trivial checks earlier in realize().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/dma/xlnx_csu_dma.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/dma/xlnx_csu_dma.c b/hw/dma/xlnx_csu_dma.c
> index 797b4fed8f5..2d19f415ef3 100644
> --- a/hw/dma/xlnx_csu_dma.c
> +++ b/hw/dma/xlnx_csu_dma.c
> @@ -626,6 +626,11 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
>      XlnxCSUDMA *s = XLNX_CSU_DMA(dev);
>      RegisterInfoArray *reg_array;
>
> +    if (!s->is_dst && !s->tx_dev) {
> +        error_setg(errp, "zynqmp.csu-dma: Stream not connected");
> +        return;
> +    }
> +
>      reg_array =
>          register_init_block32(dev, xlnx_csu_dma_regs_info[!!s->is_dst],
>                                XLNX_CSU_DMA_R_MAX,
> @@ -640,11 +645,6 @@ static void xlnx_csu_dma_realize(DeviceState *dev, Error **errp)
>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>      sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>
> -    if (!s->is_dst && !s->tx_dev) {
> -        error_setg(errp, "zynqmp.csu-dma: Stream not connected");
> -        return;
> -    }
> -
>      s->src_timer = ptimer_init(xlnx_csu_dma_src_timeout_hit,
>                                 s, PTIMER_POLICY_DEFAULT);
>
> --
> 2.31.1
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set
  2021-08-19 16:34 ` [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set Philippe Mathieu-Daudé
@ 2021-08-20  9:01   ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2021-08-20  9:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, qemu-arm, QEMU Developers, Alistair Francis

On Thu, 19 Aug 2021 at 17:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Simplify by always passing a MemoryRegion property to the device.
> Doing so we can move the AddressSpace field to the device struct,
> removing need for heap allocation.
>
> Update the Xilinx ZynqMP SoC model to pass the default system
> memory instead of a NULL value.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] hw/dma/xlnx-zdma Always expect 'dma' link property to be set
  2021-08-19 16:34 ` [PATCH v2 4/4] hw/dma/xlnx-zdma " Philippe Mathieu-Daudé
@ 2021-08-20  9:02   ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2021-08-20  9:02 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, qemu-arm, QEMU Developers, Alistair Francis

On Thu, 19 Aug 2021 at 17:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Simplify by always passing a MemoryRegion property to the device.
> Doing so we can move the AddressSpace field to the device struct,
> removing need for heap allocation.
>
> Update the Xilinx ZynqMP / Versal SoC models to pass the default
> system memory instead of a NULL value.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify
  2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2021-08-19 16:34 ` [PATCH v2 4/4] hw/dma/xlnx-zdma " Philippe Mathieu-Daudé
@ 2021-08-26 13:19 ` Peter Maydell
  4 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2021-08-26 13:19 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Edgar E. Iglesias, qemu-arm, QEMU Developers, Alistair Francis

On Thu, 19 Aug 2021 at 17:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Fix a qdev realization ordering, then simplify a pair of DMA
> devices by always passing a MemoryRegion property to the device.
> Doing so we can move the AddressSpace field to the device struct,
> removing need for heap allocation.



Applied to target-arm.next, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-08-26 13:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19 16:34 [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Philippe Mathieu-Daudé
2021-08-19 16:34 ` [PATCH v2 1/4] hw/arm/xlnx-zynqmp: Realize qspi controller *after* qspi_dma Philippe Mathieu-Daudé
2021-08-19 22:50   ` Alistair Francis
2021-08-19 16:34 ` [PATCH v2 2/4] hw/dma/xlnx_csu_dma: Run trivial checks early in realize() Philippe Mathieu-Daudé
2021-08-19 22:51   ` Alistair Francis
2021-08-19 16:34 ` [PATCH v2 3/4] hw/dma/xlnx_csu_dma: Always expect 'dma' link property to be set Philippe Mathieu-Daudé
2021-08-20  9:01   ` Peter Maydell
2021-08-19 16:34 ` [PATCH v2 4/4] hw/dma/xlnx-zdma " Philippe Mathieu-Daudé
2021-08-20  9:02   ` Peter Maydell
2021-08-26 13:19 ` [PATCH v2 0/4] hw/dma: Always expect 'dma' link property to be set to simplify Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.