All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd card
@ 2016-08-15 10:01 Haibo Chen
  2016-08-16  9:04 ` Dong Aisheng
  0 siblings, 1 reply; 3+ messages in thread
From: Haibo Chen @ 2016-08-15 10:01 UTC (permalink / raw)
  To: adrian.hunter, ulf.hansson, aisheng.dong, linux-mmc; +Cc: haibo.chen

Current driver do not clear the tuning related register after
plug out SD3.0 card. This will impact the timing and let other
SD3.0 card inserted later meet CRC error.

e.g., after plug out a SDR104 card, and then plug in a DDR50 card,
CRC error shows up:

mmc2: new ultra high speed DDR50 SDHC card at address aaaa
mmcblk2: mmc2:aaaa SE08G 7.40 GiB
mmcblk2: error -84 transferring data, sector 0, nr 8, cmd response 0x900, card status 0xb00
mmc2: tried to reset card
 mmcblk2: p1 p2

Logictally, we should reset USDHC include tuning circurt first when we
plug in a sd/mmc/sdio card.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index ea7d086..2783a4b 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -924,6 +924,30 @@ static void esdhc_reset(struct sdhci_host *host, u8 mask)
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 }
 
+static void esdhc_hw_reset_for_init(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;
+
+	sdhci_reset(host, SDHCI_RESET_ALL);
+
+	/* 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 |
+					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 unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
 {
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
@@ -958,6 +982,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.set_bus_width = esdhc_pltfm_set_bus_width,
 	.set_uhs_signaling = esdhc_set_uhs_signaling,
 	.reset = esdhc_reset,
+	.hw_reset = esdhc_hw_reset_for_init,
 };
 
 static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
@@ -1223,7 +1248,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
 
 	if (esdhc_is_usdhc(imx_data)) {
 		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
-		host->mmc->caps |= MMC_CAP_1_8V_DDR;
+		host->mmc->caps |= MMC_CAP_1_8V_DDR | MMC_CAP_HW_RESET;
 		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
 			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd card
  2016-08-15 10:01 [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd card Haibo Chen
@ 2016-08-16  9:04 ` Dong Aisheng
  2016-08-26 10:54   ` Haibo Chen
  0 siblings, 1 reply; 3+ messages in thread
From: Dong Aisheng @ 2016-08-16  9:04 UTC (permalink / raw)
  To: Haibo Chen; +Cc: adrian.hunter, ulf.hansson, aisheng.dong, linux-mmc

Hi Haibo,

On Mon, Aug 15, 2016 at 06:01:58PM +0800, Haibo Chen wrote:
> Current driver do not clear the tuning related register after
> plug out SD3.0 card. This will impact the timing and let other
> SD3.0 card inserted later meet CRC error.
> 
> e.g., after plug out a SDR104 card, and then plug in a DDR50 card,
> CRC error shows up:
> 
> mmc2: new ultra high speed DDR50 SDHC card at address aaaa
> mmcblk2: mmc2:aaaa SE08G 7.40 GiB
> mmcblk2: error -84 transferring data, sector 0, nr 8, cmd response 0x900, card status 0xb00
> mmc2: tried to reset card
>  mmcblk2: p1 p2
> 
> Logictally, we should reset USDHC include tuning circurt first when we
> plug in a sd/mmc/sdio card.

I'm thinking a bit more about the issue.

Can you check if the voltage setting will clear tuning bits during
mmc_power_up()?
Because the esdhc_writew_le(SDHCI_HOST_CONTROL2) seems will clear
it if no tuning bits set for a normal card.

And did you reproduce the issue with latest kernel?
If yes, please paste log and register dump which shows the issue
is caused by tuning bits.

Regards
Dong Aisheng

> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index ea7d086..2783a4b 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -924,6 +924,30 @@ static void esdhc_reset(struct sdhci_host *host, u8 mask)
>  	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
>  }
>  
> +static void esdhc_hw_reset_for_init(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;
> +
> +	sdhci_reset(host, SDHCI_RESET_ALL);
> +
> +	/* 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 |
> +					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 unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
>  {
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> @@ -958,6 +982,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
>  	.set_bus_width = esdhc_pltfm_set_bus_width,
>  	.set_uhs_signaling = esdhc_set_uhs_signaling,
>  	.reset = esdhc_reset,
> +	.hw_reset = esdhc_hw_reset_for_init,
>  };
>  
>  static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = {
> @@ -1223,7 +1248,7 @@ static int sdhci_esdhc_imx_probe(struct platform_device *pdev)
>  
>  	if (esdhc_is_usdhc(imx_data)) {
>  		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> -		host->mmc->caps |= MMC_CAP_1_8V_DDR;
> +		host->mmc->caps |= MMC_CAP_1_8V_DDR | MMC_CAP_HW_RESET;
>  		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
>  			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
>  
> -- 
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd card
  2016-08-16  9:04 ` Dong Aisheng
