Hi Cedric,
>
> 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 for your review.
Will update it.
Jamin
> Thanks,
>
> C.
>
>
> > + */
> > + bool wp_inverted;
> > };
> > typedef struct SDHCIState SDHCIState;
> >
