[PATCH 0/4] ARM: i.MX8: add DDRC-ECC support

[PATCH 0/4] ARM: i.MX8: add DDRC-ECC support

[Steffen Trumtrar]

The i.MX8 DDRC controller supports using inline ECC with the DDR RAM.
Inline ECC reduces the usable RAM size by 1/8: 7/8 RAM is for data and
1/8 RAM is for the ECC bits. Also, measuring random memory writes in
linux with

    stress-ng --memthrash 4 --memthrash-method chunk1 -t 1m --metrics

shows a performance decrease by ~10%.

If a board wants to support ECC, the lpddr4 RAM settings in the
according lpddr4-timing-* must be adapted to enable and configure the
ECC registers.
Also, a board_dram_ecc_scrub() function must be provided, so that the
RAM is initialized on startup.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
David Jander (3):
      arm: mach-imx: esdctl.c: Add support for imx8mp inline ECC
      drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing
      arm: boards: protonic-imx8ml: Add ECC + scrubbing

Steffen Trumtrar (1):
      ARM: i.MX: esdctl: fix spelling of ad(d)ress

 .../boards/protonic-imx8m/lpddr4-timing-prt8ml.c   | 25 ++++++-
 arch/arm/dts/imx8mp-prt8ml.dts                     | 10 ++-
 arch/arm/mach-imx/Kconfig                          |  8 ++
 arch/arm/mach-imx/esdctl.c                         | 86 ++++++++++++++++++++--
 drivers/ddr/imx/imx8m_ddr_init.c                   | 73 ++++++++++++++++++
 include/soc/imx8m/ddr.h                            | 11 +++
 6 files changed, 203 insertions(+), 10 deletions(-)
---
base-commit: f4122cb473bf8ca2d3d84cf7cd3c981d1da3309f
change-id: 20260304-v2026-02-0-topic-imx8-ecc-9206fee1f037

Best regards,
-- 
Steffen Trumtrar <s.trumtrar@pengutronix.de>

[PATCH 1/4] ARM: i.MX: esdctl: fix spelling of ad(d)ress

[Steffen Trumtrar]

address is spelled with two 'd's. Fix the spelling mistake.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/mach-imx/esdctl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index 935c3d3257..b6689b7ffc 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -262,7 +262,7 @@ static int imx_esdctl_v4_add_mem(void *esdctlbase, const struct imx_esdctl_data
 }
 
 /*
- * On i.MX6 the adress space reserved for SDRAM is 0x10000000 to 0xFFFFFFFF
+ * On i.MX6 the address space reserved for SDRAM is 0x10000000 to 0xFFFFFFFF
  * which makes the maximum supported RAM size 0xF0000000.
  */
 #define IMX6_MAX_SDRAM_SIZE 0xF0000000
@@ -380,7 +380,7 @@ static resource_size_t
 imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[DDRC_ADDRMAP_LENGTH],
 		    u8 col_max, const u8 col_b[], unsigned int col_b_num,
 		    u8 row_max, const u8 row_b[], unsigned int row_b_num,
-		    bool reduced_adress_space, unsigned int mstr)
+		    bool reduced_address_space, unsigned int mstr)
 {
 	unsigned int banks, ranks, columns, rows, active_ranks, width;
 	resource_size_t size;
@@ -459,7 +459,7 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[DDRC_ADDRMAP_LENGTH],
 		size = memory_sdram_size(columns, rows, 1 << banks, 1) >> 1;
 	size <<= ranks;
 
-	return reduced_adress_space ? size * 3 / 4 : size;
+	return reduced_address_space ? size * 3 / 4 : size;
 }
 
 static void imx_ddrc_set_mstr_device_config(u32 *mstr, unsigned bits)
@@ -505,7 +505,7 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc, unsigned buswid
 		FIELD_GET(DDRC_ADDRMAP6_ROW_B12, addrmap[6]),
 		FIELD_GET(DDRC_ADDRMAP5_ROW_B11, addrmap[5]),
 	};
-	const bool reduced_adress_space =
+	const bool reduced_address_space =
 		FIELD_GET(DDRC_ADDRMAP6_LPDDR4_6GB_12GB_24GB, addrmap[6]);
 	u32 mstr = readl(ddrc + DDRC_MSTR);
 
@@ -516,7 +516,7 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc, unsigned buswid
 	return imx_ddrc_sdram_size(ddrc, addrmap,
 				   12, ARRAY_AND_SIZE(col_b),
 				   18, ARRAY_AND_SIZE(row_b),
-				   reduced_adress_space, mstr);
+				   reduced_address_space, mstr);
 }
 
 static int _imx8m_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data,
@@ -630,7 +630,7 @@ static resource_size_t imx7d_ddrc_sdram_size(void __iomem *ddrc)
 		FIELD_GET(DDRC_ADDRMAP6_ROW_B12, addrmap[6]),
 		FIELD_GET(DDRC_ADDRMAP5_ROW_B11, addrmap[5]),
 	};
-	const bool reduced_adress_space =
+	const bool reduced_address_space =
 		FIELD_GET(DDRC_ADDRMAP6_LPDDR3_6GB_12GB, addrmap[6]);
 	u32 mstr = readl(ddrc + DDRC_MSTR);
 
@@ -640,7 +640,7 @@ static resource_size_t imx7d_ddrc_sdram_size(void __iomem *ddrc)
 	return imx_ddrc_sdram_size(ddrc, addrmap,
 				   11, ARRAY_AND_SIZE(col_b),
 				   15, ARRAY_AND_SIZE(row_b),
-				   reduced_adress_space, mstr);
+				   reduced_address_space, mstr);
 }
 
 static int imx7d_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data)

-- 
2.52.0

[PATCH 2/4] arm: mach-imx: esdctl.c: Add support for imx8mp inline ECC

[Steffen Trumtrar]

From: David Jander <david@protonic.nl>

This adds support for detecting the use of inline ECC and compute the
correct memory bank(s) in that case.
In case inline ECC is active the memory map is modified as follows:
The total memory size is reduced to 7/8th of the raw memory size.
If a reduced-address-space type RAM is used (0.75, 1.5, 3, 6... GiB), then
the whole address space is split up into 3 equal parts, separated by 1/3rd
of the raw address space, but each 7/8th that size.
The ECC area at the end of each part is not addressable and must be
excluded from the map.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/mach-imx/esdctl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index b6689b7ffc..ada79fb709 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -332,6 +332,9 @@ static int vf610_ddrmc_add_mem(void *mmdcbase, const struct imx_esdctl_data *dat
 #define DDRC_MSTR_ACTIVE_RANKS			GENMASK(27, 24)
 #define DDRC_MSTR_DEVICE_CONFIG		GENMASK(31, 30)
 
+#define DDRC_ECCCFG0				0x0070
+#define DDRC_ECCCFG0_ECC_MODE			GENMASK(2, 0)
+
 #define DDRC_ADDRMAP0_CS_BIT1			GENMASK(12,  8)
 
 #define DDRC_ADDRMAP1_BANK_B2			GENMASK(20, 16)
@@ -519,6 +522,33 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc, unsigned buswid
 				   reduced_address_space, mstr);
 }
 
