[PATCH 0/2] mmc: fsl_esdhc_imx: Auto-detect higher speeds
The driver in the Linux kernel can automatically detect high speed options like UHS, HS200, HS400 and HS400-ES without needing to add special flags to the device tree.
UHS appears to have been broken in this driver, so fix UHS support, and add auto-detection.
Adam Ford (2): mmc: fsl_esdhc_imx: Fix fsl_esdhc_wait_dat0 mmc: fsl_esdhc_imx: Use esdhc_soc_data flags to set host caps
drivers/mmc/fsl_esdhc_imx.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
According to Haibo Chen [1] - the current implementation mmc_wait_dat0, the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout. This causes UHS cards to not properly initialize to their highest rate, and default back for high-speed mode.
When reviewing [1] and comparing it to the linux driver, it appears that this function can be accomplished by turning off the clock, and waiting for the clock-standby bit to become active.
[1] - https://lists.denx.de/pipermail/u-boot/2021-January/438644.html
Reported-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 4c06361bee..e5814232a2 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1684,9 +1684,15 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state, struct fsl_esdhc_priv *priv = dev_get_priv(dev); struct fsl_esdhc *regs = priv->esdhc_regs;
+ /* + * Clear the clock-enable and wait for the bit indicating it + * is in standby. + */ + esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN); ret = readx_poll_timeout(esdhc_read32, ®s->prsstat, tmp, - !!(tmp & PRSSTAT_DAT0) == !!state, + (tmp & PRSSTAT_SDSTB), timeout_us); + return ret; }
-----Original Message----- From: Adam Ford [mailto:aford173@gmail.com] Sent: 2022年1月12日 21:54 To: u-boot@lists.denx.de Cc: Peng Fan peng.fan@nxp.com; Bough Chen haibo.chen@nxp.com; andrey.zhizhikin@leica-geosystems.com; tharvey@gateworks.com; estevam@gmail.com; sbabic@denx.de; aford@beaconembedded.com; Adam Ford aford173@gmail.com Subject: [PATCH 1/2] mmc: fsl_esdhc_imx: Fix fsl_esdhc_wait_dat0
According to Haibo Chen [1] - the current implementation mmc_wait_dat0,
the
second mmc_wait_dat0() in mmc_switch_voltage() always return timeout. This causes UHS cards to not properly initialize to their highest rate,
and
default back for high-speed mode.
When reviewing [1] and comparing it to the linux driver, it appears that
this
function can be accomplished by turning off the clock, and waiting for the clock-standby bit to become active.
[1] - https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.de nx.de%2Fpipermail%2Fu-boot%2F2021-January%2F438644.html&data=04 %7C01%7Chaibo.chen%40nxp.com%7C5de948ddd6fb41a9ab2c08d9d5d304d9 %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63777592456049525 1%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiL CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=QVXYNWR3t2N9oCr BZSnoJVquwWdJuvo0hmuP%2FZhZifc%3D&reserved=0
Reported-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index
4c06361bee..e5814232a2 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1684,9 +1684,15 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state, struct fsl_esdhc_priv *priv = dev_get_priv(dev); struct fsl_esdhc *regs = priv->esdhc_regs;
- /*
* Clear the clock-enable and wait for the bit indicating it* is in standby.*/- esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN);
Hi Adam,
For the usdhc on i.MX, the bit VENDORSPEC_CKEN is not implement by IP, instead, we use the bit 8 of register 0xc0.
By the way, for the wait_dat0 function, it not only cover the IO voltage switch process, other place also need this function. So I think this change is not correct.
Best Regards Haibo Chen
ret = readx_poll_timeout(esdhc_read32, ®s->prsstat, tmp,
!!(tmp & PRSSTAT_DAT0) == !!state,
(tmp & PRSSTAT_SDSTB), timeout_us);- return ret;
}
-- 2.32.0
According to Haibo Chen [1] - the current implementation mmc_wait_dat0, the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout. This causes UHS cards to not properly initialize to their highest rate, and default back for high-speed mode. When reviewing [1] and comparing it to the linux driver, it appears that this function can be accomplished by turning off the clock, and waiting for the clock-standby bit to become active. [1] - https://lists.denx.de/pipermail/u-boot/2021-January/438644.html Reported-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Adam Ford aford173@gmail.com diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 4c06361bee..e5814232a2 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1684,9 +1684,15 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state, struct fsl_esdhc_priv *priv = dev_get_priv(dev); struct fsl_esdhc *regs = priv->esdhc_regs;
- /*
* Clear the clock-enable and wait for the bit indicating it* is in standby.*/- esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN); ret = readx_poll_timeout(esdhc_read32, ®s->prsstat, tmp,
!!(tmp & PRSSTAT_DAT0) == !!state,
(tmp & PRSSTAT_SDSTB), timeout_us);- return ret;
}
Applied to u-boot-imx, master, thanks !
Best regards, Stefano Babic
Hi Stefano,
On Sat, Feb 19, 2022 at 10:08 AM sbabic@denx.de wrote:
According to Haibo Chen [1] - the current implementation mmc_wait_dat0, the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout. This causes UHS cards to not properly initialize to their highest rate, and default back for high-speed mode. When reviewing [1] and comparing it to the linux driver, it appears that this function can be accomplished by turning off the clock, and waiting for the clock-standby bit to become active. [1] - https://lists.denx.de/pipermail/u-boot/2021-January/438644.html Reported-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Adam Ford aford173@gmail.com diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 4c06361bee..e5814232a2 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1684,9 +1684,15 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state, struct fsl_esdhc_priv *priv = dev_get_priv(dev); struct fsl_esdhc *regs = priv->esdhc_regs;
/** Clear the clock-enable and wait for the bit indicating it* is in standby.*/esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN); ret = readx_poll_timeout(esdhc_read32, ®s->prsstat, tmp,
!!(tmp & PRSSTAT_DAT0) == !!state,
(tmp & PRSSTAT_SDSTB), timeout_us);return ret;}
Applied to u-boot-imx, master, thanks !
Bough said this patch is incorrect during the review.
Hi Fabio,
On 19.02.22 14:27, Fabio Estevam wrote:
Hi Stefano,
On Sat, Feb 19, 2022 at 10:08 AM sbabic@denx.de wrote:
According to Haibo Chen [1] - the current implementation mmc_wait_dat0, the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout. This causes UHS cards to not properly initialize to their highest rate, and default back for high-speed mode. When reviewing [1] and comparing it to the linux driver, it appears that this function can be accomplished by turning off the clock, and waiting for the clock-standby bit to become active. [1] - https://lists.denx.de/pipermail/u-boot/2021-January/438644.html Reported-by: Tim Harvey tharvey@gateworks.com Signed-off-by: Adam Ford aford173@gmail.com diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index 4c06361bee..e5814232a2 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1684,9 +1684,15 @@ static int fsl_esdhc_wait_dat0(struct udevice *dev, int state, struct fsl_esdhc_priv *priv = dev_get_priv(dev); struct fsl_esdhc *regs = priv->esdhc_regs;
/** Clear the clock-enable and wait for the bit indicating it* is in standby.*/esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN); ret = readx_poll_timeout(esdhc_read32, ®s->prsstat, tmp,
!!(tmp & PRSSTAT_DAT0) == !!state,
(tmp & PRSSTAT_SDSTB), timeout_us);return ret;Applied to u-boot-imx, master, thanks !
Bough said this patch is incorrect during the review.
Ouch, I have seen it was tested, I missed Bough's comment. I revert it and I ask Tom to wait - and nevertheless, I delegate these to Peng, this is his area of competence.
Best regards, Stefano
The Linux driver automatically can detect and enable UHS, HS200, HS400 and HS400_ES automatically without extra flags being placed into the device tree.
Right now, for U-Boot to use UHS, HS200 or HS400, the extra flags are needed in the device tree. Instead, go through the esdhc_soc_data flags and enable the host caps where applicable to automatically enable higher speeds.
Suggested-by: Fabio Estevam festevam@gmail.com Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index e5814232a2..5088e78bb6 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1305,8 +1305,29 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv, val |= ESDHC_TUNING_CMD_CRC_CHECK_DISABLE; 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; + + if (CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)) { + if (priv->flags & ESDHC_FLAG_HS200) + cfg->host_caps |= MMC_CAP(MMC_HS_200); + } + + if (CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)) { + if (priv->flags & ESDHC_FLAG_HS400) + cfg->host_caps |= MMC_CAP(MMC_HS_400); + } + + if (CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)) { + if (priv->flags & ESDHC_FLAG_HS400_ES) + cfg->host_caps |= MMC_CAP(MMC_HS_400_ES); + } + } return 0; }
The Linux driver automatically can detect and enable UHS, HS200, HS400 and HS400_ES automatically without extra flags being placed into the device tree. Right now, for U-Boot to use UHS, HS200 or HS400, the extra flags are needed in the device tree. Instead, go through the esdhc_soc_data flags and enable the host caps where applicable to automatically enable higher speeds. Suggested-by: Fabio Estevam festevam@gmail.com Signed-off-by: Adam Ford aford173@gmail.com diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index e5814232a2..5088e78bb6 100644 --- a/drivers/mmc/fsl_esdhc_imx.c +++ b/drivers/mmc/fsl_esdhc_imx.c @@ -1305,8 +1305,29 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv, val |= ESDHC_TUNING_CMD_CRC_CHECK_DISABLE; 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;if (CONFIG_IS_ENABLED(MMC_HS200_SUPPORT)) {if (priv->flags & ESDHC_FLAG_HS200)cfg->host_caps |= MMC_CAP(MMC_HS_200);}if (CONFIG_IS_ENABLED(MMC_HS400_SUPPORT)) {if (priv->flags & ESDHC_FLAG_HS400)cfg->host_caps |= MMC_CAP(MMC_HS_400);}if (CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)) {if (priv->flags & ESDHC_FLAG_HS400_ES)cfg->host_caps |= MMC_CAP(MMC_HS_400_ES);}- } return 0;
}
Applied to u-boot-imx, master, thanks !
Best regards, Stefano Babic
participants (5)
-
Adam Ford -
Bough Chen -
Fabio Estevam -
sbabic＠denx.de -
Stefano Babic