[PATCH 0/6] enhance the tuning process reliability for i.MX uSDHC controller

[PATCH 0/6] enhance the tuning process reliability for i.MX uSDHC controller

[ziniu.wang_1]

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

This patch series enhances the tuning process reliability for the i.MX
uSDHC controller by addressing several hardware-specific requirements and
improving alignment with the i.MX Reference Manual recommendations.

Haibo Chen (4):
  mmc: sdhci-esdhc-imx: explicitly reset tuning circuit via RSTT bit
  mmc: sdhci-esdhc-imx: reset async FIFO before sending manual tuning
    command
  mmc: sdhci-esdhc-imx: widen auto-tuning window for standard tuning
  mmc: sdhci-esdhc-imx: widen auto-tuning window for manual tuning

Luke Wang (2):
  mmc: sdhci-esdhc-imx: verify tuning control status after configuration
  mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning
---
This series depends on:
[PATCH v2] mmc: host: sdhci-esdhc-imx: save tuning value for the card which keep power in suspend
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 86 +++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 12 deletions(-)

-- 
2.34.1

[PATCH 1/6] mmc: sdhci-esdhc-imx: explicitly reset tuning circuit via RSTT bit

[ziniu.wang_1]

From: Haibo Chen <haibo.chen@nxp.com>

According to the i.MX Reference Manual, the RSTT bit (SYS_CTRL[28]) is
designed to reset the tuning circuit. While the Reference Manual states
that clearing EXECUTE_TUNING bit from 1 to 0 in AUTOCMD12_ERR_STATUS
can also set RSTT, this mechanism only works when the original
EXECUTE_TUNING bit was 1. When the bit is already 0, the tuning circuit
reset will not be triggered.

This explicit reset approach strengthens the tuning reliability and
aligns with the Reference Manual recommendations.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 7e8addaed697..8c2f5ae949ae 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -32,6 +32,7 @@
 
 #define ESDHC_SYS_CTRL_DTOCV_MASK	GENMASK(19, 16)
 #define ESDHC_SYS_CTRL_IPP_RST_N	BIT(23)
+#define ESDHC_SYS_CTRL_RESET_TUNING	BIT(28)
 #define	ESDHC_CTRL_D3CD			0x08
 #define ESDHC_BURST_LEN_EN_INCR		(1 << 27)
 /* VENDOR SPEC register */
@@ -1060,7 +1061,7 @@ static void esdhc_reset_tuning(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host);
-	u32 ctrl, tuning_ctrl;
+	u32 ctrl, tuning_ctrl, sys_ctrl;
 	int ret;
 
 	/* Reset the tuning circuit */
@@ -1084,6 +1085,11 @@ static void esdhc_reset_tuning(struct sdhci_host *host)
 				writel(tuning_ctrl, host->ioaddr + ESDHC_TUNING_CTRL);
 			}
 
+			/* set the reset tuning bit */
+			sys_ctrl = readl(host->ioaddr + ESDHC_SYSTEM_CONTROL);
+			sys_ctrl |= ESDHC_SYS_CTRL_RESET_TUNING;
+			writel(sys_ctrl, host->ioaddr + ESDHC_SYSTEM_CONTROL);
+
 			ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS);
 			ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL;
 			ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE;
-- 
2.34.1

[PATCH 2/6] mmc: sdhci-esdhc-imx: reset async FIFO before sending manual tuning command

[ziniu.wang_1]

From: Haibo Chen <haibo.chen@nxp.com>

During manual tuning, residual data in the async FIFO from previous
commands may impact with the tuning process. To ensure a clean state:

1. Set the RST_FIFO bit (SYS_CTRL[22]) to reset the async FIFO.
2. Poll the bit until self-cleared, confirming reset completion.

This hardening ensures the tuning command starts with a clean FIFO state,
improving the reliability of the manual tuning procedure.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 8c2f5ae949ae..8528aa6adf02 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -31,6 +31,7 @@
 #include "cqhci.h"
 
 #define ESDHC_SYS_CTRL_DTOCV_MASK	GENMASK(19, 16)