@ 2016-08-26 10:54   ` Haibo Chen
  0 siblings, 0 replies; 3+ messages in thread
From: Haibo Chen @ 2016-08-26 10:54 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: adrian.hunter, ulf.hansson, Aisheng Dong, linux-mmc

Hi Aisheng,

> -----Original Message-----
> From: Dong Aisheng [mailto:dongas86@gmail.com]
> Sent: Tuesday, August 16, 2016 5:05 PM
> To: Haibo Chen <haibo.chen@nxp.com>
> Cc: adrian.hunter@intel.com; ulf.hansson@linaro.org; Aisheng Dong
> <aisheng.dong@nxp.com>; linux-mmc@vger.kernel.org
> Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd
> card
> 
> Hi Haibo,
> 
> On Mon, Aug 15, 2016 at 06:01:58PM +0800, Haibo Chen wrote:
> > Current driver do not clear the tuning related register after plug out
> > SD3.0 card. This will impact the timing and let other
> > SD3.0 card inserted later meet CRC error.
> >
> > e.g., after plug out a SDR104 card, and then plug in a DDR50 card, CRC
> > error shows up:
> >
> > mmc2: new ultra high speed DDR50 SDHC card at address aaaa
> > mmcblk2: mmc2:aaaa SE08G 7.40 GiB
> > mmcblk2: error -84 transferring data, sector 0, nr 8, cmd response
> > 0x900, card status 0xb00
> > mmc2: tried to reset card
> >  mmcblk2: p1 p2
> >
> > Logictally, we should reset USDHC include tuning circurt first when we
> > plug in a sd/mmc/sdio card.
> 
> I'm thinking a bit more about the issue.
> 
> Can you check if the voltage setting will clear tuning bits during
> mmc_power_up()?
> Because the esdhc_writew_le(SDHCI_HOST_CONTROL2) seems will clear it if
> no tuning bits set for a normal card.
> 
> And did you reproduce the issue with latest kernel?
> If yes, please paste log and register dump which shows the issue is caused by
> tuning bits.

For the lastest kernel, DDR50 card will also execute tuning, so for DDR50 card, now no this issue. And for High-speed SD2.0 card, I tried several cards, they can work without this patch. But I dump the register, the tuning related bit set wrongly.

I test on i.MX6UL-14x14-EVK board, first, I insert one SDR104 card, then  plug out this card, and dump the register:
Reading 0x34 count starting at address 0x02190000

0x02190000:  9CE9E200 00010200 000020A0 113A0000
0x02190010:  00000900 00F15D7F 325B5900 00000900
0x02190020:  00000000 FF888008 08800022 008F000F
0x02190030:  00000000 00000000 00000000 00800000
0x02190040:  07F3B407 10401040 03000013 00000000
0x02190050:  00000000 00000000 9F046208 00000000
0x02190060:  00000000 00000200 16220000 0800FF80
0x02190070:  00000000 00000000 00000000 00000000
0x02190080:  00000000 00000000 00000000 00000000
0x02190090:  00000000 00000000 00000000 00000000
0x021900A0:  00000000 00000000 00000000 00000000
0x021900B0:  00000000 00000000 00000000 00000000
0x021900C0:  3000790B 00000000 00001006 01212801

For SDR104 card, the SMP_CLK_SEL( bit23 of 0x3c) is set, FBCLK_SEL(bit25 of 0x48) is set, and the register TUNE_CTRL_STATUS(0x68) is 0x16220000.


Then, I insert a High-speed SD2.0 card, it can work well, and I dump the register:
Reading 0x34 count starting at address 0x02190000

0x02190000:  9D0FF000 00010200 0000520F 113A0000
0x02190010:  00000900 001D8A7F 325B5900 00400E00
0x02190020:  00000000 FF888008 08800022 008F007F
0x02190030:  00000000 00000000 00000000 00800000
0x02190040:  07F3B407 10401040 03000013 00000000
0x02190050:  00000000 00000000 9F046208 00000000
0x02190060:  00000000 00000200 16220000 0800FF80
0x02190070:  00000000 00000000 00000000 00000000
0x02190080:  00000000 00000000 00000000 00000000
0x02190090:  00000000 00000000 00000000 00000000
0x021900A0:  00000000 00000000 00000000 00000000
0x021900B0:  00000000 00000000 00000000 00000000
0x021900C0:  30007909 00000000 00001006 01212801

For SD2.0 card, we do not execute tuning, but here, we can see SMP_CLK_SEL( bit23 of 0x3c) and FBCLK_SEL(bit25 of 0x48) is still set. TUNE_CTRL_STATUS(0x68) is 0x16220000.  This is not right.

Besides, now in this driver, we use TUNING_MOD_3, and use auto retuning, tuning will not execute every runtime resume. But if we change to use TUNING_MOD_1, then when usdhc runtime resume, tuning will be execute.  In this case, when plug out a SD3.0 card, it will call runtime resume, then tuning execute without a sd card, usdhc tuning circuit logic is wrong. At this time, if we insert a DDR50 card, and DDR50 card do not execute tuning(just like before), the issue describe in this path will happen, CRC error will shows up, because the tuning related bit is not clear and wrongly setted.

Now, DDR50 card will do tuning, and for i.MX usdhc, we only use TUNING_MOD_3, so the issue described in this patch is gone. But potential risk still exist in this driver. This patch still can avoid this risk.
So I will change the commit message and send V2 patch. 

Best Regards
Haibo

> 
> Regards
> Dong Aisheng
> 
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 27 ++++++++++++++++++++++++++-
> >  1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index ea7d086..2783a4b 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -924,6 +924,30 @@ static void esdhc_reset(struct sdhci_host *host, u8
> mask)
> >  	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);  }
> >
> > +static void esdhc_hw_reset_for_init(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;
> > +
> > +	sdhci_reset(host, SDHCI_RESET_ALL);
> > +
> > +	/* 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 |
> > +					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 unsigned int esdhc_get_max_timeout_count(struct sdhci_host
> > *host)  {
> >  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); @@ -958,6
> > +982,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
> >  	.set_bus_width = esdhc_pltfm_set_bus_width,
> >  	.set_uhs_signaling = esdhc_set_uhs_signaling,
> >  	.reset = esdhc_reset,
> > +	.hw_reset = esdhc_hw_reset_for_init,
> >  };
> >
> >  static const struct sdhci_pltfm_data sdhci_esdhc_imx_pdata = { @@
> > -1223,7 +1248,7 @@ static int sdhci_esdhc_imx_probe(struct
> > platform_device *pdev)
> >
> >  	if (esdhc_is_usdhc(imx_data)) {
> >  		host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > -		host->mmc->caps |= MMC_CAP_1_8V_DDR;
> > +		host->mmc->caps |= MMC_CAP_1_8V_DDR |
> MMC_CAP_HW_RESET;
> >  		if (!(imx_data->socdata->flags & ESDHC_FLAG_HS200))
> >  			host->quirks2 |= SDHCI_QUIRK2_BROKEN_HS200;
> >
> > --
> > 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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-08-26 13:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-15 10:01 [PATCH] mmc: sdhci-esdhc-imx: reset tuning circurt when insert sd card Haibo Chen
2016-08-16  9:04 ` Dong Aisheng
2016-08-26 10:54   ` Haibo Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.