Re: [PATCH v4 09/14] hw/sd/sdhci: Unify default MemoryRegionOps

[Philippe Mathieu-Daudé]

On 9/3/25 10:27, Bernhard Beschow wrote:

Am 8. März 2025 21:36:35 UTC schrieb "Philippe Mathieu-Daudé" 
<phi...@linaro.org>:

Note, sdhci_mmio_le_ops[] was missing .impl.access_size = 4.

Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
hw/sd/sdhci.c | 46 ++++++++++++++++++++--------------------------
1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index a2e7162e289..23af3958a1d 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1372,30 +1372,22 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
                        value >> shift, value >> shift);
}

-static const MemoryRegionOps sdhci_mmio_le_ops = {
-    .read = sdhci_read,
-    .write = sdhci_write,
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 4,
-        .unaligned = false
+static const MemoryRegionOps sdhci_mmio_ops[2] = {
+    [0 ... 1] = {
+        .read = sdhci_read,
+        .write = sdhci_write,
+        .impl = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+        .valid = {
+            .min_access_size = 1,
+            .max_access_size = 4,
+            .unaligned = false
+        },
     },
-    .endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-static const MemoryRegionOps sdhci_mmio_be_ops = {
-    .read = sdhci_read,
-    .write = sdhci_write,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-    .valid = {
-        .min_access_size = 1,
-        .max_access_size = 4,
-        .unaligned = false
-    },
-    .endianness = DEVICE_BIG_ENDIAN,
+    [0].endianness = DEVICE_LITTLE_ENDIAN,
+    [1].endianness = DEVICE_BIG_ENDIAN,
};

We introduced sdhci_mmio_be_ops and the endianness property specifically for 
the e500 machine. We always lied about the endianness property to be 
implemented for all SDHC types, e.g. USDHC doesn't implement it. Since you'll 
introduce a dedicated FSL_ESDHC type a few commits later, along with its own 
MemoryRegionOps, I think we should drop the endianness property and 
sdhci_mmio_ops doesn't need to become an array. Dropping the endianness 
property also results in avoiding the error conditions you have to deal with in 
the realize method.

Great idea!