On Mon, 3 Mar 2025, Bernhard Beschow wrote:
Am 1. März 2025 16:02:05 UTC schrieb BALATON Zoltan <bala...@eik.bme.hu>:
On Mon, 10 Feb 2025, BALATON Zoltan wrote:
The interrupt enable registers are not reset to 0 on Freescale eSDHC
but some bits are enabled on reset. At least some U-Boot versions seem
to expect this and not initialise these registers before expecting
interrupts. Use existing vendor property for Freescale eSDHC and set
the reset value of the interrupt registers to match Freescale
documentation.
Ping?
Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu>
---
v2: Restrict to e500. Adding a reset method in a subclass does not
work because the common reset function is called directly on register
write from the guest but there's already provision for vendor specific
behaviour which can be used to restrict this to Freescale SoCs.
hw/ppc/e500.c | 1 +
hw/sd/sdhci.c | 4 ++++
include/hw/sd/sdhci.h | 1 +
3 files changed, 6 insertions(+)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 26933e0457..560eb42a12 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1044,6 +1044,7 @@ void ppce500_init(MachineState *machine)
dev = qdev_new(TYPE_SYSBUS_SDHCI);
qdev_prop_set_uint8(dev, "sd-spec-version", 2);
qdev_prop_set_uint8(dev, "endianness", DEVICE_BIG_ENDIAN);
+ qdev_prop_set_uint8(dev, "vendor", SDHCI_VENDOR_FSL);
s = SYS_BUS_DEVICE(dev);
sysbus_realize_and_unref(s, &error_fatal);
sysbus_connect_irq(s, 0, qdev_get_gpio_in(mpicdev, MPC85XX_ESDHC_IRQ));
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 99dd4a4e95..afa3c6d448 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -307,6 +307,10 @@ static void sdhci_reset(SDHCIState *s)
s->data_count = 0;
s->stopped_state = sdhc_not_stopped;
s->pending_insert_state = false;
+ if (s->vendor == SDHCI_VENDOR_FSL) {
+ s->norintstsen = 0x013f;
+ s->errintstsen = 0x117f;
+ }
}
static void sdhci_poweron_reset(DeviceState *dev)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 38c08e2859..f722d8eb1c 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -110,6 +110,7 @@ typedef struct SDHCIState SDHCIState;
#define SDHCI_VENDOR_NONE 0
#define SDHCI_VENDOR_IMX 1
+#define SDHCI_VENDOR_FSL 2
From an i.MX point of view the sole "FSL" postfix seems confusing since
these SoCs are from the same vendor. What about "FSL_ESDHC"?
That's too long and not a vendor. How about FSL_IMX instead? But maybe
it's not really a problem as IMX is already a separate define and other
devices use the FSL prefix so maybe it's not that confusing to worth
changing.
Regards,
BALATON Zoltan
