On 10/20/20 12:20 PM, Y.b. Lu wrote: > Hi Jaehoon, > >> -----Original Message----- >> From: Jaehoon Chung <jh80.ch...@samsung.com> >> Sent: Tuesday, October 20, 2020 5:52 AM >> To: Y.b. Lu <yangbo...@nxp.com>; u-boot@lists.denx.de; Peng Fan >> <peng....@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...@nxp.com> >> >> Reviewed-by: Jaehoon Chung <jh80.ch...@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(®s->tbctl) & HS400_MODE && >>> @@ -640,17 +644,33 @@ static void esdhc_set_timing(struct fsl_esdhc_priv >> *priv, enum bus_mode mode) >>> esdhc_setbits32(®s->dllcfg0, DLL_FREQ_SEL); >>> >>> esdhc_setbits32(®s->dllcfg0, DLL_ENABLE); >>> + >>> + esdhc_setbits32(®s->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(®s->dllcfg0, DLL_RESET); >>> + >>> + start = get_timer(0); >>> + val = DLL_STS_SLV_LOCK; >>> + while (!(esdhc_read32(®s->dllstat0) & val)) { >>> + if (get_timer(start) > 1000) { >>> + printf("fsl_esdhc: delay chain lock timeout\n"); >>> + return -ETIMEDOUT; >>> + } >>> + } >>> + >>> esdhc_setbits32(®s->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(®s->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) >>> >