[PATCH v3 0/4] mmc: sdhci_am654: Fix DDR50 mode for cards without tuning support

[PATCH v3 0/4] mmc: sdhci_am654: Fix DDR50 mode for cards without tuning support

[Diogo Ivo (Schneider Electric)]

DDR50 mode is not required to support the tuning command CMD19 by the
eMMC/SD specification, meaning that calibration may fail on cards that
do not implement it. This series fixes four issues discovered when
using such cards:

  - Patch 1 moves the tuning_loop counter from a persistent struct field
    to a local variable, preventing failed tuning attempts from depleting
    the retry budget across calls.

  - Patch 2 ensures the command/data line reset is always performed
    after tuning, even on failure, preventing stale data in the
    controller buffer.

  - Patch 3 clears the ITAPDLY enable and delay values on tuning
    failure so that leftover calibration values do not interfere with
    subsequent I/O.

  - Patch 4 falls back to the DT-provided itap delay value for DDR50
    when tuning fails in this mode, making a best-effort attempt at a
    known-good delay being programmed.

Together these changes make DDR50 functional on TI J721E SoCs with
cards that lack CMD19 tuning support. This second version addresses
all comments from Sashiko, which I found to be correct.

Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
---
Changes in v3:
- Collect R-b from Judith and A-b from Adrian
- Add debug print when applying DT fallback values (Patch 4)
- Link to v2: https://patch.msgid.link/20260803-am654-sdhci-v2-0-a5ee5be86133@bootlin.com

Changes in v2:
- Add new patch moving tuning_loop to a local variable (Patch 1)
- Clear ITAPDLY array values alongside HW registers on tuning failure (Patch 3)
- Save DT-provided itap delay values for DDR50 in dedicated variables
  to preserve them across tuning cycles (Patch 4)
- Link to v1: https://patch.msgid.link/20260729-am654-sdhci-v1-0-7568108e4b9a@bootlin.com

---
Diogo Ivo (Schneider Electric) (4):
      mmc: sdhci_am654: Move tuning_loop to local variable
      mmc: sdhci_am654: Reset command and data lines on failed tuning
      mmc: sdhci_am654: Clear ITAPDLY on tuning failure
      mmc: sdhci_am654: Fallback to DT-provided itap delay on DDR50 tuning failure

 drivers/mmc/host/sdhci_am654.c | 43 +++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)
---
base-commit: f9a2394a23482bfd330911e9c8295b71724feacd
change-id: 20260728-am654-sdhci-85094d458bae

Best regards,
--  
Diogo Ivo <diogo.ivo@bootlin.com>

[PATCH v3 1/4] mmc: sdhci_am654: Move tuning_loop to local variable

[Diogo Ivo (Schneider Electric)]

The tuning_loop field in struct sdhci_am654_data is only used within
sdhci_am654_platform_execute_tuning() as a loop counter that is
initialized to 0 in sdhci_am654_init(). Since it shouldn't persist across
function calls, otherwise every failure expends its "budget", move it to a
local variable and remove the struct field along with the now-unnecessary
initialization.

Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
Reviewed-by: Judith Mendez <jm@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in v3:
 - Collect R-b from Judith and A-b from Adrian
Changes in v2:
 - New patch suggested by Sashiko
---
 drivers/mmc/host/sdhci_am654.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index d235b0aecfdb..35c3efd92112 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -151,7 +151,6 @@ struct sdhci_am654_data {
 	u32 flags;
 	u32 quirks;
 	bool dll_enable;
-	u32 tuning_loop;
 
 #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
 #define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
@@ -576,13 +575,14 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
 	unsigned char timing = host->mmc->ios.timing;
 	struct device *dev = mmc_dev(host->mmc);
+	unsigned int tuning_loop = 0;
 	int itapdly;
 
 	do {
 		itapdly = sdhci_am654_do_tuning(host, opcode);
 		if (itapdly >= 0)
 			break;
-	} while (++sdhci_am654->tuning_loop < RETRY_TUNING_MAX);
+	} while (++tuning_loop < RETRY_TUNING_MAX);
 
 	if (itapdly < 0) {
 		dev_err(dev, "Failed to find itapdly, fail tuning\n");
@@ -806,9 +806,6 @@ static int sdhci_am654_init(struct sdhci_host *host)
 	regmap_update_bits(sdhci_am654->base, CTL_CFG_3, TUNINGFORSDR50_MASK,
 			   TUNINGFORSDR50_MASK);
 
-	/* Use to re-execute tuning */
-	sdhci_am654->tuning_loop = 0;
-
 	ret = sdhci_setup_host(host);
 	if (ret)
 		return ret;

-- 
2.55.0

[PATCH v3 2/4] mmc: sdhci_am654: Reset command and data lines on failed tuning

[Diogo Ivo (Schneider Electric)]

The CMD/DATA reset after tuning should be performed regardless of
whether tuning succeeded or failed, since tuning data may remain in
the buffer in either case. Move the error return after the reset so
that the controller is always cleaned up.

Fixes: de31f6ab68a3 ("mmc: sdhci_am654: Reset Command and Data line after tuning")
Cc: stable@vger.kernel.org
Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
Reviewed-by: Judith Mendez <jm@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in v3:
 - Collect R-b from Judith and A-b from Adrian
Changes in v2:
 - No changes
---
 drivers/mmc/host/sdhci_am654.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 35c3efd92112..0db587e702ac 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -442,15 +442,13 @@ static int sdhci_am654_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	struct sdhci_host *host = mmc_priv(mmc);
 	int err = sdhci_execute_tuning(mmc, opcode);
 
-	if (err)
-		return err;
 	/*
 	 * Tuning data remains in the buffer after tuning.
 	 * Do a command and data reset to get rid of it
 	 */
 	sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
 
-	return 0;
+	return err;
 }
 
 static u32 sdhci_am654_cqhci_irq(struct sdhci_host *host, u32 intmask)