+static resource_size_t imx8mp_ddrc_sdram_size(void __iomem *ddrc,
+		unsigned int *chunks, resource_size_t *stride)
+{
+	resource_size_t size = imx8m_ddrc_sdram_size(ddrc, 32);
+	const bool reduced_address_space = FIELD_GET(
+		DDRC_ADDRMAP6_LPDDR4_6GB_12GB_24GB, readl(ddrc + DDRC_ADDRMAP(6)));
+
+	/* ECC devides the accessible address space into 1 or 3 contiguous
+	 * regions depending on reduced_address_space. For simplicity, give
+	 * barebox only one contiguous region to use.
+	 * Each region is only 7/8th the raw size due to ECC data.
+	 */
+	if (chunks)
+		*chunks = 1;
+	if (FIELD_GET(DDRC_ECCCFG0_ECC_MODE, readl(ddrc + DDRC_ECCCFG0))) {
+		if (reduced_address_space) {
+			size /= 3;
+			if (chunks)
+				*chunks = 3;
+			if (stride)
+				*stride = size;
+		}
+		size = (size * 7) / 8;
+	}
+	return size;
+}
+
 static int _imx8m_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data,
 			       unsigned int buswidth)
 {
@@ -559,6 +589,29 @@ static int imx8m_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data
 	return _imx8m_ddrc_add_mem(mmdcbase, data, 32);
 }
 
