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

[Cédric Le Goater]

On 11/4/24 04:21, 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>

---
  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 db7d547156..c675543873 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -275,6 +275,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 {
@@ -1551,6 +1555,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..25ad9ed778 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 true inverted the signal.

In case you respin, may be you could change the last sentence to :
"Set wp_inverted to invert the signal."


Thanks,

C.

+     */
+    bool wp_inverted;
  };
  typedef struct SDHCIState SDHCIState;