[U-Boot] FSL DDR3/4 wrlvl_override question

[York Sun <yorksun@freescale.com>]

Jocke,

On 09/03/2015 09:55 AM, Joakim Tjernlund wrote:
> in drivers/ddr/fsl/options.c we have:
> 
> #if defined(CONFIG_SYS_FSL_DDR3) || defined(CONFIG_SYS_FSL_DDR4)
> 	/*
> 	 * due to ddr3 dimm is fly-by topology
> 	 * we suggest to enable write leveling to
> 	 * meet the tQDSS under different loading.
> 	 */
> 	popts->wrlvl_en = 1;
> 	popts->zq_en = 1;
> 	popts->wrlvl_override = 0;
> #endif
> 
> Here one disable wrlvl_override no matter what board code wants.
> However board code still sets wrlvl_override as it needs specify
> a good start value for WRLVL_START DQS[0], just like one do for
> the other DQS[X] in WRLVL_CNTL_2/3
> 
> How about remove the above popts->wrlvl_override = 0; line?

You don't need to. This is the default to start with. See the call at the end of
this function, fsl_ddr_board_options()? It calls for board function to overwrite
the default.

> 
> 
> 
> Furthermore drivers/ddr/fsl/ctrl_regs.c I think set_ddr_wrlvl_cntl()
> should be adjusted to(or similar):
> 
> --- a/drivers/ddr/fsl/ctrl_regs.c
> +++ b/drivers/ddr/fsl/ctrl_regs.c
> @@ -2098,7 +2098,7 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>         /* WRLVL_WLR: Write leveling repeition time */
>         unsigned int wrlvl_wlr = 0;
>         /* WRLVL_START: Write leveling start time */
> -       unsigned int wrlvl_start = 0;
> +       unsigned int wrlvl_start = 8;
>  
>         /* suggest enable write leveling for DDR3 due to fly-by topology */
>         if (wrlvl_en) {
> @@ -2131,10 +2131,11 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>                  * Override the write leveling sample and start time
>                  * according to specific board
>                  */
> -               if (popts->wrlvl_override) {
> -                       wrlvl_smpl = popts->wrlvl_sample;
> -                       wrlvl_start = popts->wrlvl_start;
> -               }
> +       }
> +
> +       if (popts->wrlvl_override) {
> +               wrlvl_smpl = popts->wrlvl_sample;
> +               wrlvl_start = popts->wrlvl_start;
>         }
> 
> 
> so that board not using auto WRLVL a chance to specify these parameters.
> 
> I am no expert on DDR so I might be totally off here ...
> 

NACK. Your change is wrong. wrlvl_smpl and wrlvl_start only have meaning if
wrlvl_en is set. You can make DDR work without write leveling by setting a lot
of timing registers. It is not easy and requires extensive experience and
exercise. It is not recommended for most users. If you have to use that way,
please work with an FAE to get all the correct settings.

On the other side, wrlvl_smpl and wrlvl_start are actually mostly overwritten by
board file. You can take any Freescale board as reference, a random example,
board/freescale/t102xrdb/ddr.c.

York