+static int imx8mp_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data)
+{
+	unsigned int chunks;
+	unsigned long base;
+	resource_size_t chunksize = 0, stride = 0;
+	int ret = -ENOMEM;
+	int i;
+	char name[5];
+
+	chunksize = imx8mp_ddrc_sdram_size(mmdcbase, &chunks, &stride);
+
+	base = data->base0;
+	for (i = 0; i < chunks; i++) {
+		snprintf(name, 5, "ram%d", i);
+		ret = arm_add_mem_device(name, base, chunksize);
+		if (ret)
+			break;
+		base += stride;
+	}
+
+	return ret;
+}
+
 static int imx8mn_ddrc_add_mem(void *mmdcbase, const struct imx_esdctl_data *data)
 {
 	return _imx8m_ddrc_add_mem(mmdcbase, data, 16);
@@ -742,6 +795,11 @@ static __maybe_unused const struct imx_esdctl_data imx8mn_data = {
 	.add_mem = imx8mn_ddrc_add_mem,
 };
 
+static __maybe_unused const struct imx_esdctl_data imx8mp_data = {
+	.base0 = MX8M_DDR_CSD1_BASE_ADDR,
+	.add_mem = imx8mp_ddrc_add_mem,
+};
+
 static __maybe_unused const struct imx_esdctl_data imx9_data = {
 	.base0 = MX9_DDR_CSD1_BASE_ADDR,
 	.add_mem = imx9_ddrc_add_mem,
@@ -822,6 +880,12 @@ static __maybe_unused struct of_device_id imx_esdctl_dt_ids[] = {
 	}, {
 		.compatible = "fsl,imx8mn-ddrc",
 		.data = &imx8mn_data
+	}, {
+		.compatible = "fsl,imx8mp-ddrc",
+		.data = &imx8mp_data
+	}, {
+		.compatible = "fsl,imx8mq-ddrc",
+		.data = &imx8mp_data
 	}, {
 		.compatible = "fsl,imx93-ddrc",
 		.data = &imx9_data
@@ -1000,9 +1064,15 @@ void __noreturn vf610_barebox_entry(void *boarddata)
 
 resource_size_t imx8m_barebox_earlymem_size(unsigned buswidth)
 {
+	unsigned int chunks;
+	resource_size_t stride = 0;
 	resource_size_t size;
 
-	size = imx8m_ddrc_sdram_size(IOMEM(MX8M_DDRC_CTL_BASE_ADDR), buswidth);
+	if (IS_ENABLED(CONFIG_IMX8MP_DRAM_ECC))
+		size = imx8mp_ddrc_sdram_size(IOMEM(MX8M_DDRC_CTL_BASE_ADDR), &chunks,
+				&stride);
+	else
+		size = imx8m_ddrc_sdram_size(IOMEM(MX8M_DDRC_CTL_BASE_ADDR), buswidth);
 	/*
 	 * We artificially limit detected memory size to force malloc
 	 * pool placement to be within 4GiB address space, so as to

-- 
2.52.0

[PATCH 3/4] drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing

[Steffen Trumtrar]

From: David Jander <david@protonic.nl>

This code comes from u-boot [1] and was introducesd in commit [2].

A fix from the patch [3] is also included, which doesn't seem to be
added to u-boot, yet.

[1] https://github.com/u-boot/u-boot/
[2] commit f3acb02386f4 ("drivers: ddr: imx8mp: Add inline ECC feature support")
[3] https://patchwork.ozlabs.org/project/uboot/patch/20230123091702.7472-32-peng.fan@oss.nxp.com/

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 arch/arm/mach-imx/Kconfig        |  8 +++++
 drivers/ddr/imx/imx8m_ddr_init.c | 73 ++++++++++++++++++++++++++++++++++++++++
 include/soc/imx8m/ddr.h          | 11 ++++++
 3 files changed, 92 insertions(+)

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index d244c57580..75b04cf70f 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -1026,6 +1026,14 @@ config HABV3_IMG_CRT_DER
 
 endif
 
+config IMX8MP_DRAM_ECC
+	bool "Enable LPDDR4 ECC feature on i.MX8MP boards"
+	depends on ARCH_IMX8MP
+	help
+	  The i.MX8MP SoC supports ECC on the LPDDR4 memory. Select Y to enable
+	  this feature. The total amount of memory available will be reduced by
+	  1/8th.
+
 endmenu
 
 endif
diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
index c16e04d274..6398a2b971 100644
--- a/drivers/ddr/imx/imx8m_ddr_init.c
+++ b/drivers/ddr/imx/imx8m_ddr_init.c
@@ -45,6 +45,74 @@ static void ddr_cfg_umctl2(struct dram_controller *dram, struct dram_cfg_param *
 	}
 }
 
+#ifdef CONFIG_IMX8MP_DRAM_ECC
+void ddrc_inline_ecc_scrub(unsigned int start_address,
+			   unsigned int range_address)
+{
+	unsigned int tmp;
+
+	pr_debug("ECC scrub %08x-%08x\n", start_address, range_address);
+	/* Step1: Enable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000000);
+	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
+	reg32setbit(DDRC_ECCCFG1(0), 0x4);
+	/* Step3: Block the AXI ports from taking the transaction */
+	reg32_write(DDRC_PCTRL_0(0), 0x0);
+	/* Step4: Set scrub start address */
+	reg32_write(DDRC_SBRSTART0(0), start_address);
+	/* Step5: Set scrub range address */
+	reg32_write(DDRC_SBRRANGE0(0), range_address);
+	/* Step6: Set scrub_mode to write */
+	reg32_write(DDRC_SBRCTL(0), 0x00000014);
+	/* Step7: Set the desired pattern through SBRWDATA0 registers */
+	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
+	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32setbit(DDRC_SBRCTL(0), 0x0);
+	/* Step9: Poll SBRSTAT.scrub_done=1 */
+	tmp = reg32_read(DDRC_SBRSTAT(0));
+	while (tmp != 0x00000002)
+		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
+	/* Step10: Poll SBRSTAT.scrub_busy=0 */
+	tmp = reg32_read(DDRC_SBRSTAT(0));
+	while (tmp != 0x0)
+		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
+	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
+	clrbits_le32(DDRC_SBRCTL(0), 0x1);
+	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
+	reg32_write(DDRC_SBRCTL(0), 0xff20);
+	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32_write(DDRC_SBRCTL(0), 0xff21);
+	/* Step14: Enable AXI ports by programming */
+	reg32_write(DDRC_PCTRL_0(0), 0x1);
+	/* Step15: Disable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000001);
+}
+
+void ddrc_inline_ecc_scrub_end(unsigned int start_address,
+			       unsigned int range_address)
+{
+	pr_debug("ECC  end %08x-%08x\n", start_address, range_address);
+	/* Step1: Enable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000000);
+	/* Step2: Block the AXI ports from taking the transaction */
+	reg32_write(DDRC_PCTRL_0(0), 0x0);
+	/* Step3: Set scrub start address */
+	reg32_write(DDRC_SBRSTART0(0), start_address);
+	/* Step4: Set scrub range address */
+	reg32_write(DDRC_SBRRANGE0(0), range_address);
+	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
+	clrbits_le32(DDRC_SBRCTL(0), 0x1);
+	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
+	reg32_write(DDRC_SBRCTL(0), 0x100);
+	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
+	reg32_write(DDRC_SBRCTL(0), 0x101);
+	/* Step8: Enable AXI ports by programming */
+	reg32_write(DDRC_PCTRL_0(0), 0x1);
+	/* Step9: Disable quasi-dynamic programming */
+	reg32_write(DDRC_SWCTL(0), 0x00000001);
+}
+#endif
+
 static unsigned int g_cdd_rr_max[4];
 static unsigned int g_cdd_rw_max[4];
 static unsigned int g_cdd_wr_max[4];
@@ -642,6 +710,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
 	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
 	pr_debug("ddrmix config done\n");
 
+#ifdef CONFIG_IMX8MP_DRAM_ECC
+	if (dram->ddrc_type == DDRC_TYPE_MP)
+		board_dram_ecc_scrub();
+#endif
+
 	/* save the dram timing config into memory */
 	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
 
diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
index 5df07772b3..08ebf61da0 100644
--- a/include/soc/imx8m/ddr.h
+++ b/include/soc/imx8m/ddr.h
@@ -186,6 +186,8 @@
 #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
 #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
 #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
+#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
+#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
 
 #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
 #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
@@ -380,4 +382,13 @@ static inline void imx8m_ddr_load_train_code(enum dram_type dram_type,
 	ddr_load_train_code(&imx8m_dram_controller, dram_type, fw_type);
 }
 
+#define DDRC_PHY_REG(x)	((x) * 4)
+
+void board_dram_ecc_scrub(void);
+
+void ddrc_inline_ecc_scrub(unsigned int start_address,
+			   unsigned int range_address);
+void ddrc_inline_ecc_scrub_end(unsigned int start_address,
+			       unsigned int range_address);
+
 #endif

-- 
2.52.0

[PATCH 4/4] arm: boards: protonic-imx8ml: Add ECC + scrubbing

[Steffen Trumtrar]

From: David Jander <david@protonic.nl>

Enable ECC settings in DDRC and add inline ECC scrub on the 3 memory
regions.
The scrubbing is a simpler version than the one generated with
mscale_ddr_tool from NXP. This is much faster and seems to work equally
well.
Documentation of this procedure is nowhere to be found unfortunately, so
proving that it is correct is impossible beyond simply testing it.

Start up time increases by ~800ms with ECC scrubbing enabled.

Signed-off-by: David Jander <david@protonic.nl>
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 .../boards/protonic-imx8m/lpddr4-timing-prt8ml.c   | 25 +++++++++++++++++++++-
 arch/arm/dts/imx8mp-prt8ml.dts                     | 10 ++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boards/protonic-imx8m/lpddr4-timing-prt8ml.c b/arch/arm/boards/protonic-imx8m/lpddr4-timing-prt8ml.c
index 913db8786f..550641b462 100644
--- a/arch/arm/boards/protonic-imx8m/lpddr4-timing-prt8ml.c
+++ b/arch/arm/boards/protonic-imx8m/lpddr4-timing-prt8ml.c
@@ -15,6 +15,12 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
 	{ 0x3d400020, 0x1303 },
 	{ 0x3d400024, 0x1e84800 },
 	{ 0x3d400064, 0x7a017c },
+#ifdef CONFIG_IMX8MP_DRAM_ECC
+	{ 0x3d400070, 0x01027f54 },
+#else
+	{ 0x3d400070, 0x01027f10 },
+#endif
+	{ 0x3d400074, 0x000007b0 },
 	{ 0x3d4000d0, 0xc00307a3 },
 	{ 0x3d4000d4, 0xc50000 },
 	{ 0x3d4000dc, 0xf4003f },
@@ -47,12 +53,21 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
 
 	{ 0x3d4000f4, 0xc99 },
 	{ 0x3d400108, 0x9121c1c },
+#ifdef CONFIG_IMX8MP_DRAM_ECC
+	{ 0x3d400200, 0x13 },
+	{ 0x3d400204, 0x00050505 },
+	{ 0x3d40020c, 0x13131300 },
+	{ 0x3d400210, 0x1f1f },
+	{ 0x3d400214, 0x04040404 },
+	{ 0x3d400218, 0x68040404 },
+#else
 	{ 0x3d400200, 0x16 },
 	{ 0x3d40020c, 0x0 },
-	{ 0x3d400210, 0x1f1f },
 	{ 0x3d400204, 0x80808 },
+	{ 0x3d400210, 0x1f1f },
 	{ 0x3d400214, 0x7070707 },
 	{ 0x3d400218, 0x68070707 },
+#endif
 	{ 0x3d40021c, 0xf08 },
 	{ 0x3d400250, 0x00001705 },
 	{ 0x3d400254, 0x2c },
@@ -1119,3 +1134,11 @@ struct dram_timing_info prt8ml_dram_timing = {
 	.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
 	.fsp_table = { 4000, 400, 100, },
 };
+
+void board_dram_ecc_scrub(void)
+{
+	ddrc_inline_ecc_scrub(0x00000000, 0x1bffffff);
+	ddrc_inline_ecc_scrub(0x20000000, 0x3bffffff);
+	ddrc_inline_ecc_scrub(0x40000000, 0x5bffffff);
+	ddrc_inline_ecc_scrub_end(0x0, 0x5fffffff);
+}
diff --git a/arch/arm/dts/imx8mp-prt8ml.dts b/arch/arm/dts/imx8mp-prt8ml.dts
index 15c3e710ae..b23d64f2d1 100644
--- a/arch/arm/dts/imx8mp-prt8ml.dts
+++ b/arch/arm/dts/imx8mp-prt8ml.dts
@@ -20,8 +20,16 @@ environment-emmc {
 			status = "disabled";
 		};
 	};
-};
 
+	/* Minimum contiguous memory region is 1792MiB
+	 * The probe function of the ddrc will either expand this
+	 * to 6GiB (without ECC) or 5.25GiB (with inline ECC, in 3 regions).
+	 */
+	memory@40000000 {
+		device_type = "memory";
+		reg = <0x0 0x40000000 0x0 0x70000000>;
+	};
+};
 
 &usdhc2 {
 	#address-cells = <1>;

-- 
2.52.0

Re: [PATCH 4/4] arm: boards: protonic-imx8ml: Add ECC + scrubbing

[Ahmad Fatoum]

Hi,

On 3/4/26 12:23 PM, Steffen Trumtrar wrote:

> +
> +void board_dram_ecc_scrub(void)
> +{
> +	ddrc_inline_ecc_scrub(0x00000000, 0x1bffffff);
> +	ddrc_inline_ecc_scrub(0x20000000, 0x3bffffff);
> +	ddrc_inline_ecc_scrub(0x40000000, 0x5bffffff);
> +	ddrc_inline_ecc_scrub_end(0x0, 0x5fffffff);
> +}

This is incompatible with multi-image support. The entry point will need
to pass this board-specific information along as a parameter, either:

  - as function pointer
  - or as data: could the board just pass along 0x60000000 and have the
    common code split it up into the regions above?

Cheers,
Ahmad

> diff --git a/arch/arm/dts/imx8mp-prt8ml.dts b/arch/arm/dts/imx8mp-prt8ml.dts
> index 15c3e710ae..b23d64f2d1 100644
> --- a/arch/arm/dts/imx8mp-prt8ml.dts
> +++ b/arch/arm/dts/imx8mp-prt8ml.dts
> @@ -20,8 +20,16 @@ environment-emmc {
>  			status = "disabled";
>  		};
>  	};
> -};
>  
> +	/* Minimum contiguous memory region is 1792MiB
> +	 * The probe function of the ddrc will either expand this
> +	 * to 6GiB (without ECC) or 5.25GiB (with inline ECC, in 3 regions).
> +	 */
> +	memory@40000000 {
> +		device_type = "memory";
> +		reg = <0x0 0x40000000 0x0 0x70000000>;
> +	};
> +};
>  
>  &usdhc2 {
>  	#address-cells = <1>;
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/4] drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing

[Ahmad Fatoum]

Hi,

On 3/4/26 12:23 PM, Steffen Trumtrar wrote:
> From: David Jander <david@protonic.nl>
> 
> This code comes from u-boot [1] and was introducesd in commit [2].
> 
> A fix from the patch [3] is also included, which doesn't seem to be
> added to u-boot, yet.
> 
> [1] https://github.com/u-boot/u-boot/
> [2] commit f3acb02386f4 ("drivers: ddr: imx8mp: Add inline ECC feature support")
> [3] https://patchwork.ozlabs.org/project/uboot/patch/20230123091702.7472-32-peng.fan@oss.nxp.com/
> 
> Signed-off-by: David Jander <david@protonic.nl>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  arch/arm/mach-imx/Kconfig        |  8 +++++
>  drivers/ddr/imx/imx8m_ddr_init.c | 73 ++++++++++++++++++++++++++++++++++++++++
>  include/soc/imx8m/ddr.h          | 11 ++++++
>  3 files changed, 92 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> index d244c57580..75b04cf70f 100644
> --- a/arch/arm/mach-imx/Kconfig
> +++ b/arch/arm/mach-imx/Kconfig
> @@ -1026,6 +1026,14 @@ config HABV3_IMG_CRT_DER
>  
>  endif
>  
> +config IMX8MP_DRAM_ECC
> +	bool "Enable LPDDR4 ECC feature on i.MX8MP boards"
> +	depends on ARCH_IMX8MP
> +	help
> +	  The i.MX8MP SoC supports ECC on the LPDDR4 memory. Select Y to enable
> +	  this feature. The total amount of memory available will be reduced by
> +	  1/8th.

A generic option is too confusing when it's only affecting a single board.

Alternative suggestion:

  - make IMX8MP_DRAM_ECC a hidden symbol
  - select it from the protonic board
  - Turn the RAM timings of the protonic board into a header and
    replace #ifdef CONFIG_IMX8MP_DRAM_ECC with #ifdef USE_ECC
  - Add two source code files, that include the new header once with
    USE_ECC supported and once without
  - Add two entry points which only differ in what RAM timings are used

Also worth considering: adding the size of the region to be scrubbed or
the function doing the scrubbing into dram_timing_info.

Thoughts?

Cheers,
Ahmad

