diff mbox series

[v2,3/3] mmc: sdhci-esdhc-imx: only enable DAT[0] and CMD line auto tuning for SDIO device

Message ID 20221221112853.789675-4-haibo.chen@nxp.com (mailing list archive)
State New, archived
Headers show
Series fix the sdio device DATA/CMD CRC and Timeout issue after tuning | expand

Commit Message

Bough Chen Dec. 21, 2022, 11:28 a.m. UTC 
From: Haibo Chen <haibo.chen@nxp.com>

USDHC IP has one limitation: the tuning circuit can't handle the async
sdio device interrupt correctly. When sdio device use 4 data lines,
async sdio interrupt will use the shared DAT[1], if enable auto tuning
circuit to check these 4 data lines, include the DAT[1], this circuit
will detect this interrupt, take this as data on DAT[1], and adjust the
delay cell wrongly, finally will cause the DATA/CMD CRC error.
So for SDIO device, only enable DAT[0] and CMD line for auto tuning.

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

Comments

Adrian Hunter Dec. 21, 2022, 2:21 p.m. UTC | #1 
On 21/12/22 13:28, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> USDHC IP has one limitation: the tuning circuit can't handle the async
> sdio device interrupt correctly. When sdio device use 4 data lines,
> async sdio interrupt will use the shared DAT[1], if enable auto tuning
> circuit to check these 4 data lines, include the DAT[1], this circuit
> will detect this interrupt, take this as data on DAT[1], and adjust the
> delay cell wrongly, finally will cause the DATA/CMD CRC error.
> So for SDIO device, only enable DAT[0] and CMD line for auto tuning.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index bf8d6f60a9ee..d6ce4c8d23dc 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -448,6 +448,20 @@ static inline void usdhc_auto_tuning_mode_sel_and_en(struct sdhci_host *host)
>  		break;
>  	}
>  
> +	/*
> +	 * For USDHC, auto tuning circuit can not handle the async sdio
> +	 * device interrupt correctly. When sdio device use 4 data lines,
> +	 * async sdio interrupt will use the shared DAT[1], if enable auto
> +	 * tuning circuit check these 4 data lines, include the DAT[1],
> +	 * this circuit will detect this interrupt, take this as a data on
> +	 * DAT[1], and adjust the delay cell wrongly.
> +	 * This is the hardware design limitation, to avoid this, for sdio
> +	 * device, config the auto tuning circuit only check DAT[0] and CMD
> +	 * line.
> +	 */
> +	if (!host->mmc->card && mmc_card_sdio(host->mmc->card))

Looks like !host->mmc->card should be host->mmc->card

> +		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
> +
>  	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
>  			auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
>  			ESDHC_VEND_SPEC2);
Bough Chen Dec. 22, 2022, 2:24 a.m. UTC | #2 
> -----Original Message-----
> From: Adrian Hunter <adrian.hunter@intel.com>
> Sent: 2022年12月21日 22:21
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> linux-mmc@vger.kernel.org; kgroeneveld@lenbrook.com;
> ulf.hansson@linaro.org
> Subject: Re: [PATCH v2 3/3] mmc: sdhci-esdhc-imx: only enable DAT[0] and
> CMD line auto tuning for SDIO device
> 
> On 21/12/22 13:28, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > USDHC IP has one limitation: the tuning circuit can't handle the async
> > sdio device interrupt correctly. When sdio device use 4 data lines,
> > async sdio interrupt will use the shared DAT[1], if enable auto tuning
> > circuit to check these 4 data lines, include the DAT[1], this circuit
> > will detect this interrupt, take this as data on DAT[1], and adjust
> > the delay cell wrongly, finally will cause the DATA/CMD CRC error.
> > So for SDIO device, only enable DAT[0] and CMD line for auto tuning.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c
> > b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index bf8d6f60a9ee..d6ce4c8d23dc 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -448,6 +448,20 @@ static inline void
> usdhc_auto_tuning_mode_sel_and_en(struct sdhci_host *host)
> >  		break;
> >  	}
> >
> > +	/*
> > +	 * For USDHC, auto tuning circuit can not handle the async sdio
> > +	 * device interrupt correctly. When sdio device use 4 data lines,
> > +	 * async sdio interrupt will use the shared DAT[1], if enable auto
> > +	 * tuning circuit check these 4 data lines, include the DAT[1],
> > +	 * this circuit will detect this interrupt, take this as a data on
> > +	 * DAT[1], and adjust the delay cell wrongly.
> > +	 * This is the hardware design limitation, to avoid this, for sdio
> > +	 * device, config the auto tuning circuit only check DAT[0] and CMD
> > +	 * line.
> > +	 */
> > +	if (!host->mmc->card && mmc_card_sdio(host->mmc->card))
> 
> Looks like !host->mmc->card should be host->mmc->card

