All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] mmc: fsl_esdhc: fix up for eMMC HS400
@ 2020-10-16  3:13 Yangbo Lu
  2020-10-16  3:13 ` [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization Yangbo Lu
  2020-10-16  3:13 ` [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400 Yangbo Lu
  0 siblings, 2 replies; 7+ messages in thread
From: Yangbo Lu @ 2020-10-16  3:13 UTC (permalink / raw)
  To: u-boot

This patch-set provides fix up for eMMC HS400 for a potential
DLL lock issue during mmc rescan.

Yangbo Lu (2):
  mmc: fsl_esdhc: set sysctl register for clock initialization
  mmc: fsl_esdhc: make sure delay chain locked for HS400

 drivers/mmc/fsl_esdhc.c | 30 ++++++++++++++++++++++++++----
 include/fsl_esdhc.h     |  4 ++++
 2 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization
  2020-10-16  3:13 [PATCH 0/2] mmc: fsl_esdhc: fix up for eMMC HS400 Yangbo Lu
@ 2020-10-16  3:13 ` Yangbo Lu
  2020-10-19 21:51   ` Jaehoon Chung
  2020-10-16  3:13 ` [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400 Yangbo Lu
  1 sibling, 1 reply; 7+ messages in thread
From: Yangbo Lu @ 2020-10-16  3:13 UTC (permalink / raw)
  To: u-boot

The initial clock setting should be through sysctl register only,
while the mmc_set_clock() will call mmc_set_ios() introduce other
configurations like bus width, mode, and so on.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/mmc/fsl_esdhc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index 642784e..68130ee 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -715,7 +715,7 @@ static int esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
 
 	/* Set the initial clock speed */
-	mmc_set_clock(mmc, 400000, MMC_CLK_ENABLE);
+	set_sysctl(priv, mmc, 400000);
 
 	/* Disable the BRR and BWR bits in IRQSTAT */
 	esdhc_clrbits32(&regs->irqstaten, IRQSTATEN_BRR | IRQSTATEN_BWR);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400
  2020-10-16  3:13 [PATCH 0/2] mmc: fsl_esdhc: fix up for eMMC HS400 Yangbo Lu
  2020-10-16  3:13 ` [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization Yangbo Lu
@ 2020-10-16  3:13 ` Yangbo Lu
  2020-10-19 21:51   ` Jaehoon Chung
  1 sibling, 1 reply; 7+ messages in thread
From: Yangbo Lu @ 2020-10-16  3:13 UTC (permalink / raw)
  To: u-boot

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@nxp.com>
---
 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);
+		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)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization
  2020-10-16  3:13 ` [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization Yangbo Lu
@ 2020-10-19 21:51   ` Jaehoon Chung
  0 siblings, 0 replies; 7+ messages in thread
From: Jaehoon Chung @ 2020-10-19 21:51 UTC (permalink / raw)
  To: u-boot

Dear Yangbo,

On 10/16/20 12:13 PM, Yangbo Lu wrote:
> The initial clock setting should be through sysctl register only,
> while the mmc_set_clock() will call mmc_set_ios() introduce other
> configurations like bus width, mode, and so on.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> ---
>  drivers/mmc/fsl_esdhc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index 642784e..68130ee 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -715,7 +715,7 @@ static int esdhc_init_common(struct fsl_esdhc_priv *priv, struct mmc *mmc)
>  	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
>  
>  	/* Set the initial clock speed */
> -	mmc_set_clock(mmc, 400000, MMC_CLK_ENABLE);
> +	set_sysctl(priv, mmc, 400000);
>  
>  	/* Disable the BRR and BWR bits in IRQSTAT */
>  	esdhc_clrbits32(&regs->irqstaten, IRQSTATEN_BRR | IRQSTATEN_BWR);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400
  2020-10-16  3:13 ` [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400 Yangbo Lu
@ 2020-10-19 21:51   ` Jaehoon Chung
  2020-10-20  3:20     ` Y.b. Lu
  0 siblings, 1 reply; 7+ messages in thread
From: Jaehoon Chung @ 2020-10-19 21:51 UTC (permalink / raw)
  To: u-boot

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@nxp.com>

Reviewed-by: Jaehoon Chung <jh80.chung@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)?

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)
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400
  2020-10-19 21:51   ` Jaehoon Chung
@ 2020-10-20  3:20     ` Y.b. Lu
  2020-10-20  6:40       ` Jaehoon Chung
  0 siblings, 1 reply; 7+ messages in thread
From: Y.b. Lu @ 2020-10-20  3:20 UTC (permalink / raw)
  To: u-boot

Hi Jaehoon,

> -----Original Message-----
> From: Jaehoon Chung <jh80.chung@samsung.com>
> Sent: Tuesday, October 20, 2020 5:52 AM
> To: Y.b. Lu <yangbo.lu@nxp.com>; u-boot at lists.denx.de; Peng Fan
> <peng.fan@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@nxp.com>
> 
> Reviewed-by: Jaehoon Chung <jh80.chung@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.

> 
> 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)
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400
  2020-10-20  3:20     ` Y.b. Lu
@ 2020-10-20  6:40       ` Jaehoon Chung
  0 siblings, 0 replies; 7+ messages in thread
From: Jaehoon Chung @ 2020-10-20  6:40 UTC (permalink / raw)
  To: u-boot

On 10/20/20 12:20 PM, Y.b. Lu wrote:
> Hi Jaehoon,
> 
>> -----Original Message-----
>> From: Jaehoon Chung <jh80.chung@samsung.com>
>> Sent: Tuesday, October 20, 2020 5:52 AM
>> To: Y.b. Lu <yangbo.lu@nxp.com>; u-boot at lists.denx.de; Peng Fan
>> <peng.fan@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@nxp.com>
>>
>> Reviewed-by: Jaehoon Chung <jh80.chung@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)
>>>
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-10-20  6:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16  3:13 [PATCH 0/2] mmc: fsl_esdhc: fix up for eMMC HS400 Yangbo Lu
2020-10-16  3:13 ` [PATCH 1/2] mmc: fsl_esdhc: set sysctl register for clock initialization Yangbo Lu
2020-10-19 21:51   ` Jaehoon Chung
2020-10-16  3:13 ` [PATCH 2/2] mmc: fsl_esdhc: make sure delay chain locked for HS400 Yangbo Lu
2020-10-19 21:51   ` Jaehoon Chung
2020-10-20  3:20     ` Y.b. Lu
2020-10-20  6:40       ` Jaehoon Chung

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.