> +
>  endmenu
>  
>  endif
> diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
> index c16e04d274..6398a2b971 100644
> --- a/drivers/ddr/imx/imx8m_ddr_init.c
> +++ b/drivers/ddr/imx/imx8m_ddr_init.c
> @@ -45,6 +45,74 @@ static void ddr_cfg_umctl2(struct dram_controller *dram, struct dram_cfg_param *
>  	}
>  }
>  
> +#ifdef CONFIG_IMX8MP_DRAM_ECC
> +void ddrc_inline_ecc_scrub(unsigned int start_address,
> +			   unsigned int range_address)
> +{
> +	unsigned int tmp;
> +
> +	pr_debug("ECC scrub %08x-%08x\n", start_address, range_address);
> +	/* Step1: Enable quasi-dynamic programming */
> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> +	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
> +	reg32setbit(DDRC_ECCCFG1(0), 0x4);
> +	/* Step3: Block the AXI ports from taking the transaction */
> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> +	/* Step4: Set scrub start address */
> +	reg32_write(DDRC_SBRSTART0(0), start_address);
> +	/* Step5: Set scrub range address */
> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> +	/* Step6: Set scrub_mode to write */
> +	reg32_write(DDRC_SBRCTL(0), 0x00000014);
> +	/* Step7: Set the desired pattern through SBRWDATA0 registers */
> +	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
> +	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
> +	reg32setbit(DDRC_SBRCTL(0), 0x0);
> +	/* Step9: Poll SBRSTAT.scrub_done=1 */
> +	tmp = reg32_read(DDRC_SBRSTAT(0));
> +	while (tmp != 0x00000002)
> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
> +	/* Step10: Poll SBRSTAT.scrub_busy=0 */
> +	tmp = reg32_read(DDRC_SBRSTAT(0));
> +	while (tmp != 0x0)
> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
> +	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> +	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
> +	reg32_write(DDRC_SBRCTL(0), 0xff20);
> +	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
> +	reg32_write(DDRC_SBRCTL(0), 0xff21);
> +	/* Step14: Enable AXI ports by programming */
> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> +	/* Step15: Disable quasi-dynamic programming */
> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> +}
> +
> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> +			       unsigned int range_address)
> +{
> +	pr_debug("ECC  end %08x-%08x\n", start_address, range_address);
> +	/* Step1: Enable quasi-dynamic programming */
> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> +	/* Step2: Block the AXI ports from taking the transaction */
> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> +	/* Step3: Set scrub start address */
> +	reg32_write(DDRC_SBRSTART0(0), start_address);
> +	/* Step4: Set scrub range address */
> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> +	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> +	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
> +	reg32_write(DDRC_SBRCTL(0), 0x100);
> +	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
> +	reg32_write(DDRC_SBRCTL(0), 0x101);
> +	/* Step8: Enable AXI ports by programming */
> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> +	/* Step9: Disable quasi-dynamic programming */
> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> +}
> +#endif
> +
>  static unsigned int g_cdd_rr_max[4];
>  static unsigned int g_cdd_rw_max[4];
>  static unsigned int g_cdd_wr_max[4];
> @@ -642,6 +710,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
>  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
>  	pr_debug("ddrmix config done\n");
>  
> +#ifdef CONFIG_IMX8MP_DRAM_ECC
> +	if (dram->ddrc_type == DDRC_TYPE_MP)
> +		board_dram_ecc_scrub();
> +#endif
> +
>  	/* save the dram timing config into memory */
>  	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
>  
> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> index 5df07772b3..08ebf61da0 100644
> --- a/include/soc/imx8m/ddr.h
> +++ b/include/soc/imx8m/ddr.h
> @@ -186,6 +186,8 @@
>  #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
>  #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
>  #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
> +#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
> +#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
>  
>  #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
>  #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
> @@ -380,4 +382,13 @@ static inline void imx8m_ddr_load_train_code(enum dram_type dram_type,
>  	ddr_load_train_code(&imx8m_dram_controller, dram_type, fw_type);
>  }
>  
> +#define DDRC_PHY_REG(x)	((x) * 4)
> +
> +void board_dram_ecc_scrub(void);
> +
> +void ddrc_inline_ecc_scrub(unsigned int start_address,
> +			   unsigned int range_address);
> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> +			       unsigned int range_address);
> +
>  #endif
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/4] drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing

[David Jander]

Hi Ahmad,

On Wed, 4 Mar 2026 12:43:55 +0100
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> Hi,
> 
> On 3/4/26 12:23 PM, Steffen Trumtrar wrote:
> > From: David Jander <david@protonic.nl>
> > 
> > This code comes from u-boot [1] and was introducesd in commit [2].
> > 
> > A fix from the patch [3] is also included, which doesn't seem to be
> > added to u-boot, yet.
> > 
> > [1] https://github.com/u-boot/u-boot/
> > [2] commit f3acb02386f4 ("drivers: ddr: imx8mp: Add inline ECC feature support")
> > [3] https://patchwork.ozlabs.org/project/uboot/patch/20230123091702.7472-32-peng.fan@oss.nxp.com/
> > 
> > Signed-off-by: David Jander <david@protonic.nl>
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > ---
> >  arch/arm/mach-imx/Kconfig        |  8 +++++
> >  drivers/ddr/imx/imx8m_ddr_init.c | 73 ++++++++++++++++++++++++++++++++++++++++
> >  include/soc/imx8m/ddr.h          | 11 ++++++
> >  3 files changed, 92 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
> > index d244c57580..75b04cf70f 100644
> > --- a/arch/arm/mach-imx/Kconfig
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -1026,6 +1026,14 @@ config HABV3_IMG_CRT_DER
> >  
> >  endif
> >  
> > +config IMX8MP_DRAM_ECC
> > +	bool "Enable LPDDR4 ECC feature on i.MX8MP boards"
> > +	depends on ARCH_IMX8MP
> > +	help
> > +	  The i.MX8MP SoC supports ECC on the LPDDR4 memory. Select Y to enable
> > +	  this feature. The total amount of memory available will be reduced by
> > +	  1/8th.  
> 
> A generic option is too confusing when it's only affecting a single board.

Why should it affect only a single board? This is a general feature of this
SoC and it can be enabled for any other board that uses this SoC with LPDDR4
chips.

Even more so, I suspect this feature is present on some other SoCs as well
which use the same Synopsys DesignWare IP with this feature enabled. In that
case you'd probably want to make a more generic dram driver though. I suspect
chips like the Rockchip RK3588 or RK3576 might support this also, but I
haven't tried, and since those chips have binary dram init blobs this is
probably not easy to figure out.

Best regards,