Yes, my mistake, my local test version is correct, I will fix this.

Best Regards
Haibo Chen
> 
> > +		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
> > +
> >  	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
> >  			auto_tune_buswidth |
> ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
> >  			ESDHC_VEND_SPEC2);
Dan Carpenter Dec. 27, 2022, 5:50 p.m. UTC | #3 
Hi,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/haibo-chen-nxp-com/fix-the-sdio-device-DATA-CMD-CRC-and-Timeout-issue-after-tuning/20221221-193033
patch link:    https://lore.kernel.org/r/20221221112853.789675-4-haibo.chen%40nxp.com
patch subject: [PATCH v2 3/3] mmc: sdhci-esdhc-imx: only enable DAT[0] and CMD line auto tuning for SDIO device
config: parisc-randconfig-m031-20221225
compiler: hppa-linux-gcc (GCC) 12.1.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>

smatch warnings:
drivers/mmc/host/sdhci-esdhc-imx.c:462 usdhc_auto_tuning_mode_sel_and_en() error: we previously assumed 'host->mmc->card' could be null (see line 462)

vim +462 drivers/mmc/host/sdhci-esdhc-imx.c

45334ee13858de Haibo Chen 2021-08-18  431  /* Enable the auto tuning circuit to check the CMD line and BUS line */
235cd41770bdcb Haibo Chen 2022-12-21  432  static inline void usdhc_auto_tuning_mode_sel_and_en(struct sdhci_host *host)
45334ee13858de Haibo Chen 2021-08-18  433  {
45334ee13858de Haibo Chen 2021-08-18  434  	u32 buswidth, auto_tune_buswidth;
235cd41770bdcb Haibo Chen 2022-12-21  435  	u32 reg;
45334ee13858de Haibo Chen 2021-08-18  436  
45334ee13858de Haibo Chen 2021-08-18  437  	buswidth = USDHC_GET_BUSWIDTH(readl(host->ioaddr + SDHCI_HOST_CONTROL));
45334ee13858de Haibo Chen 2021-08-18  438  
45334ee13858de Haibo Chen 2021-08-18  439  	switch (buswidth) {
45334ee13858de Haibo Chen 2021-08-18  440  	case ESDHC_CTRL_8BITBUS:
45334ee13858de Haibo Chen 2021-08-18  441  		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_8BIT_EN;
45334ee13858de Haibo Chen 2021-08-18  442  		break;
45334ee13858de Haibo Chen 2021-08-18  443  	case ESDHC_CTRL_4BITBUS:
45334ee13858de Haibo Chen 2021-08-18  444  		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_4BIT_EN;
45334ee13858de Haibo Chen 2021-08-18  445  		break;
45334ee13858de Haibo Chen 2021-08-18  446  	default:	/* 1BITBUS */
45334ee13858de Haibo Chen 2021-08-18  447  		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
45334ee13858de Haibo Chen 2021-08-18  448  		break;
45334ee13858de Haibo Chen 2021-08-18  449  	}
45334ee13858de Haibo Chen 2021-08-18  450  
e088d1496d45c2 Haibo Chen 2022-12-21  451  	/*
e088d1496d45c2 Haibo Chen 2022-12-21  452  	 * For USDHC, auto tuning circuit can not handle the async sdio
e088d1496d45c2 Haibo Chen 2022-12-21  453  	 * device interrupt correctly. When sdio device use 4 data lines,
e088d1496d45c2 Haibo Chen 2022-12-21  454  	 * async sdio interrupt will use the shared DAT[1], if enable auto
e088d1496d45c2 Haibo Chen 2022-12-21  455  	 * tuning circuit check these 4 data lines, include the DAT[1],
e088d1496d45c2 Haibo Chen 2022-12-21  456  	 * this circuit will detect this interrupt, take this as a data on
e088d1496d45c2 Haibo Chen 2022-12-21  457  	 * DAT[1], and adjust the delay cell wrongly.
e088d1496d45c2 Haibo Chen 2022-12-21  458  	 * This is the hardware design limitation, to avoid this, for sdio
e088d1496d45c2 Haibo Chen 2022-12-21  459  	 * device, config the auto tuning circuit only check DAT[0] and CMD
e088d1496d45c2 Haibo Chen 2022-12-21  460  	 * line.
e088d1496d45c2 Haibo Chen 2022-12-21  461  	 */
e088d1496d45c2 Haibo Chen 2022-12-21 @462  	if (!host->mmc->card && mmc_card_sdio(host->mmc->card))

Presumably the ! is a mistake?  mmc_card_sdio() dereferences
host->mmc->card.

e088d1496d45c2 Haibo Chen 2022-12-21  463  		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
e088d1496d45c2 Haibo Chen 2022-12-21  464  
45334ee13858de Haibo Chen 2021-08-18  465  	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
45334ee13858de Haibo Chen 2021-08-18  466  			auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
45334ee13858de Haibo Chen 2021-08-18  467  			ESDHC_VEND_SPEC2);
235cd41770bdcb Haibo Chen 2022-12-21  468  
235cd41770bdcb Haibo Chen 2022-12-21  469  	reg = readl(host->ioaddr + ESDHC_MIX_CTRL);
235cd41770bdcb Haibo Chen 2022-12-21  470  	reg |= ESDHC_MIX_CTRL_AUTO_TUNE_EN;
235cd41770bdcb Haibo Chen 2022-12-21  471  	writel(reg, host->ioaddr + ESDHC_MIX_CTRL);
45334ee13858de Haibo Chen 2021-08-18  472  }
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index bf8d6f60a9ee..d6ce4c8d23dc 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -448,6 +448,20 @@  static inline void usdhc_auto_tuning_mode_sel_and_en(struct sdhci_host *host)
 		break;
 	}
 
+	/*
+	 * For USDHC, auto tuning circuit can not handle the async sdio
+	 * device interrupt correctly. When sdio device use 4 data lines,
+	 * async sdio interrupt will use the shared DAT[1], if enable auto
+	 * tuning circuit check these 4 data lines, include the DAT[1],
+	 * this circuit will detect this interrupt, take this as a data on
+	 * DAT[1], and adjust the delay cell wrongly.
+	 * This is the hardware design limitation, to avoid this, for sdio
+	 * device, config the auto tuning circuit only check DAT[0] and CMD
+	 * line.
+	 */
+	if (!host->mmc->card && mmc_card_sdio(host->mmc->card))
+		auto_tune_buswidth = ESDHC_VEND_SPEC2_AUTO_TUNE_1BIT_EN;
+
 	esdhc_clrset_le(host, ESDHC_VEND_SPEC2_AUTO_TUNE_MODE_MASK,
 			auto_tune_buswidth | ESDHC_VEND_SPEC2_AUTO_TUNE_CMD_EN,
 			ESDHC_VEND_SPEC2);