[PATCH] cmd: ti: Add DDRSS ECC test command
Kumar, Udit
u-kumar1 at ti.com
Wed Oct 1 11:04:53 CEST 2025
On 10/1/2025 1:50 PM, Neha Malcom Francis wrote:
> From: Georgi Vlaev <g-vlaev at ti.com>
>
> Introduce a new version of the Keystone-II "ddr" command for testing the
> inline ECC support in the DDRSS bridge available on K3 devices. The ECC
> hardware support in K3's DDRSS and the test method differ substantially
> from what we support in the K2 variant of the command. The name of the
> new command is "ddrss" and it presently supports only single controller
> testing.
what about keeping command name same as of k2 devices,
and file name as cmd/ti/ddr4.c
> The ECC test procedure follows these steps:
> 1) Flush and disable the data cache.
> 2) Disable the protected ECC Rx range.
> 3) Flip a bit in the address.
> 4) Restore the range to original.
> 5) Read the modified value (corrected).
> 6) Re-enable the data cache.
>
> This will cause the 1-bit ECC error count to increase while the read
> will return the corrected value.
code checks for 2-bit ecc as well, Please add more description when
2-bit errors are expected
> The K3 version of the command extends the syntax for the "ecc_err"
> argument by also introducing an argument for range which specifies which
> range (0, 1, 2) the address is located in.
>
> Signed-off-by: Georgi Vlaev <g-vlaev at ti.com>
> Signed-off-by: Santhosh Kumar K <s-k6 at ti.com>
> [n-francis at ti.com: Add J7 and multiple-region support, simplify logic]
> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> ---
> Test logs on J784S4-EVM (after modifying for single controller with ECC enabled)
> https://gist.github.com/nehamalcom/80437234ddd2e22007dec3d1c37dcd6a
>
> cmd/ti/Kconfig | 7 ++
> cmd/ti/Makefile | 1 +
> cmd/ti/ddrss.c | 243 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 251 insertions(+)
> create mode 100644 cmd/ti/ddrss.c
>
> diff --git a/cmd/ti/Kconfig b/cmd/ti/Kconfig
> index 9442c9993c1..0d6c86bad3e 100644
> --- a/cmd/ti/Kconfig
> +++ b/cmd/ti/Kconfig
> @@ -8,6 +8,13 @@ config CMD_DDR3
> supports memory verification, memory comapre and ecc
> verification if supported.
>
> +config CMD_DDRSS
> + bool "command for verifying DDRSS Inline ECC features"
I think you should add depends upon CONFIG_ARCH_K3
> + help
> + Support for testing DDRSS on TI platforms. This command supports
> + memory verification, memory compare and inline ECC verification
> + if supported.
> +
> config CMD_PD
> bool "command for verifying power domains"
> depends on TI_POWER_DOMAIN
> diff --git a/cmd/ti/Makefile b/cmd/ti/Makefile
> index 5f9c64f598a..d0555e7edf6 100644
> --- a/cmd/ti/Makefile
> +++ b/cmd/ti/Makefile
> @@ -2,4 +2,5 @@
> # Copyright (C) 2017 Texas Instruments Incorporated - https://www.ti.com/
>
> obj-$(CONFIG_CMD_DDR3) += ddr3.o
> +obj-$(CONFIG_CMD_DDRSS) += ddrss.o
> obj-$(CONFIG_CMD_PD) += pd.o
> diff --git a/cmd/ti/ddrss.c b/cmd/ti/ddrss.c
> new file mode 100644
> index 00000000000..d1481547938
> --- /dev/null
> +++ b/cmd/ti/ddrss.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * DDRSS: DDR 1-bit inline ECC test command
> + *
> + * Copyright (C) 2025 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include <asm/io.h>
> +#include <asm/cache.h>
> +#include <asm/global_data.h>
> +#include <command.h>
> +#include <cpu_func.h>
> +#include <linux/bitops.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
Do you plan to use gd at later stage ?
> +
> +#define K3_DDRSS_MAX_ECC_REGIONS 3
> +
> +#if (IS_ENABLED(CONFIG_SOC_K3_J784S4) ||\
> + IS_ENABLED(CONFIG_SOC_K3_J721S2) ||\
> + IS_ENABLED(CONFIG_SOC_K3_J7200) ||\
> + IS_ENABLED(CONFIG_SOC_K3_J721E))
> +#define DDRSS_BASE 0x2980000
> +#else
> +#define DDRSS_BASE 0x0f300000
> +#endif
> +
> +#define DDRSS_V2A_CTL_REG 0x0020
> +#define DDRSS_V2A_INT_RAW_REG 0x00a0
> +#define DDRSS_V2A_INT_STAT_REG 0x00a4
> +#define DDRSS_V2A_INT_ECC1BERR BIT(3)
> +#define DDRSS_V2A_INT_ECC2BERR BIT(4)
> +#define DDRSS_V2A_INT_ECCM1BERR BIT(5)
> +#define DDRSS_ECC_CTRL_REG 0x0120
> +#define DDRSS_ECC_CTRL_REG_ECC_EN BIT(0)
> +#define DDRSS_ECC_CTRL_REG_RMW_EN BIT(1)
> +#define DDRSS_ECC_CTRL_REG_ECC_CK BIT(2)
> +#define DDRSS_ECC_CTRL_REG_WR_ALLOC BIT(4)
> +#define DDRSS_ECC_R0_STR_ADDR_REG 0x0130
> +#define DDRSS_ECC_Rx_STR_ADDR_REG(x) (0x0130 + ((x) * 8))
> +#define DDRSS_ECC_Rx_END_ADDR_REG(x) (0x0134 + ((x) * 8))
> +#define DDRSS_ECC_1B_ERR_CNT_REG 0x0150
> +#define DDRSS_ECC_1B_ERR_THRSH_REG 0x0154
> +#define DDRSS_ECC_1B_ERR_ADR_REG 0x0158
> +#define DDRSS_ECC_1B_ERR_MSK_LOG_REG 0x015c
> +
> +static inline u32 ddrss_read(u32 reg)
> +{
> + return readl((unsigned long)(DDRSS_BASE + reg));
> +}
> +
> +static inline void ddrss_write(u32 value, u32 reg)
> +{
> + writel(value, (unsigned long)(DDRSS_BASE + reg));
> +}
> +
> +/* ddrss_check_ecc_status()
> + * Report the ECC state after test. Check/clear the interrupt
> + * status register, dump the ECC err counters and ECC error offset.
> + */
> +static void ddrss_check_ecc_status(void)
> +{
> + u32 ecc_1b_err_cnt, v2a_int_raw, ecc_1b_err_msk;
> + phys_addr_t ecc_1b_err_adr;
> +
> + v2a_int_raw = ddrss_read(DDRSS_V2A_INT_RAW_REG);
> +
> + /* 1-bit correctable */
> + if (v2a_int_raw & DDRSS_V2A_INT_ECC1BERR) {
> + puts("\tECC test: DDR ECC 1-bit error\n");
> +
> + /* Dump the 1-bit counter and reset it, as we want a
> + * new interrupt to be generated when above the error
> + * threshold
its interrupt or poll for status ?
> + */
> + ecc_1b_err_cnt = ddrss_read(DDRSS_ECC_1B_ERR_CNT_REG);
> + if (ecc_1b_err_cnt) {
> + printf("\tECC test: 1-bit ECC err count: %u\n",
> + ecc_1b_err_cnt & 0xffff);
> + ddrss_write(1, DDRSS_ECC_1B_ERR_CNT_REG);
> + }
in case no ecc count then does this mean test failed ?
> +
> + /* ECC fault addresses are also recorded in a 2-word deep
> + * FIFO. Calculate and report the 8-byte range of the error
> + */
> + ecc_1b_err_adr = ddrss_read(DDRSS_ECC_1B_ERR_ADR_REG);
> + ecc_1b_err_msk = ddrss_read(DDRSS_ECC_1B_ERR_MSK_LOG_REG);
> + if (ecc_1b_err_msk) {
> + if ((IS_ENABLED(CONFIG_SOC_K3_AM642)) ||
> + (IS_ENABLED(CONFIG_SOC_K3_AM625))) {
> + /* AM64/AM62:
> + * The address of the ecc error is 16-byte aligned.
> + * Each bit in 4 bit mask represents 8 bytes ECC quanta
> + * that has the 1-bit error
> + */
> + ecc_1b_err_msk &= 0xf;
> + ecc_1b_err_adr <<= 4;
> + ecc_1b_err_adr += (fls(ecc_1b_err_msk) - 1) * 8;
> + } else {
> + /* AM62A/AM62P:
> + * The address of the ecc error is 32-byte aligned.
> + * Each bit in 8 bit mask represents 8 bytes ECC quanta
> + * that has the 1-bit error
> + */
> + ecc_1b_err_msk &= 0xff;
> + ecc_1b_err_adr <<= 5;
> + ecc_1b_err_adr += (fls(ecc_1b_err_msk) - 1) * 8;
> + }
> +
> + printf("\tECC test: 1-bit error in [0x%llx:0x%llx]\n",
> + ecc_1b_err_adr, ecc_1b_err_adr + 8);
> + /* Pop the top of the addr/mask FIFOs */
> + ddrss_write(1, DDRSS_ECC_1B_ERR_ADR_REG);
> + ddrss_write(1, DDRSS_ECC_1B_ERR_MSK_LOG_REG);
> + }
> + ddrss_write(DDRSS_V2A_INT_ECC1BERR, DDRSS_V2A_INT_STAT_REG);
> + }
> +
> + /* 2-bit uncorrectable */
> + if (v2a_int_raw & DDRSS_V2A_INT_ECC2BERR) {
> + puts("\tECC test: DDR ECC 2-bit error\n");
> + ddrss_write(DDRSS_V2A_INT_ECC2BERR, DDRSS_V2A_INT_STAT_REG);
> + }
> + /* multiple 1-bit errors (uncorrectable) in multiple words */
> + if (v2a_int_raw & DDRSS_V2A_INT_ECCM1BERR) {
> + puts("\tECC test: DDR ECC multi 1-bit errors\n");
> + ddrss_write(DDRSS_V2A_INT_ECCM1BERR, DDRSS_V2A_INT_STAT_REG);
> + }
is this error case, if you are hitting uncorrectable errors ?
> +}
> +
> +/* ddrss_memory_ecc_err()
> + * Simulate an ECC error - change a 32b word at address in an ECC enabled
variables used are u64
> + * range. This removes the tested address from the ECC checks, changes a
> + * word, and then restores the ECC range as configured by k3_ddrss in R5 SPL.
> + */
> +static int ddrss_memory_ecc_err(u64 addr, u64 ecc_err, int range)
> +{
> + u64 ecc_start_addr, ecc_end_addr, ecc_temp_addr;
> + u64 val1, val2, val3;
> +
> + /* Flush and disable dcache */
> + flush_dcache_all();
> + dcache_disable();
> +
> + /* Setup a threshold for 1-bit errors to generate interrupt */
> + ddrss_write(1, DDRSS_ECC_1B_ERR_THRSH_REG);
> +
> + puts("Testing DDR ECC:\n");
> + /* Get the Rx range configuration */
> + ecc_start_addr = ddrss_read(DDRSS_ECC_Rx_STR_ADDR_REG(range));
> + ecc_end_addr = ddrss_read(DDRSS_ECC_Rx_END_ADDR_REG(range));
> +
> + /* Calculate the end of the Rx ECC region up to the tested address */
> + ecc_temp_addr = (addr - gd->ram_base) >> 16;
> +
> + puts("\tECC test: Disabling DDR ECC ...\n");
> + /* Disable entire region */
> + ddrss_write(ecc_start_addr, DDRSS_ECC_Rx_END_ADDR_REG(range));
> + ddrss_write(ecc_end_addr, DDRSS_ECC_Rx_STR_ADDR_REG(range));
> +
> + /* Inject error in the address */
> + val1 = readl((unsigned long long)addr);
> + val2 = val1 ^ ecc_err;
> + writel(val2, (unsigned long long)addr);
> + val3 = readl((unsigned long long)addr);
> +
> + /* Re-enable the ECC checks for the R0 region */
> + ddrss_write(ecc_end_addr, DDRSS_ECC_Rx_END_ADDR_REG(range));
> + ddrss_write(ecc_start_addr, DDRSS_ECC_Rx_STR_ADDR_REG(range));
> + /* Make sure the ECC range is restored before doing anything else */
> + mb();
> +
> + printf("\tECC test: addr 0x%llx, read data 0x%llx, written data 0x%llx, err pattern: 0x%llx, read after write data 0x%llx\n",
> + addr, val1, val2, ecc_err, val3);
> +
> + puts("\tECC test: Enabled DDR ECC ...\n");
> + /* Read again from the address. This creates an ECC 1-bit error
> + * condition, and returns the corrected value
> + */
> + val1 = readl((unsigned long long)addr);
> + printf("\tECC test: addr 0x%llx, read data 0x%llx\n", addr, val1);
> +
> + /* Set threshold for 1-bit errors to 0 to disable the interrupt */
> + ddrss_write(0, DDRSS_ECC_1B_ERR_THRSH_REG);
> + /* Report the ECC status */
> + ddrss_check_ecc_status();
> +
> + dcache_enable();
> +
Do we have some cases, where test may fail ?
> + return 0;
> +}
> +
> +/* ddrss_is_ecc_enabled()
> + * Report if ECC is enabled.
> + */
> +static int ddrss_is_ecc_enabled(void)
> +{
> + u32 ecc_ctrl = ddrss_read(DDRSS_ECC_CTRL_REG);
> +
> + /* Assume ECC is enabled only if all bits set by k3_ddrss are set */
> + return (ecc_ctrl & (DDRSS_ECC_CTRL_REG_ECC_EN |
> + DDRSS_ECC_CTRL_REG_RMW_EN |
> + DDRSS_ECC_CTRL_REG_WR_ALLOC |
> + DDRSS_ECC_CTRL_REG_ECC_CK));
> +}
> +
> +static int do_ddrss_test(struct cmd_tbl *cmdtp, int flag, int argc,
> + char *const argv[])
> +{
> + u64 start_addr, ecc_err, x;
x to range, please
> +
> + if (!(argc == 5 && (strncmp(argv[1], "ecc_err", 8) == 0)))
> + return cmd_usage(cmdtp);
> +
> + if (!ddrss_is_ecc_enabled()) {
> + puts("ECC not enabled. Please enable ECC any try again\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + start_addr = simple_strtoul(argv[2], NULL, 16);
> + ecc_err = simple_strtoul(argv[3], NULL, 16);
Do you want to impose some max side on ecc_err ?
as per description, 32 bits are used for
> + x = simple_strtoul(argv[4], NULL, 16);
check for range (wrong value)
> +
> + if (!((start_addr >= gd->bd->bi_dram[0].start &&
> + (start_addr <= (gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size - 1))) ||
> + (start_addr >= gd->bd->bi_dram[1].start &&
> + (start_addr <= (gd->bd->bi_dram[1].start + gd->bd->bi_dram[1].size - 1))))) {
> + puts("Address is not in the DDR range\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + ddrss_memory_ecc_err(start_addr, ecc_err, x);
results of ddrss_memory_ecc_err, should be reported back as CMD_FAIL or
CMD_PASS ,
In case, you are assuming ddrss_memory_ecc_err will always pass . Please
add in comments.
> + return 0;
> +}
> +
> +U_BOOT_CMD(ddrss, 5, 1, do_ddrss_test,
> + "DDRSS test",
> + "ecc_err <addr in hex> <bit_err in hex> <range 0/1/2> - generate bit errors\n"
> + " in DDR data at <addr>, the command will read a 32-bit data\n"
> + " from <addr>, and write (data ^ bit_err) back to <addr>\n"
> + " in range 0, 1, or 2 (if default full region ECC is enabled, choose 0)\n"
> +);
More information about the U-Boot
mailing list