From: YannickV <Y.Vossen@beckhoff.com>

The CX7200 has one SD controller connected to address 0xE0101000.
The controller connected to address 0xE0100000 can be removed.

Signed-off-by: Yannick Voßen <y.vossen@beckhoff.com>
---
 hw/arm/beckhoff_CX7200.c | 48 ++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/hw/arm/beckhoff_CX7200.c b/hw/arm/beckhoff_CX7200.c
index 89466cfdd8..bf3c66e5a4 100644
--- a/hw/arm/beckhoff_CX7200.c
+++ b/hw/arm/beckhoff_CX7200.c
@@ -207,11 +207,13 @@ static void beckhoff_cx7200_init(MachineState *machine)
     CX7200MachineState *cx7200_machine = CX7200_MACHINE(machine);
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
-    DeviceState *dev, *slcr;
+    DeviceState *carddev, *dev, *slcr;
     SysBusDevice *busdev;
     qemu_irq pic[64];
     int n;
     unsigned int smp_cpus = machine->smp.cpus;
+    DriveInfo *di;
+    BlockBackend *blk;
 
     /* max 2GB ram */
     if (machine->ram_size > 2 * GiB) {
@@ -318,33 +320,25 @@ static void beckhoff_cx7200_init(MachineState *machine)
     gem_init(0xE000B000, pic[54 - IRQ_OFFSET]);
     gem_init(0xE000C000, pic[77 - IRQ_OFFSET]);
 
-    for (n = 0; n < 2; n++) {
-        int hci_irq = n ? 79 : 56;
-        hwaddr hci_addr = n ? 0xE0101000 : 0xE0100000;
-        DriveInfo *di;
-        BlockBackend *blk;
-        DeviceState *carddev;
+    /*
+     * Compatible with:
+     * - SD Host Controller Specification Version 2.0 Part A2
+     * - SDIO Specification Version 2.0
+     * - MMC Specification Version 3.31
+     */
+    dev = qdev_new(TYPE_SYSBUS_SDHCI);
+    qdev_prop_set_uint8(dev, "sd-spec-version", 2);
+    qdev_prop_set_uint64(dev, "capareg", ZYNQ_SDHCI_CAPABILITIES);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79 - IRQ_OFFSET]);
 
-        /*
-         * Compatible with:
-         * - SD Host Controller Specification Version 2.0 Part A2
-         * - SDIO Specification Version 2.0
-         * - MMC Specification Version 3.31
-         */
-        dev = qdev_new(TYPE_SYSBUS_SDHCI);
-        qdev_prop_set_uint8(dev, "sd-spec-version", 2);
-        qdev_prop_set_uint64(dev, "capareg", ZYNQ_SDHCI_CAPABILITIES);
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-        sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, hci_addr);
-        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[hci_irq - IRQ_OFFSET]);
-
-        di = drive_get(IF_SD, 0, n);
-        blk = di ? blk_by_legacy_dinfo(di) : NULL;
-        carddev = qdev_new(TYPE_SD_CARD);
-        qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
-        qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
-                               &error_fatal);
-    }
+    di = drive_get(IF_SD, 0, 0);
+    blk = di ? blk_by_legacy_dinfo(di) : NULL;
+    carddev = qdev_new(TYPE_SD_CARD);
+    qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal);
+    qdev_realize_and_unref(carddev, qdev_get_child_bus(dev, "sd-bus"),
+                       &error_fatal);
 
     dev = qdev_new(TYPE_ZYNQ_XADC);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-- 
2.49.0

On Tue, 18 Mar 2025 at 13:11, Corvin Köhne <corvin.koehne@gmail.com> wrote:
>
> From: YannickV <Y.Vossen@beckhoff.com>
>
> The CX7200 has one SD controller connected to address 0xE0101000.
> The controller connected to address 0xE0100000 can be removed.

Hi -- I see Edgar has done some review on the first part
of this patchset, but looking quickly over the second half
I notice that it has quite a few commits like this that
are removing/changing code that was just added in an
earlier patch in the series.

Please don't structure the patchset this way. If the board
has only one SD controller, then send a patchset that puts
in one SD controller, not one that puts in 2 and then
removes 1 of them.

Similarly for most of these other "remove" or "adjust" patches.

(If you want to add a board in a way that breaks it down into
smaller patches for easier review, one way to do this is
that you start with a "skeleton" version of the board with
e.g. just the CPU and one or two devices, and then in
later patches you add more devices. But in this case the
board code looks small enough that that's not necessary.)

thanks
-- PMM