[v3] mmc: sdhci-esdhc-imx: wait for data transfer completion before reset

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

Posted by ziniu.wang_1@nxp.com 5 days, 1 hour ago

From: Luke Wang <ziniu.wang_1@nxp.com>

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 <ziniu.wang_1@nxp.com>
Reviewed-by: Bough Chen <haibo.chen@nxp.com>
---
v3: add define for timeout value
v2: amend commit message
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index a7a5df673b0f..97461e20425d 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -216,6 +216,8 @@
 #define ESDHC_FLAG_DUMMY_PAD		BIT(19)
 
 #define ESDHC_AUTO_TUNING_WINDOW	3
+/* 100ms timeout for data inhibit */
+#define ESDHC_DATA_INHIBIT_WAIT_US	100000
 
 enum wp_types {
 	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
@@ -1453,6 +1455,22 @@ 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,
+						!(present_state & SDHCI_DATA_INHIBIT), 2,
+						ESDHC_DATA_INHIBIT_WAIT_US);
+		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

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

Posted by Ulf Hansson 17 hours ago

On Thu, 11 Dec 2025 at 08:54, <ziniu.wang_1@nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> 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 <ziniu.wang_1@nxp.com>
> Reviewed-by: Bough Chen <haibo.chen@nxp.com>

Applied for next, thanks!

Kind regards
Uffe


> ---
> v3: add define for timeout value
> v2: amend commit message
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a7a5df673b0f..97461e20425d 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -216,6 +216,8 @@
>  #define ESDHC_FLAG_DUMMY_PAD           BIT(19)
>
>  #define ESDHC_AUTO_TUNING_WINDOW       3
> +/* 100ms timeout for data inhibit */
> +#define ESDHC_DATA_INHIBIT_WAIT_US     100000
>
>  enum wp_types {
>         ESDHC_WP_NONE,          /* no WP, neither controller nor gpio */
> @@ -1453,6 +1455,22 @@ 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,
> +                                               !(present_state & SDHCI_DATA_INHIBIT), 2,
> +                                               ESDHC_DATA_INHIBIT_WAIT_US);
> +               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
>

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

Posted by Adrian Hunter 3 days, 21 hours ago

On 11/12/2025 09:56, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> 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 <ziniu.wang_1@nxp.com>
> Reviewed-by: Bough Chen <haibo.chen@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> v3: add define for timeout value
> v2: amend commit message
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index a7a5df673b0f..97461e20425d 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -216,6 +216,8 @@
>  #define ESDHC_FLAG_DUMMY_PAD		BIT(19)
>  
>  #define ESDHC_AUTO_TUNING_WINDOW	3
> +/* 100ms timeout for data inhibit */
> +#define ESDHC_DATA_INHIBIT_WAIT_US	100000
>  
>  enum wp_types {
>  	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
> @@ -1453,6 +1455,22 @@ 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,
> +						!(present_state & SDHCI_DATA_INHIBIT), 2,
> +						ESDHC_DATA_INHIBIT_WAIT_US);
> +		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);