-- 
2.55.0

[PATCH v3 3/4] mmc: sdhci_am654: Clear ITAPDLY on tuning failure

[Diogo Ivo (Schneider Electric)]

When tuning fails, stale ITAPDLY values can persist and interfere with
subsequent I/O accesses, for example in DDR50 mode in cards with no tuning
support. Move the ITAPDLY enable setting out of the tuning loop to after
successful tuning, and explicitly clear ITAPDLY (delay and enable) when
tuning fails so that we are sure only working values are actually left in
hardware.

Fixes: 901d16e46296 ("mmc: sdhci_am654: Add retry tuning")
Cc: stable@vger.kernel.org
Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
Reviewed-by: Judith Mendez <jm@ti.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in v3:
 - Collect R-b from Judith and A-b from Adrian
Changes in v2:
 - As suggested by Sashiko clear the values in itap_del_sel[] and itap_del_ena[]
   so that they agree with what is written in HW and subsequent runtime_suspend()
   and runtime_resume() can never write stale values.
 - Remove now unused timing variable
---
 drivers/mmc/host/sdhci_am654.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 0db587e702ac..8ce10422bba0 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -527,7 +527,6 @@ static int sdhci_am654_do_tuning(struct sdhci_host *host,
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
-	unsigned char timing = host->mmc->ios.timing;
 	struct window fail_window[ITAPDLY_LENGTH];
 	struct device *dev = mmc_dev(host->mmc);
 	u8 curr_pass, itap;
@@ -536,11 +535,8 @@ static int sdhci_am654_do_tuning(struct sdhci_host *host,
 
 	memset(fail_window, 0, sizeof(fail_window));
 
-	/* Enable ITAPDLY */
-	sdhci_am654->itap_del_ena[timing] = 0x1;
-
 	for (itap = 0; itap < ITAPDLY_LENGTH; itap++) {
-		sdhci_am654_write_itapdly(sdhci_am654, itap, sdhci_am654->itap_del_ena[timing]);
+		sdhci_am654_write_itapdly(sdhci_am654, itap, 0x1);
 
 		curr_pass = !mmc_send_tuning(host->mmc, opcode, NULL);
 
@@ -584,10 +580,16 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 
 	if (itapdly < 0) {
 		dev_err(dev, "Failed to find itapdly, fail tuning\n");
+		sdhci_am654_write_itapdly(sdhci_am654, 0, 0);
+		sdhci_am654->itap_del_ena[timing] = 0;
+		sdhci_am654->itap_del_sel[timing] = 0;
 		return -1;
 	}
 
 	dev_dbg(dev, "Passed tuning, final itapdly=%d\n", itapdly);
+
+	/* Enable ITAPDLY */
+	sdhci_am654->itap_del_ena[timing] = 0x1;
 	sdhci_am654_write_itapdly(sdhci_am654, itapdly, sdhci_am654->itap_del_ena[timing]);
 	/* Save ITAPDLY */
 	sdhci_am654->itap_del_sel[timing] = itapdly;

-- 
2.55.0

[PATCH v3 4/4] mmc: sdhci_am654: Fallback to DT-provided itap delay on DDR50 tuning failure

[Diogo Ivo (Schneider Electric)]

DDR50 mode is not required to support the tuning command CMD19, meaning
that calibration may fail on cards that do not implement it, in which
case a known-good itap delay value should be programmed into the host
controller.

Do this by reading the (already defined) itap delay DT property for DDR50
and, if tuning fails for this mode, fall back to the DT-provided itap delay
value. If the DT does not provide a value for DDR50 fallback then this
simply disables using itapdly.

Fixes: 901d16e46296 ("mmc: sdhci_am654: Add retry tuning")
Cc: <stable@vger.kernel.org>
Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
---
Changes in v3:
 - Collect A-b from Adrian
 - Add debug print when applying DT fallback values

Changes in v2:
 - As suggested by Sashiko save values read from DT for DDR50 in separate
   variables in order to keep them across multiple tuning cycles without
   overwriting them.
---
 drivers/mmc/host/sdhci_am654.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index 8ce10422bba0..8edfab5012ed 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -126,7 +126,7 @@ static const struct timing_data td[] = {
 				   NULL,
 				   MMC_CAP_UHS_SDR104},
 	[MMC_TIMING_UHS_DDR50]	= {"ti,otap-del-sel-ddr50",
-				   NULL,
+				   "ti,itap-del-sel-ddr50",
 				   MMC_CAP_UHS_DDR50},
 	[MMC_TIMING_MMC_DDR52]	= {"ti,otap-del-sel-ddr52",
 				   "ti,itap-del-sel-ddr52",
@@ -144,6 +144,8 @@ struct sdhci_am654_data {
 	u32 otap_del_sel[ARRAY_SIZE(td)];
 	u32 itap_del_sel[ARRAY_SIZE(td)];
 	u32 itap_del_ena[ARRAY_SIZE(td)];
+	u32 itap_del_sel_dt_ddr50;
+	u32 itap_del_ena_dt_ddr50;
 	int clkbuf_sel;
 	int trm_icp;
 	int drv_strength;
@@ -579,10 +581,19 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
 	} while (++tuning_loop < RETRY_TUNING_MAX);
 
 	if (itapdly < 0) {
-		dev_err(dev, "Failed to find itapdly, fail tuning\n");
-		sdhci_am654_write_itapdly(sdhci_am654, 0, 0);
-		sdhci_am654->itap_del_ena[timing] = 0;
-		sdhci_am654->itap_del_sel[timing] = 0;
+		if (timing == MMC_TIMING_UHS_DDR50) {
+			dev_dbg(dev, "Failed DDR50 tuning, fallback to DT ITAP\n");
+			sdhci_am654->itap_del_sel[timing] = sdhci_am654->itap_del_sel_dt_ddr50;
+			sdhci_am654->itap_del_ena[timing] = sdhci_am654->itap_del_ena_dt_ddr50;
+		} else {
+			dev_err(dev, "Failed to find itapdly, fail tuning\n");
+			sdhci_am654->itap_del_ena[timing] = 0;
+			sdhci_am654->itap_del_sel[timing] = 0;
+		}
+
+		sdhci_am654_write_itapdly(sdhci_am654,
+					  sdhci_am654->itap_del_sel[timing],
+					  sdhci_am654->itap_del_ena[timing]);
 		return -1;
 	}
 
@@ -758,6 +769,11 @@ static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
 		}
 	}
 
+	sdhci_am654->itap_del_sel_dt_ddr50 =
+		sdhci_am654->itap_del_sel[MMC_TIMING_UHS_DDR50];
+	sdhci_am654->itap_del_ena_dt_ddr50 =
+		sdhci_am654->itap_del_ena[MMC_TIMING_UHS_DDR50];
+
 	return 0;
 }
 

-- 
2.55.0

Re: [PATCH v3 4/4] mmc: sdhci_am654: Fallback to DT-provided itap delay on DDR50 tuning failure

[Mendez, Judith]

Hi Diogo,

On 8/7/2026 6:07 AM, Diogo Ivo (Schneider Electric) wrote:
> DDR50 mode is not required to support the tuning command CMD19, meaning
> that calibration may fail on cards that do not implement it, in which
> case a known-good itap delay value should be programmed into the host
> controller.
> 
> Do this by reading the (already defined) itap delay DT property for DDR50
> and, if tuning fails for this mode, fall back to the DT-provided itap delay
> value. If the DT does not provide a value for DDR50 fallback then this
> simply disables using itapdly.
> 
> Fixes: 901d16e46296 ("mmc: sdhci_am654: Add retry tuning")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Reviewed-by: Judith Mendez <jm@ti.com>

> ---
> Changes in v3:
>   - Collect A-b from Adrian
>   - Add debug print when applying DT fallback values
> 
> Changes in v2:
>   - As suggested by Sashiko save values read from DT for DDR50 in separate
>     variables in order to keep them across multiple tuning cycles without
>     overwriting them.
> ---
>   drivers/mmc/host/sdhci_am654.c | 26 +++++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 8ce10422bba0..8edfab5012ed 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -126,7 +126,7 @@ static const struct timing_data td[] = {
>   				   NULL,
>   				   MMC_CAP_UHS_SDR104},
>   	[MMC_TIMING_UHS_DDR50]	= {"ti,otap-del-sel-ddr50",
> -				   NULL,
> +				   "ti,itap-del-sel-ddr50",
>   				   MMC_CAP_UHS_DDR50},
>   	[MMC_TIMING_MMC_DDR52]	= {"ti,otap-del-sel-ddr52",
>   				   "ti,itap-del-sel-ddr52",
> @@ -144,6 +144,8 @@ struct sdhci_am654_data {
>   	u32 otap_del_sel[ARRAY_SIZE(td)];
>   	u32 itap_del_sel[ARRAY_SIZE(td)];
>   	u32 itap_del_ena[ARRAY_SIZE(td)];
> +	u32 itap_del_sel_dt_ddr50;
> +	u32 itap_del_ena_dt_ddr50;
>   	int clkbuf_sel;
>   	int trm_icp;
>   	int drv_strength;
> @@ -579,10 +581,19 @@ static int sdhci_am654_platform_execute_tuning(struct sdhci_host *host,
>   	} while (++tuning_loop < RETRY_TUNING_MAX);
>   
>   	if (itapdly < 0) {
> -		dev_err(dev, "Failed to find itapdly, fail tuning\n");
> -		sdhci_am654_write_itapdly(sdhci_am654, 0, 0);
> -		sdhci_am654->itap_del_ena[timing] = 0;
> -		sdhci_am654->itap_del_sel[timing] = 0;
> +		if (timing == MMC_TIMING_UHS_DDR50) {
> +			dev_dbg(dev, "Failed DDR50 tuning, fallback to DT ITAP\n");

Its not a big deal, but it would be a bit better to use itapdly
according to the datasheet instead of itap (:

> +			sdhci_am654->itap_del_sel[timing] = sdhci_am654->itap_del_sel_dt_ddr50;
> +			sdhci_am654->itap_del_ena[timing] = sdhci_am654->itap_del_ena_dt_ddr50;
> +		} else {
> +			dev_err(dev, "Failed to find itapdly, fail tuning\n");
> +			sdhci_am654->itap_del_ena[timing] = 0;
> +			sdhci_am654->itap_del_sel[timing] = 0;
> +		}
> +
> +		sdhci_am654_write_itapdly(sdhci_am654,
> +					  sdhci_am654->itap_del_sel[timing],
> +					  sdhci_am654->itap_del_ena[timing]);
>   		return -1;
>   	}
>   
> @@ -758,6 +769,11 @@ static int sdhci_am654_get_otap_delay(struct sdhci_host *host,
>   		}
>   	}
>   
> +	sdhci_am654->itap_del_sel_dt_ddr50 =
> +		sdhci_am654->itap_del_sel[MMC_TIMING_UHS_DDR50];
> +	sdhci_am654->itap_del_ena_dt_ddr50 =
> +		sdhci_am654->itap_del_ena[MMC_TIMING_UHS_DDR50];
> +
>   	return 0;
>   }
>   
> 

Re: [PATCH v3 0/4] mmc: sdhci_am654: Fix DDR50 mode for cards without tuning support

[Ulf Hansson]

On Fri, Aug 7, 2026 at 1:08 PM Diogo Ivo (Schneider Electric)
<diogo.ivo@bootlin.com> wrote:
>
> DDR50 mode is not required to support the tuning command CMD19 by the
> eMMC/SD specification, meaning that calibration may fail on cards that
> do not implement it. This series fixes four issues discovered when
> using such cards:
>
>   - Patch 1 moves the tuning_loop counter from a persistent struct field
>     to a local variable, preventing failed tuning attempts from depleting
>     the retry budget across calls.
>
>   - Patch 2 ensures the command/data line reset is always performed
>     after tuning, even on failure, preventing stale data in the
>     controller buffer.
>
>   - Patch 3 clears the ITAPDLY enable and delay values on tuning
>     failure so that leftover calibration values do not interfere with
>     subsequent I/O.
>
>   - Patch 4 falls back to the DT-provided itap delay value for DDR50
>     when tuning fails in this mode, making a best-effort attempt at a
>     known-good delay being programmed.
>
> Together these changes make DDR50 functional on TI J721E SoCs with
> cards that lack CMD19 tuning support. This second version addresses
> all comments from Sashiko, which I found to be correct.
>
> Signed-off-by: Diogo Ivo (Schneider Electric) <diogo.ivo@bootlin.com>
> ---
> Changes in v3:
> - Collect R-b from Judith and A-b from Adrian
> - Add debug print when applying DT fallback values (Patch 4)
> - Link to v2: https://patch.msgid.link/20260803-am654-sdhci-v2-0-a5ee5be86133@bootlin.com
>

The series applied for fixes, thanks!

Note that, I added a fixes/stable tag to patch1 as well since the
other patches depend on it.

Kind regards
Uffe