> Alternative suggestion:
> 
>   - make IMX8MP_DRAM_ECC a hidden symbol
>   - select it from the protonic board
>   - Turn the RAM timings of the protonic board into a header and
>     replace #ifdef CONFIG_IMX8MP_DRAM_ECC with #ifdef USE_ECC
>   - Add two source code files, that include the new header once with
>     USE_ECC supported and once without
>   - Add two entry points which only differ in what RAM timings are used
> 
> Also worth considering: adding the size of the region to be scrubbed or
> the function doing the scrubbing into dram_timing_info.
> 
> Thoughts?
> 
> Cheers,
> Ahmad
> 
> > +
> >  endmenu
> >  
> >  endif
> > diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
> > index c16e04d274..6398a2b971 100644
> > --- a/drivers/ddr/imx/imx8m_ddr_init.c
> > +++ b/drivers/ddr/imx/imx8m_ddr_init.c
> > @@ -45,6 +45,74 @@ static void ddr_cfg_umctl2(struct dram_controller *dram, struct dram_cfg_param *
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_IMX8MP_DRAM_ECC
> > +void ddrc_inline_ecc_scrub(unsigned int start_address,
> > +			   unsigned int range_address)
> > +{
> > +	unsigned int tmp;
> > +
> > +	pr_debug("ECC scrub %08x-%08x\n", start_address, range_address);
> > +	/* Step1: Enable quasi-dynamic programming */
> > +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> > +	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
> > +	reg32setbit(DDRC_ECCCFG1(0), 0x4);
> > +	/* Step3: Block the AXI ports from taking the transaction */
> > +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> > +	/* Step4: Set scrub start address */
> > +	reg32_write(DDRC_SBRSTART0(0), start_address);
> > +	/* Step5: Set scrub range address */
> > +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> > +	/* Step6: Set scrub_mode to write */
> > +	reg32_write(DDRC_SBRCTL(0), 0x00000014);
> > +	/* Step7: Set the desired pattern through SBRWDATA0 registers */
> > +	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
> > +	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
> > +	reg32setbit(DDRC_SBRCTL(0), 0x0);
> > +	/* Step9: Poll SBRSTAT.scrub_done=1 */
> > +	tmp = reg32_read(DDRC_SBRSTAT(0));
> > +	while (tmp != 0x00000002)
> > +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
> > +	/* Step10: Poll SBRSTAT.scrub_busy=0 */
> > +	tmp = reg32_read(DDRC_SBRSTAT(0));
> > +	while (tmp != 0x0)
> > +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
> > +	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
> > +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> > +	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
> > +	reg32_write(DDRC_SBRCTL(0), 0xff20);
> > +	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
> > +	reg32_write(DDRC_SBRCTL(0), 0xff21);
> > +	/* Step14: Enable AXI ports by programming */
> > +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> > +	/* Step15: Disable quasi-dynamic programming */
> > +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> > +}
> > +
> > +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> > +			       unsigned int range_address)
> > +{
> > +	pr_debug("ECC  end %08x-%08x\n", start_address, range_address);
> > +	/* Step1: Enable quasi-dynamic programming */
> > +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> > +	/* Step2: Block the AXI ports from taking the transaction */
> > +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> > +	/* Step3: Set scrub start address */
> > +	reg32_write(DDRC_SBRSTART0(0), start_address);
> > +	/* Step4: Set scrub range address */
> > +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> > +	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
> > +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> > +	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
> > +	reg32_write(DDRC_SBRCTL(0), 0x100);
> > +	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
> > +	reg32_write(DDRC_SBRCTL(0), 0x101);
> > +	/* Step8: Enable AXI ports by programming */
> > +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> > +	/* Step9: Disable quasi-dynamic programming */
> > +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> > +}
> > +#endif
> > +
> >  static unsigned int g_cdd_rr_max[4];
> >  static unsigned int g_cdd_rw_max[4];
> >  static unsigned int g_cdd_wr_max[4];
> > @@ -642,6 +710,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
> >  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
> >  	pr_debug("ddrmix config done\n");
> >  
> > +#ifdef CONFIG_IMX8MP_DRAM_ECC
> > +	if (dram->ddrc_type == DDRC_TYPE_MP)
> > +		board_dram_ecc_scrub();
> > +#endif
> > +
> >  	/* save the dram timing config into memory */
> >  	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
> >  
> > diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> > index 5df07772b3..08ebf61da0 100644
> > --- a/include/soc/imx8m/ddr.h
> > +++ b/include/soc/imx8m/ddr.h
> > @@ -186,6 +186,8 @@
> >  #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
> >  #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
> >  #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
> > +#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
> > +#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
> >  
> >  #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
> >  #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
> > @@ -380,4 +382,13 @@ static inline void imx8m_ddr_load_train_code(enum dram_type dram_type,
> >  	ddr_load_train_code(&imx8m_dram_controller, dram_type, fw_type);
> >  }
> >  
> > +#define DDRC_PHY_REG(x)	((x) * 4)
> > +
> > +void board_dram_ecc_scrub(void);
> > +
> > +void ddrc_inline_ecc_scrub(unsigned int start_address,
> > +			   unsigned int range_address);
> > +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> > +			       unsigned int range_address);
> > +
> >  #endif
> >   
> 



-- 
David Jander

Re: [PATCH 3/4] drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing

[Ahmad Fatoum]

Hello David,

On 3/4/26 1:23 PM, David Jander wrote:
> On Wed, 4 Mar 2026 12:43:55 +0100
> Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>> +config IMX8MP_DRAM_ECC
>>> +	bool "Enable LPDDR4 ECC feature on i.MX8MP boards"
>>> +	depends on ARCH_IMX8MP
>>> +	help
>>> +	  The i.MX8MP SoC supports ECC on the LPDDR4 memory. Select Y to enable
>>> +	  this feature. The total amount of memory available will be reduced by
>>> +	  1/8th.  
>>
>> A generic option is too confusing when it's only affecting a single board.
> 
> Why should it affect only a single board? This is a general feature of this
> SoC and it can be enabled for any other board that uses this SoC with LPDDR4
> chips.

Sure and a user would expect that enabling this option would turn on
inline ECC, but it currently does so only for the Protonic board.
The i.MX8MP-EVK won't magically get ECC support with this enabled and I
think it may even fail to compile as it doesn't provide
board_dram_ecc_scrub on its own, so having this option in this form is
IMO confusing to users.

My suggestion below is thus to make ECC use a per-board (entry point)
decision instead of a build-wide decision.

> Even more so, I suspect this feature is present on some other SoCs as well
> which use the same Synopsys DesignWare IP with this feature enabled. In that
> case you'd probably want to make a more generic dram driver though. I suspect
> chips like the Rockchip RK3588 or RK3576 might support this also, but I
> haven't tried, and since those chips have binary dram init blobs this is
> probably not easy to figure out.

Interesting. While I have seen ECC documented for RK3568, I thought it
was not available for the RK3588.

