Re: [PATCH v1 5/8] hw/sd/sdhci: Introduce a new Write Protected pin inverted property

[Andrew Jeffery <andrew@codeconstruct.com.au>]

On Tue, 2024-10-29 at 17:17 +0800, 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 board
> 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_invert"
> property and set it false by default.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.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..bdf5cbfb80 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_invert) {
> +        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-invert", SDHCIState,

Can we line the name up with the mmc-controller devicetree binding
("wp-inverted")?

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/mmc-controller.yaml#n71

Andrew

> +                     wp_invert, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index 6cd2822f1d..d68f4788e7 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_invert to true inverted the signal.
> +     */
> +    bool wp_invert;
>  };
>  typedef struct SDHCIState SDHCIState;
>