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