[EXT] Re: [PATCH v2] mmc: fsl_esdhc: Do not set UHS_CAPS based on CONFIG_MMC_UHS_SUPPORT

Ye Li ye.li at nxp.com
Wed Apr 12 11:51:39 CEST 2023


Hi Fabio,

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Peng Fan
> Sent: Wednesday, April 12, 2023 4:55 PM
> To: Fabio Estevam <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>
> Cc: jh80.chung at samsung.com; aford173 at gmail.com; u-boot at lists.denx.de;
> Fabio Estevam <festevam at denx.de>; Bough Chen <haibo.chen at nxp.com>
> Subject: [EXT] Re: [PATCH v2] mmc: fsl_esdhc: Do not set UHS_CAPS based on
> CONFIG_MMC_UHS_SUPPORT
> 
> Caution: EXT Email
> 
> Hi Fabio,
> 
> On 4/11/2023 9:41 PM, Fabio Estevam wrote:
> > From: Fabio Estevam <festevam at denx.de>
> >
> > Since commit 1a7904fdfa7d ("mmc: fsl_esdhc_imx: Use esdhc_soc_data
> > flags to set host caps") the following SD card error is observed on an
> > imx7d-sdb
> > board:
> 
> What kind card do you see the issue? Have you tried the other card?
> 
> >
> > U-Boot 2023.04-00652-g487e42f7bc5e (Apr 05 2023 - 22:14:21 -0300)
> >
> > CPU:   Freescale i.MX7D rev1.0 1000 MHz (running at 792 MHz)
> > CPU:   Commercial temperature grade (0C to 95C) at 35C
> > Reset cause: POR
> > Model: Freescale i.MX7 SabreSD Board
> > Board: i.MX7D SABRESD in non-secure mode
> > DRAM:  1 GiB
> > Core:  100 devices, 19 uclasses, devicetree: separate
> > PMIC: PFUZE3000 DEV_ID=0x30 REV_ID=0x10
> > MMC:   FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2
> > Loading Environment from MMC... Card did not respond to voltage
> > select! : -110
> > *** Warning - No block device, using default environment
> >
> > Setting UHS_CAPS only based on the CONFIG_MMC_UHS_SUPPORT config
> > option is not correct because UHS_CAPS has the following definition:
> >
> >   #define UHS_CAPS (MMC_CAP(UHS_SDR12) | MMC_CAP(UHS_SDR25) | \
> >                  MMC_CAP(UHS_SDR50) | MMC_CAP(UHS_SDR104) | \
> >                  MMC_CAP(UHS_DDR50))
> >
> > and the SD card may not necessarily support all these modes.
> >
> > Remove the UHS_CAPS setting to fix the SD card regression.
> >
> > Fixes: 1a7904fdfa7d ("mmc: fsl_esdhc_imx: Use esdhc_soc_data flags to
> > set host caps")
> > Signed-off-by: Fabio Estevam <festevam at denx.de>

Remove the UHS_CAPS is wrong, it will cause UHS-I mode not work.
I just check the 7d sdb on upstream, it is due to imx7d-sdb.dts change.
When UHS is enabled in defconfig, the usdhc1 node in imx7d-sdb.dts does not configure pad for VSELECT, also
the data pad should be set to 100Mhz/200Mhz pin states. 
You can try below changes:

--- a/arch/arm/dts/imx7d-sdb.dts
+++ b/arch/arm/dts/imx7d-sdb.dts
@@ -478,8 +478,10 @@
 };
 
 &usdhc1 {
-       pinctrl-names = "default";
-       pinctrl-0 = <&pinctrl_usdhc1>;
+       pinctrl-names = "default", "state_100mhz", "state_200mhz";
+       pinctrl-0 = <&pinctrl_usdhc1>, <&pinctrl_usdhc1_gpio>;
+       pinctrl-1 = <&pinctrl_usdhc1_100mhz>, <&pinctrl_usdhc1_gpio>;
+       pinctrl-2 = <&pinctrl_usdhc1_200mhz>, <&pinctrl_usdhc1_gpio>;
        cd-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
        wp-gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
        wakeup-source;
@@ -736,6 +738,15 @@
                        >;
                };
 
