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.
>
> >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
