Re: [PATCH v5 09/14] hw/sd/sdhci: Allow SDHCI classes to register their own MemoryRegionOps

[BALATON Zoltan]

On Mon, 10 Mar 2025, Philippe Mathieu-Daudé wrote:
Add MemoryRegionOps as a class property. For now it is only
used by TYPE_IMX_USDHC.
Otherwise the default remains in little endian.

I still don't see why you need to do this via the class and not just set 
it init or realize as it's done already but it should work so

Reviewed-by: BALATON Zoltan <bala...@eik.bme.hu>

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
include/hw/sd/sdhci.h |  1 +
hw/sd/sdhci.c         | 22 ++++++++--------------
2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index dfa0c214036..108bc1993c6 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -117,6 +117,7 @@ struct SDHCIClass {
        SysBusDeviceClass sbd_parent_class;
    };

+    const MemoryRegionOps *io_ops;
    uint32_t quirks;
    uint64_t iomem_size;
};
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 3467385490d..6868bf68285 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1428,8 +1428,6 @@ void sdhci_initfn(SDHCIState *s)
    s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                     sdhci_data_transfer, s);
    s->quirks = s->sc->quirks;
-
-    s->io_ops = &sdhci_mmio_le_ops;
}

void sdhci_uninitfn(SDHCIState *s)
@@ -1447,6 +1445,7 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
    SDHCIClass *sc = s->sc;
    const char *class_name = object_get_typename(OBJECT(s));

+    s->io_ops = sc->io_ops ?: &sdhci_mmio_le_ops;
    switch (s->endianness) {
    case DEVICE_LITTLE_ENDIAN:
        /* s->io_ops is little endian by default */
@@ -1890,17 +1889,11 @@ static const MemoryRegionOps usdhc_mmio_ops = {
    .endianness = DEVICE_LITTLE_ENDIAN,
};

-static void imx_usdhc_init(Object *obj)
-{
-    SDHCIState *s = SYSBUS_SDHCI(obj);
-
-    s->io_ops = &usdhc_mmio_ops;
-}
-
static void imx_usdhc_class_init(ObjectClass *oc, void *data)
{
    SDHCIClass *sc = SYSBUS_SDHCI_CLASS(oc);

+    sc->io_ops = &usdhc_mmio_ops;
    sc->quirks = BIT(SDHCI_QUIRK_NO_BUSY_IRQ);

    sdhci_common_class_init(oc, data);
@@ -1957,11 +1950,13 @@ static const MemoryRegionOps sdhci_s3c_mmio_ops = {
    .endianness = DEVICE_LITTLE_ENDIAN,
};

-static void sdhci_s3c_init(Object *obj)
+static void sdhci_s3c_class_init(ObjectClass *oc, void *data)
{
-    SDHCIState *s = SYSBUS_SDHCI(obj);
+    SDHCIClass *sc = SYSBUS_SDHCI_CLASS(oc);

-    s->io_ops = &sdhci_s3c_mmio_ops;
+    sc->io_ops = &sdhci_s3c_mmio_ops;
+
+    sdhci_common_class_init(oc, data);
}

static const TypeInfo sdhci_types[] = {
@@ -1983,13 +1978,12 @@ static const TypeInfo sdhci_types[] = {
    {
        .name = TYPE_IMX_USDHC,
        .parent = TYPE_SYSBUS_SDHCI,
-        .instance_init = imx_usdhc_init,
        .class_init = imx_usdhc_class_init,
    },
    {
        .name = TYPE_S3C_SDHCI,
        .parent = TYPE_SYSBUS_SDHCI,
-        .instance_init = sdhci_s3c_init,
+        .class_init = sdhci_s3c_class_init,
    },
};