Hi Bernhard,
On 18/10/22 23:01, Bernhard Beschow wrote:
Will allow e500 boards to access SD cards using just their own devices.
Signed-off-by: Bernhard Beschow <shen...@gmail.com>
---
hw/sd/sdhci.c | 120 +++++++++++++++++++++++++++++++++++++++++-
include/hw/sd/sdhci.h | 3 ++
2 files changed, 122 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 306070c872..8d8ad9ff24 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1369,6 +1369,7 @@ void sdhci_initfn(SDHCIState *s)
s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer,
s);
s->io_ops = &sdhci_mmio_ops;
+ s->io_registers_map_size = SDHC_REGISTERS_MAP_SIZE;
}
void sdhci_uninitfn(SDHCIState *s)
@@ -1392,7 +1393,7 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
s->fifo_buffer = g_malloc0(s->buf_maxsz);
memory_region_init_io(&s->iomem, OBJECT(s), s->io_ops, s, "sdhci",
- SDHC_REGISTERS_MAP_SIZE);
+ s->io_registers_map_size);
I don't think we want to change this region size. [see below]
void sdhci_common_unrealize(SDHCIState *s)
@@ -1575,6 +1576,122 @@ static const TypeInfo sdhci_bus_info = {
.class_init = sdhci_bus_class_init,
};
+/* --- qdev Freescale eSDHC --- */
+
+/* Watermark Level Register */
+#define ESDHC_WML 0x44
+
+/* Control Register for DMA transfer */
+#define ESDHC_DMA_SYSCTL 0x40c
+
+#define ESDHC_REGISTERS_MAP_SIZE 0x410
My preferred approach would be to create a container region with a
size of ESDHC_REGISTERS_MAP_SIZE. Map the SDHC_REGISTERS_MAP region
in the container at offset 0, priority -1. Add 2 register regions
for ESDHC_WML and ESDHC_DMA_SYSCTL, and map them with priority 1 in
the container. ...
+static uint64_t esdhci_read(void *opaque, hwaddr offset, unsigned size)
+{
+ uint64_t ret;
+
+ switch (offset) {
+ case SDHC_SYSAD:
+ case SDHC_BLKSIZE:
+ case SDHC_ARGUMENT:
+ case SDHC_TRNMOD:
+ case SDHC_RSPREG0:
+ case SDHC_RSPREG1:
+ case SDHC_RSPREG2:
+ case SDHC_RSPREG3:
+ case SDHC_BDATA:
+ case SDHC_PRNSTS:
+ case SDHC_HOSTCTL:
+ case SDHC_CLKCON:
+ case SDHC_NORINTSTS:
+ case SDHC_NORINTSTSEN:
+ case SDHC_NORINTSIGEN:
+ case SDHC_ACMD12ERRSTS:
+ case SDHC_CAPAB:
+ case SDHC_SLOT_INT_STATUS:
+ ret = sdhci_read(opaque, offset, size);
+ break;
... Then you don't need these cases.
+ case ESDHC_WML:
+ case ESDHC_DMA_SYSCTL:
+ ret = 0;
+ qemu_log_mask(LOG_UNIMP, "ESDHC rd @0x%02" HWADDR_PRIx
+ " not implemented\n", offset);
But then I realize you only treat these 2 registers as UNIMP.
So now, I'd create 1 UNIMP region for ESDHC_WML and map it
into SDHC_REGISTERS_MAP (s->iomem) with priority 1, and add
another UNIMP region of ESDHC_REGISTERS_MAP_SIZE -
SDHC_REGISTERS_MAP_SIZE (= 0x310) and map it normally at offset
0x100 (SDHC_REGISTERS_MAP_SIZE). Look at create_unimp() in
hw/arm/bcm2835_peripherals.c.
But the ESDHC_WML register has address 0x44 and fits inside the
SDHC_REGISTERS_MAP region, so likely belong there. 0x44 is the
upper part of the SDHC_CAPAB register. These bits are undefined
on the spec v2, which I see you are setting in esdhci_init().
So this register should already return 0, otherwise we have
a bug. Thus we don't need to handle this ESDHC_WML particularly.
And your model is reduced to handling create_unimp() in esdhci_realize().
Am I missing something?
Regards,
Phil.
