diff mbox series

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

Message ID 20250409075550.3413032-7-ziniu.wang_1@nxp.com
State New
Headers show
Series enhance the tuning process reliability for i.MX uSDHC controller | expand

Commit Message

Luke Wang April 9, 2025, 7:55 a.m. UTC
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(-)

Comments

Adrian Hunter April 22, 2025, 10:53 a.m. UTC | #1
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,
Luke Wang April 22, 2025, 11:09 a.m. UTC | #2
> -----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,
diff mbox series

Patch

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,