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"?
Best regards,
Bernhard
>>
>> /*
>> * Controller does not provide transfer-complete interrupt when not
>>