+               pinctrl_usdhc1_gpio: usdhc1_gpiogrp {
+                       fsl,pins = <
+                               MX7D_PAD_SD1_CD_B__GPIO5_IO0            0x59 /* CD */
+                               MX7D_PAD_SD1_WP__GPIO5_IO1              0x59 /* WP */
+                               MX7D_PAD_SD1_RESET_B__GPIO5_IO2         0x59 /* vmmc */
+                               MX7D_PAD_GPIO1_IO08__SD1_VSELECT        0x59 /* VSELECT */
+                       >;
+               };
+
                pinctrl_usdhc1: usdhc1grp {
                        fsl,pins = <
                                MX7D_PAD_SD1_CMD__SD1_CMD               0x59
@@ -744,9 +755,28 @@
                                MX7D_PAD_SD1_DATA1__SD1_DATA1           0x59
                                MX7D_PAD_SD1_DATA2__SD1_DATA2           0x59
                                MX7D_PAD_SD1_DATA3__SD1_DATA3           0x59
-                               MX7D_PAD_SD1_CD_B__GPIO5_IO0            0x59 /* CD */
-                               MX7D_PAD_SD1_WP__GPIO5_IO1              0x59 /* WP */
-                               MX7D_PAD_SD1_RESET_B__GPIO5_IO2         0x59 /* vmmc */
+                       >;
+               };
+
+               pinctrl_usdhc1_100mhz: usdhc1grp_100mhz {
+                       fsl,pins = <
+                               MX7D_PAD_SD1_CMD__SD1_CMD               0x5a
+                               MX7D_PAD_SD1_CLK__SD1_CLK               0x1a
+                               MX7D_PAD_SD1_DATA0__SD1_DATA0           0x5a
+                               MX7D_PAD_SD1_DATA1__SD1_DATA1           0x5a
+                               MX7D_PAD_SD1_DATA2__SD1_DATA2           0x5a
+                               MX7D_PAD_SD1_DATA3__SD1_DATA3           0x5a
+                       >;
+               };
+
+               pinctrl_usdhc1_200mhz: usdhc1grp_200mhz {
+                       fsl,pins = <
+                               MX7D_PAD_SD1_CMD__SD1_CMD               0x5b
+                               MX7D_PAD_SD1_CLK__SD1_CLK               0x1b
+                               MX7D_PAD_SD1_DATA0__SD1_DATA0           0x5b
+                               MX7D_PAD_SD1_DATA1__SD1_DATA1           0x5b
+                               MX7D_PAD_SD1_DATA2__SD1_DATA2           0x5b
+                               MX7D_PAD_SD1_DATA3__SD1_DATA3           0x5b
                        >;
                };

Best regards,
Ye Li
> > ---
> > Changes since v1:
> > - Remove setting UHS_CAPS completely.
> >
> >   drivers/mmc/fsl_esdhc_imx.c | 7 -------
> >   1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > index 66caf683f7..5e7d9f41b6 100644
> > --- a/drivers/mmc/fsl_esdhc_imx.c
> > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > @@ -1258,13 +1258,6 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> >                       esdhc_write32(&regs->tuning_ctrl, val);
> >               }
> >
> > -             /*
> > -              * UHS doesn't have explicit ESDHC flags, so if it's
> > -              * not supported, disable it in config.
> > -              */
> > -             if (CONFIG_IS_ENABLED(MMC_UHS_SUPPORT))
> > -                     cfg->host_caps |= UHS_CAPS;
> 
> I am afraid this just workaround the issue you met. Actually in
> drivers/mmc/mmc, if your card not support UHS_CAPS, the caps will remove the
> UHS_CAPS.
> 
> 1762         /* Restrict card's capabilities by what the host can do */
> 1763         caps = card_caps & mmc->host_caps;
> 1764
> 1765         if (!uhs_en)
> 1766                 caps &= ~UHS_CAPS;
> 
> So would you please dump the card_caps, then we find a proper fix?
> 
> Thanks,
> Peng.
> 
> > -
> >               if (CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)) {
> >                       if (priv->flags & ESDHC_FLAG_HS200)
> >                               cfg->host_caps |=
> MMC_CAP(MMC_HS_200);


More information about the U-Boot mailing list