[PATCH v2 0/3] mmc: meson-gx: honour the busy timeout for R1b commands

[PATCH v2 0/3] mmc: meson-gx: honour the busy timeout for R1b commands

[Igor Velkov via B4 Relay]

For a command without data the driver programs a fixed 1024 ms into the
CMD_CFG timeout field and does not tell the core how long the controller
can wait for busy. Erase and cache flush then fail with -110 on eMMC.

Tested on ODROID-N2+ (S922X), 256 GB eMMC, btrfs root with discard=async,
Armbian builds in docker as load. Without the patches (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 patches (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 patches, 0 and 0 with them.

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.

---
Changes in v2:
- Split into three patches, as Neil asked: name the limit of the
  timeout field, program the R1b timeout from cmd->busy_timeout, report
  max_busy_timeout.
- Link to v1: https://lore.kernel.org/r/20261008-meson-gx-busy-timeout-v1-1-d92f3c68c409@iav.lv

---
Igor Velkov (3):
      mmc: meson-gx: name the limit of the CMD_CFG timeout field
      mmc: meson-gx: honour the busy timeout for R1b commands
      mmc: meson-gx: report the longest busy wait the controller can do

 drivers/mmc/host/meson-gx-mmc.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)
---
base-commit: fd9af27f6c2319713bb23832066585f8a59a1771
change-id: 20261005-meson-gx-busy-timeout-7e8e3fdcb3e3

Best regards,
-- 
Igor Velkov <iav@iav.lv>



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

[PATCH v2 1/3] mmc: meson-gx: name the limit of the CMD_CFG timeout field

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

The data timeout is capped at a literal 32768 ms with a comment next to
it. Give the limit a name beside the other timeout constants.

Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/mmc/host/meson-gx-mmc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 7ef16d7c03cb..06479ba38525 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 */
 #define SD_EMMC_CFG_CMD_GAP 16 /* in clock cycles */
 #define SD_EMMC_DESC_BUF_LEN PAGE_SIZE
 
@@ -212,7 +213,7 @@ 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);
 }
 
 static struct mmc_command *meson_mmc_get_next_command(struct mmc_command *cmd)

-- 
2.43.0



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

[PATCH v2 2/3] mmc: meson-gx: honour the busy timeout for R1b commands

[Igor Velkov via B4 Relay]

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 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, the range the field holds. 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>
---
 drivers/mmc/host/meson-gx-mmc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 06479ba38525..dd7d3d67f029 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -216,6 +216,20 @@ static unsigned int meson_mmc_get_timeout_msecs(struct mmc_data *data)
 	return min_t(unsigned int, timeout, SD_EMMC_CMD_TIMEOUT_MAX);
 }
 
+/* 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)
 {
 	if (cmd->opcode == MMC_SET_BLOCK_COUNT && !cmd->error)
@@ -842,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 */

-- 
2.43.0



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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mmc: meson-gx: UB and truncation when rounding huge timeout before clamping
--

commit 2982319077ff9791c1c57722fef30aa5b7026071
Author: Igor Velkov <iav@iav.lv>

mmc: meson-gx: honour the busy timeout for R1b commands

This commit programs the CMD_CFG timeout field using cmd->busy_timeout for
R1b commands. It aims to resolve an issue where the controller prematurely
times out during long operations like erase and cache flush. The driver
calculates the timeout by rounding up to a power of two and clamping it.

> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c

[ ... ]

> @@ -216,6 +216,20 @@ static unsigned int meson_mmc_get_timeout_msecs(struct mmc_data *data)
>  	return min_t(unsigned int, timeout, SD_EMMC_CMD_TIMEOUT_MAX);
>  }
>  
> +/* 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);
> +}

[Severity: Low]
Could cmd->busy_timeout hold a value large enough to cause an overflow here?

Since cmd->busy_timeout is an unsigned int representing milliseconds, it can
potentially be controlled by userspace via the MMC_IOC_CMD ioctl. If a user
provides a timeout greater than 0x80000000, passing it to
roundup_pow_of_two() calculates 1UL << 32.

On 32-bit kernels, shifting a 32-bit unsigned long by 32 bits is undefined
behavior. On 64-bit kernels, 1UL << 32 evaluates to 0x100000000, which is
truncated to 0 when assigned back to the unsigned int timeout variable.

The subsequent clamp_t() call would receive 0 (or a garbage value on 32-bit
systems) and incorrectly return the minimum timeout instead of the maximum
timeout. This might lead to a premature hardware timeout on valid long
commands.

Would it be safer to clamp the value to SD_EMMC_CMD_TIMEOUT_MAX before
rounding it to a power of two, ensuring it stays well within bounds?

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

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

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

[Igor Velkov via B4 Relay]

From: Igor Velkov <iav@iav.lv>

The driver does not set max_busy_timeout, so the core takes the host
for one that can wait for busy indefinitely and sends erase and cache
flush as R1b with busy timeouts of up to 60 s and 30 s. The CMD_CFG
timeout field holds at most 32768 ms, so the controller times out first.

Report 32768 ms as max_busy_timeout. The core then sizes discards to
fit and, for a longer wait, sends R1 and polls for busy itself.

Fixes: 51c5d8447bd7 ("MMC: meson: initial support for GX platforms")
Assisted-by: LLM
Signed-off-by: Igor Velkov <iav@iav.lv>
---
 drivers/mmc/host/meson-gx-mmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index dd7d3d67f029..8de3eb65ccb0 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;
+
 	if (mmc->caps & MMC_CAP_SDIO_IRQ)
 		mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
 

-- 
2.43.0



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

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

[sashiko-bot]

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