| Message ID | 1492509931-3388-1-git-send-email-haibo.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 18, 2017 at 06:05:31PM +0800, Haibo Chen wrote: > USDHC tuning circuit should be reset before every time card enumeration > or re-enumeration. > > SD3.0 card need tuning. For SDR104 card, when system suspend in standby > mode, and then resume back, the IO timing is still SDR104(tuned) which > may result in card re-enumeration fail in low card speed(400khz) for some > cards. And we did meet the issue that in certain probability, SDR104 > card meet mmc command CRC/Timeout error when send CMD2 during mmc bus > resume. > > This patch reset the tuning circuit when the ios timing is > MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both > mmc_power_up() and mmc_power_off() will reset the tuning circuit. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> Acked-by: Dong Aisheng <aisheng.dong@nxp.com> Regards Dong Aisheng -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18/04/17 13:05, Haibo Chen wrote: > USDHC tuning circuit should be reset before every time card enumeration > or re-enumeration. > > SD3.0 card need tuning. For SDR104 card, when system suspend in standby > mode, and then resume back, the IO timing is still SDR104(tuned) which > may result in card re-enumeration fail in low card speed(400khz) for some > cards. And we did meet the issue that in certain probability, SDR104 > card meet mmc command CRC/Timeout error when send CMD2 during mmc bus > resume. > > This patch reset the tuning circuit when the ios timing is > MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both > mmc_power_up() and mmc_power_off() will reset the tuning circuit. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index abad67a..23d8b8a 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) > } > } > > +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; > + > + /* Rest the tuning circurt */ > + 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_ACMD12_ERR); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); > + } > + } > +} > + > static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > { > u32 m; > @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > host->ops->set_clock(host, host->clock); > esdhc_set_strobe_dll(host); > break; > + case MMC_TIMING_LEGACY: > + default: > + esdhc_reset_tuning(host); > + break; > } > > esdhc_change_pinstate(host, timing); > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 April 2017 at 12:05, Haibo Chen <haibo.chen@nxp.com> wrote: > USDHC tuning circuit should be reset before every time card enumeration > or re-enumeration. > > SD3.0 card need tuning. For SDR104 card, when system suspend in standby > mode, and then resume back, the IO timing is still SDR104(tuned) which > may result in card re-enumeration fail in low card speed(400khz) for some > cards. And we did meet the issue that in certain probability, SDR104 > card meet mmc command CRC/Timeout error when send CMD2 during mmc bus > resume. > > This patch reset the tuning circuit when the ios timing is > MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both > mmc_power_up() and mmc_power_off() will reset the tuning circuit. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index abad67a..23d8b8a 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) > } > } > > +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; > + > + /* Rest the tuning circurt */ > + 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_ACMD12_ERR); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); > + } > + } > +} > + > static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > { > u32 m; > @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > host->ops->set_clock(host, host->clock); > esdhc_set_strobe_dll(host); > break; > + case MMC_TIMING_LEGACY: > + default: > + esdhc_reset_tuning(host); > + break; I understand this fixes the problem. However a more appropriate fix would be to reset tuning when ->ios.power_mode == MMC_POWER_UP, don't you think? > } > > esdhc_change_pinstate(host, timing); > -- > 1.9.1 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Thu, Apr 20, 2017 at 8:30 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 18 April 2017 at 12:05, Haibo Chen <haibo.chen@nxp.com> wrote: >> USDHC tuning circuit should be reset before every time card enumeration >> or re-enumeration. >> >> SD3.0 card need tuning. For SDR104 card, when system suspend in standby >> mode, and then resume back, the IO timing is still SDR104(tuned) which >> may result in card re-enumeration fail in low card speed(400khz) for some >> cards. And we did meet the issue that in certain probability, SDR104 >> card meet mmc command CRC/Timeout error when send CMD2 during mmc bus >> resume. >> >> This patch reset the tuning circuit when the ios timing is >> MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both >> mmc_power_up() and mmc_power_off() will reset the tuning circuit. >> >> Signed-off-by: Haibo Chen <haibo.chen@nxp.com> >> --- >> drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c >> index abad67a..23d8b8a 100644 >> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >> @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) >> } >> } >> >> +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; >> + >> + /* Rest the tuning circurt */ >> + 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_ACMD12_ERR); >> + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; >> + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); >> + } >> + } >> +} >> + >> static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >> { >> u32 m; >> @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >> host->ops->set_clock(host, host->clock); >> esdhc_set_strobe_dll(host); >> break; >> + case MMC_TIMING_LEGACY: >> + default: >> + esdhc_reset_tuning(host); >> + break; > > I understand this fixes the problem. > > However a more appropriate fix would be to reset tuning when > ->ios.power_mode == MMC_POWER_UP, don't you think? > rest_tuning seems all about the timing related stuff and it tends to be executed for all non-uhs timings setting in case there's an uhs mode running before. It looks like not quite depend on MMC_POWER_UP, that why we put it in set_uhs_signaling(). However, doing it in MMC_POWER_UP actually almost functions the same as it will set MMC_TIMING_LEGACY accordingly. To me, it looks like ok to keep in set_uhs_signaling, but if you insist, we can do it in MMC_POWER_UP, by a new platform specific .set_power(). Regards Dong Aisheng >> } >> >> esdhc_change_pinstate(host, timing); >> -- >> 1.9.1 >> > > Kind regards > Uffe > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21 April 2017 at 07:53, Dong Aisheng <dongas86@gmail.com> wrote: > Hi Ulf, > > On Thu, Apr 20, 2017 at 8:30 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >> On 18 April 2017 at 12:05, Haibo Chen <haibo.chen@nxp.com> wrote: >>> USDHC tuning circuit should be reset before every time card enumeration >>> or re-enumeration. >>> >>> SD3.0 card need tuning. For SDR104 card, when system suspend in standby >>> mode, and then resume back, the IO timing is still SDR104(tuned) which >>> may result in card re-enumeration fail in low card speed(400khz) for some >>> cards. And we did meet the issue that in certain probability, SDR104 >>> card meet mmc command CRC/Timeout error when send CMD2 during mmc bus >>> resume. >>> >>> This patch reset the tuning circuit when the ios timing is >>> MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both >>> mmc_power_up() and mmc_power_off() will reset the tuning circuit. >>> >>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com> >>> --- >>> drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ >>> 1 file changed, 26 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c >>> index abad67a..23d8b8a 100644 >>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >>> @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) >>> } >>> } >>> >>> +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; >>> + >>> + /* Rest the tuning circurt */ >>> + 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_ACMD12_ERR); >>> + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; >>> + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); >>> + } >>> + } >>> +} >>> + >>> static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >>> { >>> u32 m; >>> @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >>> host->ops->set_clock(host, host->clock); >>> esdhc_set_strobe_dll(host); >>> break; >>> + case MMC_TIMING_LEGACY: >>> + default: >>> + esdhc_reset_tuning(host); >>> + break; >> >> I understand this fixes the problem. >> >> However a more appropriate fix would be to reset tuning when >> ->ios.power_mode == MMC_POWER_UP, don't you think? >> > > rest_tuning seems all about the timing related stuff and it tends to be > executed for all non-uhs timings setting in case there's an uhs mode Yes, you are right. But this is actually why it caught my attention. Doesn't it seems weird that ->set_ios() invokes sdhci's ->set_uhs_signalling() callback, when in fact the timing isn't for UHS - but for all timings? Could you perhaps re-name the callback from ->set_uhs_signalling to ->set_timing(), to better reflect its purpose? > running before. It looks like not quite depend on MMC_POWER_UP, > that why we put it in set_uhs_signaling(). > > However, doing it in MMC_POWER_UP actually almost functions the same as > it will set MMC_TIMING_LEGACY accordingly. > > To me, it looks like ok to keep in set_uhs_signaling, but if you insist, > we can do it in MMC_POWER_UP, by a new platform specific .set_power(). Alright, it's no big deal for me. Just wanted to raise that seemed a bit weird. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 April 2017 at 12:05, Haibo Chen <haibo.chen@nxp.com> wrote: > USDHC tuning circuit should be reset before every time card enumeration > or re-enumeration. > > SD3.0 card need tuning. For SDR104 card, when system suspend in standby > mode, and then resume back, the IO timing is still SDR104(tuned) which > may result in card re-enumeration fail in low card speed(400khz) for some > cards. And we did meet the issue that in certain probability, SDR104 > card meet mmc command CRC/Timeout error when send CMD2 during mmc bus > resume. > > This patch reset the tuning circuit when the ios timing is > MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both > mmc_power_up() and mmc_power_off() will reset the tuning circuit. > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> Thanks, applied for next! Kind regards Uffe > --- > drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c > index abad67a..23d8b8a 100644 > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) > } > } > > +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; > + > + /* Rest the tuning circurt */ > + 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_ACMD12_ERR); > + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; > + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); > + } > + } > +} > + > static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > { > u32 m; > @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) > host->ops->set_clock(host, host->clock); > esdhc_set_strobe_dll(host); > break; > + case MMC_TIMING_LEGACY: > + default: > + esdhc_reset_tuning(host); > + break; > } > > esdhc_change_pinstate(host, timing); > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Fri, Apr 21, 2017 at 4:40 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 21 April 2017 at 07:53, Dong Aisheng <dongas86@gmail.com> wrote: >> Hi Ulf, >> >> On Thu, Apr 20, 2017 at 8:30 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote: >>> On 18 April 2017 at 12:05, Haibo Chen <haibo.chen@nxp.com> wrote: >>>> USDHC tuning circuit should be reset before every time card enumeration >>>> or re-enumeration. >>>> >>>> SD3.0 card need tuning. For SDR104 card, when system suspend in standby >>>> mode, and then resume back, the IO timing is still SDR104(tuned) which >>>> may result in card re-enumeration fail in low card speed(400khz) for some >>>> cards. And we did meet the issue that in certain probability, SDR104 >>>> card meet mmc command CRC/Timeout error when send CMD2 during mmc bus >>>> resume. >>>> >>>> This patch reset the tuning circuit when the ios timing is >>>> MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both >>>> mmc_power_up() and mmc_power_off() will reset the tuning circuit. >>>> >>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com> >>>> --- >>>> drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ >>>> 1 file changed, 26 insertions(+) >>>> >>>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c >>>> index abad67a..23d8b8a 100644 >>>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >>>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >>>> @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) >>>> } >>>> } >>>> >>>> +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; >>>> + >>>> + /* Rest the tuning circurt */ >>>> + 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_ACMD12_ERR); >>>> + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; >>>> + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); >>>> + } >>>> + } >>>> +} >>>> + >>>> static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >>>> { >>>> u32 m; >>>> @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) >>>> host->ops->set_clock(host, host->clock); >>>> esdhc_set_strobe_dll(host); >>>> break; >>>> + case MMC_TIMING_LEGACY: >>>> + default: >>>> + esdhc_reset_tuning(host); >>>> + break; >>> >>> I understand this fixes the problem. >>> >>> However a more appropriate fix would be to reset tuning when >>> ->ios.power_mode == MMC_POWER_UP, don't you think? >>> >> >> rest_tuning seems all about the timing related stuff and it tends to be >> executed for all non-uhs timings setting in case there's an uhs mode > > Yes, you are right. But this is actually why it caught my attention. > > Doesn't it seems weird that ->set_ios() invokes sdhci's > ->set_uhs_signalling() callback, when in fact the timing isn't for UHS > - but for all timings? > Yes, you're absolutely right at this point. > Could you perhaps re-name the callback from ->set_uhs_signalling to > ->set_timing(), to better reflect its purpose? > I was trying to follow your suggestion to change the name, however, found there's a pain that SDHCI implements timing setting in two steps: one for legacy timing while another for uhs timing (i guess that's probably why originally named set_uhs_signalling). Then only re-name callback to ->set_timing() may not so suitable as the legacy timing setting bits are still out of the callback. One way was to merge them all into one ->set_timing(), but after checking, i was a bit scared as there are executing follows exist on legacy timing setting mixed with other settings and i'm not sure the follow change won't cause any possible regression which is a bit blocking me to keep going. Another way is calling ->set_timing() twice for both legacy and uhs to ensure the sequence to be same as before. But i wonder if it's worth. Regards Dong Aisheng >> running before. It looks like not quite depend on MMC_POWER_UP, >> that why we put it in set_uhs_signaling(). >> >> However, doing it in MMC_POWER_UP actually almost functions the same as >> it will set MMC_TIMING_LEGACY accordingly. >> >> To me, it looks like ok to keep in set_uhs_signaling, but if you insist, >> we can do it in MMC_POWER_UP, by a new platform specific .set_power(). > > Alright, it's no big deal for me. Just wanted to raise that seemed a bit weird. > > Kind regards > Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c index abad67a..23d8b8a 100644 --- a/drivers/mmc/host/sdhci-esdhc-imx.c +++ b/drivers/mmc/host/sdhci-esdhc-imx.c @@ -889,6 +889,28 @@ static void esdhc_set_strobe_dll(struct sdhci_host *host) } } +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; + + /* Rest the tuning circurt */ + 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_ACMD12_ERR); + ctrl &= ~ESDHC_MIX_CTRL_SMPCLK_SEL; + writel(ctrl, host->ioaddr + SDHCI_ACMD12_ERR); + } + } +} + static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) { u32 m; @@ -932,6 +954,10 @@ static void esdhc_set_uhs_signaling(struct sdhci_host *host, unsigned timing) host->ops->set_clock(host, host->clock); esdhc_set_strobe_dll(host); break; + case MMC_TIMING_LEGACY: + default: + esdhc_reset_tuning(host); + break; } esdhc_change_pinstate(host, timing);
USDHC tuning circuit should be reset before every time card enumeration or re-enumeration. SD3.0 card need tuning. For SDR104 card, when system suspend in standby mode, and then resume back, the IO timing is still SDR104(tuned) which may result in card re-enumeration fail in low card speed(400khz) for some cards. And we did meet the issue that in certain probability, SDR104 card meet mmc command CRC/Timeout error when send CMD2 during mmc bus resume. This patch reset the tuning circuit when the ios timing is MMC_TIMING_LEGACY/MMC_TIMING_MMC_HS/MMC_TIMING_SD_HS, which means both mmc_power_up() and mmc_power_off() will reset the tuning circuit. Signed-off-by: Haibo Chen <haibo.chen@nxp.com> --- drivers/mmc/host/sdhci-esdhc-imx.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)