[PATCH v1 2/5] ddr: socfpga: Add ECC DRAM scrubbing support for Gen5/Arria10
Yuslaimi, Alif Zakuan
alif.zakuan.yuslaimi at altera.com
Thu Dec 18 07:43:15 CET 2025
Hi Brian,
On 17/12/2025 3:50 pm, Sune Brian wrote:
> [CAUTION: This email is from outside your organization. Unless you trust the sender, do not click on links or open attachments as it may be a fraudulent email attempting to steal your information and/or compromise your computer.]
>
> Hi Alif,
>
> On Wed, Dec 17, 2025 at 3:27 PM Yuslaimi, Alif Zakuan
> <alif.zakuan.yuslaimi at altera.com> wrote:
>>
>> Hi Brian,
>>
>> On 16/12/2025 5:46 pm, Sune Brian wrote:
>>> [CAUTION: This email is from outside your organization. Unless you trust the sender, do not click on links or open attachments as it may be a fraudulent email attempting to steal your information and/or compromise your computer.]
>>>
>>> Hi Alif,
>>>
>>> I have a question on sdram_arria10.c
>>> Why spl_gen5.c requires:
>>> static struct bd_info bdata __attribute__ ((section(".data")));
>>> with gd->bd = &bdata;
>>> But on arria10 this is not required, it is done on another file?
>>>
>>> Thank you,
>>> Brian
>>>
>>
>> I found that removing these will cause failure to boot U-Boot SPL after
>> trying to reboot from Linux, SPL will keep on looping during DDR
>> initialization.
>>
>> This could be because of the missing board info in the memory, leading
>> to a failed initialization as gd->bd is not set up properly after a warm
>> reset.
>>
>> SPL runs from the small OCRAM before DRAM is available. CycloneV has a
>> much more limited OCRAM space compared to Arria10.
>
> I am referring to the A10 case here. Maybe you confused my previous question.
> Why I am asking this question is because both A10 and GEN5 aka C5 use
> the same soc32.c. Hence, the ECC will require the gd->bd-> to be defined
> in order for .size or .start etc to be used.
>
> + start_addr = gd->bd->bi_dram[0].start;
> + size = gd->bd->bi_dram[0].size;
>
> So your dynamic means it is well set up before the ECC init uses those
> variables?
> As I am not finding, gd->bd is not set in the a10_spl.c at least not
> really distinct as "="
>
> Thanks,
> Brian
>
Yes, just like you have mentioned, both Arria10 and Gen5 require gd->bd
to be defined before ECC scrubbing in sdram_soc32.c.
However, unlike Gen5, Arria10 is able to leverage common SPL’s
(common/spl/spl.c) generic fallback function - spl_set_bd(), thanks to
the larger OCRAM which allows more flexibility for SPL to use U-Boot’s
generic memory allocation and initialization mechanisms.
Thanks,
Alif
>>
>> Due to this memory space limitation, there is no dynamic memory
>> allocation in Gen5 SPL, therefore bd_info must be allocated statically.
>>
>> By declaring bd_info as static and placing it in .data section, we can
>> minimize Gen5 SPL OCRAM usage, as well as making the SPL execution much
>> more simpler and robust.
>>
>> Thanks,
>> Alif
>>
>>> On Tue, Dec 16, 2025 at 4:46 PM <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 scrubbing 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.
>>>>
>>>> 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/spl_gen5.c | 4 ++
>>>> drivers/ddr/altera/Makefile | 4 +-
>>>> drivers/ddr/altera/sdram_arria10.c | 25 +++-------
>>>> drivers/ddr/altera/sdram_gen5.c | 18 ++++++++
>>>> drivers/ddr/altera/sdram_soc32.c | 74 ++++++++++++++++++++++++++++++
>>>> drivers/ddr/altera/sdram_soc32.h | 11 +++++
>>>> 6 files changed, 115 insertions(+), 21 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/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
>>>> index df79cfe0f7f..5c78a4c1fea 100644
>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>> @@ -27,6 +27,8 @@
>>>>
>>>> DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>> +static struct bd_info bdata __attribute__ ((section(".data")));
>>>> +
>>>> u32 spl_boot_device(void)
>>>> {
>>>> const u32 bsel = readl(socfpga_get_sysmgr_addr() +
>>>> @@ -146,6 +148,8 @@ void board_init_f(ulong dummy)
>>>> /* enable console uart printing */
>>>> preloader_console_init();
>>>>
>>>> + gd->bd = &bdata;
>>>> +
>>>> 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 7ed43965be5..b9e3302d714 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_TARGET_SOCFPGA_GEN5) += sdram_gen5.o sequencer.o
>>>> -obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += sdram_arria10.o
>>>> +obj-$(CONFIG_TARGET_SOCFPGA_GEN5) += sdram_soc32.o sdram_gen5.o sequencer.o
>>>> +obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += sdram_soc32.o sdram_arria10.o
>>>> obj-$(CONFIG_TARGET_SOCFPGA_STRATIX10) += sdram_soc64.o sdram_s10.o
>>>> obj-$(CONFIG_TARGET_SOCFPGA_AGILEX) += sdram_soc64.o sdram_agilex.o
>>>> obj-$(CONFIG_TARGET_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 d3305a6c82d..0e3a1d42f34 100644
>>>> --- a/drivers/ddr/altera/sdram_arria10.c
>>>> +++ b/drivers/ddr/altera/sdram_arria10.c
>>>> @@ -21,9 +21,14 @@
>>>> #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
>>>> +
>>>> static void sdram_mmr_init(void);
>>>> static u64 sdram_size_calc(void);
>>>>
>>>> @@ -192,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)
>>>> {
>>>> @@ -706,7 +693,7 @@ int ddr_calibration_sequence(void)
>>>> puts("FW: Error Configuring Firewall\n");
>>>>
>>>> if (sdram_is_ecc_enabled())
>>>> - sdram_init_ecc_bits(gd->ram_size);
>>>> + sdram_init_ecc_bits();
>>>>
>>>> return 0;
>>>> }
>>>> diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c
>>>> index 3c79bb11802..7a0a043557b 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>
>>>> @@ -17,8 +18,12 @@
>>>> #include <asm/bitops.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
>>>>
>>>> #ifdef CONFIG_XPL_BUILD
>>>>
>>>> @@ -562,6 +567,12 @@ static unsigned long sdram_calculate_size(struct socfpga_sdr_ctrl *sdr_ctrl)
>>>> return temp;
>>>> }
>>>>
>>>> +static int sdram_is_ecc_enabled(struct socfpga_sdr_ctrl *sdr_ctrl)
>>>> +{
>>>> + return !!(readl(&sdr_ctrl->ctrl_cfg) &
>>>> + SDR_CTRLGRP_CTRLCFG_ECCEN_MASK);
>>>> +}
>>>> +
>>>> static int altera_gen5_sdram_of_to_plat(struct udevice *dev)
>>>> {
>>>> struct altera_gen5_sdram_plat *plat = dev_get_plat(dev);
>>>> @@ -604,6 +615,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 (sdram_is_ecc_enabled(sdr_ctrl)) {
>>>> + /* Must set USEECCASDATA to 0 if ECC is enabled */
>>>> + clrbits_le32(&sdr_ctrl->static_cfg,
>>>> + SDR_CTRLGRP_STATICCFG_USEECCASDATA_MASK);
>>>> + sdram_init_ecc_bits();
>>>> + }
>>>> +
>>>> /* Sanity check ensure correct SDRAM size specified */
>>>> if (get_ram_size(0, sdram_size) != sdram_size) {
>>>> puts("SDRAM size check failed!\n");
>>>> diff --git a/drivers/ddr/altera/sdram_soc32.c b/drivers/ddr/altera/sdram_soc32.c
>>>> new file mode 100644
>>>> index 00000000000..b8b42ef8e03
>>>> --- /dev/null
>>>> +++ b/drivers/ddr/altera/sdram_soc32.c
>>>> @@ -0,0 +1,74 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>>>> + */
>>>> +
>>>> +#include "sdram_soc32.h"
>>>> +#include <string.h>
>>>> +#include <linux/sizes.h>
>>>> +#include <cpu_func.h>
>>>> +#include <watchdog.h>
>>>> +#include <wait_bit.h>
>>>> +#include <asm/global_data.h>
>>>> +#include <asm/system.h>
>>>> +#if !defined(CONFIG_HW_WATCHDOG)
>>>> +#include <asm/arch/reset_manager.h>
>>>> +#endif
>>>> +
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>> +
>>>> +#define PGTABLE_OFF 0x4000
>>>> +
>>>> +/* 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 (%ld MiB).\n", size >> 20);
>>>> +
>>>> + memset((void *)start_addr, 0, PGTABLE_SIZE + PGTABLE_OFF);
>>>> + gd->arch.tlb_addr = start_addr + PGTABLE_OFF;
>>>> + gd->arch.tlb_size = PGTABLE_SIZE;
>>>> + start_addr += PGTABLE_SIZE + PGTABLE_OFF;
>>>> + size -= PGTABLE_OFF + PGTABLE_SIZE;
>>>> +
>>>> + dcache_enable();
>>>> +
>>>> + while (size > 0) {
>>>> + size_init = min((phys_addr_t)SZ_1G, (phys_addr_t)size);
>>>> + memset((void *)start_addr, 0, size_init);
>>>> + size -= size_init;
>>>> + start_addr += size_init;
>>>> +
>>>> +#ifdef CONFIG_HW_WATCHDOG
>>>> + /*
>>>> + * In case the watchdog is enabled, make sure to
>>>> + * (re-)configure watchdog so that the defined timeout is
>>>> + * valid.
>>>> + */
>>>> + debug("%s: %d\n", __func__, __LINE__);
>>>> + hw_watchdog_init();
>>>> +#else
>>>> + /*
>>>> + * If the HW watchdog is NOT enabled, make sure it is not
>>>> + * running, because it is enabled in the preloader and
>>>> + * causing boot loop if is not handled.
>>>> + */
>>>> + debug("%s: %d\n", __func__, __LINE__);
>>>> + socfpga_per_reset(SOCFPGA_RESET(L4WD0), 1);
>>>> + socfpga_per_reset(SOCFPGA_RESET(L4WD0), 0);
>>>> +#endif
>>>> + }
>>>> +
>>>> + dcache_disable();
>>>> +
>>>> + printf("DDRCAL: SDRAM-ECC initialized success with %d ms\n",
>>>> + (u32)get_timer(start));
>>>> +}
>>>> diff --git a/drivers/ddr/altera/sdram_soc32.h b/drivers/ddr/altera/sdram_soc32.h
>>>> new file mode 100644
>>>> index 00000000000..85a951a5a74
>>>> --- /dev/null
>>>> +++ b/drivers/ddr/altera/sdram_soc32.h
>>>> @@ -0,0 +1,11 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * Copyright (C) 2025 Altera Corporation <www.altera.com>
>>>> + */
>>>> +
>>>> +#ifndef _SDRAM_SOC32_H_
>>>> +#define _SDRAM_SOC32_H_
>>>> +
>>>> +void sdram_init_ecc_bits(void);
>>>> +
>>>> +#endif /* _SDRAM_SOC32_H_ */
>>>> --
>>>> 2.43.7
>>>>
>>
More information about the U-Boot
mailing list