[PATCH v1 0/2] imx6 ddr initialization fixes

[PATCH v1 0/2] imx6 ddr initialization fixes

[Francesco Dolcini]

Dear all,
the following series fixes a couple of issues on the i.MX6 DDR3 memory
initialization we discovered while debugging some rare boot failures on
apalis-imx6 [0]. This is a follow-up of this RFC series [1], with the
apalis-imx6 changes removed since we do plan doing additional validation before
merging.

[0] https://lore.kernel.org/all/20211202161428.GA104937@francesco-nb.int.toradex.com/
[1] https://lore.kernel.org/u-boot/20220404085119.97792-1-francesco.dolcini@toradex.com/

Changes in v1:

Add Reviewed-by: Marek Vasut marex@denx.de
Commit message and comments improvements, no code changes.

Francesco Dolcini (2):
  mx6: ddr: Restore ralat/walat in write level calibration
  mx6: ddr: Wait before issuing the first MRS cmd

 arch/arm/mach-imx/mx6/ddr.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v1 1/2] mx6: ddr: Restore ralat/walat in write level calibration

[Francesco Dolcini]

The current DDR write level calibration routine always overwrite
the ralat/walat fields to their maximum value, just save
the existing values at the beginning of the calibration routine
and restore it at the end.

In case the delay is estimated by the user to be more than one cycle the
walat should be configured according to that, this is not
automatically done.  From the i.MX6 RM:

   The user should read the results of the associated delay-line at
   MPWLDECTRL#[WL_DL_ABS_OFFSET#] and in case the user estimates that the
   reasonable delay may be above 1 cycle then the user should indicate it at
   MPWLDECTRL#[WL_CYC_DEL#]. Moreover the user should indicate it in
   MDMISC[WALAT] field. For example, if the result of the write leveling calibration
   is 100/256 parts of a cycle, but the user estimates that the delay is above 2 cycles
   then MPWLDECTRL#[WL_CYC_DEL#] should be configured to 2, so the total
   delay will be 2 and 100/256 parts of a cycle

Probably it would just possible to not overwrite the mdmisc register in
the first place, since this is not present in the write_level_calib() example
in NXP AN4467 nor in the i.MX6 RM (44.11.6.1 Hardware Write Leveling
Calibration).

Fixes: d339f16911c7 ("arm: imx6: Add DDR3 calibration code for MX6 Q/D/DL")
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Reviewed-by: Marek Vasut <marex@denx.de>
---
Changes in v1:
- Add Reviewed-by: Marek Vasut marex@denx.de

---
 arch/arm/mach-imx/mx6/ddr.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/mx6/ddr.c b/arch/arm/mach-imx/mx6/ddr.c
