All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
@ 2022-02-22  3:28 ` haibo.chen
  2022-02-23  5:28   ` Peng Fan (OSS)
                     ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: haibo.chen @ 2022-02-22  3:28 UTC (permalink / raw)
  To: peng.fan, jh80.chung, festevam, sean.anderson, u-boot, marex,
	aford173, tharvey, andrey.zhizhikin
  Cc: uboot-imx, haibo.chen

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

After commit f132aab40327 ("Revert "mmc: fsl_esdhc_imx: use
VENDORSPEC_FRC_SDCLK_ON to control card clock output""), it
involve issue in mmc_switch_voltage(), because of the special
design of usdhc.

For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
these are reserved bits(Though RM contain the definition of these bits,
but actually internal IC logic do not implement, already confirm with
IC team). Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the card
clock output. Here is the definition of this bit in RM:

[8] FRC_SDCLK_ON
Force CLK output active
Do not set this bit to 1 unless it is necessary. Also, make sure that
this bit is cleared when uSDHC’s clock is about to be changed (frequency
change, clock source change, or delay chain tuning).
0b - CLK active or inactive is fully controlled by the hardware.
1b - Force CLK active

In default, the FRC_SDCLK_ON is 0. This means, when there is no command
or data transfer on bus, hardware will gate off the card clock. But in
some case, we need the card clock keep on. Take IO voltage 1.8v switch
as example, after IO voltage change to 1.8v, spec require gate off the
card clock for 5ms, and gate on the clock back, once detect the card
clock on, then the card will draw the dat0 to high immediately. If there
is not clock gate off/on behavior, some card will keep the dat0 to low
level. This is the reason we fail in mmc_switch_voltage().

To fix this issue, and concern that this is only the fsl usdhc hardware
design limitation, set the bit FRC_SDCLK_ON in the beginning of the
wait_dat0() and clear it in the end. To make sure the 1.8v IO voltage
switch process align with SD specification.

For standard tuning process, usdhc specification also require the card
clock keep on, so also add these behavior in fsl_esdhc_execute_tuning().

Reviewed-by: Marek Vasut <marex@denx.de>
Tested-by: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 25 ++++++++++++++++++++++---
 include/fsl_esdhc_imx.h     |  2 ++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 9299635f50..e0108144e7 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -831,13 +831,16 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
 	struct mmc *mmc = &plat->mmc;
 	u32 irqstaten = esdhc_read32(&regs->irqstaten);
 	u32 irqsigen = esdhc_read32(&regs->irqsigen);
-	int i, ret = -ETIMEDOUT;
-	u32 val, mixctrl;
+	int i, err, ret = -ETIMEDOUT;
+	u32 val, mixctrl, tmp;
 
 	/* clock tuning is not needed for upto 52MHz */
 	if (mmc->clock <= 52000000)
 		return 0;
 
+	/* make sure the card clock keep on */
+	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+
 	/* This is readw/writew SDHCI_HOST_CONTROL2 when tuning */
 	if (priv->flags & ESDHC_FLAG_STD_TUNING) {
 		val = esdhc_read32(&regs->autoc12err);
@@ -897,6 +900,12 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
 
 	esdhc_stop_tuning(mmc);
 
+	/* change to default setting, let host control the card clock */
+	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
+	if (err)
+		dev_warn(dev, "card clock not gate off as expect.\n");
+
 	return ret;
 }
 #endif
@@ -1555,14 +1564,24 @@ static int __maybe_unused fsl_esdhc_set_enhanced_strobe(struct udevice *dev)
 static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
 				int timeout_us)
 {
-	int ret;
+	int ret, err;
 	u32 tmp;
 	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
 	struct fsl_esdhc *regs = priv->esdhc_regs;
 
+	/* make sure the card clock keep on */
+	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+
 	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp,
 				!!(tmp & PRSSTAT_DAT0) == !!state,
 				timeout_us);
+
+	/* change to default setting, let host control the card clock */
+	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
+	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
+	if (err)
+		dev_warn(dev, "card clock not gate off as expect.\n");
+
 	return ret;
 }
 
diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
index 2153f29bef..b8efd2a166 100644
--- a/include/fsl_esdhc_imx.h
+++ b/include/fsl_esdhc_imx.h
@@ -37,6 +37,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)
@@ -94,6 +95,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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* RE: [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-22  3:28 ` [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
@ 2022-02-23  5:28   ` Peng Fan (OSS)
  2022-02-24  0:03   ` Jaehoon Chung
  2022-03-09 16:10   ` Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Peng Fan (OSS) @ 2022-02-23  5:28 UTC (permalink / raw)
  To: Bough Chen, jh80.chung, festevam, sean.anderson, u-boot, marex,
	aford173, tharvey, andrey.zhizhikin
  Cc: dl-uboot-imx, Bough Chen

> Subject: [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON
> when necessary
> 
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> After commit f132aab40327 ("Revert "mmc: fsl_esdhc_imx: use
> VENDORSPEC_FRC_SDCLK_ON to control card clock output""), it involve issue
> in mmc_switch_voltage(), because of the special design of usdhc.
> 
> For FSL_USDHC, it do not implement
> VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
> these are reserved bits(Though RM contain the definition of these bits, but
> actually internal IC logic do not implement, already confirm with IC team).
> Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the card clock
> output. Here is the definition of this bit in RM:
> 
> [8] FRC_SDCLK_ON
> Force CLK output active
> Do not set this bit to 1 unless it is necessary. Also, make sure that this bit is
> cleared when uSDHC’s clock is about to be changed (frequency change, clock
> source change, or delay chain tuning).
> 0b - CLK active or inactive is fully controlled by the hardware.
> 1b - Force CLK active
> 
> In default, the FRC_SDCLK_ON is 0. This means, when there is no command or
> data transfer on bus, hardware will gate off the card clock. But in some case,
> we need the card clock keep on. Take IO voltage 1.8v switch as example, after
> IO voltage change to 1.8v, spec require gate off the card clock for 5ms, and
> gate on the clock back, once detect the card clock on, then the card will draw
> the dat0 to high immediately. If there is not clock gate off/on behavior, some
> card will keep the dat0 to low level. This is the reason we fail in
> mmc_switch_voltage().
> 
> To fix this issue, and concern that this is only the fsl usdhc hardware design
> limitation, set the bit FRC_SDCLK_ON in the beginning of the
> wait_dat0() and clear it in the end. To make sure the 1.8v IO voltage switch
> process align with SD specification.
> 
> For standard tuning process, usdhc specification also require the card clock
> keep on, so also add these behavior in fsl_esdhc_execute_tuning().
> 
> Reviewed-by: Marek Vasut <marex@denx.de>
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> ---
>  drivers/mmc/fsl_esdhc_imx.c | 25 ++++++++++++++++++++++---
>  include/fsl_esdhc_imx.h     |  2 ++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 9299635f50..e0108144e7 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -831,13 +831,16 @@ static int fsl_esdhc_execute_tuning(struct udevice
> *dev, uint32_t opcode)
>  	struct mmc *mmc = &plat->mmc;
>  	u32 irqstaten = esdhc_read32(&regs->irqstaten);
>  	u32 irqsigen = esdhc_read32(&regs->irqsigen);
> -	int i, ret = -ETIMEDOUT;
> -	u32 val, mixctrl;
> +	int i, err, ret = -ETIMEDOUT;
> +	u32 val, mixctrl, tmp;
> 
>  	/* clock tuning is not needed for upto 52MHz */
>  	if (mmc->clock <= 52000000)
>  		return 0;
> 
> +	/* make sure the card clock keep on */
> +	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +
>  	/* This is readw/writew SDHCI_HOST_CONTROL2 when tuning */
>  	if (priv->flags & ESDHC_FLAG_STD_TUNING) {
>  		val = esdhc_read32(&regs->autoc12err); @@ -897,6 +900,12 @@
> static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
> 
>  	esdhc_stop_tuning(mmc);
> 
> +	/* change to default setting, let host control the card clock */
> +	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100);
> +	if (err)
> +		dev_warn(dev, "card clock not gate off as expect.\n");
> +
>  	return ret;
>  }
>  #endif
> @@ -1555,14 +1564,24 @@ static int __maybe_unused
> fsl_esdhc_set_enhanced_strobe(struct udevice *dev)  static int
> fsl_esdhc_wait_dat0(struct udevice *dev, int state,
>  				int timeout_us)
>  {
> -	int ret;
> +	int ret, err;
>  	u32 tmp;
>  	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
> 
> +	/* make sure the card clock keep on */
> +	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +
>  	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp,
>  				!!(tmp & PRSSTAT_DAT0) == !!state,
>  				timeout_us);
> +
> +	/* change to default setting, let host control the card clock */
> +	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100);
> +	if (err)
> +		dev_warn(dev, "card clock not gate off as expect.\n");
> +
>  	return ret;
>  }
> 
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h index
> 2153f29bef..b8efd2a166 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -37,6 +37,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)
> @@ -94,6 +95,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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-22  3:28 ` [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
  2022-02-23  5:28   ` Peng Fan (OSS)
