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_lin@aspeedtech.com>
> Acked-by: Cédric Le Goater <clg@redhat.com>

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

  20250108092538.11474-9-shentey@gmail.com/">https://lore.kernel.org/qemu-devel/20250108092538.11474-9-shentey@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;
>