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 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;
--
2.34.1
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 :
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;
>
Hi Cedric,
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Tuesday, January 21, 2025 6:39 PM
> To: Jamin Lin <jamin_lin@aspeedtech.com>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; Philippe Mathieu-Daudé <philmd@linaro.org>;
> Bin Meng <bmeng.cn@gmail.com>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>; open list:SD (Secure Card)
> <qemu-block@nongnu.org>; Bernhard Beschow <shentey@gmail.com>
> Cc: Troy Lee <troy_lee@aspeedtech.com>; Yunlin Tang
> <yunlin.tang@aspeedtech.com>; Cédric Le Goater <clg@redhat.com>
> Subject: Re: [PATCH v3 2/3] hw/sd/sdhci: Introduce a new Write Protected pin
> inverted property
>
> 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 :
>
>
> https://lore.kernel.org/qemu-devel/20250108092538.11474-9-shentey@gmail.
> com/
>
Thanks for your suggestion and help.
This patch is on the SD Card side.
My change is on the SDHCI (Host Controller) side.
I tried to directly set both the "wp-active-low" property and the "readonly_active_low"
variable to "true" and "false" in "sd.c".
Unfortunately, I still got the READONLY status in SDHCI.
The reason is that the SDHCI register 0x24 is not inverted.
Thanks-Jamin
root@ast2600-default:~# cat /sys/bus/mmc/drivers/mmcblk/mmc1\:f3a5/block/mmcblk1/ro
1
root@ast2600-default:~# dmesg | grep "mmc"
[ 13.131526] mmc0: SDHCI controller on 1e750100.sdhci [1e750100.sdhci] using ADMA
[ 13.212462] mmc0: Failed to initialize a non-removable card
[ 13.829166] mmc1: SDHCI controller on 1e740100.sdhci [1e740100.sdhci] using ADMA
[ 13.832527] mmc2: SDHCI controller on 1e740200.sdhci [1e740200.sdhci] using ADMA
[ 13.890041] mmc2: new high speed SD card at address cae3
[ 13.896243] mmcblk2: mmc2:cae3 QEMU! 128 MiB (ro)
[ 13.899294] mmc1: new high speed SD card at address f3a5
[ 13.915949] mmcblk1: mmc1:f3a5 QEMU! 128 MiB (ro)
root@ast2600-default:~# mount /dev/mmcblk1 /mnt
[ 107.822908] EXT4-fs (mmcblk1): mounted filesystem 3e5fa086-9be6-453a-8533-bca6cba15873 ro with ordered data mode. Quota mode: disabled.
mount: /mnt: WARNING: source write-protected, mounted read-only.
root@ast2600-default:~# touch /mnt/1111111
touch: /mnt/1111111: Read-only file system
> 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;
> >
On 14/11/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> > --- > hw/sd/sdhci.c | 6 ++++++ > include/hw/sd/sdhci.h | 5 +++++ > 2 files changed, 11 insertions(+) Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>
© 2016 - 2026 Red Hat, Inc.