[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(®s->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