Hi all,
On 2/5/21 4:23 PM, Bough Chen wrote: >> -----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 > > >> 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. I didn't have target that fsl_esdhc_imx is used. But when i have checked fsl_esdhc_imx.c, google coral dev board is using it, right? Then is it possible to produce this issue with its board? Best Regards, Jaehoon Chung > > 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 >
