[PATCH] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output

Jaehoon Chung jh80.chung at samsung.com
Fri Feb 5 08:34:36 CET 2021 

Hi all,


On 2/5/21 4:23 PM, Bough Chen wrote:
>> -----Original Message-----
>> From: ZHIZHIKIN Andrey [mailto:andrey.zhizhikin at leica-geosystems.com]
>> Sent: 2021年2月1日 19:41
>> To: Bough Chen <haibo.chen at nxp.com>; Peng Fan <peng.fan at nxp.com>;
>> u-boot at lists.denx.de
>> Cc: dl-uboot-imx <uboot-imx at nxp.com>; tharvey at 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.chen at nxp.com>
>>> Sent: Monday, February 1, 2021 11:10 AM
>>> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>; Peng Fan
>>> <peng.fan at nxp.com>; u-boot at lists.denx.de
>>> Cc: dl-uboot-imx <uboot-imx at nxp.com>; tharvey at 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.zhizhikin at leica-geosystems.com]
>>>> Sent: 2021年2月1日 17:52
>>>> To: Bough Chen <haibo.chen at nxp.com>; Peng Fan <peng.fan at nxp.com>;
>>>> u-boot at lists.denx.de
>>>> Cc: dl-uboot-imx <uboot-imx at nxp.com>; tharvey at 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.chen at nxp.com <haibo.chen at nxp.com>
>>>>> Sent: Wednesday, January 27, 2021 11:47 AM
>>>>> To: peng.fan at nxp.com; u-boot at lists.denx.de
>>>>> Cc: haibo.chen at nxp.com; uboot-imx at nxp.com;
>> tharvey at gateworks.com;
>>>>> ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
>>>>> Subject: [PATCH] mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON
>>>>> to control card clock output
>>>>>
>>>>> From: Haibo Chen <haibo.chen at 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(&regs->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 at 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 at 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.chen at 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(&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 @@
>>>> -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(&regs->vendorspec, VENDORSPEC_PEREN |
>>>>> VENDORSPEC_CKEN);
>>>>> +       esdhc_setbits32(&regs->vendorspec,
>>>> VENDORSPEC_FRC_SDCLK_ON);
>>>>>  #else
>>>>>         esdhc_setbits32(&regs->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(&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);
>>>>>
>>>>>                 /*
>>>>> @@ -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(&regs->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(&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 @@ -1045,7 +1059,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 */ @@ -1183,8 +1197,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
>

More information about the U-Boot mailing list