| Message ID | 20241104032104.2784183-3-jamin_lin@aspeedtech.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce a new Write Protected pin inverted property | expand |
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_lin@aspeedtech.com> Acked-by: Cédric Le Goater <clg@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; >
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_lin@aspeedtech.com> > > Acked-by: Cédric Le Goater <clg@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; > >
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. + */ + bool wp_inverted; }; typedef struct SDHCIState SDHCIState;
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> --- hw/sd/sdhci.c | 6 ++++++ include/hw/sd/sdhci.h | 5 +++++ 2 files changed, 11 insertions(+)