[U-Boot] [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
- add additional function erratum_a009942_check_cpo to check if the board needs tuning CPO calibration for optimal setting. - move ERRATUM_A009942(with revision to check cpo_sample option) from fsl_ddr_gen4.c to ctrl_regs.c for reuse on all DDR4/DDR3 parts. - move ERRATUM_A008378 from fsl_ddr_gen4.c to ctrl_regs.c - remove obsolete ERRATUM_A004934 which is replaced with ERRATUM_A009942.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@nxp.com --- arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 7 +- arch/powerpc/cpu/mpc85xx/cpu_init.c | 6 +- arch/powerpc/include/asm/config_mpc85xx.h | 2 - board/freescale/ls1021aqds/ls1021aqds.c | 6 +- drivers/ddr/fsl/ctrl_regs.c | 104 +++++++++++++++++++++++++++++- drivers/ddr/fsl/fsl_ddr_gen4.c | 23 ------- drivers/ddr/fsl/mpc85xx_ddr_gen3.c | 3 - include/fsl_ddr.h | 2 + include/fsl_ddr_sdram.h | 3 +- 9 files changed, 121 insertions(+), 35 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index b7a2e0c..19de15e 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -25,6 +25,9 @@ #ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include <asm/armv8/sec_firmware.h> #endif +#ifdef CONFIG_SYS_FSL_DDR +#include <fsl_ddr.h> +#endif
DECLARE_GLOBAL_DATA_PTR;
@@ -400,7 +403,9 @@ int arch_early_init_r(void) #ifdef CONFIG_SYS_FSL_ERRATUM_A009635 erratum_a009635(); #endif - +#if defined(CONFIG_SYS_FSL_ERRATUM_A009942) && defined(CONFIG_SYS_FSL_DDR) + erratum_a009942_check_cpo(); +#endif #ifdef CONFIG_MP #if defined(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) && defined(CONFIG_ARMV8_PSCI) /* Check the psci version to determine if the psci is supported */ diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c index 53b3729..c1dbd9c 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c @@ -45,7 +45,7 @@ #include <nand.h> #include <errno.h> #endif - +#include <fsl_ddr.h> #include "../../../../drivers/block/fsl_sata.h" #ifdef CONFIG_U_QE #include <fsl_qe.h> @@ -947,6 +947,10 @@ int cpu_init_r(void)
#endif /* CONFIG_SYS_FSL_USB_DUAL_PHY_ENABLE */
+#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 + erratum_a009942_check_cpo(); +#endif + #ifdef CONFIG_FMAN_ENET fman_enet_init(); #endif diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h index 6d845e8..1e62a9c 100644 --- a/arch/powerpc/include/asm/config_mpc85xx.h +++ b/arch/powerpc/include/asm/config_mpc85xx.h @@ -681,7 +681,6 @@ #define CONFIG_SYS_FSL_USB_DUAL_PHY_ENABLE #define CONFIG_SYS_FSL_USB_INTERNAL_UTMI_PHY #define CONFIG_SYS_FSL_ERRATUM_A004468 -#define CONFIG_SYS_FSL_ERRATUM_A_004934 #define CONFIG_SYS_FSL_ERRATUM_A005871 #define CONFIG_SYS_FSL_ERRATUM_A006379 #define CONFIG_SYS_FSL_ERRATUM_A007186 @@ -720,7 +719,6 @@ #define CONFIG_SYS_FSL_TBCLK_DIV 16 #define CONFIG_SYS_FSL_PCIE_COMPAT "fsl,qoriq-pcie-v2.4" #define CONFIG_SYS_FSL_USB1_PHY_ENABLE -#define CONFIG_SYS_FSL_ERRATUM_A_004934 #define CONFIG_SYS_FSL_ERRATUM_A005871 #define CONFIG_SYS_FSL_ERRATUM_A006379 #define CONFIG_SYS_FSL_ERRATUM_A007186 diff --git a/board/freescale/ls1021aqds/ls1021aqds.c b/board/freescale/ls1021aqds/ls1021aqds.c index 4eb38a7..79078d2 100644 --- a/board/freescale/ls1021aqds/ls1021aqds.c +++ b/board/freescale/ls1021aqds/ls1021aqds.c @@ -22,7 +22,7 @@ #include <spl.h> #include <fsl_devdis.h> #include <fsl_validate.h> - +#include <fsl_ddr.h> #include "../common/sleep.h" #include "../common/qixis.h" #include "ls1021aqds_qixis.h" @@ -433,7 +433,9 @@ int board_init(void) #ifdef CONFIG_SYS_FSL_ERRATUM_A010315 erratum_a010315(); #endif - +#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 + erratum_a009942_check_cpo(); +#endif major = get_soc_major_rev(); if (major == SOC_MAJOR_VER_1_0) { /* Set CCI-400 control override register to diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c index 24fd366..656de2c 100644 --- a/drivers/ddr/fsl/ctrl_regs.c +++ b/drivers/ddr/fsl/ctrl_regs.c @@ -5,14 +5,14 @@ */
/* - * Generic driver for Freescale DDR/DDR2/DDR3 memory controller. + * Generic driver for Freescale DDR/DDR2/DDR3/DDR4 memory controller. * Based on code from spd_sdram.c * Author: James Yang [at freescale.com] */
#include <common.h> #include <fsl_ddr_sdram.h> - +#include <fsl_errata.h> #include <fsl_ddr.h> #include <fsl_immap.h> #include <asm/io.h> @@ -2305,6 +2305,9 @@ compute_fsl_memctl_config_regs(const unsigned int ctrl_num, unsigned int wrlvl_en; unsigned int ip_rev = 0; unsigned int unq_mrs_en = 0; +#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 + unsigned int ddr_freq; +#endif int cs_en = 1;
memset(ddr, 0, sizeof(fsl_ddr_cfg_regs_t)); @@ -2526,5 +2529,102 @@ compute_fsl_memctl_config_regs(const unsigned int ctrl_num, ddr->debug[2] |= 0x00000200; /* set bit 22 */ #endif
+#if defined(CONFIG_SYS_FSL_ERRATUM_A008378) && defined(CONFIG_SYS_FSL_DDRC_GEN4) + /* Erratum applies when accumulated ECC is used, or DBI is enabled */ +#define IS_ACC_ECC_EN(v) ((v) & 0x4) +#define IS_DBI(v) ((((v) >> 12) & 0x3) == 0x2) + if (has_erratum_a008378()) { + if (IS_ACC_ECC_EN(ddr->ddr_sdram_cfg) || + IS_DBI(ddr->ddr_sdram_cfg_3)) + ddr->debug[28] |= (0x9 << 20); + } +#endif + +#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 + /* the POR value of debug_29 register is zero */ + ddr_freq = get_ddr_freq(ctrl_num) / 1000000; + if (ddr_freq <= 1333) + ddr->debug[28] |= 0x0080006a; + else if (ddr_freq <= 1600) + ddr->debug[28] |= 0x0070006f; + else if (ddr_freq <= 1867) + ddr->debug[28] |= 0x00700076; + else if (ddr_freq <= 2133) + ddr->debug[28] |= 0x0060007b; + if (popts->cpo_sample) + ddr->debug[28] = (ddr->debug[28] & 0xffffff00) | + popts->cpo_sample; +#endif + return check_fsl_memctl_config_regs(ddr); } + +#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 +/* + * This additional workaround of A009942 checks the condition to determine if + * the CPO value set by the existing A009942 workaround needs to be updated. + * If need, print a warning to prompt user reconfigure DDR debug_29[24:31] with + * expected optimal value, the optimal value is highly board dependent. + */ +void erratum_a009942_check_cpo(void) +{ + struct ccsr_ddr __iomem *ddr = + (struct ccsr_ddr __iomem *)(CONFIG_SYS_FSL_DDR_ADDR); + u32 cpo, cpo_e, cpo_o, cpo_target, cpo_optimal; + u32 min_cpo = 0, max_cpo = 0; + u32 sdram_cfg, i, tmp, lanes, ddr_type; + bool update_cpo = false, has_ecc = false; + + sdram_cfg = ddr_in32(&ddr->sdram_cfg); + if (sdram_cfg & SDRAM_CFG_32_BE) + lanes = 4; + else if (sdram_cfg & SDRAM_CFG_16_BE) + lanes = 2; + else + lanes = 8; + + if (sdram_cfg & SDRAM_CFG_ECC_EN) + has_ecc = true; + + /* determine the maximum and minimum CPO values */ + for (i = 9; i < 9 + lanes / 2; i++) { + cpo = ddr_in32(&ddr->debug[i]); + cpo_e = cpo >> 24; + cpo_o = (cpo >> 8) & 0xff; + tmp = min(cpo_e, cpo_o); + if (tmp < min_cpo) + min_cpo = tmp; + tmp = max(cpo_e, cpo_o); + if (tmp > max_cpo) + max_cpo = tmp; + } + + if (has_ecc) { + cpo = ddr_in32(&ddr->debug[13]); + cpo = cpo >> 24; + if (cpo << min_cpo) + min_cpo = cpo; + if (cpo > max_cpo) + max_cpo = cpo; + } + + cpo_target = ddr_in32(&ddr->debug[28]) & 0xff; + cpo_optimal = ((max_cpo + min_cpo) >> 1) + 0x27; + debug("cpo_optimal = 0x%x, cpo_target = 0x%x\n", cpo_optimal, + cpo_target); + debug("max_cpo = 0x%x, min_cpo = 0x%x\n", max_cpo, min_cpo); + + ddr_type = (sdram_cfg & SDRAM_CFG_SDRAM_TYPE_MASK) >> + SDRAM_CFG_SDRAM_TYPE_SHIFT; + if (ddr_type == SDRAM_TYPE_DDR4) + update_cpo = (min_cpo + 0x3b) < cpo_target ? true : false; + else if (ddr_type == SDRAM_TYPE_DDR3) + update_cpo = (min_cpo + 0x3f) < cpo_target ? true : false; + + if (update_cpo) { + printf("WARN: This board needs to optimize debug_29, pls set "); + printf("'popts->cpo_sample = 0x%x' in <board>/ddr.c\n", + cpo_optimal); + } +} +#endif diff --git a/drivers/ddr/fsl/fsl_ddr_gen4.c b/drivers/ddr/fsl/fsl_ddr_gen4.c index 042af09..2a5066a 100644 --- a/drivers/ddr/fsl/fsl_ddr_gen4.c +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c @@ -230,16 +230,6 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, ddr_out32(&ddr->debug[i], regs->debug[i]); } } -#ifdef CONFIG_SYS_FSL_ERRATUM_A008378 - /* Erratum applies when accumulated ECC is used, or DBI is enabled */ -#define IS_ACC_ECC_EN(v) ((v) & 0x4) -#define IS_DBI(v) ((((v) >> 12) & 0x3) == 0x2) - if (has_erratum_a008378()) { - if (IS_ACC_ECC_EN(regs->ddr_sdram_cfg) || - IS_DBI(regs->ddr_sdram_cfg_3)) - ddr_setbits32(&ddr->debug[28], 0x9 << 20); - } -#endif
#ifdef CONFIG_SYS_FSL_ERRATUM_A008511 /* Part 1 of 2 */ @@ -277,19 +267,6 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, ddr_out32(&ddr->debug[25], temp32); #endif
-#ifdef CONFIG_SYS_FSL_ERRATUM_A009942 - ddr_freq = get_ddr_freq(ctrl_num) / 1000000; - tmp = ddr_in32(&ddr->debug[28]); - if (ddr_freq <= 1333) - ddr_out32(&ddr->debug[28], tmp | 0x0080006a); - else if (ddr_freq <= 1600) - ddr_out32(&ddr->debug[28], tmp | 0x0070006f); - else if (ddr_freq <= 1867) - ddr_out32(&ddr->debug[28], tmp | 0x00700076); - else if (ddr_freq <= 2133) - ddr_out32(&ddr->debug[28], tmp | 0x0060007b); -#endif - #ifdef CONFIG_SYS_FSL_ERRATUM_A010165 ddr_freq = get_ddr_freq(ctrl_num) / 1000000; if ((ddr_freq > 1900) && (ddr_freq < 2300)) { diff --git a/drivers/ddr/fsl/mpc85xx_ddr_gen3.c b/drivers/ddr/fsl/mpc85xx_ddr_gen3.c index 653b7f0..1bfb9d4 100644 --- a/drivers/ddr/fsl/mpc85xx_ddr_gen3.c +++ b/drivers/ddr/fsl/mpc85xx_ddr_gen3.c @@ -174,9 +174,6 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, out_be32(&ddr->debug[i], regs->debug[i]); } } -#ifdef CONFIG_SYS_FSL_ERRATUM_A_004934 - out_be32(&ddr->debug[28], 0x30003000); -#endif
#ifdef CONFIG_SYS_FSL_ERRATUM_DDR_A003474 out_be32(&ddr->debug[12], 0x00000015); diff --git a/include/fsl_ddr.h b/include/fsl_ddr.h index 3351acd..0c3be0e 100644 --- a/include/fsl_ddr.h +++ b/include/fsl_ddr.h @@ -138,4 +138,6 @@ int fsl_ddr_get_dimm_params(dimm_params_t *pdimm, void update_spd_address(unsigned int ctrl_num, unsigned int slot, unsigned int *addr); + +void erratum_a009942_check_cpo(void); #endif diff --git a/include/fsl_ddr_sdram.h b/include/fsl_ddr_sdram.h index 36bd9d7..1404c57 100644 --- a/include/fsl_ddr_sdram.h +++ b/include/fsl_ddr_sdram.h @@ -374,7 +374,8 @@ typedef struct memctl_options_s { unsigned int additive_latency_override_value;
unsigned int clk_adjust; /* */ - unsigned int cpo_override; + unsigned int cpo_override; /* override timing_cfg_2[CPO]*/ + unsigned int cpo_sample; /* optimize debug_29[24:31] */ unsigned int write_data_delay; /* DQS adjust */
unsigned int cswl_override;
Enable ERRATUM_A009942 workaround for B-series and T-series platforms.
Signed-off-by: Shengzhou Liu Shengzhou.Liu@nxp.com --- arch/powerpc/include/asm/config_mpc85xx.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h index 1e62a9c..95183da 100644 --- a/arch/powerpc/include/asm/config_mpc85xx.h +++ b/arch/powerpc/include/asm/config_mpc85xx.h @@ -686,6 +686,7 @@ #define CONFIG_SYS_FSL_ERRATUM_A007186 #define CONFIG_SYS_FSL_ERRATUM_A006593 #define CONFIG_SYS_FSL_ERRATUM_A007798 +#define CONFIG_SYS_FSL_ERRATUM_A009942 #define CONFIG_SYS_CCSRBAR_DEFAULT 0xfe000000 #define CONFIG_SYS_FSL_SFP_VER_3_0 #define CONFIG_SYS_FSL_PCI_VER_3_X @@ -728,6 +729,7 @@ #define CONFIG_SYS_FSL_ERRATUM_A006384 #define CONFIG_SYS_FSL_ERRATUM_A007212 #define CONFIG_SYS_FSL_ERRATUM_A004477 +#define CONFIG_SYS_FSL_ERRATUM_A009942 #define CONFIG_SYS_CCSRBAR_DEFAULT 0xfe000000 #define CONFIG_SYS_FSL_SFP_VER_3_0
@@ -807,6 +809,7 @@ defined(CONFIG_PPC_T1020) || defined(CONFIG_PPC_T1022) #define CONFIG_SYS_FSL_SFP_VER_3_0 #define CONFIG_SYS_FSL_ERRATUM_A008378 #define CONFIG_SYS_FSL_ERRATUM_A009663 +#define CONFIG_SYS_FSL_ERRATUM_A009942
#elif defined(CONFIG_PPC_T1024) || defined(CONFIG_PPC_T1023) ||\ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013) @@ -856,6 +859,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013) #define CONFIG_SYS_FSL_SFP_VER_3_0 #define CONFIG_SYS_FSL_ERRATUM_A008378 #define CONFIG_SYS_FSL_ERRATUM_A009663 +#define CONFIG_SYS_FSL_ERRATUM_A009942
#elif defined(CONFIG_PPC_T2080) || defined(CONFIG_PPC_T2081) #define CONFIG_E6500 @@ -908,6 +912,7 @@ defined(CONFIG_PPC_T1014) || defined(CONFIG_PPC_T1013) #define CONFIG_SYS_FSL_ERRATUM_A006593 #define CONFIG_SYS_FSL_ERRATUM_A007186 #define CONFIG_SYS_FSL_ERRATUM_A006379 +#define CONFIG_SYS_FSL_ERRATUM_A009942 #define ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE #define CONFIG_SYS_FSL_SFP_VER_3_0
Fix following warning in case multiple erratum macro was not defined. warning: unused variable 'tmp' warning: unused variable 'ddr_freq'
Signed-off-by: Shengzhou Liu Shengzhou.Liu@nxp.com --- drivers/ddr/fsl/fsl_ddr_gen4.c | 43 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/ddr/fsl/fsl_ddr_gen4.c b/drivers/ddr/fsl/fsl_ddr_gen4.c index 2a5066a..03ab844 100644 --- a/drivers/ddr/fsl/fsl_ddr_gen4.c +++ b/drivers/ddr/fsl/fsl_ddr_gen4.c @@ -47,13 +47,9 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, { unsigned int i, bus_width; struct ccsr_ddr __iomem *ddr; - u32 temp_sdram_cfg; + u32 temp32; u32 total_gb_size_per_controller; int timeout; -#if defined(CONFIG_SYS_FSL_ERRATUM_A008511) || \ - defined(CONFIG_SYS_FSL_ERRATUM_A009801) - u32 temp32; -#endif
#ifdef CONFIG_SYS_FSL_ERRATUM_A008511 u32 mr6; @@ -61,11 +57,6 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, u32 vref_seq2[3] = {0xc0, 0xf0, 0x70}; /* for range 2 */ u32 *vref_seq = vref_seq1; #endif -#if defined(CONFIG_SYS_FSL_ERRATUM_A009942) | \ - defined(CONFIG_SYS_FSL_ERRATUM_A010165) - ulong ddr_freq; - u32 tmp; -#endif #ifdef CONFIG_FSL_DDR_BIST u32 mtcr, err_detect, err_sbe; u32 cs0_bnds, cs1_bnds, cs2_bnds, cs3_bnds, cs0_config; @@ -268,10 +259,10 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs, #endif
#ifdef CONFIG_SYS_FSL_ERRATUM_A010165 - ddr_freq = get_ddr_freq(ctrl_num) / 1000000; - if ((ddr_freq > 1900) && (ddr_freq < 2300)) { - tmp = ddr_in32(&ddr->debug[28]); - ddr_out32(&ddr->debug[28], tmp | 0x000a0000); + temp32 = get_ddr_freq(ctrl_num) / 1000000; + if ((temp32 > 1900) && (temp32 < 2300)) { + temp32 = ddr_in32(&ddr->debug[28]); + ddr_out32(&ddr->debug[28], temp32 | 0x000a0000); } #endif /* @@ -289,9 +280,9 @@ void fsl_ddr_set_memctl_regs(const fsl_ddr_cfg_regs_t *regs,
step2: /* Set, but do not enable the memory */ - temp_sdram_cfg = regs->ddr_sdram_cfg; - temp_sdram_cfg &= ~(SDRAM_CFG_MEM_EN); - ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg); + temp32 = regs->ddr_sdram_cfg; + temp32 &= ~(SDRAM_CFG_MEM_EN); + ddr_out32(&ddr->sdram_cfg, temp32);
/* * 500 painful micro-seconds must elapse between @@ -306,18 +297,18 @@ step2: #ifdef CONFIG_DEEP_SLEEP if (is_warm_boot()) { /* enter self-refresh */ - temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg_2); - temp_sdram_cfg |= SDRAM_CFG2_FRC_SR; - ddr_out32(&ddr->sdram_cfg_2, temp_sdram_cfg); + temp32 = ddr_in32(&ddr->sdram_cfg_2); + temp32 |= SDRAM_CFG2_FRC_SR; + ddr_out32(&ddr->sdram_cfg_2, temp32); /* do board specific memory setup */ board_mem_sleep_setup();
- temp_sdram_cfg = (ddr_in32(&ddr->sdram_cfg) | SDRAM_CFG_BI); + temp32 = (ddr_in32(&ddr->sdram_cfg) | SDRAM_CFG_BI); } else #endif - temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI; + temp32 = ddr_in32(&ddr->sdram_cfg) & ~SDRAM_CFG_BI; /* Let the controller go */ - ddr_out32(&ddr->sdram_cfg, temp_sdram_cfg | SDRAM_CFG_MEM_EN); + ddr_out32(&ddr->sdram_cfg, temp32 | SDRAM_CFG_MEM_EN); mb(); isb();
@@ -460,9 +451,9 @@ step2: #ifdef CONFIG_DEEP_SLEEP if (is_warm_boot()) { /* exit self-refresh */ - temp_sdram_cfg = ddr_in32(&ddr->sdram_cfg_2); - temp_sdram_cfg &= ~SDRAM_CFG2_FRC_SR; - ddr_out32(&ddr->sdram_cfg_2, temp_sdram_cfg); + temp32 = ddr_in32(&ddr->sdram_cfg_2); + temp32 &= ~SDRAM_CFG2_FRC_SR; + ddr_out32(&ddr->sdram_cfg_2, temp32); } #endif
On 11/04/2016 04:18 AM, Shengzhou Liu wrote:
- add additional function erratum_a009942_check_cpo to check if the board needs tuning CPO calibration for optimal setting.
- move ERRATUM_A009942(with revision to check cpo_sample option) from fsl_ddr_gen4.c to ctrl_regs.c for reuse on all DDR4/DDR3 parts.
- move ERRATUM_A008378 from fsl_ddr_gen4.c to ctrl_regs.c
- remove obsolete ERRATUM_A004934 which is replaced with ERRATUM_A009942.
Shengzhou,
There is an issue for moving the erratum 9942 workaround to ctrl_regs.c. This workaround requires setting debug register in a read-modify-write fashion. You won't be able to read the debug register in ctrl_regs.c file.
York
-----Original Message----- From: york sun Sent: Friday, November 04, 2016 11:20 PM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
On 11/04/2016 04:18 AM, Shengzhou Liu wrote:
- add additional function erratum_a009942_check_cpo to check if the board needs tuning CPO calibration for optimal setting.
- move ERRATUM_A009942(with revision to check cpo_sample option)
from
fsl_ddr_gen4.c to ctrl_regs.c for reuse on all DDR4/DDR3 parts.
- move ERRATUM_A008378 from fsl_ddr_gen4.c to ctrl_regs.c
- remove obsolete ERRATUM_A004934 which is replaced with
ERRATUM_A009942.
Shengzhou,
There is an issue for moving the erratum 9942 workaround to ctrl_regs.c. This workaround requires setting debug register in a read-modify-write fashion. You won't be able to read the debug register in ctrl_regs.c file.
York
York,
This change(moving to ctrl_regs.c) has the same effect as read-modify-write(done in fsl_ddr_gen4.c) before MEM_EN is enabled for DDRC. As I commented in code with "the POR value of debug_29 register is zero" for A009942 workaround when moving it to ctrl_regs.c, Actually only A008378 changes debug_29[8:11] bits to 9 from original POR value 0 before the implementing of A009942, and A009942 overrides debug_29[8:11] set by A008378. So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
-Shengzhou
On 11/06/2016 06:42 PM, Shengzhou Liu wrote:
-----Original Message----- From: york sun Sent: Friday, November 04, 2016 11:20 PM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
On 11/04/2016 04:18 AM, Shengzhou Liu wrote:
- add additional function erratum_a009942_check_cpo to check if the board needs tuning CPO calibration for optimal setting.
- move ERRATUM_A009942(with revision to check cpo_sample option)
from
fsl_ddr_gen4.c to ctrl_regs.c for reuse on all DDR4/DDR3 parts.
- move ERRATUM_A008378 from fsl_ddr_gen4.c to ctrl_regs.c
- remove obsolete ERRATUM_A004934 which is replaced with
ERRATUM_A009942.
Shengzhou,
There is an issue for moving the erratum 9942 workaround to ctrl_regs.c. This workaround requires setting debug register in a read-modify-write fashion. You won't be able to read the debug register in ctrl_regs.c file.
York
York,
This change(moving to ctrl_regs.c) has the same effect as read-modify-write(done in fsl_ddr_gen4.c) before MEM_EN is enabled for DDRC. As I commented in code with "the POR value of debug_29 register is zero" for A009942 workaround when moving it to ctrl_regs.c, Actually only A008378 changes debug_29[8:11] bits to 9 from original POR value 0 before the implementing of A009942, and A009942 overrides debug_29[8:11] set by A008378. So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
Shengzhou,
The presumption of POR value is not safe. It is safe for this case for now. You may have other erratum workaround popping up later using the same register, or other registers. PBI can also change registers. This sets an example to preset those registers out the scope of hardware interaction, regardless which controller is in use, or its state.
York
-----Original Message----- From: york sun Sent: Tuesday, November 08, 2016 1:04 AM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
York,
This change(moving to ctrl_regs.c) has the same effect as read-modify- write(done in fsl_ddr_gen4.c) before MEM_EN is enabled for DDRC. As I commented in code with "the POR value of debug_29 register is zero" for A009942 workaround when moving it to ctrl_regs.c, Actually only A008378 changes debug_29[8:11] bits to 9 from original POR value 0 before the implementing of A009942, and A009942 overrides debug_29[8:11] set by A008378. So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
Shengzhou,
The presumption of POR value is not safe. It is safe for this case for now. You may have other erratum workaround popping up later using the same register, or other registers. PBI can also change registers. This sets an example to preset those registers out the scope of hardware interaction, regardless which controller is in use, or its state.
York This change(move to common ctrl_regs.c for reuse on DDR4/DDR3) is only for A009942, For other erratum workaround popping up later using the same register, or other registers, we still can implement them in fsl_ddr_gen4.c or in other according to actual specific sequence requirement. I will add reading value of debug_29 register for A009942 in ctrl_regs.c in next version, which will be safe regardless how POR changes. -Shengzhou
On 11/08/2016 02:39 AM, Shengzhou Liu wrote:
-----Original Message----- From: york sun Sent: Tuesday, November 08, 2016 1:04 AM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
York,
This change(moving to ctrl_regs.c) has the same effect as read-modify- write(done in fsl_ddr_gen4.c) before MEM_EN is enabled for DDRC. As I commented in code with "the POR value of debug_29 register is zero" for A009942 workaround when moving it to ctrl_regs.c, Actually only A008378 changes debug_29[8:11] bits to 9 from original POR value 0 before the implementing of A009942, and A009942 overrides debug_29[8:11] set by A008378. So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
Shengzhou,
The presumption of POR value is not safe. It is safe for this case for now. You may have other erratum workaround popping up later using the same register, or other registers. PBI can also change registers. This sets an example to preset those registers out the scope of hardware interaction, regardless which controller is in use, or its state.
York This change(move to common ctrl_regs.c for reuse on DDR4/DDR3) is only for A009942, For other erratum workaround popping up later using the same register, or other registers, we still can implement them in fsl_ddr_gen4.c or in other according to actual specific sequence requirement. I will add reading value of debug_29 register for A009942 in ctrl_regs.c in next version, which will be safe regardless how POR changes.
You added A009942 and A008378. I don't think this is the right way. You break the isolation between calculating the register values and hardware interface. If you follow this path, you will add more and more hardware interaction into ctrl_regs.c file. Until your change you don't have to worry about any hardware condition in this file.
York
-----Original Message----- From: york sun Sent: Wednesday, November 09, 2016 1:04 AM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
On 11/08/2016 02:39 AM, Shengzhou Liu wrote:
-----Original Message----- From: york sun Sent: Tuesday, November 08, 2016 1:04 AM To: Shengzhou Liu shengzhou.liu@nxp.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] fsl/ddr: Revise erratum a009942 and clean related erratum
York,
This change(moving to ctrl_regs.c) has the same effect as read-modify- write(done in fsl_ddr_gen4.c) before MEM_EN is enabled
for DDRC.
As I commented in code with "the POR value of debug_29 register is zero" for A009942 workaround when moving it to ctrl_regs.c, Actually only A008378 changes debug_29[8:11] bits to 9 from original POR value 0 before the implementing of A009942, and A009942 overrides
debug_29[8:11] set by A008378.
So we can set debug_29 in ctrl_regs.c, it doesn't break anything.
Shengzhou,
The presumption of POR value is not safe. It is safe for this case for now. You may have other erratum workaround popping up later using the
same
register, or other registers. PBI can also change registers. This sets an example to preset those registers out the scope of hardware interaction, regardless which controller is in use, or its state.
York This change(move to common ctrl_regs.c for reuse on DDR4/DDR3) is
only
for A009942, For other erratum workaround popping up later using the same register, or other registers, we still can implement them in
fsl_ddr_gen4.c or in other according to actual specific sequence requirement.
I will add reading value of debug_29 register for A009942 in ctrl_regs.c in
next version, which will be safe regardless how POR changes.
You added A009942 and A008378. I don't think this is the right way. You break the isolation between calculating the register values and hardware interface. If you follow this path, you will add more and more hardware interaction into ctrl_regs.c file. Until your change you don't have to worry about any hardware condition in this file.
If we keep A009942 workaround in fsl_ddr_gen4.c, 1) we have to duplicate 3 same implement of A009942 separately in mpc85xx_ddr_gen3.c, arm_ddr_gen3.c and fsl_ddr_gen4.c, that is not a good idea. 2) we have to modify more code struct to introduce memctl_options_t *popts pointer in mpc85xx_ddr_gen3.c, arm_ddr_gen3.c and fsl_ddr_gen4.c to configure new option popts->cpo_sample. You had added ERRATUM_A004508 in ctrl_regs.c instead of in hardware interface, so as a special A009942, we can do it as well. Actually I had carefully thought of what you mentioned concerns before I decided to move A009942 and A008378(only for debug[28]) to common ctrl_regs.c. for future erratum and other registers except debug[28], we still keep them in files of hardware interface.
-Shengzhou
On 11/08/2016 10:04 PM, Shengzhou Liu wrote:
If we keep A009942 workaround in fsl_ddr_gen4.c,
- we have to duplicate 3 same implement of A009942 separately in mpc85xx_ddr_gen3.c, arm_ddr_gen3.c and fsl_ddr_gen4.c, that is not a good idea.
- we have to modify more code struct to introduce memctl_options_t *popts pointer in mpc85xx_ddr_gen3.c, arm_ddr_gen3.c and fsl_ddr_gen4.c to configure new option popts->cpo_sample.
You had added ERRATUM_A004508 in ctrl_regs.c instead of in hardware interface, so as a special A009942, we can do it as well. Actually I had carefully thought of what you mentioned concerns before I decided to move A009942 and A008378(only for debug[28]) to common ctrl_regs.c. for future erratum and other registers except debug[28], we still keep them in files of hardware interface.
OK. Thanks for the explanation.
York
participants (3)
-
Shengzhou Liu -
Shengzhou Liu
-
york sun