All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
@ 2022-02-11 11:16 haibo.chen
  2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: haibo.chen @ 2022-02-11 11:16 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().

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 19 ++++++++++++++++++-
 include/fsl_esdhc_imx.h     |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 9299635f50..362e3e13b6 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -832,12 +832,15 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
 	u32 irqstaten = esdhc_read32(&regs->irqstaten);
 	u32 irqsigen = esdhc_read32(&regs->irqsigen);
 	int i, ret = -ETIMEDOUT;
-	u32 val, mixctrl;
+	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,11 @@ 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);
+	if (readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100))
+		pr_warn("fsl_esdhc_imx: card clock not gate off as expect.\n");
+
 	return ret;
 }
 #endif
@@ -1560,9 +1568,18 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
 	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);
+	if (readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100))
+		pr_warn("fsl_esdhc_imx: 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] 12+ messages in thread

* [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC
  2022-02-11 11:16 [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
@ 2022-02-11 11:16 ` haibo.chen
  2022-02-13 21:53   ` Marek Vasut
  2022-02-19 13:07   ` sbabic
  2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
  2022-02-13 21:52 ` [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary Marek Vasut
  2 siblings, 2 replies; 12+ messages in thread
From: haibo.chen @ 2022-02-11 11:16 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>

Now original fsl_esdhc.c are split as fsl_esdhc.c and fsl_esdhc_imx.c.
fsl_esdhc_imx.c only cover i.MX SoC. So ARCH_MXC is redundant.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 362e3e13b6..0be7cae1e5 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -596,16 +596,12 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	int sdhc_clk = priv->sdhc_clk;
 	uint clk;
 
-	if (IS_ENABLED(ARCH_MXC)) {
 #if IS_ENABLED(CONFIG_MX53)
-		/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
-		pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
+	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
+	pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
 #else
-		pre_div = 1;
+	pre_div = 1;
 #endif
-	} else {
-		pre_div = 2;
-	}
 
 	while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div < 256)
 		pre_div *= 2;
@@ -1016,11 +1012,6 @@ static int esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
 		esdhc_write32(&regs->dllctrl, 0x0);
 	}
 
-#ifndef ARCH_MXC
-	/* Enable cache snooping */
-	esdhc_write32(&regs->scr, 0x00000040);
-#endif
-
 	if (IS_ENABLED(CONFIG_FSL_USDHC))
 		esdhc_setbits32(&regs->vendorspec,
 				VENDORSPEC_HCKEN | VENDORSPEC_IPGEN);
-- 
2.17.1


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

* [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
  2022-02-11 11:16 [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
  2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
@ 2022-02-11 11:16 ` haibo.chen
  2022-02-13 21:54   ` Marek Vasut
                     ` (2 more replies)
  2022-02-13 21:52 ` [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary Marek Vasut
  2 siblings, 3 replies; 12+ messages in thread
From: haibo.chen @ 2022-02-11 11:16 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>

The original code logic can not show the correct card clock, and also
has one risk when the div is 0. Because there is div -=1 before.

So move the operation before div -=1, and also involve ddr_pre_div
to get the correct value.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 0be7cae1e5..0ea7b0b50c 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -609,6 +609,8 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
 		div++;
 
+	mmc->clock = sdhc_clk / pre_div / div / ddr_pre_div;
+
 	pre_div >>= 1;
 	div -= 1;
 
@@ -630,7 +632,6 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 	else
 		esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
 
-	mmc->clock = sdhc_clk / pre_div / div;
 	priv->clock = clock;
 }
 
-- 
2.17.1


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

* Re: [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-11 11:16 [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
  2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
  2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
@ 2022-02-13 21:52 ` Marek Vasut
  2022-02-14  2:58   ` Bough Chen
  2 siblings, 1 reply; 12+ messages in thread
From: Marek Vasut @ 2022-02-13 21:52 UTC (permalink / raw)
  To: haibo.chen, peng.fan, jh80.chung, festevam, sean.anderson,
	u-boot, aford173, tharvey, andrey.zhizhikin
  Cc: uboot-imx

On 2/11/22 12:16, haibo.chen@nxp.com wrote:

Hi,

[...]

> @@ -897,6 +900,11 @@ 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);
> +	if (readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp & PRSSTAT_SDOFF, 100))

Please propagate the error in both cases:

ret = readx_poll..();
if (ret)
   dev_warn(...);

return ret;

> +		pr_warn("fsl_esdhc_imx: card clock not gate off as expect.\n");

btw. s@gate@gated@ , s@expect@expected@ (past tense)

With those small details fixed:

Reviewed-by: Marek Vasut <marex@denx.de>

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

* Re: [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC
  2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
@ 2022-02-13 21:53   ` Marek Vasut
  2022-02-19 13:07   ` sbabic
  1 sibling, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2022-02-13 21:53 UTC (permalink / raw)
  To: haibo.chen, peng.fan, jh80.chung, festevam, sean.anderson,
	u-boot, aford173, tharvey, andrey.zhizhikin
  Cc: uboot-imx

On 2/11/22 12:16, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> Now original fsl_esdhc.c are split as fsl_esdhc.c and fsl_esdhc_imx.c.
> fsl_esdhc_imx.c only cover i.MX SoC. So ARCH_MXC is redundant.

Reviewed-by: Marek Vasut <marex@denx.de>

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

* Re: [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
  2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
@ 2022-02-13 21:54   ` Marek Vasut
  2022-02-19 13:08   ` sbabic
  2022-07-23  2:23   ` Tim Harvey
  2 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2022-02-13 21:54 UTC (permalink / raw)
  To: haibo.chen, peng.fan, jh80.chung, festevam, sean.anderson,
	u-boot, aford173, tharvey, andrey.zhizhikin
  Cc: uboot-imx

On 2/11/22 12:16, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
> 
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.

Reviewed-by: Marek Vasut <marex@denx.de>

Thanks !

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

* RE: [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-13 21:52 ` [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary Marek Vasut
@ 2022-02-14  2:58   ` Bough Chen
  2022-02-17  0:15     ` Fabio Estevam
  0 siblings, 1 reply; 12+ messages in thread
From: Bough Chen @ 2022-02-14  2:58 UTC (permalink / raw)
  To: festevam
  Cc: dl-uboot-imx, Marek Vasut, Peng Fan, jh80.chung, sean.anderson,
	u-boot, aford173, tharvey, andrey.zhizhikin

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

Hi Fabio,

Can you help test these 3 patches on imx6qdl-pico board or imx7s board on your side to double check whether these patches enlarge the total boot time?

Best Regards
Bough Chen

> -----Original Message-----
> From: Marek Vasut [mailto:marex@denx.de]
> Sent: 2022年2月14日 5:53
> To: Bough Chen <haibo.chen@nxp.com>; Peng Fan <peng.fan@nxp.com>;
> jh80.chung@samsung.com; festevam@gmail.com; sean.anderson@seco.com;
> u-boot@lists.denx.de; aford173@gmail.com; tharvey@gateworks.com;
> andrey.zhizhikin@leica-geosystems.com
> Cc: dl-uboot-imx <uboot-imx@nxp.com>
> Subject: Re: [PATCH 1/3] mmc: fsl_esdhc_imx: use
> VENDORSPEC_FRC_SDCLK_ON when necessary
> 
> On 2/11/22 12:16, haibo.chen@nxp.com wrote:
> 
> Hi,
> 
> [...]
> 
> > @@ -897,6 +900,11 @@ 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);
> > +	if (readx_poll_timeout(esdhc_read32, &regs->prsstat, tmp, tmp &
> PRSSTAT_SDOFF, 100))
> 
> Please propagate the error in both cases:
> 
> ret = readx_poll..();
> if (ret)
>    dev_warn(...);
> 
> return ret;
> 
> > +		pr_warn("fsl_esdhc_imx: card clock not gate off as expect.\n");
> 
> btw. s@gate@gated@ , s@expect@expected@ (past tense)
> 
> With those small details fixed:
> 
> Reviewed-by: Marek Vasut <marex@denx.de>

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9551 bytes --]

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

* Re: [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary
  2022-02-14  2:58   ` Bough Chen
@ 2022-02-17  0:15     ` Fabio Estevam
  0 siblings, 0 replies; 12+ messages in thread
From: Fabio Estevam @ 2022-02-17  0:15 UTC (permalink / raw)
  To: Bough Chen
  Cc: dl-uboot-imx, Marek Vasut, Peng Fan, jh80.chung, sean.anderson,
	u-boot, aford173, tharvey, andrey.zhizhikin

Hi Bough,

On Sun, Feb 13, 2022 at 11:58 PM Bough Chen <haibo.chen@nxp.com> wrote:
>
> Hi Fabio,
>
> Can you help test these 3 patches on imx6qdl-pico board or imx7s board on your side to double check whether these patches enlarge the total boot time?

I have tested the series on an imx7s-warp and the previous issue of
slow boot is no longer present, thanks:

Tested-by: Fabio Estevam <festevam@gmail.com>

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

* [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC
  2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
  2022-02-13 21:53   ` Marek Vasut
@ 2022-02-19 13:07   ` sbabic
  1 sibling, 0 replies; 12+ messages in thread
From: sbabic @ 2022-02-19 13:07 UTC (permalink / raw)
  To: haibo.chen, u-boot

> From: Haibo Chen <haibo.chen@nxp.com>
> Now original fsl_esdhc.c are split as fsl_esdhc.c and fsl_esdhc_imx.c.
> fsl_esdhc_imx.c only cover i.MX SoC. So ARCH_MXC is redundant.
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================

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

* [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
  2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
  2022-02-13 21:54   ` Marek Vasut
@ 2022-02-19 13:08   ` sbabic
  2022-07-23  2:23   ` Tim Harvey
  2 siblings, 0 replies; 12+ messages in thread
From: sbabic @ 2022-02-19 13:08 UTC (permalink / raw)
  To: haibo.chen, u-boot

> From: Haibo Chen <haibo.chen@nxp.com>
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================

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

* Re: [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
  2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
  2022-02-13 21:54   ` Marek Vasut
  2022-02-19 13:08   ` sbabic
@ 2022-07-23  2:23   ` Tim Harvey
  2022-07-27 12:44     ` Bough Chen
  2 siblings, 1 reply; 12+ messages in thread
From: Tim Harvey @ 2022-07-23  2:23 UTC (permalink / raw)
  To: Bough Chen
  Cc: Peng Fan, Jaehoon Chung, Fabio Estevam, Sean Anderson, u-boot,
	Marek Vasut, Adam Ford, Andrey Zhizhikin, dl-uboot-imx

On Fri, Feb 11, 2022 at 3:48 AM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> The original code logic can not show the correct card clock, and also
> has one risk when the div is 0. Because there is div -=1 before.
>
> So move the operation before div -=1, and also involve ddr_pre_div
> to get the correct value.
>
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/fsl_esdhc_imx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index 0be7cae1e5..0ea7b0b50c 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -609,6 +609,8 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>         while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
>                 div++;
>
> +       mmc->clock = sdhc_clk / pre_div / div / ddr_pre_div;
> +
>         pre_div >>= 1;
>         div -= 1;
>
> @@ -630,7 +632,6 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
>         else
>                 esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>
> -       mmc->clock = sdhc_clk / pre_div / div;
>         priv->clock = clock;
>  }
>
> --
> 2.17.1
>

Haibo,

I found that this particular patch keeps an imx8mm-venice-gw7901 board
that has a viking vwsdinbdg4 eMMC from booting to Linux. While u-boot
appears to work ok, as soon as I load a kernel (from emmc or even
network) and boot to it I hang at 'starting kernel' even with early
debug turned on.

u-boot=> mmc list
FSL_SDHC: 0
FSL_SDHC: 1
FSL_SDHC: 2 (eMMC)
u-boot=> mmc dev 2
switch to partitions #0, OK
mmc2(part 0) is current device
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 45
OEM: 0
Name: DG4008
Bus Speed: 200000000
Mode: HS400ES (200MHz)
Rd Block Len: 512
MMC version 5.1
High Capacity: Yes
Capacity: 7.3 GiB
Bus Width: 8-bit DDR
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 7.3 GiB WRREL
Boot Capacity: 4 MiB ENH
RPMB Capacity: 4 MiB ENH
Boot area 0 is not write protected
Boot area 1 is not write protected

I have other boards with a Micron MTFC8GAKAJCN non HS400ES that don't
have any issue so it appears to be something to do with HS400ES
support and I find if I disable CONFIG_MMC_HS400_ES_SUPPORT or revert
this patch the issue goes away.

Any idea what might be going on here?

Best Regards,

Tim

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

* RE: [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
  2022-07-23  2:23   ` Tim Harvey
@ 2022-07-27 12:44     ` Bough Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Bough Chen @ 2022-07-27 12:44 UTC (permalink / raw)
  To: tharvey
  Cc: Peng Fan, Jaehoon Chung, Fabio Estevam, Sean Anderson, u-boot,
	Marek Vasut, Adam Ford, Andrey Zhizhikin, dl-uboot-imx



> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: 2022年7月23日 10:23
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: Peng Fan <peng.fan@nxp.com>; Jaehoon Chung
> <jh80.chung@samsung.com>; Fabio Estevam <festevam@gmail.com>; Sean
> Anderson <sean.anderson@seco.com>; u-boot <u-boot@lists.denx.de>; Marek
> Vasut <marex@denx.de>; Adam Ford <aford173@gmail.com>; Andrey Zhizhikin
> <andrey.zhizhikin@leica-geosystems.com>; dl-uboot-imx <uboot-imx@nxp.com>
> Subject: Re: [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock
> 
> On Fri, Feb 11, 2022 at 3:48 AM <haibo.chen@nxp.com> wrote:
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The original code logic can not show the correct card clock, and also
> > has one risk when the div is 0. Because there is div -=1 before.
> >
> > So move the operation before div -=1, and also involve ddr_pre_div to
> > get the correct value.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/fsl_esdhc_imx.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > index 0be7cae1e5..0ea7b0b50c 100644
> > --- a/drivers/mmc/fsl_esdhc_imx.c
> > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > @@ -609,6 +609,8 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct
> mmc *mmc, uint clock)
> >         while (sdhc_clk / (div * pre_div * ddr_pre_div) > clock && div < 16)
> >                 div++;
> >
> > +       mmc->clock = sdhc_clk / pre_div / div / ddr_pre_div;
> > +
> >         pre_div >>= 1;
> >         div -= 1;
> >
> > @@ -630,7 +632,6 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, struct
> mmc *mmc, uint clock)
> >         else
> >                 esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN |
> > SYSCTL_CKEN);
> >
> > -       mmc->clock = sdhc_clk / pre_div / div;
> >         priv->clock = clock;
> >  }
> >
> > --
> > 2.17.1
> >
> 
> Haibo,
> 
> I found that this particular patch keeps an imx8mm-venice-gw7901 board that
> has a viking vwsdinbdg4 eMMC from booting to Linux. While u-boot appears to
> work ok, as soon as I load a kernel (from emmc or even
> network) and boot to it I hang at 'starting kernel' even with early debug turned
> on.
> 
> u-boot=> mmc list
> FSL_SDHC: 0
> FSL_SDHC: 1
> FSL_SDHC: 2 (eMMC)
> u-boot=> mmc dev 2
> switch to partitions #0, OK
> mmc2(part 0) is current device
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 45
> OEM: 0
> Name: DG4008
> Bus Speed: 200000000
> Mode: HS400ES (200MHz)
> Rd Block Len: 512
> MMC version 5.1
> High Capacity: Yes
> Capacity: 7.3 GiB
> Bus Width: 8-bit DDR
> Erase Group Size: 512 KiB
> HC WP Group Size: 8 MiB
> User Capacity: 7.3 GiB WRREL
> Boot Capacity: 4 MiB ENH
> RPMB Capacity: 4 MiB ENH
> Boot area 0 is not write protected
> Boot area 1 is not write protected
> 
> I have other boards with a Micron MTFC8GAKAJCN non HS400ES that don't have
> any issue so it appears to be something to do with HS400ES support and I find if I
> disable CONFIG_MMC_HS400_ES_SUPPORT or revert this patch the issue goes
> away.
> 
> Any idea what might be going on here?

Hi Tim,

I think this should be related with the HS400ES downgrade miss. Before boot linux, uboot will do some cleanup, remove all devices. Call mmc_blk_remove-> mmc_deinit-> mmc_select_mode_and_width-> mmc_set_card_speed(mmc, MMC_HS, true);

Please make sure in your environment,  finally call mmc_set_card_speed(mmc, MMC_HS, true); which means the HS400ES mode first downgrade to HS mode, then switch to legacy mode. Otherwise, emmc mode switch may meet issue. Some emmc may hang in somewhere, we did meet that before. To debug this, you can enable the debug info, and see the detail steps of mmc related. Not sure in your side, you miss some config which cause this issue.

For why this is related with the clock related patch, I think the wrong clock config happed workaround it.

Best Regards
Haibo Chen


> 
> Best Regards,
> 
> Tim

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

end of thread, other threads:[~2022-07-27 12:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 11:16 [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary haibo.chen
2022-02-11 11:16 ` [PATCH 2/3] mmc: fsl_esdhc_imx: remove redundant ARCH_MXC haibo.chen
2022-02-13 21:53   ` Marek Vasut
2022-02-19 13:07   ` sbabic
2022-02-11 11:16 ` [PATCH 3/3] mmc: fsl_esdhc_imx: correct the actual card clock haibo.chen
2022-02-13 21:54   ` Marek Vasut
2022-02-19 13:08   ` sbabic
2022-07-23  2:23   ` Tim Harvey
2022-07-27 12:44     ` Bough Chen
2022-02-13 21:52 ` [PATCH 1/3] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON when necessary Marek Vasut
2022-02-14  2:58   ` Bough Chen
2022-02-17  0:15     ` Fabio Estevam

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.