Thread (4 messages) 4 messages, 3 authors, 2025-12-10

RE: [PATCH v2] mmc: sdhci-esdhc-imx: wait for data transfer completion before reset

From: Bough Chen <haibo.chen@nxp.com>
Date: 2025-12-05 08:12:20
Also in: imx, linux-mmc, lkml

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Luke Wang <redacted>
Sent: 2025年12月5日 15:17
To: adrian.hunter@intel.com; Bough Chen <haibo.chen@nxp.com>;
ulf.hansson@linaro.org
Cc: shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
festevam@gmail.com; linux-mmc@vger.kernel.org; imx@lists.linux.dev; dl-S32
[off-list ref]; linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org
Subject: [PATCH v2] mmc: sdhci-esdhc-imx: wait for data transfer completion
before reset

From: Luke Wang <redacted>

On IMX7ULP platforms, certain SD cards (e.g. Kingston Canvas Go! Plus) cause
system hangs and reboots during manual tuning. These cards exhibit large gaps
(~16us) between tuning command response and data transmission.
When cmd CRC errors occur during tuning, the code assumes data errors even
tuning data hasn't been fully received and then reset host data circuit.

Per IMX7ULP reference manual, reset operations (RESET_DATA/ALL) need to
make sure no active data transfers. Previously, resetting while data was in-flight
would clear data circuit, including ADMA/SDMA address, causing data to be
transmitted to incorrect memory address. This patch adds polling for data
transfer completion before executing resets.

Signed-off-by: Luke Wang <redacted>
---
v2: amend commit message
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
b/drivers/mmc/host/sdhci-esdhc-imx.c
index a7a5df673b0f..affde1936510 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1453,6 +1453,21 @@ static void esdhc_set_uhs_signaling(struct
sdhci_host *host, unsigned timing)

 static void esdhc_reset(struct sdhci_host *host, u8 mask)  {
+	u32 present_state;
+	int ret;
+
+	/*
+	 * For data or full reset, ensure any active data transfer completes
+	 * before resetting to avoid system hang.
+	 */
+	if (mask & (SDHCI_RESET_DATA | SDHCI_RESET_ALL)) {
+		ret = readl_poll_timeout_atomic(host->ioaddr + ESDHC_PRSSTAT,
present_state,
I'm okay with the patch, but I find one thing here:

I notice you use _atomic here, I guess you want to cover the case when the reset function is called in hardware irq handler sdhci_irq().
I'm not sure whether it is suitable to add delay in hardware iqr handler, I find there is also udelay in sdhci_reset(), sdhci_reset can also be called in hardware irq handler when there is cmd_crc/data_crc error.
Adrian/Ulf, do you notice this issue before? Any comments?

Regards
Haibo Chen

+						!(present_state & SDHCI_DATA_INHIBIT), 2,
100000);
+		if (ret == -ETIMEDOUT)
+			dev_warn(mmc_dev(host->mmc),
+				 "timeout waiting for data transfer completion\n");
+	}
+
 	sdhci_and_cqhci_reset(host, mask);

 	sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
--
2.34.1
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help