diff mbox series

[1/2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output

Message ID 1614762347-12380-1-git-send-email-haibo.chen@nxp.com
State Accepted
Commit 63756575b42b8b4fb3f59cbbf0cedf03331bc2d2
Delegated to: Stefano Babic
Headers show
Series [1/2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output | expand

Commit Message

Bough Chen March 3, 2021, 9:05 a.m. UTC
From: Haibo Chen <haibo.chen@nxp.com>

For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN, these
are reserved bits. Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the
card clock output.

After commit b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support"),
we meet SD3.0 card can't work at UHS mode, mmc_switch_voltage() fail because
the second mmc_wait_dat0 return -ETIMEDOUT. According to SD spec, during
voltage switch, need to gate off/on the card clock. If not set the FRC_SDCLK_ON,
after CMD11, hardware will gate off the card clock automatically, so card do
not detect the clock off/on behavior, so will draw the data0 line low until
next command.

Fixes: b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support")
Tested-by: Tim Harvey <tharvey@gateworks.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 29 +++++++++++++++++++++--------
 include/fsl_esdhc_imx.h     |  2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

Comments

ZHIZHIKIN Andrey March 3, 2021, 10:51 a.m. UTC | #1
Hello Haibo,

> -----Original Message-----
> From: haibo.chen@nxp.com <haibo.chen@nxp.com>
> Sent: Wednesday, March 3, 2021 10:06 AM
> To: peng.fan@nxp.com; u-boot@lists.denx.de; sbabic@denx.de
> Cc: haibo.chen@nxp.com; uboot-imx@nxp.com; tharvey@gateworks.com;
> ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>;
> festevam@gmail.com; ye.li@nxp.com
> Subject: [PATCH 1/2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to
> control card clock output
> 
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
> these are reserved bits. Instead, use VENDORSPEC_FRC_SDCLK_ON to gate
> on/off the card clock output.
> 
> After commit b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support"),
> we meet SD3.0 card can't work at UHS mode, mmc_switch_voltage() fail because
> the second mmc_wait_dat0 return -ETIMEDOUT. According to SD spec, during
> voltage switch, need to gate off/on the card clock. If not set the FRC_SDCLK_ON,
> after CMD11, hardware will gate off the card clock automatically, so card do not
> detect the clock off/on behavior, so will draw the data0 line low until next
> command.

I believe this patch is a RESEND of the previous one, right? I do recall I was trying to test the same patch, but was missing the second one from this series.

Maybe it make sense to deprecate the old submission then, so it's not dangling in Patchwork.

> 
> Fixes: b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> Tested-by: Tim Harvey <tharvey@gateworks.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/fsl_esdhc_imx.c | 29 +++++++++++++++++++++--------
>  include/fsl_esdhc_imx.h     |  2 ++
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index
> e0e132698e..af36558b3c 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -654,7 +654,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct
> mmc *mmc, uint clock)
>         clk = (pre_div << 8) | (div << 4);
> 
>  #ifdef CONFIG_FSL_USDHC
> -       esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
> +       esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +       ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100);
> +       if (ret)
> +               pr_warn("fsl_esdhc_imx: Internal clock never gate
> + off.\n");
>  #else
>         esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);  #endif @@ -666,7 +669,7 @@
> static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>                 pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
> 
>  #ifdef CONFIG_FSL_USDHC
> -       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
> VENDORSPEC_CKEN);
> +       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
>  #else
>         esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);  #endif @@ -
> 721,8 +724,14 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
>         struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
>         struct fsl_esdhc *regs = priv->esdhc_regs;
>         u32 val;
> +       u32 tmp;
> +       int ret;
> 
>         if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {
> +               esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +               ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100);
> +               if (ret)
> +                       pr_warn("fsl_esdhc_imx: Internal clock never
> + gate off.\n");
>                 esdhc_write32(&regs->strobe_dllctrl, ESDHC_STROBE_DLL_CTRL_RESET);
> 
>                 /*
> @@ -740,6 +749,7 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
>                         pr_warn("HS400 strobe DLL status REF not lock!\n");
>                 if (!(val & ESDHC_STROBE_DLL_STS_SLV_LOCK))
>                         pr_warn("HS400 strobe DLL status SLV not lock!\n");
> +               esdhc_setbits32(&regs->vendorspec,
> + VENDORSPEC_FRC_SDCLK_ON);
>         }
>  }
> 
> @@ -963,14 +973,18 @@ static int esdhc_set_ios_common(struct fsl_esdhc_priv
> *priv, struct mmc *mmc)  #ifdef MMC_SUPPORTS_TUNING
>         if (mmc->clk_disable) {
>  #ifdef CONFIG_FSL_USDHC
> -               esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
> +               u32 tmp;
> +
> +               esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +               ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100);
> +               if (ret)
> +                       pr_warn("fsl_esdhc_imx: Internal clock never
> + gate off.\n");
>  #else
>                 esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);  #endif
>         } else {
>  #ifdef CONFIG_FSL_USDHC
> -               esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
> -                               VENDORSPEC_CKEN);
> +               esdhc_setbits32(&regs->vendorspec,
> + VENDORSPEC_FRC_SDCLK_ON);
>  #else
>                 esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);  #endif
> @@ -1046,7 +1060,7 @@ static int esdhc_init_common(struct fsl_esdhc_priv
> *priv, struct mmc *mmc)  #ifndef CONFIG_FSL_USDHC
>         esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);  #else
> -       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_HCKEN |
> VENDORSPEC_IPGEN);
> +       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
>  #endif
> 
>         /* Set the initial clock speed */ @@ -1184,8 +1198,7 @@ static int
> fsl_esdhc_init(struct fsl_esdhc_priv *priv,
>         esdhc_write32(&regs->autoc12err, 0);
>         esdhc_write32(&regs->clktunectrlstatus, 0);  #else
> -       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
> -                       VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
> +       esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
>  #endif
> 
>         if (priv->vs18_enable)
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h index
> 45ed635a77..b092034464 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -39,6 +39,7 @@
>  #define VENDORSPEC_HCKEN       0x00001000
>  #define VENDORSPEC_IPGEN       0x00000800
>  #define VENDORSPEC_INIT                0x20007809
> +#define VENDORSPEC_FRC_SDCLK_ON 0x00000100
> 
>  #define IRQSTAT                        0x0002e030
>  #define IRQSTAT_DMAE           (0x10000000)
> @@ -96,6 +97,7 @@
>  #define PRSSTAT_CINS           (0x00010000)
>  #define PRSSTAT_BREN           (0x00000800)
>  #define PRSSTAT_BWEN           (0x00000400)
> +#define PRSSTAT_SDOFF          (0x00000080)
>  #define PRSSTAT_SDSTB          (0X00000008)
>  #define PRSSTAT_DLA            (0x00000004)
>  #define PRSSTAT_CICHB          (0x00000002)
> --
> 2.17.1

-- andrey
Stefano Babic April 9, 2021, 11:24 a.m. UTC | #2
> From: Haibo Chen <haibo.chen@nxp.com>
> For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN, these
> are reserved bits. Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the
> card clock output.
> After commit b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support"),
> we meet SD3.0 card can't work at UHS mode, mmc_switch_voltage() fail because
> the second mmc_wait_dat0 return -ETIMEDOUT. According to SD spec, during
> voltage switch, need to gate off/on the card clock. If not set the FRC_SDCLK_ON,
> after CMD11, hardware will gate off the card clock automatically, so card do
> not detect the clock off/on behavior, so will draw the data0 line low until
> next command.
> Fixes: b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> Tested-by: Tim Harvey <tharvey@gateworks.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index e0e132698e..af36558b3c 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -654,7 +654,10 @@  static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	clk = (pre_div << 8) | (div << 4);
 
 #ifdef CONFIG_FSL_USDHC
-	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
+	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
+	if (ret)
+		pr_warn("fsl_esdhc_imx: Internal clock never gate off.\n");
 #else
 	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
 #endif
@@ -666,7 +669,7 @@  static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 		pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
 
 #ifdef CONFIG_FSL_USDHC
-	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
+	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
 #else
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 #endif
@@ -721,8 +724,14 @@  static void esdhc_set_strobe_dll(struct mmc *mmc)
 	struct fsl_esdhc_priv *priv = dev_get_priv(mmc->dev);
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 	u32 val;
+	u32 tmp;
+	int ret;
 
 	if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {
+		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+		ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
+		if (ret)
+			pr_warn("fsl_esdhc_imx: Internal clock never gate off.\n");
 		esdhc_write32(&regs->strobe_dllctrl, ESDHC_STROBE_DLL_CTRL_RESET);
 
 		/*
@@ -740,6 +749,7 @@  static void esdhc_set_strobe_dll(struct mmc *mmc)
 			pr_warn("HS400 strobe DLL status REF not lock!\n");
 		if (!(val & ESDHC_STROBE_DLL_STS_SLV_LOCK))
 			pr_warn("HS400 strobe DLL status SLV not lock!\n");
+		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
 	}
 }
 
@@ -963,14 +973,18 @@  static int esdhc_set_ios_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
 #ifdef MMC_SUPPORTS_TUNING
 	if (mmc->clk_disable) {
 #ifdef CONFIG_FSL_USDHC
-		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_CKEN);
+		u32 tmp;
+
+		esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+		ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
+		if (ret)
+			pr_warn("fsl_esdhc_imx: Internal clock never gate off.\n");
 #else
 		esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
 #endif
 	} else {
 #ifdef CONFIG_FSL_USDHC
-		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
-				VENDORSPEC_CKEN);
+		esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
 #else
 		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 #endif
@@ -1046,7 +1060,7 @@  static int esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
 #ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
 #else
-	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_HCKEN | VENDORSPEC_IPGEN);
+	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
 #endif
 
 	/* Set the initial clock speed */
@@ -1184,8 +1198,7 @@  static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
 	esdhc_write32(&regs->autoc12err, 0);
 	esdhc_write32(&regs->clktunectrlstatus, 0);
 #else
-	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_PEREN |
-			VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
+	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
 #endif
 
 	if (priv->vs18_enable)
diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
index 45ed635a77..b092034464 100644
--- a/include/fsl_esdhc_imx.h
+++ b/include/fsl_esdhc_imx.h
@@ -39,6 +39,7 @@ 
 #define VENDORSPEC_HCKEN	0x00001000
 #define VENDORSPEC_IPGEN	0x00000800
 #define VENDORSPEC_INIT		0x20007809
+#define VENDORSPEC_FRC_SDCLK_ON 0x00000100
 
 #define IRQSTAT			0x0002e030
 #define IRQSTAT_DMAE		(0x10000000)
@@ -96,6 +97,7 @@ 
 #define PRSSTAT_CINS		(0x00010000)
 #define PRSSTAT_BREN		(0x00000800)
 #define PRSSTAT_BWEN		(0x00000400)
+#define PRSSTAT_SDOFF		(0x00000080)
 #define PRSSTAT_SDSTB		(0X00000008)
 #define PRSSTAT_DLA		(0x00000004)
 #define PRSSTAT_CICHB		(0x00000002)