Shengzhou,

Subject is inaccurate. Not only UDIMM for DDR4.

On 03/07/2016 10:01 PM, Shengzhou Liu wrote:
> Add support of command/address parity for DDR4 UDIMM or discrete memory.
> It requires to configurate corresponding MR5[2:0] and TIMING_CFG_7[PAR_LAT].
> Parity can be turned on/off by hwconfig, e.g. hwconfig=fsl_ddr:parity=on.
> 
> Signed-off-by: Shengzhou Liu <shengzhou....@nxp.com>
> ---
>  doc/README.fsl-ddr          |  8 +++++++
>  drivers/ddr/fsl/ctrl_regs.c | 52 
> +++++++++++++++++++++++++++++++++++++++------
>  drivers/ddr/fsl/options.c   | 12 +++++++++--
>  include/fsl_ddr_sdram.h     | 11 +++++++++-
>  4 files changed, 73 insertions(+), 10 deletions(-)
> 
> diff --git a/doc/README.fsl-ddr b/doc/README.fsl-ddr
> index cd71ec8..80f91d3 100644
> --- a/doc/README.fsl-ddr
> +++ b/doc/README.fsl-ddr
> @@ -143,6 +143,14 @@ platform
>  
>  
> hwconfig=fsl_ddr:addr_hash=true,ctlr_intlv=cacheline,bank_intlv=cs0_cs1_cs2_cs3,ecc=on
>  
> +
> +Memory address parity on/off
> +============================
> +address parity can be turned on/off by hwconfig, it's disabled by default.
> +Syntax is:
> +hwconfig=fsl_ddr:parity=on
> +
> +
>  Table for dynamic ODT for DDR3
>  ==============================
>  For single-slot system with quad-rank DIMM and dual-slot system, dynamic ODT 
> may
> diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
> index 0bfcd34..1f5b9fc 100644
> --- a/drivers/ddr/fsl/ctrl_regs.c
> +++ b/drivers/ddr/fsl/ctrl_regs.c
> @@ -895,12 +895,10 @@ static void set_ddr_sdram_cfg_2(const unsigned int 
> ctrl_num,
>       slow = get_ddr_freq(ctrl_num) < 1249000000;
>  #endif
>  
> -     if (popts->registered_dimm_en) {
> +     ap_en = popts->ap_en;
> +
> +     if (popts->registered_dimm_en)
>               rcw_en = 1;
> -             ap_en = popts->ap_en;
> -     } else {
> -             ap_en = 0;
> -     }
>  

This change removes the protection. popts->ap_en can be set by board ddr
setting. Without checking popts->registered_dimm_en, this could set ap_en
incorrectly. This was written for DDR3 RDIMM. Since DDR4 allows address parity
for UDIMM, I think it is better to check for DDR3.


>       x4_en = popts->x4_en ? 1 : 0;
>  
> @@ -1135,6 +1133,7 @@ static void set_ddr_sdram_mode_9(fsl_ddr_cfg_regs_t 
> *ddr,
>       unsigned short esdmode5;        /* Extended SDRAM mode 5 */
>       int rtt_park = 0;
>       bool four_cs = false;
> +     const unsigned int mclk_ps = get_memory_clk_period_ps(0);
>  
>  #if CONFIG_CHIP_SELECTS_PER_CTRL == 4
>       if ((ddr->cs[0].config & SDRAM_CS_CONFIG_EN) &&
> @@ -1150,6 +1149,19 @@ static void set_ddr_sdram_mode_9(fsl_ddr_cfg_regs_t 
> *ddr,
>               esdmode5 = 0x00000400;  /* Data mask enabled */
>       }
>  
> +     /* set command/address parity latency */
> +     if (ddr->ddr_sdram_cfg_2 & SDRAM_CFG2_AP_EN) {
> +             if (mclk_ps >= 935) {
> +                     /* for DDR4-1600/1866/2133 */
> +                     esdmode5 |= DDR_MR5_CA_PARITY_LAT_4_CLK;
> +             } else if (mclk_ps >= 833) {
> +                     /* for DDR4-2400 */
> +                     esdmode5 |= DDR_MR5_CA_PARITY_LAT_5_CLK;
> +             } else {
> +                     printf("parity: mclk_ps = %d not supported\n", mclk_ps);
> +             }
> +     }
> +
>       ddr->ddr_sdram_mode_9 = (0
>                                | ((esdmode4 & 0xffff) << 16)
>                                | ((esdmode5 & 0xffff) << 0)
> @@ -1170,6 +1182,19 @@ static void set_ddr_sdram_mode_9(fsl_ddr_cfg_regs_t 
> *ddr,
>                       } else {
>                               esdmode5 = 0x00000400;
>                       }
> +
> +                     if (ddr->ddr_sdram_cfg_2 & SDRAM_CFG2_AP_EN) {
> +                             if (mclk_ps >= 935) {
> +                                     /* for DDR4-1600/1866/2133 */
> +                                     esdmode5 |= DDR_MR5_CA_PARITY_LAT_4_CLK;
> +                             } else if (mclk_ps >= 833) {
> +                                     /* for DDR4-2400 */
> +                                     esdmode5 |= DDR_MR5_CA_PARITY_LAT_5_CLK;
> +                             } else {
> +                                     printf("parity: the speed reserved\n");
> +                             }
> +                     }
> +
>                       switch (i) {
>                       case 1:
>                               ddr->ddr_sdram_mode_11 = (0
> @@ -1925,12 +1950,25 @@ static void set_timing_cfg_7(const unsigned int 
> ctrl_num,
>                            const common_timing_params_t *common_dimm)
>  {
>       unsigned int txpr, tcksre, tcksrx;
> -     unsigned int cke_rst, cksre, cksrx, par_lat, cs_to_cmd;
> +     unsigned int cke_rst, cksre, cksrx, par_lat = 0, cs_to_cmd;
> +     const unsigned int mclk_ps = get_memory_clk_period_ps(ctrl_num);
>  
>       txpr = max(5U, picos_to_mclk(ctrl_num, common_dimm->trfc1_ps + 10000));
>       tcksre = max(5U, picos_to_mclk(ctrl_num, 10000));
>       tcksrx = max(5U, picos_to_mclk(ctrl_num, 10000));
> -     par_lat = 0;
> +
> +     if (ddr->ddr_sdram_cfg_2 & SDRAM_CFG2_AP_EN) {
> +             if (mclk_ps >= 935) {
> +                     /* parity latency 4 clocks in case of 1600/1866/2133 */
> +                     par_lat = 4;
> +             } else if (mclk_ps >= 833) {
> +                     /* parity latency 5 clocks for DDR4-2400 */
> +                     par_lat = 5;
> +             } else {
> +                     printf("parity: mclk_ps = %d not supported\n", mclk_ps);
> +             }
> +     }
> +
>       cs_to_cmd = 0;
>  
>       if (txpr <= 200)
> diff --git a/drivers/ddr/fsl/options.c b/drivers/ddr/fsl/options.c
> index 791d644..9ea80af 100644
> --- a/drivers/ddr/fsl/options.c
> +++ b/drivers/ddr/fsl/options.c
> @@ -1002,8 +1002,16 @@ unsigned int populate_memctl_options(const 
> common_timing_params_t *common_dimm,
>       popts->twot_en = 0;
>       popts->threet_en = 0;
>  
> -     /* for RDIMM, address parity enable */
> -     popts->ap_en = 1;
> +     /* for RDIMM and DDR4 UDIMM/discrete memory, address parity enable */
> +     popts->ap_en = 0; /* 0 = disable,  1 = enable */
> +
> +     if (hwconfig_sub_f("fsl_ddr", "parity", buf)) {
> +             if (hwconfig_subarg_cmp_f("fsl_ddr", "parity", "on", buf)) {
> +                     if (popts->registered_dimm_en ||
> +                         (CONFIG_FSL_SDRAM_TYPE == SDRAM_TYPE_DDR4))
> +                             popts->ap_en = 1;
> +             }
> +     }

I don't remember why I enabled AP for all DDR3 RDIMM. Now you are making it
optional. Please double check this.

York

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to