[PATCH v2 4/7] mmc: sdhci-of-k1: add comprehensive SDR tuning support

Iker Pedrosa posted 7 patches 1 month ago
There is a newer version of this series
[PATCH v2 4/7] mmc: sdhci-of-k1: add comprehensive SDR tuning support
Posted by Iker Pedrosa 1 month ago
Implement software tuning algorithm to enable UHS-I SDR modes for SD
card operation. This adds both TX and RX delay line tuning based on the
SpacemiT K1 controller capabilities.

Key features:
- Conditional tuning: only tune when SD card is present and for
  high-speed modes (≥100MHz)
- TX tuning: configure transmit delay line with default values
  (dline_reg=0, delaycode=127) to ensure optimal signal output timing
- RX tuning: test full delay range (0-255) with window detection
  algorithm to find optimal receive timing
- Retry mechanism: multiple fallback delays within optimal window for
  improved reliability
- Complete register support: add delay line control and configuration
  register definitions for fine-grained timing control

Signed-off-by: Iker Pedrosa <ikerpedrosam@gmail.com>
---
 drivers/mmc/host/sdhci-of-k1.c | 119 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
index 79cb7c8d0b6d9c4206bf01721651c8efe8a173c9..d903851b9be0e1d21a2b30636f5e63a52cad0dc2 100644
--- a/drivers/mmc/host/sdhci-of-k1.c
+++ b/drivers/mmc/host/sdhci-of-k1.c
@@ -84,6 +84,12 @@
 #define  SDHC_TX_DLINE_REG_MASK         GENMASK(23, 16)
 
 #define SPACEMIT_RX_DLINE_REG		9
+#define SPACEMIT_RX_TUNE_DELAY_MIN	0x0
+#define SPACEMIT_RX_TUNE_DELAY_MAX	0xFF
+#define SPACEMIT_RX_TUNE_DELAY_STEP	0x1
+
+#define SPACEMIT_TX_TUNING_DLINE_REG	0x00
+#define SPACEMIT_TX_TUNING_DELAYCODE	127
 
 struct spacemit_sdhci_host {
 	struct clk *clk_core;
@@ -251,6 +257,118 @@ static unsigned int spacemit_sdhci_clk_get_max_clock(struct sdhci_host *host)
 	return clk_get_rate(pltfm_host->clk);
 }
 
