Re: [Qemu-devel] [PATCH v3 2/2] sdhci: add quirk property for card insert interrupt status on Raspberry Pi

[Peter Maydell]

On 24 February 2016 at 22:47, Andrew Baumann
<address@hidden> wrote:
> This quirk is a workaround for the following hardware behaviour, on
> which UEFI (specifically, the bootloader for Windows on Pi2) depends:
>
> 1. at boot with an SD card present, the interrupt status/enable
>    registers are initially zero
> 2. upon enabling it in the interrupt enable register, the card insert
>    bit in the interrupt status register is immediately set
> 3. after a subsequent controller reset, the card insert interrupt does
>    not fire, even if enabled in the interrupt enable register
>
> The implementation uses a pending_insert bool, which can be set via a
> property (enabling the quirk) and is cleared and remains clear once
> the interrupt has been delivered.
>
> Signed-off-by: Andrew Baumann <address@hidden>
> ---
> There's a fairly extensive discussion of the hardware behaviour that
> this patch seeks to model in the thread at:
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00605.html
>
> v3: changed to use subsection for vmstate, to preserve backward
> compatibility
>
> v2: changed implementation to use pending_insert bool rather than
> masking norintsts at read time, since the older version diverges from
> actual hardware behaviour when an interrupt is masked without being
> acked
>
>  hw/sd/sdhci.c         | 33 ++++++++++++++++++++++++++++++++-
>  include/hw/sd/sdhci.h |  1 +
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index f175b30..db34d78 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -204,6 +204,7 @@ static void sdhci_reset(SDHCIState *s)
>
>      s->data_count = 0;
>      s->stopped_state = sdhc_not_stopped;
> +    s->pending_insert = false;

You seem to be trying to use this both to store the device
property value and as the current state of the device.
I don't think that will work, because if we do a system
reset then the device state should go back to what it was
when QEMU was first started (meaning it goes back to whatever
the property value was set to), and at the moment you have
no mechanism for that.

The patch seems OK otherwise (not having looked into the
depths of the hardware behaviour).

thanks
-- PMM