Re: [PATCH v2] hw/sd/sdhci: Set reset value of interrupt registers

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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 <balaton@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),
   ...

Then SoC code sets it:

   qdev_prop_set_uint16(dev, "norintstsen", 0x013f);
   ...

WDYT?

> +    }
>   }