Re: [PATCH v2 3/3] mmc: meson-gx: report the longest busy wait the controller can do

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mmc: meson-gx: max_busy_timeout patch unmasks pre-existing logic bug causing skipped busy waits
--

commit 8392473ffe7fe05ea31a587e9d682a2201cb340c
Author: Igor Velkov <iav@iav.lv>

mmc: meson-gx: report the longest busy wait the controller can do

The driver does not currently set max_busy_timeout, leading the core to
assume it can wait indefinitely and send commands with timeouts that exceed
the controller's maximum capability. This patch explicitly sets
max_busy_timeout to force the core to size discards appropriately and fall
back to software polling for long waits.

> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index dd7d3d67f0290..8de3eb65ccb07 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -1183,6 +1183,9 @@ static int meson_mmc_probe(struct platform_device *pdev)
>  
>  	mmc->caps |= MMC_CAP_CMD23;
>  
> +	/* The longest busy wait the CMD_CFG timeout field can hold. */
> +	mmc->max_busy_timeout = SD_EMMC_CMD_TIMEOUT_MAX;
> +

[Severity: High]
This problem wasn't introduced by this patch, but does setting the
max_busy_timeout here unmask a bug in the software polling fallback?

When a long-running command exceeds this timeout, the MMC core drops the
hardware R1B busy wait and relies on software polling using
meson_mmc_card_busy().

However, meson_mmc_card_busy() requires all 4 DAT lines to be low:

drivers/mmc/host/meson-gx-mmc.c:meson_mmc_card_busy() {
	...
	regval = readl(host->regs + SD_EMMC_STATUS);

	/* We are only interrested in lines 0 to 3, so mask the other ones */
	return !(FIELD_GET(STATUS_DATI, regval) & 0xf);
}

Since the MMC/SD spec dictates that only DAT0 is pulled low during busy while
DAT1-3 remain high, wouldn't this check immediately return false?

If so, the core would instantly send the next command while the card is still
busy erasing or flushing.

Could meson_mmc_card_busy() be updated to only check DAT0?

>  	if (mmc->caps & MMC_CAP_SDIO_IRQ)
>  		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-meson-gx-busy-timeout-v2-0-8bde1adfb5b4@iav.lv?part=3

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic