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

[Shawn Lin <shawn.lin@linux.dev>]

Hi Ulf,

在 2026/03/09 星期一 22:57, Ulf Hansson 写道:
> On Fri, 6 Mar 2026 at 13:48, Shawn Lin <shawn.lin@linux.dev> wrote:
>>
>> From: Luke Wang <ziniu.wang_1@nxp.com>
>>
>> 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@nxp.com>
>> [Shawn: reword the commit msg and drop HS400 change]
>> Signed-off-by: Shawn Lin <shawn.lin@linux.dev>
>> ---
>>
>>   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
>