[Qemu-devel] [PATCH] sdhci: Revert "add optional quirk property to disable card insertion/removal interrupts"

[Qemu-devel] [PATCH] sdhci: Revert "add optional quirk property to disable card insertion/removal interrupts"

[Andrew Baumann]

This reverts commit 723697551a7e926abe7d3c7f2966012b8075143d.

This change was poorly tested on my part. It squelched card insertion
interrupts on reset, but that was not necessary because sdhci_reset()
clears all the registers (via the call to memset), so the subsequent
sdhci_insert_eject_cb() call never sees the card insert interrupt
enabled. However, not calling the insert_eject_cb results in prnsts
remaining 0, when it actually needs to be updated to indicate card
presence and R/O status.

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
 hw/sd/sdhci.c         | 5 +----
 include/hw/sd/sdhci.h | 1 -
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 7acb4d7..dd83e89 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -193,9 +193,7 @@ static void sdhci_reset(SDHCIState *s)
      * initialization */
     memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
 
-    if (!s->noeject_quirk) {
-        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
-    }
+    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
     s->data_count = 0;
     s->stopped_state = sdhc_not_stopped;
 }
@@ -1278,7 +1276,6 @@ static Property sdhci_sysbus_properties[] = {
     DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
             SDHC_CAPAB_REG_DEFAULT),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
-    DEFINE_PROP_BOOL("noeject-quirk", SDHCIState, noeject_quirk, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index ffd1f80..e78d938 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -77,7 +77,6 @@ typedef struct SDHCIState {
     uint32_t buf_maxsz;
     uint16_t data_count;   /* current element in FIFO buffer */
     uint8_t  stopped_state;/* Current SDHC state */
-    bool     noeject_quirk;/* Quirk to disable card insert/remove interrupts */
     /* Buffer Data Port Register - virtual access point to R and W buffers */
     /* Software Reset Register - always reads as 0 */
     /* Force Event Auto CMD12 Error Interrupt Reg - write only */
-- 
2.5.3

Re: [Qemu-devel] [PATCH] sdhci: Revert "add optional quirk property to disable card insertion/removal interrupts"

[Peter Crosthwaite]

On Thu, Dec 31, 2015 at 1:10 PM, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> This reverts commit 723697551a7e926abe7d3c7f2966012b8075143d.
>
> This change was poorly tested on my part. It squelched card insertion
> interrupts on reset, but that was not necessary because sdhci_reset()
> clears all the registers (via the call to memset), so the subsequent
> sdhci_insert_eject_cb() call never sees the card insert interrupt
> enabled. However, not calling the insert_eject_cb results in prnsts
> remaining 0, when it actually needs to be updated to indicate card
> presence and R/O status.
>

Do both bits 16 and 18 of prnsts work in real hw? A bug in the
debouncing logic (which I think is the only difference between the
two) could explain some of these strange symptoms.

Regards,
Peter

> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
>  hw/sd/sdhci.c         | 5 +----
>  include/hw/sd/sdhci.h | 1 -
>  2 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 7acb4d7..dd83e89 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -193,9 +193,7 @@ static void sdhci_reset(SDHCIState *s)
>       * initialization */
>      memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
>
> -    if (!s->noeject_quirk) {
> -        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> -    }
> +    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
>      s->data_count = 0;
>      s->stopped_state = sdhc_not_stopped;
>  }
> @@ -1278,7 +1276,6 @@ static Property sdhci_sysbus_properties[] = {
>      DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
>              SDHC_CAPAB_REG_DEFAULT),
>      DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
> -    DEFINE_PROP_BOOL("noeject-quirk", SDHCIState, noeject_quirk, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index ffd1f80..e78d938 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -77,7 +77,6 @@ typedef struct SDHCIState {
>      uint32_t buf_maxsz;
>      uint16_t data_count;   /* current element in FIFO buffer */
>      uint8_t  stopped_state;/* Current SDHC state */
> -    bool     noeject_quirk;/* Quirk to disable card insert/remove interrupts */
>      /* Buffer Data Port Register - virtual access point to R and W buffers */
>      /* Software Reset Register - always reads as 0 */
>      /* Force Event Auto CMD12 Error Interrupt Reg - write only */
> --
> 2.5.3
>