diff mbox series

[v5,05/14] hw/sd/sdhci: Include 'pending-insert-quirk' property in quirk bitmask

Message ID 20250310000620.70120-6-philmd@linaro.org
State New
Headers show
Series hw/sd/sdhci: Set reset value of interrupt registers | expand

Commit Message

Philippe Mathieu-Daudé March 10, 2025, 12:06 a.m. UTC 
Import Linux's SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET quirk definition.

Replace 'pending_insert_quirk' boolean (originally introduce in commit
0a7ac9f9e72 "sdhci: quirk property for card insert interrupt status
on Raspberry Pi") by a bit in quirk bitmask.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sdhci.h | 5 ++++-
 hw/sd/sdhci.c         | 8 ++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

Comments

BALATON Zoltan March 10, 2025, 1:39 p.m. UTC | #1 
On Mon, 10 Mar 2025, Philippe Mathieu-Daudé wrote:
> Import Linux's SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET quirk definition.
>
> Replace 'pending_insert_quirk' boolean (originally introduce in commit
> 0a7ac9f9e72 "sdhci: quirk property for card insert interrupt status
> on Raspberry Pi") by a bit in quirk bitmask.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/sd/sdhci.h | 5 ++++-
> hw/sd/sdhci.c         | 8 ++++----
> 2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index d2e4f0f0050..2e6e719df7b 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -34,6 +34,8 @@
>  * SD/MMC host controller state
>  *
>  * QEMU interface:
> + *  + QOM property "pending-insert-quirk" re-enables pending "card inserted"
> + *    IRQ after reset (used by the Raspberry Pi controllers).
>  *  + QOM property "wp-inverted-quirk" inverts the Write Protect pin
>  *    polarity (by default the polarity is active low for detecting SD
>  *    card to be protected).
> @@ -101,7 +103,6 @@ struct SDHCIState {
>     /* RO Host Controller Version Register always reads as 0x2401 */
>
>     /* Configurable properties */
> -    bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
>     uint32_t quirks;
>     uint8_t endianness;
>     uint8_t sd_spec_version;
> @@ -118,6 +119,8 @@ enum {
>     SDHCI_QUIRK_NO_BUSY_IRQ                     = 14,
>     /* Controller reports inverted write-protect state */
>     SDHCI_QUIRK_INVERTED_WRITE_PROTECT          = 16,
> +    /* Controller losing signal/interrupt enable states after reset */
> +    SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET        = 19,
> };
>
> #define TYPE_PCI_SDHCI "sdhci-pci"
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 19c600d5bfc..d1b1b187874 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -320,7 +320,7 @@ static void sdhci_poweron_reset(DeviceState *dev)
>
>     sdhci_reset(s);
>
> -    if (s->pending_insert_quirk) {
> +    if (s->quirks & BIT(SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)) {
>         s->pending_insert_state = true;
>     }
> }
> @@ -1307,7 +1307,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>          * appears when first enabled after power on
>          */
>         if ((s->norintstsen & SDHC_NISEN_INSERT) && s->pending_insert_state) {
> -            assert(s->pending_insert_quirk);
> +            assert(s->quirks & BIT(SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET));
>             s->norintsts |= SDHC_NIS_INSERT;
>             s->pending_insert_state = false;
>         }
> @@ -1557,8 +1557,8 @@ static const Property sdhci_sysbus_properties[] = {
>     DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
>     DEFINE_PROP_BIT("wp-inverted-quirk", SDHCIState, quirks,
>                     SDHCI_QUIRK_INVERTED_WRITE_PROTECT, false),
> -    DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
> -                     false),
> +    DEFINE_PROP_BIT("pending-insert-quirk", SDHCIState, quirks,
> +                    SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET, false),

Now I see why you need bit defines instead of mask value. Other comments 
about renaming and adjusting comment still apply. I'm not sure if adding 
and enum for bit numbers and keeping the defines for the QUIRK to avoid 
mistake of forgetting BIT at usage might be better. Otherwise this looks 
good.

Regards,
BALATON Zoltan

>     DEFINE_PROP_LINK("dma", SDHCIState,
>                      dma_mr, TYPE_MEMORY_REGION, MemoryRegion *),
> };
>
diff mbox series

Patch

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index d2e4f0f0050..2e6e719df7b 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -34,6 +34,8 @@ 
  * SD/MMC host controller state
  *
  * QEMU interface:
+ *  + QOM property "pending-insert-quirk" re-enables pending "card inserted"
+ *    IRQ after reset (used by the Raspberry Pi controllers).
  *  + QOM property "wp-inverted-quirk" inverts the Write Protect pin
  *    polarity (by default the polarity is active low for detecting SD
  *    card to be protected).
@@ -101,7 +103,6 @@  struct SDHCIState {
     /* RO Host Controller Version Register always reads as 0x2401 */
 
     /* Configurable properties */
-    bool pending_insert_quirk; /* Quirk for Raspberry Pi card insert int */
     uint32_t quirks;
     uint8_t endianness;
     uint8_t sd_spec_version;
@@ -118,6 +119,8 @@  enum {
     SDHCI_QUIRK_NO_BUSY_IRQ                     = 14,
     /* Controller reports inverted write-protect state */
     SDHCI_QUIRK_INVERTED_WRITE_PROTECT          = 16,
+    /* Controller losing signal/interrupt enable states after reset */
+    SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET        = 19,
 };
 
 #define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 19c600d5bfc..d1b1b187874 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -320,7 +320,7 @@  static void sdhci_poweron_reset(DeviceState *dev)
 
     sdhci_reset(s);
 
-    if (s->pending_insert_quirk) {
+    if (s->quirks & BIT(SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)) {
         s->pending_insert_state = true;
     }
 }
@@ -1307,7 +1307,7 @@  sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
          * appears when first enabled after power on
          */
         if ((s->norintstsen & SDHC_NISEN_INSERT) && s->pending_insert_state) {
-            assert(s->pending_insert_quirk);
+            assert(s->quirks & BIT(SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET));
             s->norintsts |= SDHC_NIS_INSERT;
             s->pending_insert_state = false;
         }
@@ -1557,8 +1557,8 @@  static const Property sdhci_sysbus_properties[] = {
     DEFINE_SDHCI_COMMON_PROPERTIES(SDHCIState),
     DEFINE_PROP_BIT("wp-inverted-quirk", SDHCIState, quirks,
                     SDHCI_QUIRK_INVERTED_WRITE_PROTECT, false),
-    DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
-                     false),
+    DEFINE_PROP_BIT("pending-insert-quirk", SDHCIState, quirks,
+                    SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET, false),
     DEFINE_PROP_LINK("dma", SDHCIState,
                      dma_mr, TYPE_MEMORY_REGION, MemoryRegion *),
 };