[PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400

[Jaehoon Chung]

On 10/20/20 12:20 PM, Y.b. Lu wrote:
> Hi Jaehoon,
> 
>> -----Original Message-----
>> From: Jaehoon Chung <jh80.chung at samsung.com>
>> Sent: Tuesday, October 20, 2020 5:52 AM
>> To: Y.b. Lu <yangbo.lu at nxp.com>; u-boot at lists.denx.de; Peng Fan
>> <peng.fan at nxp.com>
>> Subject: Re: [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for
>> HS400
>>
>> Dear Yangbo,
>>
>> On 10/16/20 12:13 PM, Yangbo Lu wrote:
>>> For eMMC HS400 mode, the DLL reset is a required step for mmc rescan.
>>> This step has not been documented in reference manual, but the RM will
>>> be fixed sooner or later.
>>>
>>> This patch is to add the step of DLL reset, and make sure delay chain
>>> locked for HS400.
>>>
>>> Fixes: db8f93672b42 ("mmc: fsl_esdhc: support eMMC HS400 mode")
>>> Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung at samsung.com>
>>
>> Just added minor comments.
>>
>>
>>> ---
>>>  drivers/mmc/fsl_esdhc.c | 28 +++++++++++++++++++++++++---
>>>  include/fsl_esdhc.h     |  4 ++++
>>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
>>> index 68130ee..a18316e 100644
>>> --- a/drivers/mmc/fsl_esdhc.c
>>> +++ b/drivers/mmc/fsl_esdhc.c
>>> @@ -70,7 +70,9 @@ struct fsl_esdhc {
>>>  	uint	sdtimingctl;	/* SD timing control register */
>>>  	char    reserved8[20];	/* reserved */
>>>  	uint	dllcfg0;	/* DLL config 0 register */
>>> -	char    reserved9[680];	/* reserved */
>>> +	char	reserved9[12];	/* reserved */
>>> +	uint	dllstat0;	/* DLL status 0 register */
>>> +	char    reserved10[664];/* reserved */
>>>  	uint    esdhcctl;	/* eSDHC control register */
>>>  };
>>>
>>> @@ -617,9 +619,11 @@ static void esdhc_exit_hs400(struct fsl_esdhc_priv
>> *priv)
>>>  	esdhc_tuning_block_enable(priv, false);
>>>  }
>>>
>>> -static void esdhc_set_timing(struct fsl_esdhc_priv *priv, enum bus_mode
>> mode)
>>> +static int esdhc_set_timing(struct fsl_esdhc_priv *priv, enum bus_mode
>> mode)
>>>  {
>>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>>> +	ulong start;
>>> +	u32 val;
>>>
>>>  	/* Exit HS400 mode before setting any other mode */
>>>  	if (esdhc_read32(&regs->tbctl) & HS400_MODE &&
>>> @@ -640,17 +644,33 @@ static void esdhc_set_timing(struct fsl_esdhc_priv
>> *priv, enum bus_mode mode)
>>>  			esdhc_setbits32(&regs->dllcfg0, DLL_FREQ_SEL);
>>>
>>>  		esdhc_setbits32(&regs->dllcfg0, DLL_ENABLE);
>>> +
>>> +		esdhc_setbits32(&regs->dllcfg0, DLL_RESET);
>>> +		udelay(1);
>>
>> Could you add a light comment why need to put udelay(1)?
> 
> Actually this is just per fixed reference manual.
> I sent out v2 patch, to explain in commit message in case some one what to know why.
> 
> "    In previous commit to support eMMC HS400,
>       db8f936 mmc: fsl_esdhc: support eMMC HS400 mode
> 
>     the steps to configure DLL could be found in commit message,
>       13. Set DLLCFG0[DLL_ENABLE] and DLLCFG0[DLL_FREQ_SEL].
>       14. Wait for delay chain to lock.
> 
>     these would be fixed as,
>       13.   Set DLLCFG0[DLL_ENABLE] and DLLCFG0[DLL_FREQ_SEL].
>       13.1  Write DLLCFG0[DLL_RESET] to 1 and wait for 1us,
>             then write DLLCFG0[DLL_RESET]
>       14.   Wait for delay chain to lock."
> 
> Thanks.

Thanks you for kindly explanation.

Best Regards,
Jaehoon Chung

> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> +		esdhc_clrbits32(&regs->dllcfg0, DLL_RESET);
>>> +
>>> +		start = get_timer(0);
>>> +		val = DLL_STS_SLV_LOCK;
>>> +		while (!(esdhc_read32(&regs->dllstat0) & val)) {
>>> +			if (get_timer(start) > 1000) {
>>> +				printf("fsl_esdhc: delay chain lock timeout\n");
>>> +				return -ETIMEDOUT;
>>> +			}
>>> +		}
>>> +
>>>  		esdhc_setbits32(&regs->tbctl, HS400_WNDW_ADJUST);
>>>
>>>  		esdhc_clock_control(priv, false);
>>>  		esdhc_flush_async_fifo(priv);
>>>  	}
>>>  	esdhc_clock_control(priv, true);
>>> +	return 0;
>>>  }
>>>
>>>  static int esdhc_set_ios_common(struct fsl_esdhc_priv *priv, struct mmc
>> *mmc)
>>>  {
>>>  	struct fsl_esdhc *regs = priv->esdhc_regs;
>>> +	int ret;
>>>
>>>  	if (priv->is_sdhc_per_clk) {
>>>  		/* Select to use peripheral clock */
>>> @@ -667,7 +687,9 @@ static int esdhc_set_ios_common(struct
>> fsl_esdhc_priv *priv, struct mmc *mmc)
>>>  		set_sysctl(priv, mmc, mmc->clock);
>>>
>>>  	/* Set timing */
>>> -	esdhc_set_timing(priv, mmc->selected_mode);
>>> +	ret = esdhc_set_timing(priv, mmc->selected_mode);
>>> +	if (ret)
>>> +		return ret;
>>>
>>>  	/* Set the bus width */
>>>  	esdhc_clrbits32(&regs->proctl, PROCTL_DTW_4 | PROCTL_DTW_8);
>>> diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h
>>> index e6f1c75..850a304 100644
>>> --- a/include/fsl_esdhc.h
>>> +++ b/include/fsl_esdhc.h
>>> @@ -187,8 +187,12 @@
>>>
>>>  /* DLL config 0 register */
>>>  #define DLL_ENABLE		0x80000000
>>> +#define DLL_RESET		0x40000000
>>>  #define DLL_FREQ_SEL		0x08000000
>>>
>>> +/* DLL status 0 register */
>>> +#define DLL_STS_SLV_LOCK	0x08000000
>>> +
>>>  #define MAX_TUNING_LOOP		40
>>>
>>>  #define HOSTVER_VENDOR(x)	(((x) >> 8) & 0xff)
>>>
>