+#define ESDHC_SYS_CTRL_RST_FIFO		BIT(22)
 #define ESDHC_SYS_CTRL_IPP_RST_N	BIT(23)
 #define ESDHC_SYS_CTRL_RESET_TUNING	BIT(28)
 #define	ESDHC_CTRL_D3CD			0x08
@@ -1149,7 +1150,7 @@ static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 {
-	u32 reg;
+	u32 reg, sys_ctrl;
 	u8 sw_rst;
 	int ret;
 
@@ -1173,6 +1174,16 @@ static void esdhc_prepare_tuning(struct sdhci_host *host, u32 val)
 	dev_dbg(mmc_dev(host->mmc),
 		"tuning with delay 0x%x ESDHC_TUNE_CTRL_STATUS 0x%x\n",
 			val, readl(host->ioaddr + ESDHC_TUNE_CTRL_STATUS));
+
+	/* set RST_FIFO to reset the async FIFO, and wat it to self-clear */
+	sys_ctrl = readl(host->ioaddr + ESDHC_SYSTEM_CONTROL);
+	sys_ctrl |= ESDHC_SYS_CTRL_RST_FIFO;
+	writel(sys_ctrl, host->ioaddr + ESDHC_SYSTEM_CONTROL);
+	ret = readl_poll_timeout(host->ioaddr + ESDHC_SYSTEM_CONTROL, sys_ctrl,
+				 !(sys_ctrl & ESDHC_SYS_CTRL_RST_FIFO), 10, 100);
+	if (ret == -ETIMEDOUT)
+		dev_warn(mmc_dev(host->mmc),
+			 "warning! RST_FIFO not clear in 100us\n");
 }
 
 static void esdhc_post_tuning(struct sdhci_host *host)
-- 
2.34.1

[PATCH 3/6] mmc: sdhci-esdhc-imx: widen auto-tuning window for standard tuning

[ziniu.wang_1]

From: Haibo Chen <haibo.chen@nxp.com>

Expand the auto-tuning window width from 2 to 3 for standard tuning to
account for sampling point shifts caused by temperature change. This change
is based on hardware recommendation, providing 50% more margin for the
auto-tuning logic to locate valid sampling points.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 8528aa6adf02..d91b65439544 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -108,6 +108,7 @@
 
 #define ESDHC_TUNING_CTRL		0xcc
 #define ESDHC_STD_TUNING_EN		(1 << 24)
+#define ESDHC_TUNING_WINDOW_MASK	GENMASK(22, 20)
 /* NOTE: the minimum valid tuning start tap for mx6sl is 1 */
 #define ESDHC_TUNING_START_TAP_DEFAULT	0x1
 #define ESDHC_TUNING_START_TAP_MASK	0x7f
@@ -209,6 +210,8 @@
 /* The IP does not have GPIO CD wake capabilities */
 #define ESDHC_FLAG_SKIP_CD_WAKE		BIT(18)
 
+#define ESDHC_AUTO_TUNING_WINDOW	3
+
 enum wp_types {
 	ESDHC_WP_NONE,		/* no WP, neither controller nor gpio */
 	ESDHC_WP_CONTROLLER,	/* mmc controller internal WP */
@@ -1560,6 +1563,16 @@ static void sdhci_esdhc_imx_hwinit(struct sdhci_host *host)
 					<< ESDHC_TUNING_STEP_SHIFT;
 			}
 
