| Message ID | 1666947869-7904-1-git-send-email-haibo.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci-esdhc-imx: reset the tuning logic before execute tuning | expand |
Hi Haibo, On Fri, Oct 28, 2022 at 6:25 AM <haibo.chen@nxp.com> wrote: > > From: Haibo Chen <haibo.chen@nxp.com> > > For standard tuning method on usdhc, the previous tuning result can > impact current tuning result, let current tuning can't set the correct > delay cell. And from the logic, this is also reasonable for manual > tuning method. So reset the tuning logic before execute tuning. > To avoid compile issue, this patch also move the esdhc_reset_tuning() > upper. > > Find this issue when support SDIO WiFi in band wakeup feature. After > system resume back, will do re-tuning, but then meet data CRC error. > > Do not meet this issue on SD/eMMC, because we already call > esdhc_reset_tuning() when config the legency ios, and SD/eMMC need > to re-init when system resume back, but SDIO device don't do re-init > if it has MMC_PM_KEEP_POWER pm_flags. Fixes tag missing?
> -----Original Message----- > From: Fabio Estevam <festevam@gmail.com> > Sent: 2022年10月28日 18:18 > To: Bough Chen <haibo.chen@nxp.com> > Cc: adrian.hunter@intel.com; ulf.hansson@linaro.org; > linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Sherry Sun > <sherry.sun@nxp.com>; shawnguo@kernel.org; s.hauer@pengutronix.de; > kernel@pengutronix.de > Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset the tuning logic before > execute tuning > > Hi Haibo, > > On Fri, Oct 28, 2022 at 6:25 AM <haibo.chen@nxp.com> wrote: > > > > From: Haibo Chen <haibo.chen@nxp.com> > > > > For standard tuning method on usdhc, the previous tuning result can > > impact current tuning result, let current tuning can't set the correct > > delay cell. And from the logic, this is also reasonable for manual > > tuning method. So reset the tuning logic before execute tuning. > > To avoid compile issue, this patch also move the esdhc_reset_tuning() > > upper. > > > > Find this issue when support SDIO WiFi in band wakeup feature. After > > system resume back, will do re-tuning, but then meet data CRC error. > > > > Do not meet this issue on SD/eMMC, because we already call > > esdhc_reset_tuning() when config the legency ios, and SD/eMMC need to > > re-init when system resume back, but SDIO device don't do re-init if > > it has MMC_PM_KEEP_POWER pm_flags. > > Fixes tag missing? Hi Fabio, This is a bug fix patch, but hard to find a fix tag. In this patch, I just add the tuning reset in function usdhc_execute_tuning(), but for this usdhc_execute_tuning(), it is involve in the commit de3e1dd09b72 ("mmc: sdhci: usdhc: do not do tuning for DDR50 mode."). if fix tag point to this commit number, logically, seem incorrect. But if point to an older commit, like d9370424c948 ("mmc: sdhci-esdhc-imx: reset tuning circuit when power on mmc card"), it's hard to port current patch to the linux version between these two commit number, because no usdhc_execute_tuning() there. So I just not add the fix tag here. Best Regards Haibo Chen
On 28/10/22 12:04, haibo.chen@nxp.com wrote: > From: Haibo Chen <haibo.chen@nxp.com> > > For standard tuning method on usdhc, the previous tuning result can > impact current tuning result, let current tuning can't set the correct > delay cell. And from the logic, this is also reasonable for manual > tuning method. So reset the tuning logic before execute tuning. > To avoid compile issue, this patch also move the esdhc_reset_tuning() > upper. > > Find this issue when support SDIO WiFi in band wakeup feature. After > system resume back, will do re-tuning, but then meet data CRC error. > > Do not meet this issue on SD/eMMC, because we already call > esdhc_reset_tuning() when config the legency ios, and SD/eMMC need > to re-init when system resume back, but SDIO device don't do re-init > if it has MMC_PM_KEEP_POWER pm_flags. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 82 ++++++++++++++++-------------- > 1 file changed, 44 insertions(+), 38 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index b073e79dcd99..4559599d897d 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -1012,6 +1012,44 @@ static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width) > SDHCI_HOST_CONTROL); > } > > +static void esdhc_reset_tuning(struct sdhci_host *host) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > + u32 ctrl; > + int ret; > + > + /* Reset the tuning circuit */ > + if (esdhc_is_usdhc(imx_data)) { > + if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { > + ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > + writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); > + } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { > + ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; > + writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); > + /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ > + ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, > + ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); > + if (ret == -ETIMEDOUT) > + dev_warn(mmc_dev(host->mmc), > + "Warning! clear execute tuning bit failed\n"); > + /* > + * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the > + * usdhc IP internal logic flag execute_tuning_with_clr_buf, which > + * will finally make sure the normal data transfer logic correct. > + */ > + ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); > + ctrl |= SDHCI_INT_DATA_AVAIL; > + writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); > + } > + } > +} > + > static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > { > struct sdhci_host *host = mmc_priv(mmc); > @@ -1023,6 +1061,12 @@ static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > if (host->timing == MMC_TIMING_UHS_DDR50) > return 0; > > + /* > + * Reset tuning circuit logic. If not, the previous tuning result > + * will impact current tuning, make current tuning can't set the > + * correct delay cell. > + */ > + esdhc_reset_tuning(host); > return sdhci_execute_tuning(mmc, opcode); > } > > @@ -1196,44 +1240,6 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) > "warning! HS400 strobe DLL status REF/SLV not lock in 50us, STROBE DLL status is %x!\n", v); > } > > -static void esdhc_reset_tuning(struct sdhci_host *host) > -{ > - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > - struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > - u32 ctrl; > - int ret; > - > - /* Reset the tuning circuit */ > - if (esdhc_is_usdhc(imx_data)) { > - if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { > - ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); > - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > - ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > - writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > - writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); > - } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { > - ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); > - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > - ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; > - writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); > - /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ > - ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, > - ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); > - if (ret == -ETIMEDOUT) > - dev_warn(mmc_dev(host->mmc), > - "Warning! clear execute tuning bit failed\n"); > - /* > - * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the > - * usdhc IP internal logic flag execute_tuning_with_clr_buf, which > - * will finally make sure the normal data transfer logic correct. > - */ > - ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); > - ctrl |= SDHCI_INT_DATA_AVAIL; > - writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); > - } > - } > -} > - > static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > { > u32 m;
On Fri, 28 Oct 2022 at 11:25, <haibo.chen@nxp.com> wrote: > > From: Haibo Chen <haibo.chen@nxp.com> > > For standard tuning method on usdhc, the previous tuning result can > impact current tuning result, let current tuning can't set the correct > delay cell. And from the logic, this is also reasonable for manual > tuning method. So reset the tuning logic before execute tuning. > To avoid compile issue, this patch also move the esdhc_reset_tuning() > upper. > > Find this issue when support SDIO WiFi in band wakeup feature. After > system resume back, will do re-tuning, but then meet data CRC error. > > Do not meet this issue on SD/eMMC, because we already call > esdhc_reset_tuning() when config the legency ios, and SD/eMMC need > to re-init when system resume back, but SDIO device don't do re-init > if it has MMC_PM_KEEP_POWER pm_flags. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> Applied for next, thanks! Kind regards Uffe > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 82 ++++++++++++++++-------------- > 1 file changed, 44 insertions(+), 38 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index b073e79dcd99..4559599d897d 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -1012,6 +1012,44 @@ static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width) > SDHCI_HOST_CONTROL); > } > > +static void esdhc_reset_tuning(struct sdhci_host *host) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > + u32 ctrl; > + int ret; > + > + /* Reset the tuning circuit */ > + if (esdhc_is_usdhc(imx_data)) { > + if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { > + ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > + writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); > + } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { > + ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; > + writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); > + /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ > + ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, > + ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); > + if (ret == -ETIMEDOUT) > + dev_warn(mmc_dev(host->mmc), > + "Warning! clear execute tuning bit failed\n"); > + /* > + * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the > + * usdhc IP internal logic flag execute_tuning_with_clr_buf, which > + * will finally make sure the normal data transfer logic correct. > + */ > + ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); > + ctrl |= SDHCI_INT_DATA_AVAIL; > + writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); > + } > + } > +} > + > static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > { > struct sdhci_host *host = mmc_priv(mmc); > @@ -1023,6 +1061,12 @@ static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > if (host->timing == MMC_TIMING_UHS_DDR50) > return 0; > > + /* > + * Reset tuning circuit logic. If not, the previous tuning result > + * will impact current tuning, make current tuning can't set the > + * correct delay cell. > + */ > + esdhc_reset_tuning(host); > return sdhci_execute_tuning(mmc, opcode); > } > > @@ -1196,44 +1240,6 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) > "warning! HS400 strobe DLL status REF/SLV not lock in 50us, STROBE DLL status is %x!\n", v); > } > > -static void esdhc_reset_tuning(struct sdhci_host *host) > -{ > - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > - struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); > - u32 ctrl; > - int ret; > - > - /* Reset the tuning circuit */ > - if (esdhc_is_usdhc(imx_data)) { > - if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { > - ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); > - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > - ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > - writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > - writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); > - } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { > - ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); > - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > - ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; > - writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); > - /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ > - ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, > - ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); > - if (ret == -ETIMEDOUT) > - dev_warn(mmc_dev(host->mmc), > - "Warning! clear execute tuning bit failed\n"); > - /* > - * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the > - * usdhc IP internal logic flag execute_tuning_with_clr_buf, which > - * will finally make sure the normal data transfer logic correct. > - */ > - ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); > - ctrl |= SDHCI_INT_DATA_AVAIL; > - writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); > - } > - } > -} > - > static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > { > u32 m; > -- > 2.34.1 >
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index b073e79dcd99..4559599d897d 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -1012,6 +1012,44 @@ static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width) SDHCI_HOST_CONTROL); } +static void esdhc_reset_tuning(struct sdhci_host *host) +{ + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); + u32 ctrl; + int ret; + + /* Reset the tuning circuit */ + if (esdhc_is_usdhc(imx_data)) { + if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { + ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); + writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); + } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { + ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; + ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; + writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); + /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ + ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, + ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); + if (ret == -ETIMEDOUT) + dev_warn(mmc_dev(host->mmc), + "Warning! clear execute tuning bit failed\n"); + /* + * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the + * usdhc IP internal logic flag execute_tuning_with_clr_buf, which + * will finally make sure the normal data transfer logic correct. + */ + ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); + ctrl |= SDHCI_INT_DATA_AVAIL; + writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); + } + } +} + static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) { struct sdhci_host *host = mmc_priv(mmc); @@ -1023,6 +1061,12 @@ static int usdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) if (host->timing == MMC_TIMING_UHS_DDR50) return 0; + /* + * Reset tuning circuit logic. If not, the previous tuning result + * will impact current tuning, make current tuning can't set the + * correct delay cell. + */ + esdhc_reset_tuning(host); return sdhci_execute_tuning(mmc, opcode); } @@ -1196,44 +1240,6 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) "warning! HS400 strobe DLL status REF/SLV not lock in 50us, STROBE DLL status is %x!\n", v); } -static void esdhc_reset_tuning(struct sdhci_host *host) -{ - struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); - struct pltfm_imx_data *imx_data = sdhci_pltfm_priv(pltfm_host); - u32 ctrl; - int ret; - - /* Reset the tuning circuit */ - if (esdhc_is_usdhc(imx_data)) { - if (imx_data->socdata->flags & ESDHC_FLAG_MAN_TUNING) { - ctrl = readl(host->ioaddr + ESDHC_MIX_CTRL); - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; - ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; - writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); - writel(0, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); - } else if (imx_data->socdata->flags & ESDHC_FLAG_STD_TUNING) { - ctrl = readl(host->ioaddr + SDHCI_AUTO_CMD_STATUS); - ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; - ctrl &= ~ESDHC_MIX_CTRL_EXE_TUNE; - writel(ctrl, host->ioaddr + SDHCI_AUTO_CMD_STATUS); - /* Make sure ESDHC_MIX_CTRL_EXE_TUNE cleared */ - ret = readl_poll_timeout(host->ioaddr + SDHCI_AUTO_CMD_STATUS, - ctrl, !(ctrl & ESDHC_MIX_CTRL_EXE_TUNE), 1, 50); - if (ret == -ETIMEDOUT) - dev_warn(mmc_dev(host->mmc), - "Warning! clear execute tuning bit failed\n"); - /* - * SDHCI_INT_DATA_AVAIL is W1C bit, set this bit will clear the - * usdhc IP internal logic flag execute_tuning_with_clr_buf, which - * will finally make sure the normal data transfer logic correct. - */ - ctrl = readl(host->ioaddr + SDHCI_INT_STATUS); - ctrl |= SDHCI_INT_DATA_AVAIL; - writel(ctrl, host->ioaddr + SDHCI_INT_STATUS); - } - } -} - static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) { u32 m;