| Message ID | 1492429538-14996-1-git-send-email-haibo.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/04/17 14:45, 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> > --- > 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..8960833 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); > + u16 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; So ESDHC_MIX_CTRL_SMPCLK_SEL is bit-23 but ctrl is 16-bit? > + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > + writel(0 << 8, host->ioaddr + ESDHC_TUNE_CTRL_STATUS); 0 << 8 is still zero. > + } 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
> -----Original Message----- > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc- > owner@vger.kernel.org] On Behalf Of Adrian Hunter > Sent: Tuesday, April 18, 2017 3:16 PM > To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org; A.S. Dong > <aisheng.dong@nxp.com> > Cc: linux-mmc@vger.kernel.org > Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circuit when power > on mmc card > > On 17/04/17 14:45, 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> > > --- > > 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..8960833 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); > > + u16 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; > > So ESDHC_MIX_CTRL_SMPCLK_SEL is bit-23 but ctrl is 16-bit? > The bit-23 of both register ESDHC_MIX_CTRL and SDHCI_ACMD12_ERR is named ESDHC_MIX_CTRL_SMPCLK_SEL , define whether tuned clock or fixed clock is used to sample data. The bit-23 of register ESDHC_MIX_CTRL works when STD_TUNING_EN is 0, which means use MAN tuning. The bit-23 of register SDHCI_ACMD12_ERR works when STD_TUNING_EN is set. > > + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > > + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > > + writel(0 << 8, host->ioaddr + > ESDHC_TUNE_CTRL_STATUS); > > 0 << 8 is still zero. Yes, this code want to clear the bit8~bit14 of register ESDHC_TUNE_CTRL_STATUS Here is the define of these bits in RM: [14-8] DLY_CELL_SET_PRE[6:0] Set the number of delay cells on the feedback clock between the feedback clock and CLK_PRE. Haibo > > > + } 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 -- 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 11:16, Bough Chen wrote: >> -----Original Message----- >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc- >> owner@vger.kernel.org] On Behalf Of Adrian Hunter >> Sent: Tuesday, April 18, 2017 3:16 PM >> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org; A.S. Dong >> <aisheng.dong@nxp.com> >> Cc: linux-mmc@vger.kernel.org >> Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circuit when power >> on mmc card >> >> On 17/04/17 14:45, 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> >>> --- >>> 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..8960833 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); >>> + u16 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; >> >> So ESDHC_MIX_CTRL_SMPCLK_SEL is bit-23 but ctrl is 16-bit? >> > > The bit-23 of both register ESDHC_MIX_CTRL and SDHCI_ACMD12_ERR is named ESDHC_MIX_CTRL_SMPCLK_SEL , define whether tuned clock or fixed clock is used to sample data. > The bit-23 of register ESDHC_MIX_CTRL works when STD_TUNING_EN is 0, which means use MAN tuning. > The bit-23 of register SDHCI_ACMD12_ERR works when STD_TUNING_EN is set. I meant it looks very weird to try to change bit-23 in a 16-bit variable. Should 'ctrl' be u32? > >>> + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; >>> + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); >>> + writel(0 << 8, host->ioaddr + >> ESDHC_TUNE_CTRL_STATUS); >> >> 0 << 8 is still zero. > > Yes, this code want to clear the bit8~bit14 of register ESDHC_TUNE_CTRL_STATUS > Here is the define of these bits in RM: > [14-8] DLY_CELL_SET_PRE[6:0] Set the number of delay cells on the feedback clock between the feedback clock and CLK_PRE. I just meant use 0 not 0 << 8. > > Haibo >> >>> + } 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 > -- 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
> -----Original Message----- > From: Adrian Hunter [mailto:adrian.hunter@intel.com] > Sent: Tuesday, April 18, 2017 5:38 PM > To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org; A.S. Dong > <aisheng.dong@nxp.com> > Cc: linux-mmc@vger.kernel.org > Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circuit when power > on mmc card > > On 18/04/17 11:16, Bough Chen wrote: > >> -----Original Message----- > >> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc- > >> owner@vger.kernel.org] On Behalf Of Adrian Hunter > >> Sent: Tuesday, April 18, 2017 3:16 PM > >> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org; A.S. > >> Dong <aisheng.dong@nxp.com> > >> Cc: linux-mmc@vger.kernel.org > >> Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circuit when > >> power on mmc card > >> > >> On 17/04/17 14:45, 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> > >>> --- > >>> 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..8960833 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); > >>> + u16 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; > >> > >> So ESDHC_MIX_CTRL_SMPCLK_SEL is bit-23 but ctrl is 16-bit? > >> > > > > The bit-23 of both register ESDHC_MIX_CTRL and SDHCI_ACMD12_ERR is > named ESDHC_MIX_CTRL_SMPCLK_SEL , define whether tuned clock or fixed > clock is used to sample data. > > The bit-23 of register ESDHC_MIX_CTRL works when STD_TUNING_EN is 0, > which means use MAN tuning. > > The bit-23 of register SDHCI_ACMD12_ERR works when STD_TUNING_EN is > set. > > I meant it looks very weird to try to change bit-23 in a 16-bit variable. > Should 'ctrl' be u32? > Oh, yes, my mistake. Will change in V2 > > > >>> + ctrl &= ~ESDHC_MIX_CTRL_FBCLK_SEL; > >>> + writel(ctrl, host->ioaddr + ESDHC_MIX_CTRL); > >>> + writel(0 << 8, host->ioaddr + > >> ESDHC_TUNE_CTRL_STATUS); > >> > >> 0 << 8 is still zero. > > > > Yes, this code want to clear the bit8~bit14 of register > > ESDHC_TUNE_CTRL_STATUS Here is the define of these bits in RM: > > [14-8] DLY_CELL_SET_PRE[6:0] Set the number of delay cells on the feedback > clock between the feedback clock and CLK_PRE. > > I just meant use 0 not 0 << 8. Yes, seems better. Haibo > > > > > Haibo > >> > >>> + } 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 > > -- 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..8960833 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); + u16 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 << 8, 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(+)