diff mbox series

[v5,04/14] hw/sd/sdhci: Include 'wp-inverted' property in quirk bitmask

Message ID 20250310000620.70120-5-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_INVERTED_WRITE_PROTECT quirk definition.

Replace 'wp_inverted' boolean by a bit in quirk bitmask.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/sd/sdhci.h | 16 ++++++++++------
 hw/arm/aspeed.c       |  2 +-
 hw/sd/sdhci.c         |  6 +++---
 3 files changed, 14 insertions(+), 10 deletions(-)

Comments

BALATON Zoltan March 10, 2025, 1:36 p.m. UTC | #1 
On Mon, 10 Mar 2025, Philippe Mathieu-Daudé wrote:
> Import Linux's SDHCI_QUIRK_INVERTED_WRITE_PROTECT quirk definition.
>
> Replace 'wp_inverted' boolean by a bit in quirk bitmask.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/sd/sdhci.h | 16 ++++++++++------
> hw/arm/aspeed.c       |  2 +-
> hw/sd/sdhci.c         |  6 +++---
> 3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index 096d607f4b7..d2e4f0f0050 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -30,7 +30,14 @@
> #include "hw/sd/sd.h"
> #include "qom/object.h"
>
> -/* SD/MMC host controller state */
> +/*
> + * SD/MMC host controller state
> + *
> + * QEMU interface:
> + *  + 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).
> + */
> struct SDHCIState {
>     /*< private >*/
>     union {
> @@ -99,11 +106,6 @@ struct SDHCIState {
>     uint8_t endianness;
>     uint8_t sd_spec_version;
>     uint8_t uhs_mode;
> -    /*
> -     * Write Protect pin default active low for detecting SD card
> -     * to be protected. Set wp_inverted to invert the signal.
> -     */
> -    bool wp_inverted;
> };
> typedef struct SDHCIState SDHCIState;
>
> @@ -114,6 +116,8 @@ typedef struct SDHCIState SDHCIState;

Now that we have two adjust comment above here to say "These defines"

> enum {
>     /* Controller does not provide transfer-complete interrupt when not busy. */
>     SDHCI_QUIRK_NO_BUSY_IRQ                     = 14,
> +    /* Controller reports inverted write-protect state */
> +    SDHCI_QUIRK_INVERTED_WRITE_PROTECT          = 16,
> };

and I'd say keep this defines that also matches what Linux has.

> #define TYPE_PCI_SDHCI "sdhci-pci"
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 98bf071139b..daee2376d50 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -412,7 +412,7 @@ static void aspeed_machine_init(MachineState *machine)
>     if (amc->sdhci_wp_inverted) {
>         for (i = 0; i < bmc->soc->sdhci.num_slots; i++) {
>             object_property_set_bool(OBJECT(&bmc->soc->sdhci.slots[i]),
> -                                     "wp-inverted", true, &error_abort);
> +                                     "wp-inverted-quirk", true, &error_abort);

Why rename it? That would break command lines that use this.

Regards,
BALATON Zoltan

>         }
>     }
>     if (machine->kernel_filename) {
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 1dc942a0e06..19c600d5bfc 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -274,7 +274,7 @@ static void sdhci_set_readonly(DeviceState *dev, bool level)
> {
>     SDHCIState *s = (SDHCIState *)dev;
>
> -    if (s->wp_inverted) {
> +    if (s->quirks & BIT(SDHCI_QUIRK_INVERTED_WRITE_PROTECT)) {
>         level = !level;
>     }
>
> @@ -1555,12 +1555,12 @@ void sdhci_common_class_init(ObjectClass *klass, const void *data)
>
> 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_LINK("dma", SDHCIState,
>                      dma_mr, TYPE_MEMORY_REGION, MemoryRegion *),
> -    DEFINE_PROP_BOOL("wp-inverted", SDHCIState,
> -                     wp_inverted, false),
> };
>
> static void sdhci_sysbus_init(Object *obj)
>
diff mbox series

Patch

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 096d607f4b7..d2e4f0f0050 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -30,7 +30,14 @@ 
 #include "hw/sd/sd.h"
 #include "qom/object.h"
 
-/* SD/MMC host controller state */
+/*
+ * SD/MMC host controller state
+ *
+ * QEMU interface:
+ *  + 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).
+ */
 struct SDHCIState {
     /*< private >*/
     union {
@@ -99,11 +106,6 @@  struct SDHCIState {
     uint8_t endianness;
     uint8_t sd_spec_version;
     uint8_t uhs_mode;
-    /*
-     * Write Protect pin default active low for detecting SD card
-     * to be protected. Set wp_inverted to invert the signal.
-     */
-    bool wp_inverted;
 };
 typedef struct SDHCIState SDHCIState;
 
@@ -114,6 +116,8 @@  typedef struct SDHCIState SDHCIState;
 enum {
     /* Controller does not provide transfer-complete interrupt when not busy. */
     SDHCI_QUIRK_NO_BUSY_IRQ                     = 14,
+    /* Controller reports inverted write-protect state */
+    SDHCI_QUIRK_INVERTED_WRITE_PROTECT          = 16,
 };
 
 #define TYPE_PCI_SDHCI "sdhci-pci"
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 98bf071139b..daee2376d50 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -412,7 +412,7 @@  static void aspeed_machine_init(MachineState *machine)
     if (amc->sdhci_wp_inverted) {
         for (i = 0; i < bmc->soc->sdhci.num_slots; i++) {
             object_property_set_bool(OBJECT(&bmc->soc->sdhci.slots[i]),
-                                     "wp-inverted", true, &error_abort);
+                                     "wp-inverted-quirk", true, &error_abort);
         }
     }
     if (machine->kernel_filename) {
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 1dc942a0e06..19c600d5bfc 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -274,7 +274,7 @@  static void sdhci_set_readonly(DeviceState *dev, bool level)
 {
     SDHCIState *s = (SDHCIState *)dev;
 
-    if (s->wp_inverted) {
+    if (s->quirks & BIT(SDHCI_QUIRK_INVERTED_WRITE_PROTECT)) {
         level = !level;
     }
 
@@ -1555,12 +1555,12 @@  void sdhci_common_class_init(ObjectClass *klass, const void *data)
 
 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_LINK("dma", SDHCIState,
                      dma_mr, TYPE_MEMORY_REGION, MemoryRegion *),
-    DEFINE_PROP_BOOL("wp-inverted", SDHCIState,
-                     wp_inverted, false),
 };
 
 static void sdhci_sysbus_init(Object *obj)