By the way, Daniel (Cc'd) has been working on reverse engineering the
Rockchip DRAM init blob, so maybe that's moving within reach :)

Cheers,
Ahmad

> 
> Best regards,
> 
>> Alternative suggestion:
>>
>>   - make IMX8MP_DRAM_ECC a hidden symbol
>>   - select it from the protonic board
>>   - Turn the RAM timings of the protonic board into a header and
>>     replace #ifdef CONFIG_IMX8MP_DRAM_ECC with #ifdef USE_ECC
>>   - Add two source code files, that include the new header once with
>>     USE_ECC supported and once without
>>   - Add two entry points which only differ in what RAM timings are used
>>
>> Also worth considering: adding the size of the region to be scrubbed or
>> the function doing the scrubbing into dram_timing_info.
>>
>> Thoughts?
>>
>> Cheers,
>> Ahmad
>>
>>> +
>>>  endmenu
>>>  
>>>  endif
>>> diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
>>> index c16e04d274..6398a2b971 100644
>>> --- a/drivers/ddr/imx/imx8m_ddr_init.c
>>> +++ b/drivers/ddr/imx/imx8m_ddr_init.c
>>> @@ -45,6 +45,74 @@ static void ddr_cfg_umctl2(struct dram_controller *dram, struct dram_cfg_param *
>>>  	}
>>>  }
>>>  
>>> +#ifdef CONFIG_IMX8MP_DRAM_ECC
>>> +void ddrc_inline_ecc_scrub(unsigned int start_address,
>>> +			   unsigned int range_address)
>>> +{
>>> +	unsigned int tmp;
>>> +
>>> +	pr_debug("ECC scrub %08x-%08x\n", start_address, range_address);
>>> +	/* Step1: Enable quasi-dynamic programming */
>>> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
>>> +	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
>>> +	reg32setbit(DDRC_ECCCFG1(0), 0x4);
>>> +	/* Step3: Block the AXI ports from taking the transaction */
>>> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
>>> +	/* Step4: Set scrub start address */
>>> +	reg32_write(DDRC_SBRSTART0(0), start_address);
>>> +	/* Step5: Set scrub range address */
>>> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
>>> +	/* Step6: Set scrub_mode to write */
>>> +	reg32_write(DDRC_SBRCTL(0), 0x00000014);
>>> +	/* Step7: Set the desired pattern through SBRWDATA0 registers */
>>> +	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
>>> +	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
>>> +	reg32setbit(DDRC_SBRCTL(0), 0x0);
>>> +	/* Step9: Poll SBRSTAT.scrub_done=1 */
>>> +	tmp = reg32_read(DDRC_SBRSTAT(0));
>>> +	while (tmp != 0x00000002)
>>> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
>>> +	/* Step10: Poll SBRSTAT.scrub_busy=0 */
>>> +	tmp = reg32_read(DDRC_SBRSTAT(0));
>>> +	while (tmp != 0x0)
>>> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
>>> +	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
>>> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
>>> +	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
>>> +	reg32_write(DDRC_SBRCTL(0), 0xff20);
>>> +	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
>>> +	reg32_write(DDRC_SBRCTL(0), 0xff21);
>>> +	/* Step14: Enable AXI ports by programming */
>>> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
>>> +	/* Step15: Disable quasi-dynamic programming */
>>> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
>>> +}
>>> +
>>> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
>>> +			       unsigned int range_address)
>>> +{
>>> +	pr_debug("ECC  end %08x-%08x\n", start_address, range_address);
>>> +	/* Step1: Enable quasi-dynamic programming */
>>> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
>>> +	/* Step2: Block the AXI ports from taking the transaction */
>>> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
>>> +	/* Step3: Set scrub start address */
>>> +	reg32_write(DDRC_SBRSTART0(0), start_address);
>>> +	/* Step4: Set scrub range address */
>>> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
>>> +	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
>>> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
>>> +	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
>>> +	reg32_write(DDRC_SBRCTL(0), 0x100);
>>> +	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
>>> +	reg32_write(DDRC_SBRCTL(0), 0x101);
>>> +	/* Step8: Enable AXI ports by programming */
>>> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
>>> +	/* Step9: Disable quasi-dynamic programming */
>>> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
>>> +}
>>> +#endif
>>> +
>>>  static unsigned int g_cdd_rr_max[4];
>>>  static unsigned int g_cdd_rw_max[4];
>>>  static unsigned int g_cdd_wr_max[4];
>>> @@ -642,6 +710,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
>>>  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
>>>  	pr_debug("ddrmix config done\n");
>>>  
>>> +#ifdef CONFIG_IMX8MP_DRAM_ECC
>>> +	if (dram->ddrc_type == DDRC_TYPE_MP)
>>> +		board_dram_ecc_scrub();
>>> +#endif
>>> +
>>>  	/* save the dram timing config into memory */
>>>  	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
>>>  
>>> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
>>> index 5df07772b3..08ebf61da0 100644
>>> --- a/include/soc/imx8m/ddr.h
>>> +++ b/include/soc/imx8m/ddr.h
>>> @@ -186,6 +186,8 @@
>>>  #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
>>>  #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
>>>  #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
>>> +#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
>>> +#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
>>>  
>>>  #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
>>>  #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
>>> @@ -380,4 +382,13 @@ static inline void imx8m_ddr_load_train_code(enum dram_type dram_type,
>>>  	ddr_load_train_code(&imx8m_dram_controller, dram_type, fw_type);
>>>  }
>>>  
>>> +#define DDRC_PHY_REG(x)	((x) * 4)
>>> +
>>> +void board_dram_ecc_scrub(void);
>>> +
>>> +void ddrc_inline_ecc_scrub(unsigned int start_address,
>>> +			   unsigned int range_address);
>>> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
>>> +			       unsigned int range_address);
>>> +
>>>  #endif
>>>   
>>
> 
> 
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 3/4] drivers: ddr: imx8m: ddr_init.c: support ECC scrubbing

[David Jander]

On Wed, 4 Mar 2026 13:33:42 +0100
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> Hello David,
> 
> On 3/4/26 1:23 PM, David Jander wrote:
> > On Wed, 4 Mar 2026 12:43:55 +0100
> > Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:  
> >>> +config IMX8MP_DRAM_ECC
> >>> +	bool "Enable LPDDR4 ECC feature on i.MX8MP boards"
> >>> +	depends on ARCH_IMX8MP
> >>> +	help
> >>> +	  The i.MX8MP SoC supports ECC on the LPDDR4 memory. Select Y to enable
> >>> +	  this feature. The total amount of memory available will be reduced by
> >>> +	  1/8th.    
> >>
> >> A generic option is too confusing when it's only affecting a single board.  
> > 
> > Why should it affect only a single board? This is a general feature of this
> > SoC and it can be enabled for any other board that uses this SoC with LPDDR4
> > chips.  
> 
> Sure and a user would expect that enabling this option would turn on
> inline ECC, but it currently does so only for the Protonic board.
> The i.MX8MP-EVK won't magically get ECC support with this enabled and I
> think it may even fail to compile as it doesn't provide
> board_dram_ecc_scrub on its own, so having this option in this form is
> IMO confusing to users.
> 
> My suggestion below is thus to make ECC use a per-board (entry point)
> decision instead of a build-wide decision.

Oh, sorry for the missing context on my side. Ideally the ecc scrubbing
function should be made generic, so that all boards can use it. There is
already enough code duplication going on IMHO... why add even more?
Or, likewise, if you add more code to one copy of lpddr4-timing.c, shouldn't
it be added to all of them?

> > Even more so, I suspect this feature is present on some other SoCs as well
> > which use the same Synopsys DesignWare IP with this feature enabled. In that
> > case you'd probably want to make a more generic dram driver though. I suspect
> > chips like the Rockchip RK3588 or RK3576 might support this also, but I
> > haven't tried, and since those chips have binary dram init blobs this is
> > probably not easy to figure out.  
> 
> Interesting. While I have seen ECC documented for RK3568, I thought it
> was not available for the RK3588.

Hmm... yes the RK3568 probably also supports it. They all have the same
Synopsys DesignWare IP with different configurations. The 88 and 76 have a
newer version that adds LPDDR5 and drops (LP)DDR3 support.
Not sure, but I suspect i.MX935 and i.MX95 might use the same.

