Re: [PATCH] mmc: meson-gx: honour the busy timeout for R1b commands

[Neil Armstrong <neil.armstrong@linaro.org>]

On 10/8/26 06:26, Igor Velkov via B4 Relay wrote:
> From: Igor Velkov <iav@iav.lv>
> 
> For a command without data the driver programs a fixed 1024 ms into the
> CMD_CFG timeout field, and for R1b the controller waits for the card to
> release busy within that time. The driver does not set max_busy_timeout,
> so the core sends erase and cache flush as R1b with busy timeouts of up
> to 60 s and 30 s, and the controller gives up after one second:
> 
>    mmc_erase: erase error -110, status 0x0
>    mmc1: cache flush error -110
> 
> Program the timeout from cmd->busy_timeout, rounded up to a power of
> two and kept within 1024..32768 ms, and report 32768 ms, the largest
> value the field holds, as max_busy_timeout. The core then sizes discards
> to fit and, for a longer wait, sends R1 and polls for busy itself.
> sdhci takes its command timeout from cmd->busy_timeout in the same way.
> 
> Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
> Assisted-by: LLM
> Signed-off-by: Igor Velkov <iav@iav.lv>
> ---
> Tested on ODROID-N2+ (S922X), 256 GB eMMC, btrfs root with discard=async, Armbian builds in docker as load.
> Without the patch (7.3-rc6): 262 errors in 3 h and 68 in 2.5 h in two runs; the first came 4 and 43 min into the build.
> With the patch (7.3-rc6): 0 in 2.4 h.
> On 7.1 with the previous eMMC module, a synthetic test (20 GiB write + fstrim) gave 48 errors without the patch, 0 and 0 with it.
> 
> The timeout is rounded up to a power of two: the field holds powers of two only, and the timeout is a lower bound.
> MMC_CAP_WAIT_WHILE_BUSY is not set: I have not checked that the controller waits for busy on every R1b command, HS400 switches included.
> ---
>   drivers/mmc/host/meson-gx-mmc.c | 22 ++++++++++++++++++++--
>   1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 7ef16d7c03cb..8de3eb65ccb0 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -125,6 +125,7 @@
>   #define SD_EMMC_CFG_RESP_TIMEOUT 256 /* in clock cycles */
>   #define SD_EMMC_CMD_TIMEOUT 1024 /* in ms */
>   #define SD_EMMC_CMD_TIMEOUT_DATA 4096 /* in ms */
> +#define SD_EMMC_CMD_TIMEOUT_MAX 32768 /* in ms, 2^15: limit of CMD_CFG_TIMEOUT_MASK */

This is a cleanup and be sent separately

>   #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
>   #define SD_EMMC_DESC_BUF_LEN PAGE_SIZE
>   
> @@ -212,7 +213,21 @@ static unsigned int meson_mmc_get_timeout_msecs(struct mmc_data *data)
>   
>   	timeout = roundup_pow_of_two(timeout);
>   
> -	return min(timeout, 32768U); /* max. 2^15 ms */
> +	return min_t(unsigned int, timeout, SD_EMMC_CMD_TIMEOUT_MAX);
> +}

Same here

> +
> +/* For R1b the controller waits for busy to be released within this timeout. */
> +static unsigned int meson_mmc_get_cmd_timeout_msecs(struct mmc_command *cmd)
> +{
> +	unsigned int timeout = cmd->busy_timeout;
> +
> +	if (!timeout)
> +		return SD_EMMC_CMD_TIMEOUT;
> +
> +	timeout = roundup_pow_of_two(timeout);
> +
> +	return clamp_t(unsigned int, timeout, SD_EMMC_CMD_TIMEOUT,
> +		       SD_EMMC_CMD_TIMEOUT_MAX);
>   }
>   
>   static struct mmc_command *meson_mmc_get_next_command(struct mmc_command *cmd)
> @@ -841,7 +856,7 @@ static void meson_mmc_start_cmd(struct mmc_host *mmc, struct mmc_command *cmd)
>   		cmd_data = host->bounce_dma_addr & CMD_DATA_MASK;
>   	} else {
>   		cmd_cfg |= FIELD_PREP(CMD_CFG_TIMEOUT_MASK,
> -				      ilog2(SD_EMMC_CMD_TIMEOUT));
> +				      ilog2(meson_mmc_get_cmd_timeout_msecs(cmd)));
>   	}
>   
>   	/* Last descriptor */
> @@ -1168,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;

So you seem to fix 2 issues:
- for timeouts larger than 32768U, we don't wait enough, so ok this should be in a single changeset
- we don't wait the right time for r1b, so you add meson_mmc_get_cmd_timeout_msecs(), ok
   so please sent it in a separate change aswell

> +
>   	if (mmc->caps & MMC_CAP_SDIO_IRQ)
>   		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
>   
> 
> ---
> base-commit: fd9af27f6c2319713bb23832066585f8a59a1771
> change-id: 20261005-meson-gx-busy-timeout-7e8e3fdcb3e3
> 
> Best regards,

Thanks,
Neil

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