[PATCH v2 3/3] ddr: socfpga: Add ECC DRAM scrubbing support for Gen5/Arria10
Chee, Tien Fong
tien.fong.chee at altera.com
Thu May 7 04:54:52 CEST 2026
Hi Alif,
On 6/5/2026 1:19 pm, Yuslaimi, Alif Zakuan wrote:
> Hi Tien Fong,
>
> On 5/5/2026 2:54 pm, Chee, Tien Fong wrote:
>> Hi Alif,
>>
>>
>> On 28/4/2026 11:32 am, alif.zakuan.yuslaimi at altera.com wrote:
>>> From: Alif Zakuan Yuslaimi <alif.zakuan.yuslaimi at altera.com>
>>>
>>> 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 <tien.fong.chee at altera.com>
>>> Signed-off-by: Alif Zakuan Yuslaimi <alif.zakuan.yuslaimi at altera.com>
>>> ---
>>> 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.
OK
>
>>> +#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.
OK
>
>>> +#if !defined(CONFIG_HW_WATCHDOG)
>>
>>
>> #if !IS_ENABLED(CONFIG_WATCHDOG)
i think reset_manager.h is included to support socfpga_per_reset, using
this,
#if !IS_ENABLED(CONFIG_WATCHDOG) && !CONFIG_IS_ENABLED(WDT)
>>
>>
>>> +#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.
Yes for no watchdog enabled scenario, it should be
#if !IS_ENABLED(CONFIG_WATCHDOG) && !CONFIG_IS_ENABLED(WDT)
/*
* No watchdog driver is active in U-Boot. The bootrom or a
* previous boot stage may have left L4WD0 running. Assert and
* deassert its reset to stop it before the long ECC scrub,
* preventing a spurious watchdog reset during DDR init.
*/
socfpga_per_reset(SOCFPGA_RESET(L4WD0), 1);
socfpga_per_reset(SOCFPGA_RESET(L4WD0), 0);
#endif
>
>>> + /*
>>> + * 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
>>> +}
Best regards,
Tien Fong
More information about the U-Boot
mailing list