Re: [PATCH 2/2] hw/sd/sdhci: Extract uSDHC-specific quirk

On Tue, Jul 28, 2026 at 4:15 PM Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> Hi Bin,
>
> On 21/7/26 12:42, Philippe Mathieu-Daudé wrote:
> > 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?
>
> What is your final tought, should we proceed with v2?

Yes, I think so.

Regards,
Bin