Re: [PATCH v3 2/3] hw/sd/sdhci: Introduce a new Write Protected pin inverted property

[Cédric Le Goater]

Jamin,

+Bernhard

On 11/14/24 10:48, Jamin Lin wrote:

The Write Protect pin of SDHCI model is default active low to match the SDHCI
spec. So, write enable the bit 19 should be 1 and write protected the bit 19
should be 0 at the Present State Register (0x24). However, some boards are
design Write Protected pin active high. In other words, write enable the bit 19
should be 0 and write protected the bit 19 should be 1 at the
Present State Register (0x24). To support it, introduces a new "wp-inverted"
property and set it false by default.

Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
Acked-by: Cédric Le Goater <c...@redhat.com>

When you have some time, could you please check that this change fits
the aspeed needs :

  https://lore.kernel.org/qemu-devel/20250108092538.11474-9-shen...@gmail.com/

It should be merged shortly.

Thanks,

C.

---
  hw/sd/sdhci.c         | 6 ++++++
  include/hw/sd/sdhci.h | 5 +++++
  2 files changed, 11 insertions(+)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 37875c02c3..06d1e24086 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -274,6 +274,10 @@ static void sdhci_set_readonly(DeviceState *dev, bool 
level)
  {
      SDHCIState *s = (SDHCIState *)dev;

+ if (s->wp_inverted) {

+        level = !level;
+    }
+
      if (level) {
          s->prnsts &= ~SDHC_WRITE_PROTECT;
      } else {
@@ -1550,6 +1554,8 @@ static Property sdhci_sysbus_properties[] = {
                       false),
      DEFINE_PROP_LINK("dma", SDHCIState,
                       dma_mr, TYPE_MEMORY_REGION, MemoryRegion *),
+    DEFINE_PROP_BOOL("wp-inverted", SDHCIState,
+                     wp_inverted, false),
      DEFINE_PROP_END_OF_LIST(),
  };

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h

index 6cd2822f1d..38c08e2859 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -100,6 +100,11 @@ struct SDHCIState {
      uint8_t sd_spec_version;
      uint8_t uhs_mode;
      uint8_t vendor;        /* For vendor specific functionality */
+    /*
+     * 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;