+			/*
+			 * Config the tuning window to the hardware suggested value 3.
+			 * This tuning window is used for auto tuning logic. The default
+			 * tuning window is 2, here change to 3 make the window a bit
+			 * wider, give auto tuning enough space to handle the sample
+			 * point shift cause by temperature change.
+			 */
+			 tmp &= ~ESDHC_TUNING_WINDOW_MASK;
+			 tmp |= FIELD_PREP(ESDHC_TUNING_WINDOW_MASK, ESDHC_AUTO_TUNING_WINDOW);
+
 			/* Disable the CMD CRC check for tuning, if not, need to
 			 * add some delay after every tuning command, because
 			 * hardware standard tuning logic will directly go to next
-- 
2.34.1

[PATCH 4/6] mmc: sdhci-esdhc-imx: widen auto-tuning window for manual tuning

[ziniu.wang_1]

From: Haibo Chen <haibo.chen@nxp.com>

Expand the auto-tuning window width from 0 to 3 for manual tuning to
account for sampling point shifts caused by temperature change. This change
is based on hardware recommendation, providing enough margin for the
auto-tuning logic to locate valid sampling points.

When config the manual tuning final sample delay, need deduct the auto
tuning window width according to the IP logic.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index d91b65439544..46da155fe582 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -85,7 +85,8 @@
 #define  ESDHC_TUNE_CTRL_MAX		((1 << 7) - 1)
 #define  ESDHC_TUNE_CTRL_STATUS_TAP_SEL_PRE_MASK	GENMASK(30, 24)
 #define  ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK	GENMASK(14, 8)
-
+#define  ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_OUT_MASK	GENMASK(7, 4)
+#define  ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_POST_MASK	GENMASK(3, 0)
 /* strobe dll register */
 #define ESDHC_STROBE_DLL_CTRL		0x70
 #define ESDHC_STROBE_DLL_CTRL_ENABLE	(1 << 0)
@@ -243,6 +244,7 @@ struct esdhc_platform_data {
 	unsigned int tuning_start_tap;	/* The start delay cell point in tuning procedure */
 	unsigned int strobe_dll_delay_target;	/* The delay cell for strobe pad (read clock) */
 	unsigned int saved_tuning_delay_cell;	/* save the value of tuning delay cell */
+	unsigned int saved_auto_tuning_window;  /* save the auto tuning window width */
 };
 
 struct esdhc_soc_data {
@@ -1206,6 +1208,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int min, max, avg, ret;
 	int win_length, target_min, target_max, target_win_length;
+	u32 clk_tune_ctrl_status;
 
 	min = ESDHC_TUNE_CTRL_MIN;
 	max = ESDHC_TUNE_CTRL_MIN;
@@ -1245,6 +1248,23 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 	/* use average delay to get the best timing */
 	avg = (target_min + target_max) / 2;
 	esdhc_prepare_tuning(host, avg);
+
+	/*
+	 * adjust the delay according to tuning window, make preparation
+	 * for the auto-tuning logic. According to hardware suggest, need
+	 * to config the auto tuning window width to 3, to make the auto
+	 * tuning logic have enough space to handle the sample point shift
+	 * caused by temperature change.
+	 */
+	clk_tune_ctrl_status = FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,
+					  avg - ESDHC_AUTO_TUNING_WINDOW) |
+			       FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_OUT_MASK,
+					  ESDHC_AUTO_TUNING_WINDOW) |
+			       FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_POST_MASK,
+					  ESDHC_AUTO_TUNING_WINDOW);
+
+	writel(clk_tune_ctrl_status, host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
+
 	ret = mmc_send_tuning(host->mmc, opcode, NULL);
 	esdhc_post_tuning(host);
 
@@ -1659,7 +1679,11 @@ static void sdhc_esdhc_tuning_restore(struct sdhci_host *host)
 		writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
 
 		writel(FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK,
-				  imx_data->boarddata.saved_tuning_delay_cell),
+				  imx_data->boarddata.saved_tuning_delay_cell) |
+		       FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_OUT_MASK,
+				  ESDHC_AUTO_TUNING_WINDOW) |
+		       FIELD_PREP(ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_POST_MASK,
+				  ESDHC_AUTO_TUNING_WINDOW),
 		       host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
 	}
 }
-- 
2.34.1

[PATCH 5/6] mmc: sdhci-esdhc-imx: verify tuning control status after configuration

[ziniu.wang_1]

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

Enhance manual tuning configuration reliability by adding tuning control
status checks per the i.MX Reference Manual recommendations.

Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 46da155fe582..fd0ad0ad1519 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -83,6 +83,7 @@
 #define  ESDHC_TUNE_CTRL_STEP		1
 #define  ESDHC_TUNE_CTRL_MIN		0
 #define  ESDHC_TUNE_CTRL_MAX		((1 << 7) - 1)