@ 2022-02-24  0:03   ` Jaehoon Chung
  2022-03-09 16:10   ` Tom Rini
  2 siblings, 0 replies; 5+ messages in thread
From: Jaehoon Chung @ 2022-02-24  0:03 UTC (permalink / raw)
  To: haibo.chen, peng.fan, festevam, sean.anderson, u-boot, marex,
	aford173, tharvey, andrey.zhizhikin
  Cc: uboot-imx

On 2/22/22 12:28, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> After commit f132aab40327 ("Revert "mmc: fsl_esdhc_imx: use
> VENDORSPEC_FRC_SDCLK_ON to control card clock output""), it
> involve issue in mmc_switch_voltage(), because of the special
> design of usdhc.
> 
> For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
> these are reserved bits(Though RM contain the definition of these bits,
> but actually internal IC logic do not implement, already confirm with
> IC team). Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the card
> clock output. Here is the definition of this bit in RM:
> 
> [8] FRC_SDCLK_ON
> Force CLK output active
> Do not set this bit to 1 unless it is necessary. Also, make sure that
> this bit is cleared when uSDHC’s clock is about to be changed (frequency
> change, clock source change, or delay chain tuning).
> 0b - CLK active or inactive is fully controlled by the hardware.
> 1b - Force CLK active
> 
> In default, the FRC_SDCLK_ON is 0. This means, when there is no command
> or data transfer on bus, hardware will gate off the card clock. But in
> some case, we need the card clock keep on. Take IO voltage 1.8v switch
> as example, after IO voltage change to 1.8v, spec require gate off the
> card clock for 5ms, and gate on the clock back, once detect the card
> clock on, then the card will draw the dat0 to high immediately. If there
> is not clock gate off/on behavior, some card will keep the dat0 to low
> level. This is the reason we fail in mmc_switch_voltage().
> 
> To fix this issue, and concern that this is only the fsl usdhc hardware
> design limitation, set the bit FRC_SDCLK_ON in the beginning of the
> wait_dat0() and clear it in the end. To make sure the 1.8v IO voltage
> switch process align with SD specification.
> 
> For standard tuning process, usdhc specification also require the card
> clock keep on, so also add these behavior in fsl_esdhc_execute_tuning().
> 
> Reviewed-by: Marek Vasut <marex@denx.de>
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/fsl_esdhc_imx.c | 25 ++++++++++++++++++++++---
>  include/fsl_esdhc_imx.h     |  2 ++
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 9299635f50..e0108144e7 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -831,13 +831,16 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
>  	struct mmc *mmc = &plat->mmc;
>  	u32 irqstaten = esdhc_read32(&regs->irqstaten);
>  	u32 irqsigen = esdhc_read32(&regs->irqsigen);
> -	int i, ret = -ETIMEDOUT;
> -	u32 val, mixctrl;
> +	int i, err, ret = -ETIMEDOUT;
> +	u32 val, mixctrl, tmp;
>  
>  	/* clock tuning is not needed for upto 52MHz */
>  	if (mmc->clock <= 52000000)
>  		return 0;
>  
> +	/* make sure the card clock keep on */
> +	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +
>  	/* This is readw/writew SDHCI_HOST_CONTROL2 when tuning */
>  	if (priv->flags & ESDHC_FLAG_STD_TUNING) {
>  		val = esdhc_read32(&regs->autoc12err);
> @@ -897,6 +900,12 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
>  
>  	esdhc_stop_tuning(mmc);
>  
> +	/* change to default setting, let host control the card clock */
> +	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
> +	if (err)
> +		dev_warn(dev, "card clock not gate off as expect.\n");
> +
>  	return ret;
>  }
>  #endif
> @@ -1555,14 +1564,24 @@ static int __maybe_unused fsl_esdhc_set_enhanced_strobe(struct udevice *dev)
>  static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
>  				int timeout_us)
>  {
> -	int ret;
> +	int ret, err;
>  	u32 tmp;
>  	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>  
> +	/* make sure the card clock keep on */
> +	esdhc_setbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +
>  	ret = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp,
>  				!!(tmp & PRSSTAT_DAT0) == !!state,
>  				timeout_us);
> +
> +	/* change to default setting, let host control the card clock */
> +	esdhc_clrbits32(&regs->vendorspec, VENDORSPEC_FRC_SDCLK_ON);
> +	err = readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100);
> +	if (err)
> +		dev_warn(dev, "card clock not gate off as expect.\n");
> +
>  	return ret;
>  }
>  
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
> index 2153f29bef..b8efd2a166 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -37,6 +37,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)
> @@ -94,6 +95,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)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-22  3:28 ` [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
  2022-02-23  5:28   ` Peng Fan (OSS)
  2022-02-24  0:03   ` Jaehoon Chung
@ 2022-03-09 16:10   ` Tom Rini
  2022-03-14  9:31     ` Jaehoon Chung
  2 siblings, 1 reply; 5+ messages in thread
From: Tom Rini @ 2022-03-09 16:10 UTC (permalink / raw)
  To: haibo.chen, Jaehoon Chung, Peng Fan, Stefano Babic
  Cc: peng.fan, jh80.chung, festevam, sean.anderson, u-boot, marex,
	aford173, tharvey, andrey.zhizhikin, uboot-imx

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

On Tue, Feb 22, 2022 at 11:28:18AM +0800, haibo.chen@nxp.com wrote:

> From: Haibo Chen <haibo.chen@nxp.com>
> 
> After commit f132aab40327 ("Revert "mmc: fsl_esdhc_imx: use
> VENDORSPEC_FRC_SDCLK_ON to control card clock output""), it
> involve issue in mmc_switch_voltage(), because of the special
> design of usdhc.
> 
> For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
> these are reserved bits(Though RM contain the definition of these bits,
> but actually internal IC logic do not implement, already confirm with
> IC team). Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the card
> clock output. Here is the definition of this bit in RM:
> 
> [8] FRC_SDCLK_ON
> Force CLK output active
> Do not set this bit to 1 unless it is necessary. Also, make sure that
> this bit is cleared when uSDHC’s clock is about to be changed (frequency
> change, clock source change, or delay chain tuning).
> 0b - CLK active or inactive is fully controlled by the hardware.
> 1b - Force CLK active
> 
> In default, the FRC_SDCLK_ON is 0. This means, when there is no command
> or data transfer on bus, hardware will gate off the card clock. But in
> some case, we need the card clock keep on. Take IO voltage 1.8v switch
> as example, after IO voltage change to 1.8v, spec require gate off the
> card clock for 5ms, and gate on the clock back, once detect the card
> clock on, then the card will draw the dat0 to high immediately. If there
> is not clock gate off/on behavior, some card will keep the dat0 to low
> level. This is the reason we fail in mmc_switch_voltage().
> 
> To fix this issue, and concern that this is only the fsl usdhc hardware
> design limitation, set the bit FRC_SDCLK_ON in the beginning of the
> wait_dat0() and clear it in the end. To make sure the 1.8v IO voltage
> switch process align with SD specification.
> 
> For standard tuning process, usdhc specification also require the card
> clock keep on, so also add these behavior in fsl_esdhc_execute_tuning().
> 
> Reviewed-by: Marek Vasut <marex@denx.de>
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/fsl_esdhc_imx.c | 25 ++++++++++++++++++++++---
>  include/fsl_esdhc_imx.h     |  2 ++
>  2 files changed, 24 insertions(+), 3 deletions(-)

Is this going to get picked up in the imx or mmc trees soon, or should I
take this directly?  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-03-09 16:10   ` Tom Rini
@ 2022-03-14  9:31     ` Jaehoon Chung
  0 siblings, 0 replies; 5+ messages in thread
From: Jaehoon Chung @ 2022-03-14  9:31 UTC (permalink / raw)
  To: Tom Rini, haibo.chen, Peng Fan, Stefano Babic
  Cc: festevam, sean.anderson, u-boot, marex, aford173, tharvey,
	andrey.zhizhikin, uboot-imx

On 3/10/22 01:10, Tom Rini wrote:
> On Tue, Feb 22, 2022 at 11:28:18AM +0800, haibo.chen@nxp.com wrote:
> 
>> From: Haibo Chen <haibo.chen@nxp.com>
>>
>> After commit f132aab40327 ("Revert "mmc: fsl_esdhc_imx: use
>> VENDORSPEC_FRC_SDCLK_ON to control card clock output""), it
>> involve issue in mmc_switch_voltage(), because of the special
>> design of usdhc.
>>
>> For FSL_USDHC, it do not implement VENDORSPEC_CKEN/PEREN/HCKEN/IPGEN,
>> these are reserved bits(Though RM contain the definition of these bits,
>> but actually internal IC logic do not implement, already confirm with
>> IC team). Instead, use VENDORSPEC_FRC_SDCLK_ON to gate on/off the card
>> clock output. Here is the definition of this bit in RM:
>>
>> [8] FRC_SDCLK_ON
>> Force CLK output active
>> Do not set this bit to 1 unless it is necessary. Also, make sure that
>> this bit is cleared when uSDHC’s clock is about to be changed (frequency
>> change, clock source change, or delay chain tuning).
>> 0b - CLK active or inactive is fully controlled by the hardware.
>> 1b - Force CLK active
>>
>> In default, the FRC_SDCLK_ON is 0. This means, when there is no command
>> or data transfer on bus, hardware will gate off the card clock. But in
>> some case, we need the card clock keep on. Take IO voltage 1.8v switch
>> as example, after IO voltage change to 1.8v, spec require gate off the
>> card clock for 5ms, and gate on the clock back, once detect the card
>> clock on, then the card will draw the dat0 to high immediately. If there
>> is not clock gate off/on behavior, some card will keep the dat0 to low
>> level. This is the reason we fail in mmc_switch_voltage().
>>
>> To fix this issue, and concern that this is only the fsl usdhc hardware
>> design limitation, set the bit FRC_SDCLK_ON in the beginning of the
>> wait_dat0() and clear it in the end. To make sure the 1.8v IO voltage
>> switch process align with SD specification.
>>
>> For standard tuning process, usdhc specification also require the card
>> clock keep on, so also add these behavior in fsl_esdhc_execute_tuning().
>>
>> Reviewed-by: Marek Vasut <marex@denx.de>
>> Tested-by: Fabio Estevam <festevam@gmail.com>
>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>> Reviewed-by: Peng Fan <peng.fan@nxp.com>
>> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/fsl_esdhc_imx.c | 25 ++++++++++++++++++++++---
>>  include/fsl_esdhc_imx.h     |  2 ++
>>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> Is this going to get picked up in the imx or mmc trees soon, or should I
> take this directly?  Thanks!

Applied to u-boot-mmc. Thanks! Sorry for late.

Best Regards,
Jaehoon Chung

> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-14  9:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20220222033658epcas1p4d233f0d3c4a5c59f50609c7589c72379@epcas1p4.samsung.com>
2022-02-22  3:28 ` [PATCH v2] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
2022-02-23  5:28   ` Peng Fan (OSS)
2022-02-24  0:03   ` Jaehoon Chung
2022-03-09 16:10   ` Tom Rini
2022-03-14  9:31     ` Jaehoon Chung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.