> By the way, Daniel (Cc'd) has been working on reverse engineering the
> Rockchip DRAM init blob, so maybe that's moving within reach :)

This is pretty cool!
Keep me posted with news about this. I have a lot of questions about this blob
with regards to hardware design details. I don't think I'll get answers from
Rockchip besides the usual "copy our reference design", but this effort might
help a lot.

Best regards,

> > 
> > Best regards,
> >   
> >> Alternative suggestion:
> >>
> >>   - make IMX8MP_DRAM_ECC a hidden symbol
> >>   - select it from the protonic board
> >>   - Turn the RAM timings of the protonic board into a header and
> >>     replace #ifdef CONFIG_IMX8MP_DRAM_ECC with #ifdef USE_ECC
> >>   - Add two source code files, that include the new header once with
> >>     USE_ECC supported and once without
> >>   - Add two entry points which only differ in what RAM timings are used
> >>
> >> Also worth considering: adding the size of the region to be scrubbed or
> >> the function doing the scrubbing into dram_timing_info.
> >>
> >> Thoughts?
> >>
> >> Cheers,
> >> Ahmad
> >>  
> >>> +
> >>>  endmenu
> >>>  
> >>>  endif
> >>> diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
> >>> index c16e04d274..6398a2b971 100644
> >>> --- a/drivers/ddr/imx/imx8m_ddr_init.c
> >>> +++ b/drivers/ddr/imx/imx8m_ddr_init.c
> >>> @@ -45,6 +45,74 @@ static void ddr_cfg_umctl2(struct dram_controller *dram, struct dram_cfg_param *
> >>>  	}
> >>>  }
> >>>  
> >>> +#ifdef CONFIG_IMX8MP_DRAM_ECC
> >>> +void ddrc_inline_ecc_scrub(unsigned int start_address,
> >>> +			   unsigned int range_address)
> >>> +{
> >>> +	unsigned int tmp;
> >>> +
> >>> +	pr_debug("ECC scrub %08x-%08x\n", start_address, range_address);
> >>> +	/* Step1: Enable quasi-dynamic programming */
> >>> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> >>> +	/* Step2: Set ECCCFG1.ecc_parity_region_lock to 1 */
> >>> +	reg32setbit(DDRC_ECCCFG1(0), 0x4);
> >>> +	/* Step3: Block the AXI ports from taking the transaction */
> >>> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> >>> +	/* Step4: Set scrub start address */
> >>> +	reg32_write(DDRC_SBRSTART0(0), start_address);
> >>> +	/* Step5: Set scrub range address */
> >>> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> >>> +	/* Step6: Set scrub_mode to write */
> >>> +	reg32_write(DDRC_SBRCTL(0), 0x00000014);
> >>> +	/* Step7: Set the desired pattern through SBRWDATA0 registers */
> >>> +	reg32_write(DDRC_SBRWDATA0(0), 0x55aa55aa);
> >>> +	/* Step8: Enable the SBR by programming SBRCTL.scrub_en=1 */
> >>> +	reg32setbit(DDRC_SBRCTL(0), 0x0);
> >>> +	/* Step9: Poll SBRSTAT.scrub_done=1 */
> >>> +	tmp = reg32_read(DDRC_SBRSTAT(0));
> >>> +	while (tmp != 0x00000002)
> >>> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x2;
> >>> +	/* Step10: Poll SBRSTAT.scrub_busy=0 */
> >>> +	tmp = reg32_read(DDRC_SBRSTAT(0));
> >>> +	while (tmp != 0x0)
> >>> +		tmp = reg32_read(DDRC_SBRSTAT(0)) & 0x1;
> >>> +	/* Step11: Disable SBR by programming SBRCTL.scrub_en=0 */
> >>> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> >>> +	/* Step12: Prepare for normal scrub operation(Read) and set scrub_interval*/
> >>> +	reg32_write(DDRC_SBRCTL(0), 0xff20);
> >>> +	/* Step13: Enable the SBR by programming SBRCTL.scrub_en=1 */
> >>> +	reg32_write(DDRC_SBRCTL(0), 0xff21);
> >>> +	/* Step14: Enable AXI ports by programming */
> >>> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> >>> +	/* Step15: Disable quasi-dynamic programming */
> >>> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> >>> +}
> >>> +
> >>> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> >>> +			       unsigned int range_address)
> >>> +{
> >>> +	pr_debug("ECC  end %08x-%08x\n", start_address, range_address);
> >>> +	/* Step1: Enable quasi-dynamic programming */
> >>> +	reg32_write(DDRC_SWCTL(0), 0x00000000);
> >>> +	/* Step2: Block the AXI ports from taking the transaction */
> >>> +	reg32_write(DDRC_PCTRL_0(0), 0x0);
> >>> +	/* Step3: Set scrub start address */
> >>> +	reg32_write(DDRC_SBRSTART0(0), start_address);
> >>> +	/* Step4: Set scrub range address */
> >>> +	reg32_write(DDRC_SBRRANGE0(0), range_address);
> >>> +	/* Step5: Disable SBR by programming SBRCTL.scrub_en=0 */
> >>> +	clrbits_le32(DDRC_SBRCTL(0), 0x1);
> >>> +	/* Step6: Prepare for normal scrub operation(Read) and set scrub_interval */
> >>> +	reg32_write(DDRC_SBRCTL(0), 0x100);
> >>> +	/* Step7: Enable the SBR by programming SBRCTL.scrub_en=1 */
> >>> +	reg32_write(DDRC_SBRCTL(0), 0x101);
> >>> +	/* Step8: Enable AXI ports by programming */
> >>> +	reg32_write(DDRC_PCTRL_0(0), 0x1);
> >>> +	/* Step9: Disable quasi-dynamic programming */
> >>> +	reg32_write(DDRC_SWCTL(0), 0x00000001);
> >>> +}
> >>> +#endif
> >>> +
> >>>  static unsigned int g_cdd_rr_max[4];
> >>>  static unsigned int g_cdd_rw_max[4];
> >>>  static unsigned int g_cdd_wr_max[4];
> >>> @@ -642,6 +710,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
> >>>  	reg32_write(DDRC_PCTRL_0(0), 0x00000001);
> >>>  	pr_debug("ddrmix config done\n");
> >>>  
> >>> +#ifdef CONFIG_IMX8MP_DRAM_ECC
> >>> +	if (dram->ddrc_type == DDRC_TYPE_MP)
> >>> +		board_dram_ecc_scrub();
> >>> +#endif
> >>> +
> >>>  	/* save the dram timing config into memory */
> >>>  	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
> >>>  
> >>> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> >>> index 5df07772b3..08ebf61da0 100644
> >>> --- a/include/soc/imx8m/ddr.h
> >>> +++ b/include/soc/imx8m/ddr.h
> >>> @@ -186,6 +186,8 @@
> >>>  #define DDRC_SBRWDATA0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf2c)
> >>>  #define DDRC_SBRWDATA1(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf30)
> >>>  #define DDRC_PDCH(X)             (DDRC_IPS_BASE_ADDR(X) + 0xf34)
> >>> +#define DDRC_SBRSTART0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf38)
> >>> +#define DDRC_SBRRANGE0(X)        (DDRC_IPS_BASE_ADDR(X) + 0xf40)
> >>>  
> >>>  #define DDRC_FREQ1_DERATEEN(X)         (DDRC_IPS_BASE_ADDR(X) + 0x2020)
> >>>  #define DDRC_FREQ1_DERATEINT(X)        (DDRC_IPS_BASE_ADDR(X) + 0x2024)
> >>> @@ -380,4 +382,13 @@ static inline void imx8m_ddr_load_train_code(enum dram_type dram_type,
> >>>  	ddr_load_train_code(&imx8m_dram_controller, dram_type, fw_type);
> >>>  }
> >>>  
> >>> +#define DDRC_PHY_REG(x)	((x) * 4)
> >>> +
> >>> +void board_dram_ecc_scrub(void);
> >>> +
> >>> +void ddrc_inline_ecc_scrub(unsigned int start_address,
> >>> +			   unsigned int range_address);
> >>> +void ddrc_inline_ecc_scrub_end(unsigned int start_address,
> >>> +			       unsigned int range_address);
> >>> +
> >>>  #endif
> >>>     
> >>  
> > 
> > 
> >   
> 



-- 
David Jander
Protonic Holland.
tel.: +31 (0) 229 212928
De Factorij 36 / 1689 AL Zwaag