+#define  ESDHC_TUNE_CTRL_STATUS_TAP_SEL_MASK		GENMASK(30, 16)
 #define  ESDHC_TUNE_CTRL_STATUS_TAP_SEL_PRE_MASK	GENMASK(30, 24)
 #define  ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_PRE_MASK	GENMASK(14, 8)
 #define  ESDHC_TUNE_CTRL_STATUS_DLY_CELL_SET_OUT_MASK	GENMASK(7, 4)
@@ -1208,7 +1209,7 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 {
 	int min, max, avg, ret;
 	int win_length, target_min, target_max, target_win_length;
-	u32 clk_tune_ctrl_status;
+	u32 clk_tune_ctrl_status, temp;
 
 	min = ESDHC_TUNE_CTRL_MIN;
 	max = ESDHC_TUNE_CTRL_MIN;
@@ -1264,6 +1265,13 @@ static int esdhc_executing_tuning(struct sdhci_host *host, u32 opcode)
 					  ESDHC_AUTO_TUNING_WINDOW);
 
 	writel(clk_tune_ctrl_status, host->ioaddr + ESDHC_TUNE_CTRL_STATUS);
+	ret = readl_poll_timeout(host->ioaddr + ESDHC_TUNE_CTRL_STATUS, temp,
+				 clk_tune_ctrl_status ==
+				 FIELD_GET(ESDHC_TUNE_CTRL_STATUS_TAP_SEL_MASK, temp),
+				 1, 10);
+	if (ret == -ETIMEDOUT)
+		dev_warn(mmc_dev(host->mmc),
+			 "clock tuning control status not set in 10us\n");
 
 	ret = mmc_send_tuning(host->mmc, opcode, NULL);
 	esdhc_post_tuning(host);
-- 
2.34.1

[PATCH 6/6] mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning

[ziniu.wang_1]

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

Current standard tuning has some limitations:

1. Standard tuning only try 40 times to find first pass window, but this
pass window maybe not the best pass window.

2. Sometimes there are two tuning pass windows and the gap between
those two windows may only have one cell. If tuning step > 1, the gap may
just be skipped and host assumes those two windows as a continuous
windows. This will cause a bad delay cell near the gap to be selected.

3. Standard tuning logic need to detect at least one success and failure
to pass the tuning. If all cells in the tuning window pass, the hardware
will not set the SDHCI_CTRL_TUNED_CLK bit, causing tuning failed.

4. Standard tuning logic only check the CRC, do not really compare the data
pattern. If data pins are connected incorrectly, standard will not detect
this kind of issue.

Switch to manual tuning to avoid those limitations

Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index fd0ad0ad1519..9b66e07ed8e7 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -275,35 +275,35 @@ static const struct esdhc_soc_data usdhc_imx6q_data = {
 };
 
 static const struct esdhc_soc_data usdhc_imx6sl_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_ERR004536
 			| ESDHC_FLAG_HS200
 			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
 };
 
 static const struct esdhc_soc_data usdhc_imx6sll_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static const struct esdhc_soc_data usdhc_imx6sx_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
 			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
 };
 
 static const struct esdhc_soc_data usdhc_imx6ull_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_ERR010450
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
 };
 
 static const struct esdhc_soc_data usdhc_imx7d_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
@@ -319,7 +319,7 @@ static struct esdhc_soc_data usdhc_s32g2_data = {
 };
 
 static struct esdhc_soc_data usdhc_imx7ulp_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
@@ -332,7 +332,7 @@ static struct esdhc_soc_data usdhc_imxrt1050_data = {
 };
 
 static struct esdhc_soc_data usdhc_imx8qxp_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
