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