On Mon, 3 Mar 2025, Philippe Mathieu-Daudé wrote:
Hi Zoltan,
On 10/2/25 17:03, 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.
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;
I'd rather do like capareg, and add:
DEFINE_PROP_UINT16("norintstsen", _state, norintstsen, 0),
...
I don't see what you mean. capareg does not seem to be set via a property.
Then SoC code sets it:
qdev_prop_set_uint16(dev, "norintstsen", 0x013f);
...
WDYT?
I think it may be overkill to add properties for this if there are no
other vendor or variant that needs this. Also properties are for something
the user may want to set as those can be changed with QEMU command line
and these reset values aren't something the user should change so I think
this patch is the simplest solution now.
Regards,
BALATON Zoltan