index f872bfdab315..181df941cfb7 100644
--- a/arch/arm/mach-imx/mx6/ddr.c
+++ b/arch/arm/mach-imx/mx6/ddr.c
@@ -108,7 +108,7 @@ int mmdc_do_write_level_calibration(struct mx6_ddr_sysinfo const *sysinfo)
 {
 	struct mmdc_p_regs *mmdc0 = (struct mmdc_p_regs *)MMDC_P0_BASE_ADDR;
 	struct mmdc_p_regs *mmdc1 = (struct mmdc_p_regs *)MMDC_P1_BASE_ADDR;
-	u32 esdmisc_val, zq_val;
+	u32 esdmisc_val, zq_val, mdmisc_val;
 	u32 errors = 0;
 	u32 ldectrl[4] = {0};
 	u32 ddr_mr1 = 0x4;
@@ -131,6 +131,9 @@ int mmdc_do_write_level_calibration(struct mx6_ddr_sysinfo const *sysinfo)
 	/* disable Adopt power down timer */
 	setbits_le32(&mmdc0->mapsr, 0x1);
 
+	/* Save old RALAT and WALAT values */
+	mdmisc_val = readl(&mmdc0->mdmisc);
+
 	debug("Starting write leveling calibration.\n");
 
 	/*
@@ -217,6 +220,9 @@ int mmdc_do_write_level_calibration(struct mx6_ddr_sysinfo const *sysinfo)
 	writel(esdmisc_val, &mmdc0->mdref);
 	writel(zq_val, &mmdc0->mpzqhwctrl);
 
+	/* restore WALAT/RALAT */
+	writel(mdmisc_val, &mmdc0->mdmisc);
+
 	debug("\tMMDC_MPWLDECTRL0 after write level cal: 0x%08x\n",
 	      readl(&mmdc0->mpwldectrl0));
 	debug("\tMMDC_MPWLDECTRL1 after write level cal: 0x%08x\n",
-- 
2.25.1

Re: [PATCH v1 1/2] mx6: ddr: Restore ralat/walat in write level calibration

[Fabio Estevam]

Hi Francesco,

On Wed, Apr 6, 2022 at 8:53 AM Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> The current DDR write level calibration routine always overwrite
> the ralat/walat fields to their maximum value, just save
> the existing values at the beginning of the calibration routine
> and restore it at the end.
>
> In case the delay is estimated by the user to be more than one cycle the
> walat should be configured according to that, this is not
> automatically done.  From the i.MX6 RM:
>
>    The user should read the results of the associated delay-line at
>    MPWLDECTRL#[WL_DL_ABS_OFFSET#] and in case the user estimates that the
>    reasonable delay may be above 1 cycle then the user should indicate it at
>    MPWLDECTRL#[WL_CYC_DEL#]. Moreover the user should indicate it in
>    MDMISC[WALAT] field. For example, if the result of the write leveling calibration
>    is 100/256 parts of a cycle, but the user estimates that the delay is above 2 cycles
>    then MPWLDECTRL#[WL_CYC_DEL#] should be configured to 2, so the total
>    delay will be 2 and 100/256 parts of a cycle
>
> Probably it would just possible to not overwrite the mdmisc register in
> the first place, since this is not present in the write_level_calib() example
> in NXP AN4467 nor in the i.MX6 RM (44.11.6.1 Hardware Write Leveling
> Calibration).
>
> Fixes: d339f16911c7 ("arm: imx6: Add DDR3 calibration code for MX6 Q/D/DL")
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Reviewed-by: Marek Vasut <marex@denx.de>

Reviewed-by: Fabio Estevam <festevam@denx.de>

[PATCH v1 1/2] mx6: ddr: Restore ralat/walat in write level calibration

[sbabic]

> The current DDR write level calibration routine always overwrite
> the ralat/walat fields to their maximum value, just save
> the existing values at the beginning of the calibration routine
> and restore it at the end.
> In case the delay is estimated by the user to be more than one cycle the
> walat should be configured according to that, this is not
> automatically done.  From the i.MX6 RM:
>    The user should read the results of the associated delay-line at
>    MPWLDECTRL#[WL_DL_ABS_OFFSET#] and in case the user estimates that the
>    reasonable delay may be above 1 cycle then the user should indicate it at
>    MPWLDECTRL#[WL_CYC_DEL#]. Moreover the user should indicate it in
>    MDMISC[WALAT] field. For example, if the result of the write leveling calibration
>    is 100/256 parts of a cycle, but the user estimates that the delay is above 2 cycles
>    then MPWLDECTRL#[WL_CYC_DEL#] should be configured to 2, so the total
>    delay will be 2 and 100/256 parts of a cycle
> Probably it would just possible to not overwrite the mdmisc register in
> the first place, since this is not present in the write_level_calib() example
> in NXP AN4467 nor in the i.MX6 RM (44.11.6.1 Hardware Write Leveling
> Calibration).
> Fixes: d339f16911c7 ("arm: imx6: Add DDR3 calibration code for MX6 Q/D/DL")
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Reviewed-by: Fabio Estevam <festevam@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================

[PATCH v1 2/2] mx6: ddr: Wait before issuing the first MRS cmd

[Francesco Dolcini]

Wait 1ms before issuing the first MRS command to write DDR3 Mode
registers.

There is a requirement to wait a minimum time before issuing command to
the DDR3 device, according to the JEDEC standard this time is 500us
(after RESET_n is de-asserted until CKE becomes active) + tXPR (Reset
CKE Exit time, maximum value 360ns).

It seems that for some reason this is not enforced by the MMDC
controller.

Without this change we experienced random memory initialization failures
with about 2% boot failure rate on specific problematic boards, after
this change we were able to do more than 10.000 power-cycle without a
single failure.

Fixes: fe0f7f7842e1 ("mx6: add mmdc configuration for MX6Q/MX6DL")
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Reviewed-by: Marek Vasut <marex@denx.de>
---
Changes in v1:
- Add Reviewed-by: Marek Vasut marex@denx.de
- Commit message and comments improvements, no code changes.
---
 arch/arm/mach-imx/mx6/ddr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-imx/mx6/ddr.c b/arch/arm/mach-imx/mx6/ddr.c
index 181df941cfb7..73a637c42d6c 100644
--- a/arch/arm/mach-imx/mx6/ddr.c
+++ b/arch/arm/mach-imx/mx6/ddr.c
@@ -1526,6 +1526,11 @@ void mx6_ddr3_cfg(const struct mx6_ddr_sysinfo *sysinfo,
 			((sysinfo->ncs == 2) ? 1 : 0) << 30; /* SDE_1 for CS1 */
 
 	/* Step 8: Write Mode Registers to Init DDR3 devices */
+	mdelay(1); /* Wait before issuing the first MRS command.
+		    * Minimum wait time is (tXPR + 500us),
+		    * with max tXPR value 360ns, and 500us wait required after
+		    * RESET_n is de-asserted.
+		    */
 	for (cs = 0; cs < sysinfo->ncs; cs++) {
 		/* MR2 */
 		val = (sysinfo->rtt_wr & 3) << 9 | (ddr3_cfg->SRT & 1) << 7 |
-- 
2.25.1

Re: [PATCH v1 2/2] mx6: ddr: Wait before issuing the first MRS cmd

[Fabio Estevam]

Hi Francesco,

On Wed, Apr 6, 2022 at 8:53 AM Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> Wait 1ms before issuing the first MRS command to write DDR3 Mode
> registers.
>
> There is a requirement to wait a minimum time before issuing command to
> the DDR3 device, according to the JEDEC standard this time is 500us
> (after RESET_n is de-asserted until CKE becomes active) + tXPR (Reset
> CKE Exit time, maximum value 360ns).
>
> It seems that for some reason this is not enforced by the MMDC
> controller.
>
> Without this change we experienced random memory initialization failures
> with about 2% boot failure rate on specific problematic boards, after
> this change we were able to do more than 10.000 power-cycle without a
> single failure.

Glad you fixed this problem. Not an easy one!

Reviewed-by: Fabio Estevam <festevam@denx.de>

[PATCH v1 2/2] mx6: ddr: Wait before issuing the first MRS cmd

[sbabic]

> Wait 1ms before issuing the first MRS command to write DDR3 Mode
> registers.
> There is a requirement to wait a minimum time before issuing command to
> the DDR3 device, according to the JEDEC standard this time is 500us
> (after RESET_n is de-asserted until CKE becomes active) + tXPR (Reset
> CKE Exit time, maximum value 360ns).
> It seems that for some reason this is not enforced by the MMDC
> controller.
> Without this change we experienced random memory initialization failures
> with about 2% boot failure rate on specific problematic boards, after
> this change we were able to do more than 10.000 power-cycle without a
> single failure.
> Fixes: fe0f7f7842e1 ("mx6: add mmdc configuration for MX6Q/MX6DL")
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Reviewed-by: Marek Vasut <marex@denx.de>
> Reviewed-by: Fabio Estevam <festevam@denx.de>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================