[U-Boot] [PATCH 0/5] arm: omap: dra7: Fix ECC test and memory priming

This series fixes ecc address calculation and updates the ddr ecc enabling sequence .
Krunal Bhargav (3): arm: omap: emif-common: Disable interleaving arm: omap: emif-common: Fix memory priming for ECC cmd: ti: ddr3: Move the print statement after test
Lokesh Vutla (2): arm: omap: emif-common: Fix ecc address calculation cmd: ti: ddr3: Fix ecc address calculation
arch/arm/mach-omap2/emif-common.c | 53 +++++++++++++++++++------------ cmd/ti/ddr3.c | 17 +++++----- 2 files changed, 41 insertions(+), 29 deletions(-)

ecc_address_range registers contains the start address and end address of the DDR address space. But the ddr driver is assuming the register contains the start address and size of the DDR address space. Because of this the ecc enabling is failing for the 2nd range of ecc addresses. Fix this calculation.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- arch/arm/mach-omap2/emif-common.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/emif-common.c b/arch/arm/mach-omap2/emif-common.c index b384343a3f..04bbfd84a2 100644 --- a/arch/arm/mach-omap2/emif-common.c +++ b/arch/arm/mach-omap2/emif-common.c @@ -348,7 +348,7 @@ static void dra7_reset_ddr_data(u32 base, u32 size) static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) { struct emif_reg_struct *emif = (struct emif_reg_struct *)base; - u32 rgn, size; + u32 rgn, rgn_start, size;
/* ECC available only on dra76x EMIF1 */ if ((base != EMIF1_BASE) || !is_dra76x()) @@ -362,22 +362,22 @@ static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) writel(regs->emif_ecc_ctrl_reg, &emif->emif_ecc_ctrl_reg);
/* Set region1 memory with 0 */ - rgn = ((regs->emif_ecc_address_range_1 & - EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16) + - CONFIG_SYS_SDRAM_BASE; + rgn_start = (regs->emif_ecc_address_range_1 & + EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16; + rgn = rgn_start + CONFIG_SYS_SDRAM_BASE; size = (regs->emif_ecc_address_range_1 & - EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0x10000; + EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0x10000 - rgn_start;
if (regs->emif_ecc_ctrl_reg & EMIF_ECC_REG_ECC_ADDR_RGN_1_EN_MASK) dra7_reset_ddr_data(rgn, size);
/* Set region2 memory with 0 */ - rgn = ((regs->emif_ecc_address_range_2 & - EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16) + - CONFIG_SYS_SDRAM_BASE; + rgn_start = (regs->emif_ecc_address_range_2 & + EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16; + rgn = rgn_start + CONFIG_SYS_SDRAM_BASE; size = (regs->emif_ecc_address_range_2 & - EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0x10000; + EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0x10000 - rgn_start;
if (regs->emif_ecc_ctrl_reg & EMIF_ECC_REG_ECC_ADDR_RGN_2_EN_MASK)

On Mon, Sep 16, 2019 at 01:47:15PM +0530, Lokesh Vutla wrote:
ecc_address_range registers contains the start address and end address of the DDR address space. But the ddr driver is assuming the register contains the start address and size of the DDR address space. Because of this the ecc enabling is failing for the 2nd range of ecc addresses. Fix this calculation.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Applied to u-boot/master, thanks!

ecc_address_range registers contains the start address and end address of the DDR address space. But the ddr cmd driver is assuming the register contains the start address and size of the DDR address space. Because of this some valid ecc addresses are errored out as invalid address. Fix this calculation.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- cmd/ti/ddr3.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/cmd/ti/ddr3.c b/cmd/ti/ddr3.c index b82cbe152d..34f870ab43 100644 --- a/cmd/ti/ddr3.c +++ b/cmd/ti/ddr3.c @@ -242,8 +242,8 @@ static int is_addr_valid(u32 addr) if (ecc_ctrl & EMIF_ECC_REG_ECC_ADDR_RGN_1_EN_MASK) { start_addr = ((range & EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16) + CONFIG_SYS_SDRAM_BASE; - end_addr = start_addr + (range & EMIF_ECC_REG_ECC_END_ADDR_MASK) - + 0xFFFF; + end_addr = (range & EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0xFFFF + + CONFIG_SYS_SDRAM_BASE; if ((addr >= start_addr) && (addr <= end_addr)) /* addr within ecc address range 1 */ return 1; @@ -254,8 +254,8 @@ static int is_addr_valid(u32 addr) range = readl(&emif->emif_ecc_address_range_2); start_addr = ((range & EMIF_ECC_REG_ECC_START_ADDR_MASK) << 16) + CONFIG_SYS_SDRAM_BASE; - end_addr = start_addr + (range & EMIF_ECC_REG_ECC_END_ADDR_MASK) - + 0xFFFF; + end_addr = (range & EMIF_ECC_REG_ECC_END_ADDR_MASK) + 0xFFFF + + CONFIG_SYS_SDRAM_BASE; if ((addr >= start_addr) && (addr <= end_addr)) /* addr within ecc address range 2 */ return 1;

On Mon, Sep 16, 2019 at 01:47:16PM +0530, Lokesh Vutla wrote:
ecc_address_range registers contains the start address and end address of the DDR address space. But the ddr cmd driver is assuming the register contains the start address and size of the DDR address space. Because of this some valid ecc addresses are errored out as invalid address. Fix this calculation.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Applied to u-boot/master, thanks!

From: Krunal Bhargav k-bhargav@ti.com
If ECC is enabled, we need to ensure interleaving is disabled for higher address space.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- arch/arm/mach-omap2/emif-common.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-omap2/emif-common.c b/arch/arm/mach-omap2/emif-common.c index 04bbfd84a2..9bdaa388c9 100644 --- a/arch/arm/mach-omap2/emif-common.c +++ b/arch/arm/mach-omap2/emif-common.c @@ -355,6 +355,9 @@ static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) return;
if (regs->emif_ecc_ctrl_reg & EMIF_ECC_CTRL_REG_ECC_EN_MASK) { + /* Disable high-order interleaving */ + clrbits_le32(MA_PRIORITY, MA_HIMEM_INTERLEAVE_UN_MASK); + writel(regs->emif_ecc_address_range_1, &emif->emif_ecc_address_range_1); writel(regs->emif_ecc_address_range_2,

On Mon, Sep 16, 2019 at 01:47:17PM +0530, Lokesh Vutla wrote:
From: Krunal Bhargav k-bhargav@ti.com
If ECC is enabled, we need to ensure interleaving is disabled for higher address space.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Applied to u-boot/master, thanks!

From: Krunal Bhargav k-bhargav@ti.com
Before the priming begins, we need to disable RMW (Read Modify Write) and disable ECC verification for read accesses. By default, the EMIF tool enables RMW and read accesses in the EMIF_ECC_CTRL_REG.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- arch/arm/mach-omap2/emif-common.c | 34 +++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/emif-common.c b/arch/arm/mach-omap2/emif-common.c index 9bdaa388c9..290f9dcdb0 100644 --- a/arch/arm/mach-omap2/emif-common.c +++ b/arch/arm/mach-omap2/emif-common.c @@ -348,7 +348,7 @@ static void dra7_reset_ddr_data(u32 base, u32 size) static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) { struct emif_reg_struct *emif = (struct emif_reg_struct *)base; - u32 rgn, rgn_start, size; + u32 rgn, rgn_start, size, ctrl_reg;
/* ECC available only on dra76x EMIF1 */ if ((base != EMIF1_BASE) || !is_dra76x()) @@ -358,11 +358,28 @@ static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) /* Disable high-order interleaving */ clrbits_le32(MA_PRIORITY, MA_HIMEM_INTERLEAVE_UN_MASK);
+#ifdef CONFIG_DRA7XX + /* Clear the status flags and other history */ + writel(readl(&emif->emif_1b_ecc_err_cnt), + &emif->emif_1b_ecc_err_cnt); + writel(0xffffffff, &emif->emif_1b_ecc_err_dist_1); + writel(0x2, &emif->emif_1b_ecc_err_addr_log); + writel(0x1, &emif->emif_2b_ecc_err_addr_log); + writel(EMIF_INT_WR_ECC_ERR_SYS_MASK | + EMIF_INT_TWOBIT_ECC_ERR_SYS_MASK | + EMIF_INT_ONEBIT_ECC_ERR_SYS_MASK, + &emif->emif_irqstatus_sys); +#endif writel(regs->emif_ecc_address_range_1, &emif->emif_ecc_address_range_1); writel(regs->emif_ecc_address_range_2, &emif->emif_ecc_address_range_2); - writel(regs->emif_ecc_ctrl_reg, &emif->emif_ecc_ctrl_reg); + + /* Disable RMW and ECC verification for read accesses */ + ctrl_reg = (regs->emif_ecc_ctrl_reg & + ~EMIF_ECC_REG_RMW_EN_MASK) | + EMIF_ECC_CTRL_REG_ECC_VERIFY_DIS_MASK; + writel(ctrl_reg, &emif->emif_ecc_ctrl_reg);
/* Set region1 memory with 0 */ rgn_start = (regs->emif_ecc_address_range_1 & @@ -386,17 +403,8 @@ static void dra7_enable_ecc(u32 base, const struct emif_regs *regs) EMIF_ECC_REG_ECC_ADDR_RGN_2_EN_MASK) dra7_reset_ddr_data(rgn, size);
-#ifdef CONFIG_DRA7XX - /* Clear the status flags and other history */ - writel(readl(&emif->emif_1b_ecc_err_cnt), - &emif->emif_1b_ecc_err_cnt); - writel(0xffffffff, &emif->emif_1b_ecc_err_dist_1); - writel(0x1, &emif->emif_2b_ecc_err_addr_log); - writel(EMIF_INT_WR_ECC_ERR_SYS_MASK | - EMIF_INT_TWOBIT_ECC_ERR_SYS_MASK | - EMIF_INT_ONEBIT_ECC_ERR_SYS_MASK, - &emif->emif_irqstatus_sys); -#endif + /* Default value enables RMW and ECC verification */ + writel(regs->emif_ecc_ctrl_reg, &emif->emif_ecc_ctrl_reg); } }

On Mon, Sep 16, 2019 at 01:47:18PM +0530, Lokesh Vutla wrote:
From: Krunal Bhargav k-bhargav@ti.com
Before the priming begins, we need to disable RMW (Read Modify Write) and disable ECC verification for read accesses. By default, the EMIF tool enables RMW and read accesses in the EMIF_ECC_CTRL_REG.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Applied to u-boot/master, thanks!

From: Krunal Bhargav k-bhargav@ti.com
If the ECC is enabled over the entire memory region, we need to ensure the printf/put calls do not modify the stack after ECC is disabled. Moved the printf/put statements after ECC is enabled.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- cmd/ti/ddr3.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/cmd/ti/ddr3.c b/cmd/ti/ddr3.c index 34f870ab43..448a7f54a9 100644 --- a/cmd/ti/ddr3.c +++ b/cmd/ti/ddr3.c @@ -202,10 +202,6 @@ static int ddr_memory_ecc_err(u32 addr, u32 ecc_err) writel(val2, addr);
val3 = readl(addr); - printf("\tECC test: addr 0x%x, read data 0x%x, written data 0x%x, err pattern: 0x%x, read after write data 0x%x\n", - addr, val1, val2, ecc_err, val3); - - puts("\tECC test: Enabling DDR ECC ...\n"); #ifdef CONFIG_ARCH_KEYSTONE ecc_ctrl = ECC_START_ADDR1 | (ECC_END_ADDR1 << 16); writel(ecc_ctrl, EMIF1_BASE + KS2_DDR3_ECC_ADDR_RANGE1_OFFSET); @@ -214,6 +210,11 @@ static int ddr_memory_ecc_err(u32 addr, u32 ecc_err) writel(ecc_ctrl, &emif->emif_ecc_ctrl_reg); #endif
+ printf("\tECC test: addr 0x%x, read data 0x%x, written data 0x%x, err pattern: 0x%x, read after write data 0x%x\n", + addr, val1, val2, ecc_err, val3); + + puts("\tECC test: Enabled DDR ECC ...\n"); + val1 = readl(addr); printf("\tECC test: addr 0x%x, read data 0x%x\n", addr, val1);

On Mon, Sep 16, 2019 at 01:47:19PM +0530, Lokesh Vutla wrote:
From: Krunal Bhargav k-bhargav@ti.com
If the ECC is enabled over the entire memory region, we need to ensure the printf/put calls do not modify the stack after ECC is disabled. Moved the printf/put statements after ECC is enabled.
Signed-off-by: Krunal Bhargav k-bhargav@ti.com Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
Applied to u-boot/master, thanks!
participants (2)
-
Lokesh Vutla
-
Tom Rini