@@ -341,7 +341,7 @@ static struct esdhc_soc_data usdhc_imx8qxp_data = {
 };
 
 static struct esdhc_soc_data usdhc_imx8mm_data = {
-	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
+	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
 			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
 			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
 			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
-- 
2.34.1

Re: [PATCH 6/6] mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning

[Adrian Hunter]

On 9/04/25 10:55, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> Current standard tuning has some limitations:
> 
> 1. Standard tuning only try 40 times to find first pass window, but this
> pass window maybe not the best pass window.
> 
> 2. Sometimes there are two tuning pass windows and the gap between
> those two windows may only have one cell. If tuning step > 1, the gap may
> just be skipped and host assumes those two windows as a continuous
> windows. This will cause a bad delay cell near the gap to be selected.
> 
> 3. Standard tuning logic need to detect at least one success and failure
> to pass the tuning. If all cells in the tuning window pass, the hardware
> will not set the SDHCI_CTRL_TUNED_CLK bit, causing tuning failed.
> 
> 4. Standard tuning logic only check the CRC, do not really compare the data
> pattern. If data pins are connected incorrectly, standard will not detect
> this kind of issue.
> 
> Switch to manual tuning to avoid those limitations

Is it necessary to have standard tuning at all then?

> 
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index fd0ad0ad1519..9b66e07ed8e7 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -275,35 +275,35 @@ static const struct esdhc_soc_data usdhc_imx6q_data = {
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6sl_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_ERR004536
>  			| ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6sll_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_HS400
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6sx_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
>  			| ESDHC_FLAG_BROKEN_AUTO_CMD23,
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx6ull_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_ERR010450
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
>  };
>  
>  static const struct esdhc_soc_data usdhc_imx7d_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_HS400
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> @@ -319,7 +319,7 @@ static struct esdhc_soc_data usdhc_s32g2_data = {
>  };
>  
>  static struct esdhc_soc_data usdhc_imx7ulp_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,
> @@ -332,7 +332,7 @@ static struct esdhc_soc_data usdhc_imxrt1050_data = {
>  };
>  
>  static struct esdhc_soc_data usdhc_imx8qxp_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE
> @@ -341,7 +341,7 @@ static struct esdhc_soc_data usdhc_imx8qxp_data = {
>  };
>  
>  static struct esdhc_soc_data usdhc_imx8mm_data = {
> -	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> +	.flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
>  			| ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
>  			| ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
>  			| ESDHC_FLAG_STATE_LOST_IN_LPMODE,

RE: [EXT] Re: [PATCH 6/6] mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning

[Luke Wang]

> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: Tuesday, April 22, 2025 6:53 PM
> To: Luke Wang <ziniu.wang_1@nxp.com>; ulf.hansson@linaro.org; Bough
> Chen <haibo.chen@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; imx@lists.linux.dev; linux-
> mmc@vger.kernel.org; dl-S32 <S32@nxp.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 6/6] mmc: sdhci-esdhc-imx: switch standard tuning
> to manual tuning
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 9/04/25 10:55, ziniu.wang_1@nxp.com wrote:
> > From: Luke Wang <ziniu.wang_1@nxp.com>
> >
> > Current standard tuning has some limitations:
> >
> > 1. Standard tuning only try 40 times to find first pass window, but this
> > pass window maybe not the best pass window.
> >
> > 2. Sometimes there are two tuning pass windows and the gap between
> > those two windows may only have one cell. If tuning step > 1, the gap may
> > just be skipped and host assumes those two windows as a continuous
> > windows. This will cause a bad delay cell near the gap to be selected.
> >
> > 3. Standard tuning logic need to detect at least one success and failure
> > to pass the tuning. If all cells in the tuning window pass, the hardware
> > will not set the SDHCI_CTRL_TUNED_CLK bit, causing tuning failed.
> >
> > 4. Standard tuning logic only check the CRC, do not really compare the data
> > pattern. If data pins are connected incorrectly, standard will not detect
> > this kind of issue.
> >
> > Switch to manual tuning to avoid those limitations
> 
> Is it necessary to have standard tuning at all then?

One advantage of standard tuning is that it can be attempted up to 40 times. Usually, one standard tuning takes about 20 to 30 milliseconds to complete. However, in order to obtain a better window through manual tuning, 127 tuning attempts will be made, which takes approximately over 100 milliseconds. 

The four limitations mentioned are all practical issues encountered on different i.MX platforms

> 
> >
> > Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> > index fd0ad0ad1519..9b66e07ed8e7 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -275,35 +275,35 @@ static const struct esdhc_soc_data
> usdhc_imx6q_data = {
> >  };
> >
> >  static const struct esdhc_soc_data usdhc_imx6sl_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_ERR004536
> >                       | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_BROKEN_AUTO_CMD23,
> >  };
> >
> >  static const struct esdhc_soc_data usdhc_imx6sll_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_HS400
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE,
> >  };
> >
> >  static const struct esdhc_soc_data usdhc_imx6sx_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE
> >                       | ESDHC_FLAG_BROKEN_AUTO_CMD23,
> >  };
> >
> >  static const struct esdhc_soc_data usdhc_imx6ull_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_ERR010450
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE,
> >  };
> >
> >  static const struct esdhc_soc_data usdhc_imx7d_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_HS400
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE
> > @@ -319,7 +319,7 @@ static struct esdhc_soc_data usdhc_s32g2_data = {
> >  };
> >
> >  static struct esdhc_soc_data usdhc_imx7ulp_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_PMQOS | ESDHC_FLAG_HS400
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE,
> > @@ -332,7 +332,7 @@ static struct esdhc_soc_data
> usdhc_imxrt1050_data = {
> >  };
> >
> >  static struct esdhc_soc_data usdhc_imx8qxp_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE
> > @@ -341,7 +341,7 @@ static struct esdhc_soc_data usdhc_imx8qxp_data
> = {
> >  };
> >
> >  static struct esdhc_soc_data usdhc_imx8mm_data = {
> > -     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_STD_TUNING
> > +     .flags = ESDHC_FLAG_USDHC | ESDHC_FLAG_MAN_TUNING
> >                       | ESDHC_FLAG_HAVE_CAP1 | ESDHC_FLAG_HS200
> >                       | ESDHC_FLAG_HS400 | ESDHC_FLAG_HS400_ES
> >                       | ESDHC_FLAG_STATE_LOST_IN_LPMODE,

Re: [PATCH 0/6] enhance the tuning process reliability for i.MX uSDHC controller

[Adrian Hunter]

On 9/04/25 10:55, ziniu.wang_1@nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
> 
> This patch series enhances the tuning process reliability for the i.MX
> uSDHC controller by addressing several hardware-specific requirements and
> improving alignment with the i.MX Reference Manual recommendations.
> 
> Haibo Chen (4):
>   mmc: sdhci-esdhc-imx: explicitly reset tuning circuit via RSTT bit
>   mmc: sdhci-esdhc-imx: reset async FIFO before sending manual tuning
>     command
>   mmc: sdhci-esdhc-imx: widen auto-tuning window for standard tuning
>   mmc: sdhci-esdhc-imx: widen auto-tuning window for manual tuning
> 
> Luke Wang (2):
>   mmc: sdhci-esdhc-imx: verify tuning control status after configuration
>   mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning
> ---
> This series depends on:
> [PATCH v2] mmc: host: sdhci-esdhc-imx: save tuning value for the card which keep power in suspend
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 86 +++++++++++++++++++++++++-----
>  1 file changed, 74 insertions(+), 12 deletions(-)
> 

I had one question about patch 6 but otherwise, for
all 6 patches:

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

Re: [PATCH 0/6] enhance the tuning process reliability for i.MX uSDHC controller

[Ulf Hansson]

On Wed, 9 Apr 2025 at 09:54, <ziniu.wang_1@nxp.com> wrote:
>
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> This patch series enhances the tuning process reliability for the i.MX
> uSDHC controller by addressing several hardware-specific requirements and
> improving alignment with the i.MX Reference Manual recommendations.
>
> Haibo Chen (4):
>   mmc: sdhci-esdhc-imx: explicitly reset tuning circuit via RSTT bit
>   mmc: sdhci-esdhc-imx: reset async FIFO before sending manual tuning
>     command
>   mmc: sdhci-esdhc-imx: widen auto-tuning window for standard tuning
>   mmc: sdhci-esdhc-imx: widen auto-tuning window for manual tuning
>
> Luke Wang (2):
>   mmc: sdhci-esdhc-imx: verify tuning control status after configuration
>   mmc: sdhci-esdhc-imx: switch standard tuning to manual tuning
> ---
> This series depends on:
> [PATCH v2] mmc: host: sdhci-esdhc-imx: save tuning value for the card which keep power in suspend
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 86 +++++++++++++++++++++++++-----
>  1 file changed, 74 insertions(+), 12 deletions(-)
>
> --
> 2.34.1
>

Applied for next, thanks!

Kind regards
Uffe