Re: [PATCH 1/4] mmc: core: fix timing selection for 1-bit bus width

[Shawn Lin]

Hi Luke

在 2026/03/02 星期一 16:00, ziniu.wang_1@xxxxxxx 写道:
> From: Luke Wang <ziniu.wang_1@xxxxxxx>
>
> When 1-bit bus width is used with HS200/HS400 capabilities set,
> mmc_select_hs200() returns 0 without actually switching. This
> causes mmc_select_timing() to skip mmc_select_hs(), leaving eMMC
> in legacy mode (26MHz) instead of High Speed SDR (52MHz).
>
> Per JEDEC eMMC spec section 5.3.2, 1-bit width supports High Speed
> SDR. Drop incompatible HS200/HS400/UHS/DDR caps early so timing
> selection falls through to mmc_select_hs() correctly.

I've tested this patch, and it works as intended.

Unrelated to your specific change, however, I'm not convinced that
mmc_validate_host_caps() is doing what its name suggests, at least not
comprehensively. For example, the SD card path handles 1-bit bus with
UHS support correctly, thanks to the mmc_host_uhs() check inside the
SD/SDIO code. This makes the validation logic feel scattered across
different layers IMO. It would be even better if you could consolidate 
all these checks in one place maybe, but anyway

Tested-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>


> Signed-off-by: Luke Wang <ziniu.wang_1@xxxxxxx>
> ---
>   drivers/mmc/core/host.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 88c95dbfd9cf..18b9c3174e1f 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -617,17 +617,26 @@ EXPORT_SYMBOL(devm_mmc_alloc_host);
>   static int mmc_validate_host_caps(struct mmc_host *host)
>   {
>   	struct device *dev = host->parent;
> -	u32 caps = host->caps, caps2 = host->caps2;
>   
> -	if (caps & MMC_CAP_SDIO_IRQ && !host->ops->enable_sdio_irq) {
> +	if (host->caps & MMC_CAP_SDIO_IRQ && !host->ops->enable_sdio_irq) {
>   		dev_warn(dev, "missing ->enable_sdio_irq() ops\n");
>   		return -EINVAL;
>   	}
>   
> -	if (caps2 & (MMC_CAP2_HS400_ES | MMC_CAP2_HS400) &&
> -	    !(caps & MMC_CAP_8_BIT_DATA) && !(caps2 & MMC_CAP2_NO_MMC)) {
> +	/* UHS/DDR/HS200/HS400 modes require at least 4-bit bus */
> +	if (!(host->caps & (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA)) &&
> +	    ((host->caps & (MMC_CAP_UHS | MMC_CAP_DDR)) ||
> +	     (host->caps2 & (MMC_CAP2_HS200 | MMC_CAP2_HS400_ES | MMC_CAP2_HS400)))) {
> +		dev_warn(dev, "drop UHS/DDR/HS200/HS400 support since 1-bit bus only\n");
> +		host->caps &= ~(MMC_CAP_UHS | MMC_CAP_DDR);
> +		host->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400_ES | MMC_CAP2_HS400);
> +	}
> +
> +	if (host->caps2 & (MMC_CAP2_HS400_ES | MMC_CAP2_HS400) &&
> +	    !(host->caps & MMC_CAP_8_BIT_DATA) &&
> +	    !(host->caps2 & MMC_CAP2_NO_MMC)) {
>   		dev_warn(dev, "drop HS400 support since no 8-bit bus\n");
> -		host->caps2 = caps2 & ~MMC_CAP2_HS400_ES & ~MMC_CAP2_HS400;
> +		host->caps2 &= ~(MMC_CAP2_HS400_ES | MMC_CAP2_HS400);
>   	}
>   
>   	return 0;