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

[Tim Harvey]

On Thu, Feb 4, 2021 at 11:23 PM Bough Chen <haibo.chen at nxp.com> 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
>
>

Haibo,

I can confirm that this fixes UHS-I on the IMX8MM_EVK and I can now
get SDR104 on the cards I have here. I would include this with your
previous patch as without it your patch breaks MMC in my findings.

Tested-By: Tim Harvey <tharvey at gateworks.com>

Thanks for keeping up on this!

Tim