[U-Boot] [PATCH] ARM: mx6: ddr: Add write leveling correction code

[Eric Nelson]

Hi Marek,

Thanks for this update and the detailed notes.

On 03/29/2018 06:04 PM, Marek Vasut wrote:
> When the DDR calibration is enabled, a situation may happen that it
> will fail on a few select boards out of a whole production lot. In
> particular, after the first write leveling stage, the MPWLDECTRLx
> registers will contain a value 0x1nn , for nn usually being 0x7f or
> slightly lower.
> 
> What this means is that the HW write leveling detected that the DQS
> rising edge on one or more bundles arrives slightly _after_ CLK and
> therefore when the DDR DRAM samples CLK on the DQS rising edge, the
> CLK signal is already high (cfr. AN4467 rev2 Figure 7 on page 18).
> 
> The HW write leveling then ends up adding almost an entire cycle (thus
> the 0x17f) to the DQS delay, which indeed aligns it, but also triggers
> subsequent calibration failure in DQS gating due to this massive offset.
> 
> There are two observations here:
> - If the MPWLDECTRLx value is corrected from 0x17f to 0x0 , then the
>    DQS gating passes, the entire calibration passes as well and the
>    DRAM is perfectly stable even under massive load.
> - When using the NXP DRAM calibrator for iMX6/7, the value 0x17f or so
>    in MPWLDECTRx register is not there, but it is replaced by 0x0 as one
>    would expect.
> 
> Someone from NXP finally explains why, quoting [1]:
> 
>      "
>      Having said all that, the DDR Stress Test does something that we
>      do not advertise to the users. The Stress Test iself looks at the
>      values of the MPWLDECTRL0/1 fields before reporting results, and
>      if it sees any filed with a value greater than 200/256 delay
>      (reported as half-cycle = 0x1 and ABS_OFFSET > 0x48), the DDR
>      Stress test will reset the Write Leveling delay for this lane
>      to 0x000 and not report it in the log.
> 
>      The reason that the DDR Stress test does this is because a delay
>      of more than 78% a clock cycle means that the DQS edge is arriving
>      within the JEDEC tolerence of 25% of the clock edge. In most cases,
>      DQS is arriving < 5% tCK of the SDCLK edge in the early case, and
>      it does not make sense to delay the DQS strobe almost a full clock
>      cycle and add extra latency to each Write burst just to make the
>      two edges align exactly. In this case, we are guilty of making a
>      decision for the customer without telling them we are doing it so
>      that we don't have to provide the above explanation to every customer.
>      They don't need to know it.
>      "
> 
> This patch adds the correction described above, that is if the MPWLDECTRx
> value is over 0x148, the value is corrected back to 0x0.
> 
> [1] https://community.nxp.com/thread/456246
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Stefano Babic <sbabic at denx.de>
> ---
>   arch/arm/mach-imx/mx6/ddr.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/mx6/ddr.c b/arch/arm/mach-imx/mx6/ddr.c
> index 43b77cfa41..6e5e40dd1a 100644
> --- a/arch/arm/mach-imx/mx6/ddr.c
> +++ b/arch/arm/mach-imx/mx6/ddr.c
> @@ -85,6 +85,23 @@ static void modify_dg_result(u32 *reg_st0, u32 *reg_st1, u32 *reg_ctrl)
>   	writel(val_ctrl, reg_ctrl);
>   }
>   
> +static void correct_mpwldectr_result(void *reg)
> +{
> +	/* Limit is 200/256 of CK, which is WL_HC_DELx | 0x48. */
> +	const unsigned int limit = 0x148;
> +	u32 val = readl(reg);
> +	u32 old = val;
> +

Nit: I think "val &= 0xffff0000" would be slightly easier to read
instead of the "<< 16":

> +	if ((val & 0x17f) > limit)
> +		val &= 0xffff << 16;
> +
> +	if (((val >> 16) & 0x17f) > limit)
> +		val &= 0xffff;
> +
> +	if (old != val)
> +		writel(val, reg);
> +}
> +
>   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;
> @@ -176,6 +193,13 @@ int mmdc_do_write_level_calibration(struct mx6_ddr_sysinfo const *sysinfo)
>   		errors |= 4;
>   	}
>   
> +	correct_mpwldectr_result(&mmdc0->mpwldectrl0);
> +	correct_mpwldectr_result(&mmdc0->mpwldectrl1);
> +	if (sysinfo->dsize == 2) {
> +		correct_mpwldectr_result(&mmdc1->mpwldectrl0);
> +		correct_mpwldectr_result(&mmdc1->mpwldectrl1);
> +	}
> +
>   	/*
>   	 * User should issue MRS command to exit write leveling mode
>   	 * through Load Mode Register command
> 

Otherwise,

Reviewed-by: Eric Nelson <eric at nelint.com>