Hello Haibo,
> -----Original Message-----
> From: Bough Chen <haibo.c...@nxp.com>
> Sent: Friday, February 5, 2021 8:24 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhi...@leica-geosystems.com>; Peng Fan
> <peng....@nxp.com>; u-boot@lists.denx.de
> Cc: dl-uboot-imx <uboot-...@nxp.com>; thar...@gateworks.com
> Subject: RE: [PATCH] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to
> control card clock output
>
> > -----Original Message-----
> > From: ZHIZHIKIN Andrey [mailto:andrey.zhizhi...@leica-geosystems.com]
> > Sent: 2021年2月1日 19:41
> > To: Bough Chen <haibo.c...@nxp.com>; Peng Fan <peng....@nxp.com>;
> > u-boot@lists.denx.de
> > Cc: dl-uboot-imx <uboot-...@nxp.com>; thar...@gateworks.com
> > Subject: RE: [PATCH] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON
> > to control card clock output
> >
> > Hello Haibo,
> >
> > > -----Original Message-----
> > > From: Bough Chen <haibo.c...@nxp.com>
> > > Sent: Monday, February 1, 2021 11:10 AM
> > > To: ZHIZHIKIN Andrey <andrey.zhizhi...@leica-geosystems.com>; Peng
> > > Fan <peng....@nxp.com>; u-boot@lists.denx.de
> > > Cc: dl-uboot-imx <uboot-...@nxp.com>; thar...@gateworks.com
> > > Subject: RE: [PATCH] mmc: fsl_esdhc_imx: use
> > VENDORSPEC_FRC_SDCLK_ON
> > > to control card clock output
> > >
> > > > -----Original Message-----
> > > > From: ZHIZHIKIN Andrey
> > > > [mailto:andrey.zhizhi...@leica-geosystems.com]
> > > > Sent: 2021年2月1日 17:52
> > > > To: Bough Chen <haibo.c...@nxp.com>; Peng Fan <peng....@nxp.com>;
> > > > u-boot@lists.denx.de
> > > > Cc: dl-uboot-imx <uboot-...@nxp.com>; thar...@gateworks.com
> > > > Subject: RE: [PATCH] mmc: fsl_esdhc_imx: use
> > VENDORSPEC_FRC_SDCLK_ON
> > > > to control card clock output
> > > >
> > > > Hello Haibo,
> > > >
> > > > > -----Original Message-----
> > > > > From: haibo.c...@nxp.com <haibo.c...@nxp.com>
> > > > > Sent: Wednesday, January 27, 2021 11:47 AM
> > > > > To: peng....@nxp.com; u-boot@lists.denx.de
> > > > > Cc: haibo.c...@nxp.com; uboot-...@nxp.com;
> > thar...@gateworks.com;
> > > > > ZHIZHIKIN Andrey <andrey.zhizhi...@leica-geosystems.com>
> > > > > Subject: [PATCH] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON
> > > > > to control card clock output
> > > > >
> > > > > From: Haibo Chen <haibo.c...@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")
> > > >
> > > > This patch does not fix the switch of uSDHC to 1v8...
> > > >
> > > > I've applied it locally on the imx8mmevk, and had following log
> > > > during the boot when tried to query SD Card info:
> > > > -----------------------------------------------
> > > > U-Boot SPL 2021.01-01004-gb852007333 (Feb 01 2021 - 09:45:42
> > > > +0100) Normal Boot
> > > > WDT: Started with servicing (60s timeout)
> > > > Trying to boot from MMC1
> > > > NOTICE: BL31: v2.2(release):rel_imx_5.4.70_2.3.0-0-gf1d7187f2
> > > > NOTICE: BL31: Built : 22:29:05, Jan 17 2021
> > > >
> > > >
> > > > U-Boot 2021.01-01004-gb852007333 (Feb 01 2021 - 09:45:42 +0100)
> > > >
> > > > CPU: Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > Reset cause: POR
> > > > Model: FSL i.MX8MM EVK board
> > > > DRAM: 2 GiB
> > > > WDT: Started with servicing (60s timeout)
> > > > MMC: FSL_SDHC: 1, FSL_SDHC: 2
> > > > Loading Environment from MMC... Run CMD11 1.8V switch Card did not
> > > > respond to voltage select! : -110
> > >
> > > This do not align with my test result. Can you help identify which
> > > function first return the timeout on your side?
> >
> > I would have a look at the exact function, but it seems to me that it
> > would be
> > wait_dat0() since if I revert the patch introducing it - high speed
> > mode switch is not timing out.
> >
> > > Or can you try a different SD card?
> >
> > It is rather strange, it seems like it is dependent on the SD Card used.
> >
> > So far, I've tried 3 SD Cards I had on hands, one of which being operable:
> > == Working:
> > "Transcend 32GB"
> > Manufacturer ID: 74
> > OEM: 4a60
> > Name: USDU1
> >
> > == Failed:
> > 1. (Kingston 32GB)
> > Manufacturer ID: 41
> > OEM: 3432
> > Name: SD32G
> >
> > 2. (Intenso 32 GB)
> > Manufacturer ID: 9f
> > OEM: 5449
> > Name: 00000
> >
>
> Hi Andrey,
>
> With this patch, can you also add the following change to test again, I double
> check the code logic, only the following code do not follow the SD spec for
> voltage switch.
> Seems the two failed SD cards follow the sd voltage spec strictly.
>
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index
> da33ee8253..20337b0ed4 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -513,7 +513,7 @@ static int esdhc_send_cmd_common(struct
> fsl_esdhc_priv *priv, struct mmc *mmc,
> err = -ETIMEDOUT;
> goto out;
> }
> -
> +#if 0
> /* Switch voltage to 1.8V if CMD11 succeeded */
> if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
> esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
> @@ -522,6 +522,7 @@ static int esdhc_send_cmd_common(struct
> fsl_esdhc_priv *priv, struct mmc *mmc,
> /* Sleep for 5 ms - max time for card to switch to 1.8V */
> udelay(5000);
> }
> +#endif
>
I've tried this change and can confirm: the failed Kingston 32GB card from the
list above
operates in high-speed mode.
u-boot=> mmc dev 1
switch to partitions #0, OK
mmc1 is current device
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 41
OEM: 3432
Name: SD32G
Bus Speed: 200000000
Mode: UHS SDR104 (208MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 29.3 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
>
> > This raises a concern that a regular off-the-shelf SD Card might
> > exhibit this issue or might not - it is not tight to a one specific SD
> > Card. Hence I believe it is quite important to look at it further, as
> > there might be a lot of reports from various users.
>
> Yes, I will take care of that, but I think current issue is not caused by
> this, because
> uboot already print out the log " Run CMD11 1.8V", which means the S18A is set
> in the response of ACMD41.
> Currently, common code only delay 2ms for power cycle, but some boards may
> need more time, few i.mx boards need 3.8ms ~4.7ms. for my new imx8mm
> board, need 2.7ms to let VDD change to 0.5v from 3.3v.
> And spec need to keep VDD under 0.5v for at least 1ms, so totally need about
> 3.7ms for the power cycle. According to the log, seems you use an old version
> board, not sure about the actual delay time on your board.
>
> In Linux, regulator driver support the compatible "off-on-delay-us", let me
> double check whether uboot also support this feature.
This binding does exist in u-boot, and I've se it to 20 msec in commit
247bbeb74c
("ARM: dts: imx8m: increase off-on delay on the SD Vcc regulator").
>
> Best Regards
> Haibo
>
>
>
> >
> > Please note, that for all SD Cards I've tested NVCC_SD2 is indeed
> > physically switched to 1v8.
> >
> > >
> > > Best Regards
> > > Haibo
> > >
> > > > *** Warning - No block device, using default environment
> > > >
> > > > In: serial
> > > > Out: serial
> > > > Err: serial
> > > > Net: eth0: ethernet@30be0000
> > > > Hit any key to stop autoboot: 0
> > > > u-boot=> mmc dev 1
> > > > Card did not respond to voltage select! : -110 u-boot=> mmc info
> > > > MMC Device
> > > > 0 not found no mmc device at slot 0 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: 100
> > > > Name: DG401
> > > > Bus Speed: 200000000
> > > > Mode: HS400ES (200MHz)
> > > > Rd Block Len: 512
> > > > MMC version 5.1
> > > > High Capacity: Yes
> > > > Capacity: 14.7 GiB
> > > > Bus Width: 8-bit DDR
> > > > Erase Group Size: 512 KiB
> > > > HC WP Group Size: 8 MiB
> > > > User Capacity: 14.7 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
> > > > -----------------------------------------------
> > > >
> > > > Note, that eMMC is not affected and is operating in HS400ES mode
> > > > without any issues.
> > > >
> > > > Reverting patch b5874b552ffa ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > > support") with this patch applied in tree does switch the SD Card
> > > > to high-speed mode, following log is produced:
> > > > -----------------------------------------------
> > > > U-Boot SPL 2021.01-01005-gb8aeb689a2 (Feb 01 2021 - 10:38:01
> > > > +0100) Normal Boot
> > > > WDT: Started with servicing (60s timeout)
> > > > Trying to boot from MMC1
> > > > NOTICE: BL31: v2.2(release):rel_imx_5.4.70_2.3.0-0-gf1d7187f2
> > > > NOTICE: BL31: Built : 22:29:05, Jan 17 2021
> > > >
> > > >
> > > > U-Boot 2021.01-01005-gb8aeb689a2 (Feb 01 2021 - 10:38:01 +0100)
> > > >
> > > > CPU: Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > Reset cause: POR
> > > > Model: FSL i.MX8MM EVK board
> > > > DRAM: 2 GiB
> > > > WDT: Started with servicing (60s timeout)
> > > > MMC: FSL_SDHC: 1, FSL_SDHC: 2
> > > > Loading Environment from MMC... Run CMD11 1.8V switch
> > > > *** Warning - bad CRC, using default environment
> > > >
> > > > In: serial
> > > > Out: serial
> > > > Err: serial
> > > > Net: eth0: ethernet@30be0000
> > > > Hit any key to stop autoboot: 0
> > > > u-boot=> mmc dev 1
> > > > Run CMD11 1.8V switch
> > > > switch to partitions #0, OK
> > > > mmc1 is current device
> > > > u-boot=> mmc info
> > > > Device: FSL_SDHC
> > > > Manufacturer ID: 41
> > > > OEM: 3432
> > > > Name: SD32G
> > > > Bus Speed: 200000000
> > > > Mode: UHS SDR104 (208MHz)
> > > > Rd Block Len: 512
> > > > SD version 3.0
> > > > High Capacity: Yes
> > > > Capacity: 29.3 GiB
> > > > Bus Width: 4-bit
> > > > Erase Group Size: 512 Bytes
> > > > 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: 100
> > > > Name: DG401
> > > > Bus Speed: 200000000
> > > > Mode: HS400ES (200MHz)
> > > > Rd Block Len: 512
> > > > MMC version 5.1
> > > > High Capacity: Yes
> > > > Capacity: 14.7 GiB
> > > > Bus Width: 8-bit DDR
> > > > Erase Group Size: 512 KiB
> > > > HC WP Group Size: 8 MiB
> > > > User Capacity: 14.7 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 guess high-level modifications are also required, since the
> > > > uSDHC does not follow the SD Specification in regards to voltage
> > > > switching, which is currently implemented in U-Boot, this patch
> > > > alone does not solve the
> > > mode switch.
> > > >
> > > > > Signed-off-by: Haibo Chen <haibo.c...@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
> > > > > 8ac859797f..da33ee8253 100644
> > > > > --- a/drivers/mmc/fsl_esdhc_imx.c
> > > > > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > > > > @@ -653,7 +653,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(®s->vendorspec, VENDORSPEC_CKEN);
> > > > > + esdhc_clrbits32(®s->vendorspec,
> > > > VENDORSPEC_FRC_SDCLK_ON);
> > > > > + ret = readx_poll_timeout(esdhc_read32, ®s->prsstat,
> > > > > + tmp, tmp &
> > > > > PRSSTAT_SDOFF, 100);
> > > > > + if (ret)
> > > > > + pr_warn("fsl_esdhc_imx: Internal clock never
> > > > > + gate off.\n");
> > > > > #else
> > > > > esdhc_clrbits32(®s->sysctl, SYSCTL_CKEN); #endif @@
> > > > -665,7
> > > > > +668,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(®s->vendorspec, VENDORSPEC_PEREN |
> > > > > VENDORSPEC_CKEN);
> > > > > + esdhc_setbits32(®s->vendorspec,
> > > > VENDORSPEC_FRC_SDCLK_ON);
> > > > > #else
> > > > > esdhc_setbits32(®s->sysctl, SYSCTL_PEREN |
> > > > > SYSCTL_CKEN); #endif @@ -
> > > > > 720,8 +723,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(®s->vendorspec,
> > > > VENDORSPEC_FRC_SDCLK_ON);
> > > > > + ret = readx_poll_timeout(esdhc_read32,
> > > > > + ®s->prsstat, tmp, tmp &
> > > > > PRSSTAT_SDOFF, 100);
> > > > > + if (ret)
> > > > > + pr_warn("fsl_esdhc_imx: Internal clock
> > > > > + never gate off.\n");
> > > > > esdhc_write32(®s->strobe_dllctrl,
> > > > > ESDHC_STROBE_DLL_CTRL_RESET);
> > > > >
> > > > > /*
> > > > > @@ -739,6 +748,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(®s->vendorspec,
> > > > > + VENDORSPEC_FRC_SDCLK_ON);
> > > > > }
> > > > > }
> > > > >
> > > > > @@ -962,14 +972,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(®s->vendorspec,
> > > > VENDORSPEC_CKEN);
> > > > > + u32 tmp;
> > > > > +
> > > > > + esdhc_clrbits32(®s->vendorspec,
> > > > VENDORSPEC_FRC_SDCLK_ON);
> > > > > + ret = readx_poll_timeout(esdhc_read32,
> > > > > + ®s->prsstat, tmp, tmp &
> > > > > PRSSTAT_SDOFF, 100);
> > > > > + if (ret)
> > > > > + pr_warn("fsl_esdhc_imx: Internal clock
> > > > > + never gate off.\n");
> > > > > #else
> > > > > esdhc_clrbits32(®s->sysctl, SYSCTL_CKEN);
> > #endif
> > > > > } else {
> > > > > #ifdef CONFIG_FSL_USDHC
> > > > > - esdhc_setbits32(®s->vendorspec,
> > > > VENDORSPEC_PEREN |
> > > > > - VENDORSPEC_CKEN);
> > > > > + esdhc_setbits32(®s->vendorspec,
> > > > > + VENDORSPEC_FRC_SDCLK_ON);
> > > > > #else
> > > > > esdhc_setbits32(®s->sysctl, SYSCTL_PEREN |
> > > > > SYSCTL_CKEN); #endif @@ -1045,7 +1059,7 @@ static int
> > > > > esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
> > > > > #ifndef
> > > > CONFIG_FSL_USDHC
> > > > > esdhc_setbits32(®s->sysctl, SYSCTL_HCKEN |
> > > > > SYSCTL_IPGEN);
> > > > #else
> > > > > - esdhc_setbits32(®s->vendorspec, VENDORSPEC_HCKEN |
> > > > > VENDORSPEC_IPGEN);
> > > > > + esdhc_setbits32(®s->vendorspec,
> > > > VENDORSPEC_FRC_SDCLK_ON);
> > > > > #endif
> > > > >
> > > > > /* Set the initial clock speed */ @@ -1183,8 +1197,7 @@
> > > > > static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> > > > > esdhc_write32(®s->autoc12err, 0);
> > > > > esdhc_write32(®s->clktunectrlstatus, 0); #else
> > > > > - esdhc_setbits32(®s->vendorspec, VENDORSPEC_PEREN |
> > > > > - VENDORSPEC_HCKEN |
> > VENDORSPEC_IPGEN |
> > > > VENDORSPEC_CKEN);
> > > > > + esdhc_setbits32(®s->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
