Re: [PATCH] mmc: sdhci-esdhc-imx: reset the tuning logic before execute tuning

[Adrian Hunter <adrian.hunter@intel.com>]

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;