On 21/7/26 12:29, Bernhard Beschow wrote:
Am 21. Juli 2026 08:38:19 UTC schrieb Bin Meng <[email protected]>:
Hi Bernhard,
On Tue, Jul 21, 2026 at 5:11 AM Bernhard Beschow <[email protected]> wrote:
Am 20. Juli 2026 14:50:24 UTC schrieb Bin Meng <[email protected]>:
Hi Bin,
Thanks for yor reviews and R-b tags!
On Mon, Jul 20, 2026 at 5:19 AM Bernhard Beschow <[email protected]> wrote:
In Linux, the ESDHC_MIX_CTRL qirk is guarded by esdhc_is_usdhc() while
the eSDHC code path uses the standard SDHC interface. Extract the quirk
into a new `usdhc_write()` function.
Fixes file system corruption on emulated i.MX53 where Linux'
esdhc_is_usdhc() returns false. The same likely happens on e500 and
imx25-pdk machines.
Fixes: 75e98bc4f859 ("hw/sd/sdhci: Add TYPE_FSL_ESDHC_BE")
cc: qemu-stable
Signed-off-by: Bernhard Beschow <[email protected]>
---
hw/sd/sdhci.c | 73 +++++++++++++++++++++++++++++++--------------------
1 file changed, 44 insertions(+), 29 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index c86dfa281f..e58a610397 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1795,34 +1795,6 @@ esdhc_write(void *opaque, hwaddr offset, uint64_t val,
unsigned size)
sdhci_write(opaque, offset, value, size);
break;
- case ESDHC_MIX_CTRL:
- /*
- * So, when SD/MMC stack in Linux tries to write to "Transfer
- * Mode Register", ESDHC i.MX quirk code will translate it
- * into a write to ESDHC_MIX_CTRL, so we do the opposite in
- * order to get where we started
- *
- * Note that Auto CMD23 Enable bit is located in a wrong place
- * on i.MX, but since it is not used by QEMU we do not care.
- *
- * We don't want to call sdhci_write(.., SDHC_TRNMOD, ...)
- * here because it will result in a call to
- * sdhci_send_command(s) which we don't want.
- *
- */
- s->trnmod = value & UINT16_MAX;
- break;
- case SDHC_TRNMOD:
Looks like this register is not eSDHC specific?
Not quite sure what you mean exactly. The SDHC_TRNMOD isn't eSDHC specific but
as per the comment above trnmod partial handling is deferred here, i.e. needs
to be intercepted.
If I read the Linux driver correctly, I think we should do something
like below when writing to SDHC_TRNMOD:
case SDHC_TRNMOD:
if (is_usdhc) {
val |= s->trnmod;
}
sdhci_write(opaque, offset, val, size);
break;
Because for eSDHC, the transfer mode is already passed in the value,
and oring previous s->trnmod may cause some bits being set to old
previous value unexpectedly. Please double check.
By moving out of esdhc_write() into a dedicated usdhc_write() method we achieve
exactly that, no?
Indeed this is kind of hidden, see below.
Best regards,
Bernhard
I looked at linux
driver source codes and suspect we may need some more turning here
other than uSDHC only.
Yeah, there is certainly a lot of room for improvement. I have some
clock-related and SDMA buffer boundary patches in the pipeline which are
necessary for satisfying U-Boot. But that's a story for another day.
Best regards,
Bernhard
- /*
- * Similar to above, but this time a write to "Command
- * Register" will be translated into a 4-byte write to
- * "Transfer Mode register" where lower 16-bit of value would
- * be set to zero. So what we do is fill those bits with
- * cached value from s->trnmod and let the SDHCI
- * infrastructure handle the rest
- */
- sdhci_write(opaque, offset, val | s->trnmod, size);
- break;
case SDHC_BLKSIZE:
/*
* ESDHCI does not implement "Host SDMA Buffer Boundary", and
@@ -1891,9 +1863,52 @@ static void fsl_esdhc_le_init(Object *obj)
qdev_prop_set_uint8(dev, "sd-spec-version", 2);
}
+static void
+usdhc_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
+{
+ SDHCIState *s = SYSBUS_SDHCI(opaque);
+ uint32_t value = (uint32_t)val;
+
+ switch (offset) {
+ case ESDHC_MIX_CTRL:
+ /*
+ * So, when SD/MMC stack in Linux tries to write to "Transfer
+ * Mode Register", uSDHC i.MX quirk code will translate it
+ * into a write to ESDHC_MIX_CTRL, so we do the opposite in
+ * order to get where we started.
+ *
+ * Note that Auto CMD23 Enable bit is located in a wrong place
+ * on i.MX, but since it is not used by QEMU we do not care.
+ *
+ * We don't want to call sdhci_write(.., SDHC_TRNMOD, ...)
+ * here because it will result in a call to
+ * sdhci_send_command(s) which we don't want.
+ *
+ */
+ s->trnmod = value & UINT16_MAX;
+ break;
+
+ case SDHC_TRNMOD:
+ /*
+ * Similar to above, but this time a write to "Command
+ * Register" will be translated into a 4-byte write to
+ * "Transfer Mode register" where lower 16-bit of value would
+ * be set to zero. So what we do is fill those bits with
+ * cached value from s->trnmod and let the SDHCI
+ * infrastructure handle the rest
+ */
+ sdhci_write(opaque, offset, val | s->trnmod, size);
^^^^^^^^^^^^^^^
+ break;
+
+ default:
+ esdhc_write(opaque, offset, val, size);
+ break;
+ }
+}
+
static const MemoryRegionOps usdhc_mmio_ops = {
.read = esdhc_read,
- .write = esdhc_write,
+ .write = usdhc_write,
.valid = {
.min_access_size = 1,
.max_access_size = 4,
--
Regards,
Bin
