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

Philippe Mathieu-Daudé posted 14 patches 11 months ago
[PATCH v5 09/14] hw/sd/sdhci: Allow SDHCI classes to register their own MemoryRegionOps
Posted by Philippe Mathieu-Daudé 11 months ago
Add MemoryRegionOps as a class property. For now it is only
used by TYPE_IMX_USDHC.
Otherwise the default remains in little endian.

Signed-off-by: Philippe Mathieu-Daudé <philmd@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,
     },
 };
 
-- 
2.47.1


Re: [PATCH v5 09/14] hw/sd/sdhci: Allow SDHCI classes to register their own MemoryRegionOps
Posted by BALATON Zoltan 11 months ago
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 <balaton@eik.bme.hu>

> Signed-off-by: Philippe Mathieu-Daudé <philmd@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,
>     },
> };
>
>