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

[Marek Vasut <marex@denx.de>]

On 4/4/22 10:51, Francesco Dolcini 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 [1] 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>
> ---
>   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..08e2f0f130a6 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",

Reviewed-by: Marek Vasut <marex@denx.de>