Re: [PATCH 1/2] mmc: core: Validate UHS/DDR/HS200 timing selection for 1-bit bus width

[Shawn Lin]

Hi Ulf,

在 2026/03/09 星期一 22:57, Ulf Hansson 写道:
> On Fri, 6 Mar 2026 at 13:48, Shawn Lin <shawn.lin@xxxxxxxxx> wrote:
> > From: Luke Wang <ziniu.wang_1@xxxxxxx>
> >
> > UHS/DDR/HS200 modes require at least 4-bit bus support. Host controllers
> > that lack relevant capability registers rely on paring properties provided
> > by firmware, which may incorrectly set these modes. Now that mmc_validate_host_caps()
> > has been introduced to validate such configuration violations, let's also
> > add checks for UHS/DDR/HS200 modes.
> >
> > This fixes an issue where, if the HS200/HS400 property is set while only a
> > 1-bit bus width is used, mmc_select_hs200() returns 0 without actually performing
> > the mode switch. Consequently, mmc_select_timing() proceeds without falling back to
> > mmc_select_hs(), leaving the eMMC device operating in legacy mode (26 MHz) instead
> > of switching to High Speed mode (52 MHz).
> >
> > Signed-off-by: Luke Wang <ziniu.wang_1@xxxxxxx>
> > [Shawn: reword the commit msg and drop HS400 change]
> > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxx>
> > ---
> >
> >   drivers/mmc/core/host.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> > index 88c95db..d1d4870 100644
> > --- a/drivers/mmc/core/host.c
> > +++ b/drivers/mmc/core/host.c
> > @@ -624,6 +624,15 @@ static int mmc_validate_host_caps(struct mmc_host *host)
> >                  return -EINVAL;
> >          }
> >
> > +       /* UHS/DDR/HS200 modes require at least 4-bit bus */
> > +       if (!(caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)) &&
> > +           ((caps & (MMC_CAP_UHS | MMC_CAP_DDR)) || (caps2 & MMC_CAP2_HS200))) {
> > +               dev_warn(dev, "drop UHS/DDR/HS200 support since 1-bit bus only\n");
> > +               host->caps = caps = caps & ~(MMC_CAP_UHS | MMC_CAP_DDR);
> > +               host->caps2 = caps2 = caps2 & ~MMC_CAP2_HS200;
>
> Hmm.
>
> How about just updating the temporary variables, caps and caps2, here?
>
> Then at the end of the function when we have validated all "caps", we
> can update the variables in the host, like:
> host->caps = caps;
> host->caps2 = caps2;

It looks better. Will update v2 based on your suggestion.

> > +       }
> > +
> > +       /* HS400 and HS400ES modes require 8-bit bus */
> >          if (caps2 & (MMC_CAP2_HS400_ES | MMC_CAP2_HS400) &&
> >              !(caps & MMC_CAP_8_BIT_DATA) && !(caps2 & MMC_CAP2_NO_MMC)) {
> >                  dev_warn(dev, "drop HS400 support since no 8-bit bus\n");
> > --
> > 2.7.4
>
> Kind regards
> Uffe