Hi Tien Fong,
On 5/5/2026 2:54 pm, Chee, Tien Fong wrote:
Hi Alif,
On 28/4/2026 11:32 am, [email protected] wrote:
From: Alif Zakuan Yuslaimi <[email protected]>
The SDRAM must first be rewritten by zeroes if ECC is used to initialize
the ECC metadata. Make the CPU overwrite the DRAM with zeroes in such a
case.
This implementation turns the caches on temporarily, then overwrites the
whole RAM with zeroes, flushes the caches and turns them off again.
This provides satisfactory performance.
Move common code sdram_init_ecc_bits() to new common file sdram_soc32.c.
Preparation for Gen5 uses the same memory initialization function as
Arria10.
New Kconfig is introduced to enable this implementation only on the
default
Arria10 and CycloneV boards as this will increase the SPL size which
will exceed some Gen5 devices' SPL size limit.
Signed-off-by: Tien Fong Chee <[email protected]>
Signed-off-by: Alif Zakuan Yuslaimi <[email protected]>
---
arch/arm/mach-socfpga/Kconfig | 13 ++++-
arch/arm/mach-socfpga/spl_a10.c | 4 ++
arch/arm/mach-socfpga/spl_gen5.c | 17 ++++++
drivers/ddr/altera/Makefile | 4 +-
drivers/ddr/altera/sdram_arria10.c | 34 +++++-------
drivers/ddr/altera/sdram_gen5.c | 41 ++++++++++++--
drivers/ddr/altera/sdram_soc32.c | 85 ++++++++++++++++++++++++++++++
drivers/ddr/altera/sdram_soc32.h | 15 ++++++
8 files changed, 187 insertions(+), 26 deletions(-)
create mode 100644 drivers/ddr/altera/sdram_soc32.c
create mode 100644 drivers/ddr/altera/sdram_soc32.h
diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/
Kconfig
index 830585a72cc..dd71691b724 100644
--- a/arch/arm/mach-socfpga/Kconfig
+++ b/arch/arm/mach-socfpga/Kconfig
@@ -6,6 +6,13 @@ config ERR_PTR_OFFSET
config NR_DRAM_BANKS
default 1
+config SOCFPGA_ECC_SUPPORT
+ bool "Enable ECC support for DRAM"
+ help
+ Adds CPU-based ECC support for DRAM at boot. This will initialize
+ all DRAM ECC metadata to zero, preventing false ECC errors and
+ improving reliability.
+
config SOCFPGA_DRAM_SIZE_CHECK
bool "Enable DRAM size checking for safety"
help
@@ -105,6 +112,7 @@ config ARCH_SOCFPGA_ARRIA10
select ETH_DESIGNWARE_SOCFPGA
imply FPGA_SOCFPGA
imply SPL_USE_TINY_PRINTF
+ select SOCFPGA_ECC_SUPPORT
config SOCFPGA_ARRIA10_ALWAYS_REPROGRAM
bool "Always reprogram Arria 10 FPGA"
@@ -117,6 +125,9 @@ config SOCFPGA_ARRIA10_ALWAYS_REPROGRAM
config ARCH_SOCFPGA_CYCLONE5
bool
select ARCH_SOCFPGA_GEN5
+ select SOCFPGA_ECC_SUPPORT if \
+ !TARGET_SOCFPGA_TERASIC_SOCKIT && !TARGET_SOCFPGA_EBV_SOCRATES \
+ && !TARGET_SOCFPGA_SOFTING_VINING_FPGA
select SOCFPGA_DRAM_SIZE_CHECK if !TARGET_SOCFPGA_TERASIC_SOCKIT \
&& !TARGET_SOCFPGA_EBV_SOCRATES && \
!TARGET_SOCFPGA_SOFTING_VINING_FPGA
@@ -124,7 +135,7 @@ config ARCH_SOCFPGA_CYCLONE5
config ARCH_SOCFPGA_GEN5
bool
select SPL_ALTERA_SDRAM
- select SPL_CACHE if SPL
+ select SPL_CACHE if SPL && SOCFPGA_ECC_SUPPORT
imply FPGA_SOCFPGA
imply SPL_SIZE_LIMIT_SUBTRACT_GD
imply SPL_SIZE_LIMIT_SUBTRACT_MALLOC
diff --git a/arch/arm/mach-socfpga/spl_a10.c b/arch/arm/mach-socfpga/
spl_a10.c
index c20376f7f8e..4d0696bbaf6 100644
--- a/arch/arm/mach-socfpga/spl_a10.c
+++ b/arch/arm/mach-socfpga/spl_a10.c
@@ -25,6 +25,7 @@
#include <asm/sections.h>
#include <fdtdec.h>
#include <watchdog.h>
+#include <wdt.h>
#include <asm/arch/pinmux.h>
#include <asm/arch/fpga_manager.h>
#include <mmc.h>
@@ -265,6 +266,9 @@ void board_init_f(ulong dummy)
/* Configure the clock based on handoff */
cm_basic_init(gd->fdt_blob);
+ if (CONFIG_IS_ENABLED(WDT))
+ initr_watchdog();
+
#ifdef CONFIG_HW_WATCHDOG
/* release osc1 watchdog timer 0 from reset */
socfpga_reset_deassert_osc1wd0();
Why not remove this?
Thanks for pointing this out. I must have missed this out for Arria10.
I will remove this in v3.
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/
spl_gen5.c
index 08b756db2ca..530863b1564 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -6,6 +6,7 @@
#include <hang.h>
#include <init.h>
#include <log.h>
+#include <asm/global_data.h>
#include <asm/io.h>
#include <asm/utils.h>
#include <image.h>
@@ -21,9 +22,17 @@
#include <debug_uart.h>
#include <fdtdec.h>
#include <watchdog.h>
+#include <wdt.h>
#include <dm/uclass.h>
#include <linux/bitops.h>
+DECLARE_GLOBAL_DATA_PTR;
+
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT) || \
+ IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK)
+static struct bd_info bdata __attribute__ ((section(".data")));
+#endif
+
u32 spl_boot_device(void)
{
const u32 bsel = readl(socfpga_get_sysmgr_addr() +
@@ -106,6 +115,9 @@ void board_init_f(ulong dummy)
if (cm_basic_init(cm_default_cfg))
hang();
+ if (CONFIG_IS_ENABLED(WDT))
+ initr_watchdog();
+
/* Enable bootrom to configure IOs. */
sysmgr_config_warmrstcfgio(1);
@@ -143,6 +155,11 @@ void board_init_f(ulong dummy)
/* enable console uart printing */
preloader_console_init();
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT) || \
+ IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK)
+ gd->bd = &bdata;
+#endif
+
ret = uclass_get_device(UCLASS_RAM, 0, &dev);
if (ret) {
debug("DRAM init failed: %d\n", ret);
diff --git a/drivers/ddr/altera/Makefile b/drivers/ddr/altera/Makefile
index 8259ab04a7e..ece6a131897 100644
--- a/drivers/ddr/altera/Makefile
+++ b/drivers/ddr/altera/Makefile
@@ -7,8 +7,8 @@
# Copyright (C) 2014-2025 Altera Corporation <www.altera.com>
ifdef CONFIG_$(PHASE_)ALTERA_SDRAM
-obj-$(CONFIG_ARCH_SOCFPGA_GEN5) += sdram_gen5.o sequencer.o
-obj-$(CONFIG_ARCH_SOCFPGA_ARRIA10) += sdram_arria10.o
+obj-$(CONFIG_ARCH_SOCFPGA_GEN5) += sdram_soc32.o sdram_gen5.o
sequencer.o
+obj-$(CONFIG_ARCH_SOCFPGA_ARRIA10) += sdram_soc32.o sdram_arria10.o
obj-$(CONFIG_ARCH_SOCFPGA_STRATIX10) += sdram_soc64.o sdram_s10.o
obj-$(CONFIG_ARCH_SOCFPGA_AGILEX) += sdram_soc64.o sdram_agilex.o
obj-$(CONFIG_ARCH_SOCFPGA_N5X) += sdram_soc64.o sdram_n5x.o
diff --git a/drivers/ddr/altera/sdram_arria10.c b/drivers/ddr/altera/
sdram_arria10.c
index c281f711fdf..09d3526603e 100644
--- a/drivers/ddr/altera/sdram_arria10.c
+++ b/drivers/ddr/altera/sdram_arria10.c
@@ -22,9 +22,13 @@
#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/kernel.h>
+#include <linux/sizes.h>
+#include "sdram_soc32.h"
DECLARE_GLOBAL_DATA_PTR;
+#define PGTABLE_OFF 0x4000
Dead code, remove PGTABLE_OFF, only sdram_soc32.c uses it
Sure, I will clean this up in v3.
+
static void sdram_mmr_init(void);
static u64 sdram_size_calc(void);
@@ -193,24 +197,6 @@ static int sdram_is_ecc_enabled(void)
ALT_ECC_HMC_OCP_ECCCTL_ECC_EN_SET_MSK);
}
-/* Initialize SDRAM ECC bits to avoid false DBE */
-static void sdram_init_ecc_bits(u32 size)
-{
- icache_enable();
-
- memset(0, 0, 0x8000);
- gd->arch.tlb_addr = 0x4000;
- gd->arch.tlb_size = PGTABLE_SIZE;
-
- dcache_enable();
-
- printf("DDRCAL: Scrubbing ECC RAM (%i MiB).\n", size >> 20);
- memset((void *)0x8000, 0, size - 0x8000);
- flush_dcache_all();
- printf("DDRCAL: Scrubbing ECC RAM done.\n");
- dcache_disable();
-}
-
/* Function to startup the SDRAM*/
static int sdram_startup(void)
{
@@ -735,8 +721,16 @@ int ddr_calibration_sequence(void)
if (of_sdram_firewall_setup(gd->fdt_blob))
puts("FW: Error Configuring Firewall\n");
- if (sdram_is_ecc_enabled())
- sdram_init_ecc_bits(gd->ram_size);
+ if (sdram_is_ecc_enabled()) {
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
+ sdram_init_ecc_bits();
+ }
+#else
+ puts("DDR: Enable CONFIG_SOCFPGA_ECC_SUPPORT when SDRAM ");
+ puts("ECC is enabled.\n");
+ hang();
+ }
+#endif
confusing pattern, moves the } outside the preprocessor block entirely
to here
Thanks for the suggestion, I will implement this in v3.
sdram_size_check();
diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/
sdram_gen5.c
index 1c3c70ea8ae..76effb264e2 100644
--- a/drivers/ddr/altera/sdram_gen5.c
+++ b/drivers/ddr/altera/sdram_gen5.c
@@ -2,6 +2,7 @@
/*
* Copyright Altera Corporation (C) 2014-2015
*/
+#include <cpu_func.h>
#include <dm.h>
#include <errno.h>
#include <div64.h>
@@ -19,8 +20,11 @@
#include <asm/global_data.h>
#include <asm/io.h>
#include <dm/device_compat.h>
-
+#include <linux/sizes.h>
#include "sequencer.h"
+#include "sdram_soc32.h"
+
+#define PGTABLE_OFF 0x4000
Dead code, remove PGTABLE_OFF, only sdram_soc32.c uses it
I will clean this up in v3.
#ifdef CONFIG_XPL_BUILD
@@ -566,6 +570,19 @@ static unsigned long sdram_calculate_size(struct
socfpga_sdr_ctrl *sdr_ctrl)
return temp;
}
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
+static int sdram_is_ecc_enabled(struct socfpga_sdr_ctrl *sdr_ctrl)
+{
+ return !!(readl(&sdr_ctrl->ctrl_cfg) &
+ SDR_CTRLGRP_CTRLCFG_ECCEN_MASK);
+}
+#else
+static int sdram_is_ecc_enabled(struct socfpga_sdr_ctrl *sdr_ctrl)
+{
+ return 0;
4 spaces (WRONG, should be \t)
ERROR: code indent should use tabs where possible from checkpatch
I must have missed this checkpatch error. Thanks for pointing this out.
I will clean this up in v3.
+}
+#endif
+
static int altera_gen5_sdram_of_to_plat(struct udevice *dev)
{
struct altera_gen5_sdram_plat *plat = dev_get_plat(dev);
@@ -608,10 +625,13 @@ static int altera_gen5_sdram_probe(struct
udevice *dev)
sdram_size = sdram_calculate_size(sdr_ctrl);
debug("SDRAM: %ld MiB\n", sdram_size >> 20);
-#if IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK)
+#if IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK) || \
+ IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
/* setup the dram info within bd */
dram_init_banksize();
+#endif
+#if IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK)
if (sdram_size != gd->bd->bi_dram[0].size) {
printf("DDR: Warning: DRAM size from device tree (%lu MiB)\n",
(ulong)(gd->bd->bi_dram[0].size >> 20));
@@ -626,8 +646,23 @@ static int altera_gen5_sdram_probe(struct udevice
*dev)
}
#endif
+ if (sdram_is_ecc_enabled(sdr_ctrl)) {
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
+ /* Must set USEECCASDATA to 0 if ECC is enabled */
+ clrbits_le32(&sdr_ctrl->static_cfg,
+ SDR_CTRLGRP_STATICCFG_USEECCASDATA_MASK);
+ sdram_init_ecc_bits();
+#else
+ puts("DDR: Enable CONFIG_SOCFPGA_ECC_SUPPORT when SDRAM ");
+ puts("ECC is enabled.\n");
+ puts("DDR: Without scrub, false ECC errors may occur.\n");
+ hang();
+#endif
+}
WRONG, should be \t
I wil clean this up in v3.
+
/* Sanity check ensure correct SDRAM size specified */
-#if IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK)
+#if IS_ENABLED(CONFIG_SOCFPGA_DRAM_SIZE_CHECK) || \
+ IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
if (get_ram_size(0, gd->bd->bi_dram[0].size) !=
gd->bd->bi_dram[0].size) {
#else
diff --git a/drivers/ddr/altera/sdram_soc32.c b/drivers/ddr/altera/
sdram_soc32.c
new file mode 100644
index 00000000000..7556d4933f4
--- /dev/null
+++ b/drivers/ddr/altera/sdram_soc32.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2026 Altera Corporation <www.altera.com>
+ */
+
+#include "sdram_soc32.h"
+#include <string.h>
+#include <hang.h>
+#include <linux/sizes.h>
+#include <cpu_func.h>
+#include <watchdog.h>
+#include <wait_bit.h>
Unused header
This header file is needed for get_timer() function declaration.
Removing this header will cause compilation failure.
+#include <asm/global_data.h>
+#include <asm/system.h>
Unused header, please check!
This header file is needed to define PGTABLE_SIZE.
Removing this header will cause compilation failure.
+#if !defined(CONFIG_HW_WATCHDOG)
#if !IS_ENABLED(CONFIG_WATCHDOG)
+#include <asm/arch/reset_manager.h>
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define PGTABLE_OFF 0x4000
+#define PGTABLE_RESERVE (PGTABLE_OFF + PGTABLE_SIZE)
+
+#if IS_ENABLED(CONFIG_SOCFPGA_ECC_SUPPORT)
+static void socfpga_prepare_watchdog_for_long_op(void)
+{
+#if !IS_ENABLED(CONFIG_WATCHDOG)
#if IS_ENABLED(CONFIG_WATCHDOG) || IS_ENABLED(CONFIG_WDT)
Shouldn't it be "#if !IS_ENABLED(CONFIG_WATCHDOG) ||
!IS_ENABLED(CONFIG_WDT)" instead? This is for no DM watchdog enabled
scenario.
+ /*
+ * No DM watchdog support enabled. Previous boot stage may have left
+ * L4WD0 running, so stop it once before long DDR scrub operation.
+ */
+ socfpga_per_reset(SOCFPGA_RESET(L4WD0), 1);
+ socfpga_per_reset(SOCFPGA_RESET(L4WD0), 0);
+#endif
+}
+
+/* Initialize SDRAM ECC bits to avoid false DBE */
+void sdram_init_ecc_bits(void)
+{
+ u32 start;
+ phys_addr_t start_addr;
+ phys_size_t size, size_init;
+
+ start = get_timer(0);
+
+ start_addr = gd->bd->bi_dram[0].start;
+ size = gd->bd->bi_dram[0].size;
+
+ printf("DDRCAL: Scrubbing ECC RAM (%lu MiB).\n",
+ (ulong)(size >> 20));
+
+ if (size <= PGTABLE_RESERVE) {
+ printf("DDRCAL: Error: DRAM size %#llx smaller than scrub
reserve %#x\n",
+ (unsigned long long)size, PGTABLE_RESERVE);
+ hang();
+ }
+
+ memset((void *)start_addr, 0, PGTABLE_RESERVE);
+ gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
+ gd->arch.tlb_size = PGTABLE_SIZE;
+ start_addr += PGTABLE_RESERVE;
+ size -= PGTABLE_RESERVE;
+
+ dcache_enable();
+
+ socfpga_prepare_watchdog_for_long_op();
+
+ while (size > 0) {
+ size_init = min_t(phys_size_t, (phys_size_t)SZ_1G, size);
+ memset((void *)start_addr, 0, size_init);
+ size -= size_init;
+ start_addr += size_init;
+
+#if IS_ENABLED(CONFIG_WATCHDOG)
Can remove the guard, just call schedule() unconditionally, it is a no-
op when nothing is pending
Best regards,
Tien Fong
Noted, I will implement this in v3.
Thanks,
Alif