+static int spacemit_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
+{
+	int ret = 0;
+	int i;
+	bool pass_window[SPACEMIT_RX_TUNE_DELAY_MAX + 1] = {false};
+	int pass_len = 0, pass_start = 0, max_pass_len = 0, max_pass_start = 0;
+	u8 final_delay;
+	struct mmc_host *mmc = host->mmc;
+	struct mmc_ios ios = mmc->ios;
+
+	/*
+	 * Tuning is required for SDR50/SDR104, HS200/HS400 cards and
+	 * if clock frequency is greater than 100MHz in these modes.
+	 */
+	if (host->clock < 100 * 1000 * 1000 ||
+	    !(ios.timing == MMC_TIMING_MMC_HS200 ||
+	      ios.timing == MMC_TIMING_UHS_SDR50 ||
+	      ios.timing == MMC_TIMING_UHS_SDR104))
+		return 0;
+
+	if (!(mmc->caps2 & MMC_CAP2_NO_SD) && !mmc->ops->get_cd(mmc))
+		return 0;
+
+	if (mmc->caps2 & MMC_CAP2_NO_MMC) {
+		spacemit_sdhci_set_tx_dline_reg(host, SPACEMIT_TX_TUNING_DLINE_REG);
+		spacemit_sdhci_set_tx_delay(host, SPACEMIT_TX_TUNING_DELAYCODE);
+		spacemit_sdhci_tx_tuning_prepare(host);
+
+		dev_dbg(mmc_dev(host->mmc), "TX tuning: dline_reg=%d, delaycode=%d\n",
+			SPACEMIT_TX_TUNING_DLINE_REG, SPACEMIT_TX_TUNING_DELAYCODE);
+	}
+
+	spacemit_sdhci_prepare_tuning(host);
+
+	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
+	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {
+		spacemit_sdhci_set_rx_delay(host, i);
+
+		ret = mmc_send_tuning(host->mmc, opcode, NULL);
+		pass_window[i] = (ret == 0);
+
+		dev_dbg(mmc_dev(host->mmc), "RX delay %d: %s\n",
+			i, pass_window[i] ? "pass" : "fail");
+	}
+
+	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
+	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {
+		if (pass_window[i]) {
+			if (pass_len == 0)
+				pass_start = i;
+			pass_len++;
+		} else {
+			if (pass_len > max_pass_len) {
+				max_pass_len = pass_len;
+				max_pass_start = pass_start;
+			}
+			pass_len = 0;
+		}
+	}
+
+	if (pass_len > max_pass_len) {
+		max_pass_len = pass_len;
+		max_pass_start = pass_start;
+	}
+
+	if (max_pass_len < 3) {
+		dev_err(mmc_dev(host->mmc), "Tuning failed: no stable window found\n");
+		return -EIO;
+	}
+
+	final_delay = max_pass_start + max_pass_len / 2;
+	spacemit_sdhci_set_rx_delay(host, final_delay);
+	ret = mmc_send_tuning(host->mmc, opcode, NULL);
+	if (ret) {
+		u8 retry_delays[] = {
+			max_pass_start + max_pass_len / 4,
+			max_pass_start + (3 * max_pass_len) / 4,
+			max_pass_start,
+			max_pass_start + max_pass_len - 1
+		};
+		int retry_count = ARRAY_SIZE(retry_delays);
+
+		dev_warn(mmc_dev(mmc), "Primary delay %d failed, trying alternatives\n",
+			 final_delay);
+
+		for (i = 0; i < retry_count; i++) {
+			if (retry_delays[i] >= SPACEMIT_RX_TUNE_DELAY_MIN &&
+			    retry_delays[i] <= SPACEMIT_RX_TUNE_DELAY_MAX) {
+				spacemit_sdhci_set_rx_delay(host, retry_delays[i]);
+				ret = mmc_send_tuning(host->mmc, opcode, NULL);
+				if (!ret) {
+					final_delay = retry_delays[i];
+					dev_info(mmc_dev(mmc), "Retry successful with delay %d\n",
+						 final_delay);
+					break;
+				}
+			}
+		}
+
+		if (ret) {
+			dev_err(mmc_dev(mmc), "All retry attempts failed\n");
+			return -EIO;
+		}
+	}
+
+	dev_dbg(mmc_dev(host->mmc),
+		"Tuning successful: window %d-%d, using delay %d\n",
+		max_pass_start, max_pass_start + max_pass_len - 1, final_delay);
+
+	return 0;
+}
+
 static int spacemit_sdhci_pre_select_hs400(struct mmc_host *mmc)
 {
 	struct sdhci_host *host = mmc_priv(mmc);
@@ -344,6 +462,7 @@ static const struct sdhci_ops spacemit_sdhci_ops = {
 	.set_clock		= spacemit_sdhci_set_clock,
 	.set_uhs_signaling	= spacemit_sdhci_set_uhs_signaling,
 	.voltage_switch         = spacemit_sdhci_voltage_switch,
+	.platform_execute_tuning = spacemit_sdhci_execute_tuning,
 };
 
 static const struct sdhci_pltfm_data spacemit_sdhci_k1_pdata = {

-- 
2.53.0

Re: [PATCH v2 4/7] mmc: sdhci-of-k1: add comprehensive SDR tuning support
Posted by Yao Zi 3 weeks, 5 days ago
On Mon, Mar 09, 2026 at 12:40:14PM +0100, Iker Pedrosa wrote:
> Implement software tuning algorithm to enable UHS-I SDR modes for SD
> card operation. This adds both TX and RX delay line tuning based on the
> SpacemiT K1 controller capabilities.
> 
> Key features:
> - Conditional tuning: only tune when SD card is present and for
>   high-speed modes (≥100MHz)
> - TX tuning: configure transmit delay line with default values
>   (dline_reg=0, delaycode=127) to ensure optimal signal output timing
> - RX tuning: test full delay range (0-255) with window detection
>   algorithm to find optimal receive timing
> - Retry mechanism: multiple fallback delays within optimal window for
>   improved reliability
> - Complete register support: add delay line control and configuration
>   register definitions for fine-grained timing control
> 
> Signed-off-by: Iker Pedrosa <ikerpedrosam@gmail.com>

Sorry for raising this late: I think PATCH 3 and PATCH 4 (this one)
should also be squashed together to form a complete functionality, I
also doubt otherwise whether compilers complain about unused static
functions with only PATCH 3 applied.

> ---
>  drivers/mmc/host/sdhci-of-k1.c | 119 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> index 79cb7c8d0b6d9c4206bf01721651c8efe8a173c9..d903851b9be0e1d21a2b30636f5e63a52cad0dc2 100644
> --- a/drivers/mmc/host/sdhci-of-k1.c
> +++ b/drivers/mmc/host/sdhci-of-k1.c
> @@ -84,6 +84,12 @@
>  #define  SDHC_TX_DLINE_REG_MASK         GENMASK(23, 16)
>  
>  #define SPACEMIT_RX_DLINE_REG		9
> +#define SPACEMIT_RX_TUNE_DELAY_MIN	0x0
> +#define SPACEMIT_RX_TUNE_DELAY_MAX	0xFF
> +#define SPACEMIT_RX_TUNE_DELAY_STEP	0x1

I think the STEP constant isn't very helpful, to me its purpose is
quite obvious in spacemit_sdhci_execute_tuning(), and you could simplify
the tuning function without adding confusing magic numbers -- since the
constant is literally one.

> +#define SPACEMIT_TX_TUNING_DLINE_REG	0x00
> +#define SPACEMIT_TX_TUNING_DELAYCODE	127

...

> +static int spacemit_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> +	int ret = 0;
> +	int i;
> +	bool pass_window[SPACEMIT_RX_TUNE_DELAY_MAX + 1] = {false};
> +	int pass_len = 0, pass_start = 0, max_pass_len = 0, max_pass_start = 0;
> +	u8 final_delay;
> +	struct mmc_host *mmc = host->mmc;
> +	struct mmc_ios ios = mmc->ios;
> +
> +	/*
> +	 * Tuning is required for SDR50/SDR104, HS200/HS400 cards and
> +	 * if clock frequency is greater than 100MHz in these modes.
> +	 */
> +	if (host->clock < 100 * 1000 * 1000 ||
> +	    !(ios.timing == MMC_TIMING_MMC_HS200 ||
> +	      ios.timing == MMC_TIMING_UHS_SDR50 ||
> +	      ios.timing == MMC_TIMING_UHS_SDR104))
> +		return 0;
> +
> +	if (!(mmc->caps2 & MMC_CAP2_NO_SD) && !mmc->ops->get_cd(mmc))
> +		return 0;

Is this check really necessary? Shouldn't this be handled in MMC core
instead?

> +	if (mmc->caps2 & MMC_CAP2_NO_MMC) {
> +		spacemit_sdhci_set_tx_dline_reg(host, SPACEMIT_TX_TUNING_DLINE_REG);
> +		spacemit_sdhci_set_tx_delay(host, SPACEMIT_TX_TUNING_DELAYCODE);
> +		spacemit_sdhci_tx_tuning_prepare(host);
> +
> +		dev_dbg(mmc_dev(host->mmc), "TX tuning: dline_reg=%d, delaycode=%d\n",
> +			SPACEMIT_TX_TUNING_DLINE_REG, SPACEMIT_TX_TUNING_DELAYCODE);
> +	}
> +
> +	spacemit_sdhci_prepare_tuning(host);
> +
> +	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
> +	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {

The last expression could be simply i++ if you drop
SPACEMIT_RX_TUNE_DELAY_STEP, this is still quite readable. Same for the
loop below.

> +		spacemit_sdhci_set_rx_delay(host, i);
> +
> +		ret = mmc_send_tuning(host->mmc, opcode, NULL);
> +		pass_window[i] = (ret == 0);
> +
> +		dev_dbg(mmc_dev(host->mmc), "RX delay %d: %s\n",
> +			i, pass_window[i] ? "pass" : "fail");
> +	}
> +
> +	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
> +	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {
> +		if (pass_window[i]) {
> +			if (pass_len == 0)
> +				pass_start = i;
> +			pass_len++;
> +		} else {
> +			if (pass_len > max_pass_len) {
> +				max_pass_len = pass_len;
> +				max_pass_start = pass_start;
> +			}
> +			pass_len = 0;
> +		}
> +	}

It seems pass_window is only used once in the later loop, why not
calculate and maintain the best window and its size on the fly? Then you
could get avoid of the pass_window buffer, and refactor a loop away.
Something like,

	int current_size, max_window, max_size;

	current_size = 0;
	max_size = 0;
	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i < SPACEMIT_RX_TUNE_DELAY_MAX;
	     i++) {
		ret = /* try tuning ... */

		if (ret) {
			if (max_size < current_size) {
				max_window = i - current_size;
				max_size = current_size;
			}

			current_size = 0;
		} else {
			current_size++;
		}
	}

	if (current_size > max_size) {
		max_window = i - current_size;
		max_size = current_size;
		current_size = 0;
	}

should work, too, and is simpler.

Regards,
Yao Zi
Re: [PATCH v2 4/7] mmc: sdhci-of-k1: add comprehensive SDR tuning support
Posted by Adrian Hunter 3 weeks, 5 days ago
On 09/03/2026 13:40, Iker Pedrosa wrote:
> Implement software tuning algorithm to enable UHS-I SDR modes for SD
> card operation. This adds both TX and RX delay line tuning based on the
> SpacemiT K1 controller capabilities.

Need to say something about the HS200 support since it is
eMMC only

> 
> Key features:
> - Conditional tuning: only tune when SD card is present and for
>   high-speed modes (≥100MHz)
> - TX tuning: configure transmit delay line with default values
>   (dline_reg=0, delaycode=127) to ensure optimal signal output timing
> - RX tuning: test full delay range (0-255) with window detection
>   algorithm to find optimal receive timing
> - Retry mechanism: multiple fallback delays within optimal window for
>   improved reliability
> - Complete register support: add delay line control and configuration
>   register definitions for fine-grained timing control
> 
> Signed-off-by: Iker Pedrosa <ikerpedrosam@gmail.com>
> ---
>  drivers/mmc/host/sdhci-of-k1.c | 119 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> index 79cb7c8d0b6d9c4206bf01721651c8efe8a173c9..d903851b9be0e1d21a2b30636f5e63a52cad0dc2 100644
> --- a/drivers/mmc/host/sdhci-of-k1.c
> +++ b/drivers/mmc/host/sdhci-of-k1.c
> @@ -84,6 +84,12 @@
>  #define  SDHC_TX_DLINE_REG_MASK         GENMASK(23, 16)
>  
>  #define SPACEMIT_RX_DLINE_REG		9
> +#define SPACEMIT_RX_TUNE_DELAY_MIN	0x0
> +#define SPACEMIT_RX_TUNE_DELAY_MAX	0xFF
> +#define SPACEMIT_RX_TUNE_DELAY_STEP	0x1
> +
> +#define SPACEMIT_TX_TUNING_DLINE_REG	0x00
> +#define SPACEMIT_TX_TUNING_DELAYCODE	127
>  
>  struct spacemit_sdhci_host {
>  	struct clk *clk_core;
> @@ -251,6 +257,118 @@ static unsigned int spacemit_sdhci_clk_get_max_clock(struct sdhci_host *host)
>  	return clk_get_rate(pltfm_host->clk);
>  }
>  
> +static int spacemit_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> +	int ret = 0;
> +	int i;
> +	bool pass_window[SPACEMIT_RX_TUNE_DELAY_MAX + 1] = {false};
> +	int pass_len = 0, pass_start = 0, max_pass_len = 0, max_pass_start = 0;
> +	u8 final_delay;
> +	struct mmc_host *mmc = host->mmc;
> +	struct mmc_ios ios = mmc->ios;

Prefer to arrange local definitions/declarations in order of
descending line length e.g.

	int pass_len = 0, pass_start = 0, max_pass_len = 0, max_pass_start = 0;
	bool pass_window[SPACEMIT_RX_TUNE_DELAY_MAX + 1] = {false};
	struct mmc_host *mmc = host->mmc;
	struct mmc_ios ios = mmc->ios;
	u8 final_delay;
	int ret = 0;
	int i;

> +
> +	/*
> +	 * Tuning is required for SDR50/SDR104, HS200/HS400 cards and
> +	 * if clock frequency is greater than 100MHz in these modes.
> +	 */
> +	if (host->clock < 100 * 1000 * 1000 ||
> +	    !(ios.timing == MMC_TIMING_MMC_HS200 ||
> +	      ios.timing == MMC_TIMING_UHS_SDR50 ||
> +	      ios.timing == MMC_TIMING_UHS_SDR104))
> +		return 0;
> +
> +	if (!(mmc->caps2 & MMC_CAP2_NO_SD) && !mmc->ops->get_cd(mmc))
> +		return 0;
> +
> +	if (mmc->caps2 & MMC_CAP2_NO_MMC) {
> +		spacemit_sdhci_set_tx_dline_reg(host, SPACEMIT_TX_TUNING_DLINE_REG);
> +		spacemit_sdhci_set_tx_delay(host, SPACEMIT_TX_TUNING_DELAYCODE);
> +		spacemit_sdhci_tx_tuning_prepare(host);
> +
> +		dev_dbg(mmc_dev(host->mmc), "TX tuning: dline_reg=%d, delaycode=%d\n",
> +			SPACEMIT_TX_TUNING_DLINE_REG, SPACEMIT_TX_TUNING_DELAYCODE);
> +	}
> +
> +	spacemit_sdhci_prepare_tuning(host);
> +
> +	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
> +	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {
> +		spacemit_sdhci_set_rx_delay(host, i);
> +
> +		ret = mmc_send_tuning(host->mmc, opcode, NULL);
> +		pass_window[i] = (ret == 0);
> +
> +		dev_dbg(mmc_dev(host->mmc), "RX delay %d: %s\n",
> +			i, pass_window[i] ? "pass" : "fail");
> +	}
> +
> +	for (i = SPACEMIT_RX_TUNE_DELAY_MIN; i <= SPACEMIT_RX_TUNE_DELAY_MAX;
> +	     i += SPACEMIT_RX_TUNE_DELAY_STEP) {
> +		if (pass_window[i]) {
> +			if (pass_len == 0)
> +				pass_start = i;
> +			pass_len++;
> +		} else {
> +			if (pass_len > max_pass_len) {
> +				max_pass_len = pass_len;
> +				max_pass_start = pass_start;
> +			}
> +			pass_len = 0;
> +		}
> +	}
> +
> +	if (pass_len > max_pass_len) {
> +		max_pass_len = pass_len;
> +		max_pass_start = pass_start;
> +	}
> +
> +	if (max_pass_len < 3) {
> +		dev_err(mmc_dev(host->mmc), "Tuning failed: no stable window found\n");
> +		return -EIO;
> +	}
> +
> +	final_delay = max_pass_start + max_pass_len / 2;
> +	spacemit_sdhci_set_rx_delay(host, final_delay);
> +	ret = mmc_send_tuning(host->mmc, opcode, NULL);
> +	if (ret) {
> +		u8 retry_delays[] = {
> +			max_pass_start + max_pass_len / 4,
> +			max_pass_start + (3 * max_pass_len) / 4,
> +			max_pass_start,
> +			max_pass_start + max_pass_len - 1
> +		};
> +		int retry_count = ARRAY_SIZE(retry_delays);
> +
> +		dev_warn(mmc_dev(mmc), "Primary delay %d failed, trying alternatives\n",
> +			 final_delay);
> +
> +		for (i = 0; i < retry_count; i++) {
> +			if (retry_delays[i] >= SPACEMIT_RX_TUNE_DELAY_MIN &&
> +			    retry_delays[i] <= SPACEMIT_RX_TUNE_DELAY_MAX) {
> +				spacemit_sdhci_set_rx_delay(host, retry_delays[i]);
> +				ret = mmc_send_tuning(host->mmc, opcode, NULL);
> +				if (!ret) {
> +					final_delay = retry_delays[i];
> +					dev_info(mmc_dev(mmc), "Retry successful with delay %d\n",
> +						 final_delay);
> +					break;
> +				}
> +			}
> +		}
> +
> +		if (ret) {
> +			dev_err(mmc_dev(mmc), "All retry attempts failed\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	dev_dbg(mmc_dev(host->mmc),
> +		"Tuning successful: window %d-%d, using delay %d\n",
> +		max_pass_start, max_pass_start + max_pass_len - 1, final_delay);
> +
> +	return 0;
> +}
> +
>  static int spacemit_sdhci_pre_select_hs400(struct mmc_host *mmc)
>  {
>  	struct sdhci_host *host = mmc_priv(mmc);
> @@ -344,6 +462,7 @@ static const struct sdhci_ops spacemit_sdhci_ops = {
>  	.set_clock		= spacemit_sdhci_set_clock,
>  	.set_uhs_signaling	= spacemit_sdhci_set_uhs_signaling,
>  	.voltage_switch         = spacemit_sdhci_voltage_switch,
> +	.platform_execute_tuning = spacemit_sdhci_execute_tuning,
>  };
>  
>  static const struct sdhci_pltfm_data spacemit_sdhci_k1_pdata = {
>