Re: [PATCH v4 01/15] misc: fs_loader: Add QSPI RAW offset loading support

Re: [PATCH v4 01/15] misc: fs_loader: Add QSPI RAW offset loading support

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Add support for loading data from a QSPI RAW flash offset through the
> generic firmware loader. Consumers include the DDR calibration backup
> restore path, and optionally FPGA bitstream loading from QSPI.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   doc/device-tree-bindings/misc/fs_loader.txt |  8 ++
>   drivers/misc/fs_loader.c                    | 99 +++++++++++++++++----
>   include/fs_loader.h                         | 33 ++++++-
>   3 files changed, 120 insertions(+), 20 deletions(-)
>
> diff --git a/doc/device-tree-bindings/misc/fs_loader.txt b/doc/device-tree-bindings/misc/fs_loader.txt
> index 542be4b25a0..8ee59db24d5 100644
> --- a/doc/device-tree-bindings/misc/fs_loader.txt
> +++ b/doc/device-tree-bindings/misc/fs_loader.txt
> @@ -12,6 +12,7 @@ Required properties:
>   	   required for ubi and mounting.
>   - ubivol: which volume of ubi the image loading from, this property is required
>   	  for ubi and mounting.
> +- sfconfig: should only contain "bus number" and "chip selection".
>   
>   Example of storage device and partition search set for mmc, usb, sata and
>   ubi in device tree source as shown in below:
> @@ -46,3 +47,10 @@ ubi in device tree source as shown in below:
>   		mtdpart = "UBI",
>   		ubivol = "ubi0";
>   	};
> +
> +	Example for qspi:
> +	fs_loader4: fs-loader@4 {
> +		bootph-all;
> +		compatible = "u-boot,fs-loader";
> +		sfconfig = <0 0>;
> +	};
> diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> index 6af4c7f15e7..293db94dd29 100644
> --- a/drivers/misc/fs_loader.c
> +++ b/drivers/misc/fs_loader.c
> @@ -19,6 +19,7 @@
>   #include <mapmem.h>
>   #include <malloc.h>
>   #include <spl.h>
> +#include <spi_flash.h>
>   
>   #ifdef CONFIG_CMD_UBIFS
>   #include <ubi_uboot.h>
> @@ -121,16 +122,31 @@ static int _request_firmware_prepare(struct udevice *dev,
>   				    const char *name, void *dbuf,
>   				    size_t size, u32 offset)
>   {
> -	if (!name || name[0] == '\0')
> -		return -EINVAL;
> -
>   	struct firmware *firmwarep = dev_get_priv(dev);
> +	struct device_plat *plat = dev_get_plat(dev);
> +	char *endptr;
> +	u32 fw_offset;
>   
>   	if (!firmwarep)
>   		return -ENOMEM;
>   
> +	if (plat->data_type == DATA_RAW) {
> +		if (!name)
> +			return -EINVAL;
> +
> +		fw_offset = simple_strtoul(name, &endptr, 16);
> +		if (name == endptr || *endptr != '\0')
> +			return -EINVAL;
> +
> +		firmwarep->offset = fw_offset + offset;
> +	} else {
> +		if (!name || name[0] == '\0')
> +			return -EINVAL;
> +
> +		firmwarep->offset = offset;
> +	}
> +
>   	firmwarep->name = name;
> -	firmwarep->offset = offset;
>   	firmwarep->data = dbuf;
>   	firmwarep->size = size;
>   
> @@ -146,13 +162,17 @@ static int _request_firmware_prepare(struct udevice *dev,
>   static int fw_get_filesystem_firmware(struct udevice *dev)
>   {
>   	loff_t actread = 0;
> -	char *storage_interface, *dev_part, *ubi_mtdpart, *ubi_volume;
> -	int ret;
> +	char *storage_interface = NULL, *dev_part = NULL;
> +	char *ubi_mtdpart = NULL, *ubi_volume = NULL;
> +	int ret = 0;
> +	struct device_plat *plat = dev_get_plat(dev);
>   
> -	storage_interface = env_get("storage_interface");
> -	dev_part = env_get("fw_dev_part");
> -	ubi_mtdpart = env_get("fw_ubi_mtdpart");
> -	ubi_volume = env_get("fw_ubi_volume");
> +	if (CONFIG_IS_ENABLED(ENV_SUPPORT)) {
> +		storage_interface = env_get("storage_interface");
> +		dev_part = env_get("fw_dev_part");
> +		ubi_mtdpart = env_get("fw_ubi_mtdpart");
> +		ubi_volume = env_get("fw_ubi_volume");
> +	}
>   
>   	if (storage_interface && dev_part) {
>   		ret = fs_set_blk_dev(storage_interface, dev_part, FS_TYPE_ANY);
> @@ -167,7 +187,8 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
>   		else
>   			ret = -ENODEV;
>   	} else {
> -		ret = select_fs_dev(dev_get_plat(dev));
> +		if (plat->data_type == DATA_FS)
> +			ret = select_fs_dev(dev_get_plat(dev));
>   	}
>   
>   	if (ret)
> @@ -178,8 +199,20 @@ static int fw_get_filesystem_firmware(struct udevice *dev)
>   	if (!firmwarep)
>   		return -ENOMEM;
>   
> -	ret = fs_read(firmwarep->name, (ulong)map_to_sysmem(firmwarep->data),
> -			firmwarep->offset, firmwarep->size, &actread);
> +	if (plat->data_type == DATA_FS)
> +		ret = fs_read(firmwarep->name,
> +			      (ulong)map_to_sysmem(firmwarep->data),
> +			      firmwarep->offset, firmwarep->size, &actread);
> +	else if (plat->data_type == DATA_RAW) {
> +		if (IS_ENABLED(CONFIG_SPI_FLASH)) {
> +			ret = spi_flash_read_dm(plat->flash, firmwarep->offset,
> +						firmwarep->size,
> +						(void *)firmwarep->data);
> +			actread = firmwarep->size;
> +		} else {
> +			ret = -ENOSYS;
> +		}
> +	}
>   
>   	if (ret) {
>   		debug("Error: %d Failed to read %s from flash %lld != %zu.\n",
> @@ -228,6 +261,7 @@ int request_firmware_into_buf(struct udevice *dev,
>   static int fs_loader_of_to_plat(struct udevice *dev)
>   {
>   	u32 phandlepart[2];
> +	u32 sfconfig[2];
>   
>   	ofnode fs_loader_node = dev_ofnode(dev);
>   
> @@ -247,6 +281,15 @@ static int fs_loader_of_to_plat(struct udevice *dev)
>   
>   		plat->ubivol = (char *)ofnode_read_string(
>   				 fs_loader_node, "ubivol");
> +
> +		if (!ofnode_read_u32_array(fs_loader_node, "sfconfig", sfconfig,
> +					   2)) {
> +			plat->data_type = DATA_RAW;
> +			plat->sfconfig.bus = sfconfig[0];
> +			plat->sfconfig.cs = sfconfig[1];
> +		} else {
> +			plat->data_type = DATA_FS;
> +		}
>   	}
>   
>   	return 0;
> @@ -254,11 +297,30 @@ static int fs_loader_of_to_plat(struct udevice *dev)
>   
>   static int fs_loader_probe(struct udevice *dev)
>   {
> -#if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(BLK)
> -	int ret;
> +	int ret = 0;
>   	struct device_plat *plat = dev_get_plat(dev);
>   
> -	if (plat->phandlepart.phandle) {
> +	if (IS_ENABLED(CONFIG_SPI_FLASH) &&
> +	    plat->data_type == DATA_RAW && !plat->flash) {
> +		debug("bus = %d\ncs = %d\n",
> +		      plat->sfconfig.bus, plat->sfconfig.cs);
> +
> +		ret = spi_flash_probe_bus_cs(plat->sfconfig.bus,
> +					     plat->sfconfig.cs,
> +					     &plat->flash);
> +		if (ret) {
> +			debug("fs_loader: Failed to initialize SPI flash at ");
> +			debug("%u:%u (error %d)\n", plat->sfconfig.bus,
> +			      plat->sfconfig.cs, ret);
> +			return -ENODEV;
> +		}
> +
> +		if (!plat->flash)
> +			return -EINVAL;
> +	}
> +
> +	if (CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(BLK) &&
> +	    plat->phandlepart.phandle) {
>   		ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
>   		struct udevice *parent_dev = NULL;
>   
> @@ -269,15 +331,14 @@ static int fs_loader_probe(struct udevice *dev)
>   			ret = blk_get_from_parent(parent_dev, &dev);
>   			if (ret) {
>   				debug("fs_loader: No block device: %d\n",
> -					ret);
> +				      ret);
>   
>   				return ret;
>   			}
>   		}
>   	}
> -#endif
>   
> -	return 0;
> +	return ret;
>   };
>   
>   static const struct udevice_id fs_loader_ids[] = {
> diff --git a/include/fs_loader.h b/include/fs_loader.h
> index 7e16e0f7030..f9c073fee6e 100644
> --- a/include/fs_loader.h
> +++ b/include/fs_loader.h
> @@ -23,18 +23,49 @@ struct phandle_part {
>   };
>   
>   /**
> - * struct phandle_part - A place for storing all supported storage devices
> + * struct sf_config - A place for storing serial flash configuration
> + *
> + * This holds information about bus and chip-select of a serial flash
> + * configuration.
> + *
> + * @bus: SPI bus number.
> + * @cs: SPI chip selection.
> + */
> +struct sf_config {
> +	u32 bus;
> +	u32 cs;
> +};
> +
> +/**
> + * enum data_flags - Flag to indicate data as RAW or as filesystem
> + *
> + * DATA_RAW: Data stored as RAW.
> + * DATA_FS: DATA stored as filesystem.
> + */
> +enum data_flags {
> +	DATA_RAW, /* Stored in raw */
> +	DATA_FS,  /* Stored within a file system */
> +};
> +
> +/**
> + * struct device_plat - A place for storing all supported storage devices
>    *
>    * This holds information about all supported storage devices for driver use.
>    *
>    * @phandlepart: Attribute data for block device.
>    * @mtdpart: MTD partition for ubi partition.
>    * @ubivol: UBI volume-name for ubifsmount.
> + * @data_type: Data type (RAW or filesystem).
> + * @sfconfig: Serial flash configuration.
> + * @flash: SPI flash device used for RAW offset reads.
>    */
>   struct device_plat {
>   	struct phandle_part phandlepart;
>   	char *mtdpart;
>   	char *ubivol;
> +	enum data_flags data_type;
> +	struct sf_config sfconfig;
> +	struct udevice *flash;
>   };
>   
>   /**


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 02/15] arm: dts: socfpga: n5x: add DDR retention calibration backup nodes

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:

> Add the device tree nodes that the N5X DDR retention restore path depends
> on. Reintroduce the fs-loader node and the &sdr DDR calibration backup
> properties (firmware-loader, intel,ddrcal-qspi-offset and
> intel,ddrcal-ddr-offset) that were dropped during the switch to the
> upstream Linux DT config. These describe where the backup calibration
> data is stored and the firmware-loader instance used to read it back in
> SPL.
>
> Document the intel,sdr-ctl-n5x retention properties in
> doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt, including the QSPI
> calibration-restore trust model (System Manager retention bit plus flash
> integrity checks) and fail-closed behaviour, and add an F: entry under
> ARM ALTERA SOCFPGA in MAINTAINERS for that binding path.
>
> Use bootph-all in place of the legacy u-boot,dm-pre-reloc so the nodes
> are bound in the pre-relocation SPL phase on the migrated device tree.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   MAINTAINERS                                   |  1 +
>   arch/arm/dts/socfpga_n5x_socdk-u-boot.dtsi    | 13 ++++++
>   .../ram/intel,sdr-ctl-n5x.txt                 | 44 +++++++++++++++++++
>   3 files changed, 58 insertions(+)
>   create mode 100644 doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 420a3bbf730..3f83a3b11a1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -181,6 +181,7 @@ M:	Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>   M:	Tien Fong Chee <tien.fong.chee@altera.com>
>   S:	Maintained
>   T:	git https://git.u-boot-project.org/u-boot/custodians/u-boot-socfpga.git
> +F:	doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt
>   N:	altera
>   N:	agilex
>   N:	socfpga
> diff --git a/arch/arm/dts/socfpga_n5x_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_n5x_socdk-u-boot.dtsi
> index 3de75f36cbb..ea95d04d37d 100644
> --- a/arch/arm/dts/socfpga_n5x_socdk-u-boot.dtsi
> +++ b/arch/arm/dts/socfpga_n5x_socdk-u-boot.dtsi
> @@ -8,6 +8,12 @@
>   #include "socfpga_n5x-u-boot.dtsi"
>   
>   /{
> +	fs_loader0: fs-loader {
> +		bootph-all;
> +		compatible = "u-boot,fs-loader";
> +		sfconfig = <0 0>;
> +	};
> +
>   	chosen {
>   		u-boot,spl-boot-order = &mmc;
>   	};
> @@ -36,6 +42,13 @@
>   	};
>   };
>   
> +&sdr {
> +	intel,ddrcal-qspi-offset = "0x7000000";
> +	intel,ddrcal-ddr-offset = "0x100000";
> +	firmware-loader = <&fs_loader0>;
> +	bootph-all;
> +};
> +
>   &flash0 {
>   	bootph-all;
>   	cdns,page-size = <256>;
> diff --git a/doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt b/doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt
> new file mode 100644
> index 00000000000..a86e7efdfb9
> --- /dev/null
> +++ b/doc/device-tree-bindings/ram/intel,sdr-ctl-n5x.txt
> @@ -0,0 +1,44 @@
> +* Intel N5X SDRAM controller
> +
> +Required properties:
> +--------------------
> +- compatible: must be "intel,sdr-ctl-n5x"
> +
> +Optional properties (DDR retention):
> +------------------------------------
> +- intel,ddrcal-qspi-offset: string, hex byte offset of the calibration
> +  backup in QSPI (e.g. "0x7000000"). Used by SPL to load the backup
> +  via the firmware loader.
> +- intel,ddrcal-ddr-offset: string, hex address in DRAM where a valid
> +  backup is copied after a successful restore (e.g. "0x100000").
> +- firmware-loader: phandle to a "u-boot,fs-loader" node used to read
> +  the QSPI backup. If omitted, the loader under /chosen is used.
> +
> +Trust model (QSPI calibration restore):
> +---------------------------------------
> +Skipping PHY recalibration after a configuration reset is gated by the
> +System Manager DDR retention bit together with the reset type.
> +The QSPI-sourced calibration backup is treated as untrusted until
> +integrity checks pass. Restore is fail-closed: on any missing DT
> +property, flash read failure, header magic mismatch, oversized
> +data_len that would overflow the HPS handoff region, CRC32 mismatch
> +over the calibration payload, or SHA384 mismatch over the DDR
> +configuration image, U-Boot discards the backup and runs a full PHY
> +calibration instead.
> +
> +Example:
> +--------
> +
> +	fs_loader0: fs-loader {
> +		bootph-all;
> +		compatible = "u-boot,fs-loader";
> +		sfconfig = <0 0>;
> +	};
> +
> +	sdr {
> +		compatible = "intel,sdr-ctl-n5x";
> +		intel,ddrcal-qspi-offset = "0x7000000";
> +		intel,ddrcal-ddr-offset = "0x100000";
> +		firmware-loader = <&fs_loader0>;
> +		bootph-all;
> +	};


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 03/15] arm: socfpga: n5x: enable QSPI mailbox before DDR init

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> Move the mbox_qspi_open() call ahead of DRAM initialization so the QSPI
> controller is handed over to the HPS and its reference clock is
> configured before the DDR retention path reads the calibration backup
> from QSPI during DDR init.
>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   arch/arm/mach-socfpga/spl_n5x.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-socfpga/spl_n5x.c b/arch/arm/mach-socfpga/spl_n5x.c
> index 381aca6d32f..bb2d6187ffc 100644
> --- a/arch/arm/mach-socfpga/spl_n5x.c
> +++ b/arch/arm/mach-socfpga/spl_n5x.c
> @@ -105,6 +105,9 @@ void board_init_f(ulong dummy)
>   		hang();
>   	}
>   
> +	if (IS_ENABLED(CONFIG_CADENCE_QSPI))
> +		mbox_qspi_open();
> +
>   #if CONFIG_IS_ENABLED(ALTERA_SDRAM)
>   	ret = uclass_get_device(UCLASS_RAM, 0, &dev);
>   	if (ret) {
> @@ -112,8 +115,4 @@ void board_init_f(ulong dummy)
>   		hang();
>   	}
>   #endif
> -
> -#ifdef CONFIG_CADENCE_QSPI
> -	mbox_qspi_open();
> -#endif
>   }


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 04/15] ddr: altera: n5x: Add self-refresh support in DDR4

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Enable self-refresh support in DDR4 for retaining DDR4 content with
> minimal power when reset is triggered. On a configuration reset with
> retention enabled, the trained PHY calibration data is restored from a
> backup instead of running a full calibration, so the DDR contents are
> preserved across the reset.
>
> The same INIT0.skip_dram_init / PWRCTL self-refresh entry settings that
> LPDDR4 requires are also applied to DDR4, as some DDR4 chips need them to
> enter and exit retention correctly.
>
> The backup calibration data is copied to DDR only when DDR retention is
> enabled, and the backup header length ('cal->header.data_len') is
> validated against the HPS handoff region so a corrupted or oversized
> value cannot overflow into the handoff data before it is consumed.
>
> Look up the DDR controller node using the "intel,sdr-ctl-n5x" compatible
> string so it matches the node defined in the device tree.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   .../include/mach/system_manager_soc64.h       |   3 +-
>   drivers/ddr/altera/sdram_n5x.c                | 869 ++++++++++++++++--
>   2 files changed, 790 insertions(+), 82 deletions(-)

[...]

Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 05/15] configs: socfpga: n5x: enable required configs for DDR retention

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> DDR retention lets N5X skip a full DDR re-calibration across a
> configuration reset by reusing previously calibrated PHY data, which
> significantly shortens bring-up time. To do this the SPL must, early in
> boot and before DRAM is usable, locate a known-good backup of the
> calibration data, verify it, and replay it into the DDR PHY before
> handoff. Enable the configs that this restore path depends on:
>
> - CONFIG_FS_LOADER / CONFIG_SPL_FS_LOADER: the generic firmware loader is
>    used by the DDR driver (request_firmware_into_buf()) to pull the backup
>    calibration header and data from the QSPI/NAND RAW partition into the
>    OCRAM scratch buffer (SOC64_OCRAM_PHY_BACKUP_BASE). The loader device is
>    resolved through the "firmware-loader" phandle on the DDR node, falling
>    back to /chosen. SPL_FS_LOADER is required because the restore runs in
>    SPL, before DRAM is available. The N5X RAW path binds SPI flash from DT
>    (sfconfig), so CONFIG_SPL_ENV_SUPPORT is not required.
>
> - CONFIG_SPL_CRC32: the retention path checksums the calibration payload
>    with crc32_wd_buf() when storing and validating the QSPI backup. SPL
>    must build lib/crc32.c (obj-$(CONFIG_SPL_CRC32)), which is not selected
>    by SHA384.
>
> - CONFIG_SHA384: the calibration backup is bound to the current DDR
>    configuration with a SHA-384 digest (sha384_csum_wd() over the DDR
>    handoff image and the mem-clk table). On store the digest is written
>    into the backup header; on restore it is recomputed and compared
>    (is_ddrconfig_hash_match()), so a backup that no longer matches the
>    running DDR / mem-clk configuration is rejected. Enabling SHA384 selects
>    the SHA-512 implementation it is built on, so CONFIG_SHA512_ALGO does
>    not need to be listed explicitly.
>
> Init order and failure modes: the restore is best-effort and fully
> guarded. The buffer load is bounds-checked against the HPS handoff region
> (SOC64_HANDOFF_BASE) to prevent overflow, the header magic must match, a
> CRC32 over the data must match, and finally the SHA-384 config hash must
> match. If any check fails (missing loader, read error, bad magic, CRC or
> hash mismatch) the backup is discarded and DDR falls back to a normal
> full calibration, so an invalid or stale backup never corrupts bring-up.
>
> These symbols are enabled only after the fs_loader RAW-partition support
> and the N5X self-refresh / retention restore code that consume them, so
> the series stays bisectable. The defconfig is kept in canonical
> savedefconfig order.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   configs/socfpga_n5x_defconfig | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/configs/socfpga_n5x_defconfig b/configs/socfpga_n5x_defconfig
> index 2d9456ef435..2119d8f4f4f 100644
> --- a/configs/socfpga_n5x_defconfig
> +++ b/configs/socfpga_n5x_defconfig
> @@ -72,6 +72,8 @@ CONFIG_SPL_DM_SEQ_ALIAS=y
>   CONFIG_DWAPB_GPIO=y
>   CONFIG_DM_I2C=y
>   CONFIG_SYS_I2C_DW=y
> +CONFIG_FS_LOADER=y
> +CONFIG_SPL_FS_LOADER=y
>   CONFIG_SYS_MMC_MAX_BLK_COUNT=256
>   CONFIG_MMC_DW=y
>   CONFIG_SPI_FLASH_ISSI=y
> @@ -93,4 +95,5 @@ CONFIG_WDT=y
>   # CONFIG_SPL_USE_TINY_PRINTF is not set
>   CONFIG_PANIC_HANG=y
>   CONFIG_SPL_CRC32=y
> +CONFIG_SHA384=y
>   # CONFIG_TOOLS_MKEFICAPSULE is not set


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 06/15] ddr: altera: n5x: set DDR init progress bit for watchdog recovery

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Bit[30] of the boot scratch cold 8 register tracks DDR init progress. SDM
> inspects it when the watchdog fires due to a DDR init hang, so that it can
> run a clean reset of the DDR subsystem before the next attempt.
>
> Set the bit when SDRAM init starts and clear it once the basic memory test
> has passed.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 4ac22c536fc..4da0617e37c 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -420,6 +420,18 @@ enum data_process {
>   	LOADING
>   };
>   
> +void ddr_init_inprogress(bool start)
> +{
> +	if (start)
> +		setbits_le32(socfpga_get_sysmgr_addr() +
> +			     SYSMGR_SOC64_BOOT_SCRATCH_COLD8,
> +			     ALT_SYSMGR_SCRATCH_REG_8_DDR_PROGRESS_MASK);
> +	else
> +		clrbits_le32(socfpga_get_sysmgr_addr() +
> +			     SYSMGR_SOC64_BOOT_SCRATCH_COLD8,
> +			     ALT_SYSMGR_SCRATCH_REG_8_DDR_PROGRESS_MASK);
> +}
> +
>   bool is_ddr_retention_enabled(u32 reg)
>   {
>   	if (reg & ALT_SYSMGR_SCRATCH_REG_0_DDR_RETENTION_MASK)
> @@ -2820,6 +2832,7 @@ int sdram_mmr_init_full(struct udevice *dev)
>   
>   	if (!is_ddr_init_skipped(reg)) {
>   		printf("SDRAM init in progress ...\n");
> +		ddr_init_inprogress(true);
>   
>   		/*
>   		 * Polling reset complete, must be high to ensure DDR
> @@ -2970,6 +2983,9 @@ int sdram_mmr_init_full(struct udevice *dev)
>   
>   	sdram_size_check(&bd);
>   
> +	/* Marking end of ddr init with passing basic memory test */
> +	ddr_init_inprogress(false);
> +
>   	sdram_set_firewall(&bd);
>   
>   	if (is_ddr_retention_enabled(reg)) {


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 07/15] ddr: altera: n5x: re-init DDR when double-bit ECC error is detected

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> A double-bit ECC error (DBE) means the retained DDR contents can no longer
> be trusted. Detect the DBE status and force a full DDR init and
> calibration in that case instead of taking the retention / warm-reset skip
> path.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 33 ++++++++++++++++++++++++---------
>   1 file changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 4da0617e37c..1bbd5bb0082 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -420,6 +420,17 @@ enum data_process {
>   	LOADING
>   };
>   
> +bool is_ddr_dbe_triggered(void)
> +{
> +	u32 reg = readl(socfpga_get_sysmgr_addr() +
> +			SYSMGR_SOC64_BOOT_SCRATCH_COLD8);
> +
> +	if (reg & ALT_SYSMGR_SCRATCH_REG_8_DDR_DBE_MASK)
> +		return true;
> +
> +	return false;
> +}
> +
>   void ddr_init_inprogress(bool start)
>   {
>   	if (start)
> @@ -478,17 +489,20 @@ bool is_ddr_init_skipped(u32 reg)
>   
>   	reset_type_print(reset_t);
>   
> -	if (reset_t == WARM_RESET) {
> -		debug("%s: DDR init is skipped\n", __func__);
> -		return true;
> -	}
> -
> -	if (reset_t == COLD_RESET) {
> -		if (is_ddr_retention_enabled(reg)) {
> -			debug("%s: DDR retention bit is set\n", __func__);
> +	if (!is_ddr_dbe_triggered()) {
> +		if (reset_t == WARM_RESET) {
>   			debug("%s: DDR init is skipped\n", __func__);
>   			return true;
>   		}
> +
> +		if (reset_t == COLD_RESET) {
> +			if (is_ddr_retention_enabled(reg)) {
> +				debug("%s: DDR retention bit is set\n",
> +				      __func__);
> +				debug("%s: DDR init is skipped\n", __func__);
> +				return true;
> +			}
> +		}
>   	}
>   
>   	debug("%s: DDR init is required\n", __func__);
> @@ -500,7 +514,8 @@ bool is_ddr_calibration_skipped(u32 reg)
>   	enum reset_type reset_t = get_reset_type(reg);
>   
>   	if ((reset_t == NCONFIG || reset_t == JTAG_CONFIG ||
> -	     reset_t == RSU_RECONFIG) && is_ddr_retention_enabled(reg)) {
> +	     reset_t == RSU_RECONFIG) && is_ddr_retention_enabled(reg) &&
> +	     !is_ddr_dbe_triggered()) {
>   		debug("%s: DDR retention bit is set\n", __func__);
>   		return true;
>   	}


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 08/15] ddr: altera: n5x: re-init DDR after watchdog reset from init hang

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> If the previous DDR init hung and was recovered by a watchdog reset, the
> DDR state is undefined and must not be reused. Detect the init-hang
> indication and force a full DDR init and calibration instead of taking the
> retention skip path.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 1bbd5bb0082..858f2e76aa9 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -420,6 +420,17 @@ enum data_process {
>   	LOADING
>   };
>   
> +bool is_ddr_init_hang(void)
> +{
> +	u32 reg = readl(socfpga_get_sysmgr_addr() +
> +			SYSMGR_SOC64_BOOT_SCRATCH_COLD8);
> +
> +	if (reg & ALT_SYSMGR_SCRATCH_REG_8_DDR_PROGRESS_MASK)
> +		return true;
> +
> +	return false;
> +}
> +
>   bool is_ddr_dbe_triggered(void)
>   {
>   	u32 reg = readl(socfpga_get_sysmgr_addr() +
> @@ -483,13 +494,13 @@ void reset_type_print(enum reset_type reset_t)
>   	}
>   }
>   
> -bool is_ddr_init_skipped(u32 reg)
> +bool is_ddr_init_skipped(u32 reg, bool is_ddr_hang_be4_rst)
>   {
>   	enum reset_type reset_t = get_reset_type(reg);
>   
>   	reset_type_print(reset_t);
>   
> -	if (!is_ddr_dbe_triggered()) {
> +	if (!is_ddr_dbe_triggered() && !is_ddr_hang_be4_rst) {
>   		if (reset_t == WARM_RESET) {
>   			debug("%s: DDR init is skipped\n", __func__);
>   			return true;
> @@ -509,13 +520,13 @@ bool is_ddr_init_skipped(u32 reg)
>   	return false;
>   }
>   
> -bool is_ddr_calibration_skipped(u32 reg)
> +bool is_ddr_calibration_skipped(u32 reg, bool is_ddr_hang_be4_rst)
>   {
>   	enum reset_type reset_t = get_reset_type(reg);
>   
>   	if ((reset_t == NCONFIG || reset_t == JTAG_CONFIG ||
>   	     reset_t == RSU_RECONFIG) && is_ddr_retention_enabled(reg) &&
> -	     !is_ddr_dbe_triggered()) {
> +	     !is_ddr_dbe_triggered() && !is_ddr_hang_be4_rst) {
>   		debug("%s: DDR retention bit is set\n", __func__);
>   		return true;
>   	}
> @@ -1598,7 +1609,7 @@ static bool is_cal_bak_data_valid(void)
>   }
>   
>   static int init_phy(struct ddr_handoff *ddr_handoff_info,
> -		    bool *need_calibrate)
> +		    bool *need_calibrate, bool is_ddr_hang_be4_rst)
>   {
>   	u32 reg = readl(socfpga_get_sysmgr_addr() +
>   			SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> @@ -1627,7 +1638,7 @@ static int init_phy(struct ddr_handoff *ddr_handoff_info,
>   			ddr_handoff_info->phy_handoff_length,
>   			ddr_handoff_info->phy_base);
>   
> -	if (is_ddr_calibration_skipped(reg)) {
> +	if (is_ddr_calibration_skipped(reg, is_ddr_hang_be4_rst)) {
>   		if (is_cal_bak_data_valid())
>   			*need_calibrate = false;
>   		else
> @@ -2820,6 +2831,7 @@ int sdram_mmr_init_full(struct udevice *dev)
>   	u32 reg = readl(socfpga_get_sysmgr_addr() +
>   			SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>   	int ret;
> +	bool is_ddr_hang_be4_rst = is_ddr_init_hang();
>   	bool need_calibrate = true;
>   	ulong ddr_offset;
>   	char *endptr;
> @@ -2845,7 +2857,7 @@ int sdram_mmr_init_full(struct udevice *dev)
>   	 */
>   	writel(SOC64_CRAM_PHY_BACKUP_SKIP_MAGIC, SOC64_OCRAM_PHY_BACKUP_BASE);
>   
> -	if (!is_ddr_init_skipped(reg)) {
> +	if (!is_ddr_init_skipped(reg, is_ddr_hang_be4_rst)) {
>   		printf("SDRAM init in progress ...\n");
>   		ddr_init_inprogress(true);
>   
> @@ -2887,7 +2899,8 @@ int sdram_mmr_init_full(struct udevice *dev)
>   		printf("DDR controller configuration is completed\n");
>   
>   		/* Initialize DDR PHY */
> -		ret = init_phy(&ddr_handoff_info, &need_calibrate);
> +		ret = init_phy(&ddr_handoff_info, &need_calibrate,
> +			       is_ddr_hang_be4_rst);
>   		if (ret) {
>   			debug("%s: Failed to inilialize DDR PHY\n", __func__);
>   			return ret;


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 09/15] ddr: altera: n5x: return error on invalid DDR controller type

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Return an error immediately when an invalid DDR controller type is passed
> in, so that uninitialised local variables such as val and upd_val are
> never used.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 858f2e76aa9..43d654341cc 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -2450,6 +2450,9 @@ static int set_cal_res_to_umctl2(struct ddr_handoff *handoff,
>   
>   		upd_val = GET_LOWB_DATA(readw((uintptr_t)(handoff->phy_base +
>   					DMEM_MB_CDD_CHB_WW_0_1_OFFSET)));
> +	} else {
> +		debug("%s: Invalid DDR controller type\n", __func__);
> +		return -ENXIO;
>   	}
>   
>   	upd_val = max(val, upd_val);


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 10/15] ddr: altera: n5x: use correct cast for reset status register read

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Klocwork reported a potential value overflow from an incorrect typecast.
> It cannot actually happen here because the result never exceeds 32 bits,
> but correct the casting of the reset-status register read for the sake of
> good coding practice.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 43d654341cc..d1f0a0d312e 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -2869,9 +2869,9 @@ int sdram_mmr_init_full(struct udevice *dev)
>   		 * subsystem in complete reset state before init DDR clock
>   		 * and DDR controller
>   		 */
> -		ret = wait_for_bit_le32((const void *)((uintptr_t)(readl
> +		ret = wait_for_bit_le32((const void *)((uintptr_t)readl
>   					(ddr_handoff_info.mem_reset_base) +
> -					MEM_RST_MGR_STATUS)),
> +					MEM_RST_MGR_STATUS),
>   					MEM_RST_MGR_STATUS_RESET_COMPLETE,
>   					true, TIMEOUT_200MS, false);
>   		if (ret) {
> @@ -2894,8 +2894,8 @@ int sdram_mmr_init_full(struct udevice *dev)
>   
>   		/* Release the controller from reset */
>   		setbits_le32((uintptr_t)
> -			     (readl(ddr_handoff_info.mem_reset_base) +
> -			     MEM_RST_MGR_STATUS), MEM_RST_MGR_STATUS_AXI_RST |
> +			     readl(ddr_handoff_info.mem_reset_base) +
> +			     MEM_RST_MGR_STATUS, MEM_RST_MGR_STATUS_AXI_RST |
>   			     MEM_RST_MGR_STATUS_CONTROLLER_RST |
>   			     MEM_RST_MGR_STATUS_RESET_COMPLETE);
>   


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 11/15] ddr: altera: n5x: fix debug log typo

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> From: Tien Fong Chee <tien.fong.chee@altera.com>
>
> Fix a typo in the calibration-result debug log messages.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@altera.com>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index d1f0a0d312e..244bf8ccb8c 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -2341,7 +2341,7 @@ static void set_cal_res_to_rankctrl(u32 reg_addr, u16 update_value,
>   
>   	debug("max value divided by 2 is 0x%x\n", update_value);
>   	debug("umclt2 register 0x%x value is 0%x before ", reg_addr, reg);
> -	debug("update with train result\n");
> +	debug("updating with train result\n");
>   
>   	value = (reg & mask) >> shift;
>   
> @@ -2362,7 +2362,7 @@ static void set_cal_res_to_rankctrl(u32 reg_addr, u16 update_value,
>   
>   	reg = readl((uintptr_t)reg_addr);
>   	debug("umclt2 register 0x%x value is 0%x before ", reg_addr, reg);
> -	debug("update with train result\n");
> +	debug("updating with train result\n");
>   }
>   
>   /* helper function for updating train result to register */
> @@ -2375,7 +2375,7 @@ static void set_cal_res_to_reg(u32 reg_addr, u16 update_value, u32 mask,
>   
>   	debug("max value divided by 2 is 0x%x\n", update_value);
>   	debug("umclt2 register 0x%x value is 0%x before ", reg_addr, reg);
> -	debug("update with train result\n");
> +	debug("updating with train result\n");
>   
>   	value = (reg & mask) >> shift;
>   
> @@ -2386,7 +2386,7 @@ static void set_cal_res_to_reg(u32 reg_addr, u16 update_value, u32 mask,
>   
>   	reg = readl((uintptr_t)reg_addr);
>   	debug("umclt2 register 0x%x value is 0%x before ", reg_addr, reg);
> -	debug("update with train result\n");
> +	debug("updating with train result\n");
>   }
>   
>   static u16 get_max_txdqsdlytg0_ux_p0(struct ddr_handoff *handoff, u32 reg,


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 12/15] ddr: altera: n5x: rename is_ddr_init_skipped() and quieten reset log

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> Rename is_ddr_init_skipped() to is_ddr_init() and invert its return value
> so the helper reads naturally at the call site (true means DDR must be
> initialised). The decision is otherwise unchanged: init is required on
> POR, skipped on a warm reset, and skipped on a cold reset only when the
> retention bit is set.
>
> Switch the reset-type reporting from printf() to debug() so the reset
> cause is not printed on every boot, and add the NCONFIG, JTAG_CONFIG and
> RSU_RECONFIG cases. Define reset_type_debug_print() above its first caller
> so the new call site does not trigger an implicit-declaration warning.
>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 56 +++++++++++++---------------------
>   1 file changed, 22 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 244bf8ccb8c..be512cd33b1 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -468,17 +468,17 @@ static enum reset_type get_reset_type(u32 reg)
>   		ALT_SYSMGR_SCRATCH_REG_0_DDR_RESET_TYPE_SHIFT;
>   }
>   
> -void reset_type_print(enum reset_type reset_t)
> +void reset_type_debug_print(u32 boot_scratch_cold0_reg)
>   {
> -	switch (reset_t) {
> +	switch (get_reset_type(boot_scratch_cold0_reg)) {
>   	case POR_RESET:
> -		printf("%s: POR is triggered\n", __func__);
> +		debug("%s: POR is triggered\n", __func__);
>   		break;
>   	case WARM_RESET:
> -		printf("%s: Warm reset is triggered\n", __func__);
> +		debug("%s: Warm reset is triggered\n", __func__);
>   		break;
>   	case COLD_RESET:
> -		printf("%s: Cold reset is triggered\n", __func__);
> +		debug("%s: Cold reset is triggered\n", __func__);
>   		break;
>   	case NCONFIG:
>   		printf("%s: NCONFIG is triggered\n", __func__);
> @@ -490,34 +490,39 @@ void reset_type_print(enum reset_type reset_t)
>   		printf("%s: RSU_RECONFIG is triggered\n", __func__);
>   		break;
>   	default:
> -		printf("%s: Invalid reset type\n", __func__);
> +		debug("%s: Invalid reset type\n", __func__);
>   	}
>   }
>   
> -bool is_ddr_init_skipped(u32 reg, bool is_ddr_hang_be4_rst)
> +bool is_ddr_init(bool is_ddr_hang_be4_rst)
>   {
> -	enum reset_type reset_t = get_reset_type(reg);
> +	u32 reg = readl(socfpga_get_sysmgr_addr() +
> +			SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>   
> -	reset_type_print(reset_t);
> +	reset_type_debug_print(reg);
>   
>   	if (!is_ddr_dbe_triggered() && !is_ddr_hang_be4_rst) {
> -		if (reset_t == WARM_RESET) {
> -			debug("%s: DDR init is skipped\n", __func__);
> +		if (get_reset_type(reg) == POR_RESET) {
> +			debug("%s: DDR init is required\n", __func__);
>   			return true;
>   		}
>   
> -		if (reset_t == COLD_RESET) {
> +		if (get_reset_type(reg) == WARM_RESET) {
> +			debug("%s: DDR init is skipped\n", __func__);
> +			return false;
> +		}
> +
> +		if (get_reset_type(reg) == COLD_RESET) {
>   			if (is_ddr_retention_enabled(reg)) {
> -				debug("%s: DDR retention bit is set\n",
> -				      __func__);
> +				debug("%s: DDR retention bit is set\n", __func__);
>   				debug("%s: DDR init is skipped\n", __func__);
> -				return true;
> +				return false;
>   			}
>   		}
>   	}
>   
>   	debug("%s: DDR init is required\n", __func__);
> -	return false;
> +	return true;
>   }
>   
>   bool is_ddr_calibration_skipped(u32 reg, bool is_ddr_hang_be4_rst)
> @@ -2811,23 +2816,6 @@ static int ddr_post_config(struct ddr_handoff *handoff, bool *need_calibrate)
>   	return ret;
>   }
>   
> -void reset_type_debug_print(u32 boot_scratch_cold0_reg)
> -{
> -	switch (get_reset_type(boot_scratch_cold0_reg)) {
> -	case POR_RESET:
> -		debug("%s: POR is triggered\n", __func__);
> -		break;
> -	case WARM_RESET:
> -		debug("%s: Warm reset is triggered\n", __func__);
> -		break;
> -	case COLD_RESET:
> -		debug("%s: Cold reset is triggered\n", __func__);
> -		break;
> -	default:
> -		debug("%s: Invalid reset type\n", __func__);
> -	}
> -}
> -
>   int sdram_mmr_init_full(struct udevice *dev)
>   {
>   	u32 user_backup[2], user_backup_2nd[2];
> @@ -2860,7 +2848,7 @@ int sdram_mmr_init_full(struct udevice *dev)
>   	 */
>   	writel(SOC64_CRAM_PHY_BACKUP_SKIP_MAGIC, SOC64_OCRAM_PHY_BACKUP_BASE);
>   
> -	if (!is_ddr_init_skipped(reg, is_ddr_hang_be4_rst)) {
> +	if (is_ddr_init(is_ddr_hang_be4_rst)) {
>   		printf("SDRAM init in progress ...\n");
>   		ddr_init_inprogress(true);
>   


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 13/15] ddr: altera: n5x: drop always-false unsigned less-than-zero checks

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> The *_handoff_length fields are unsigned, so the "< 0" checks on them in
> populate_ddr_handoff() can never be true. Remove these dead comparisons.
>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 32 ++++++--------------------------
>   1 file changed, 6 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index be512cd33b1..558415d7931 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>   /*
> - * Copyright (C) 2020-2022 Intel Corporation <www.intel.com>
> + * Copyright (C) 2020-2023 Intel Corporation <www.intel.com>
>    *
>    */
>   
> @@ -1731,12 +1731,6 @@ int populate_ddr_handoff(struct ddr_handoff *handoff)
>   	debug("%s: Umctl2 handoff length in word(32-bit) = 0x%x\n", __func__,
>   	      (u32)handoff->cntlr_handoff_length);
>   
> -	/* Wrong format on user setting data */
> -	if (handoff->cntlr_handoff_length < 0) {
> -		debug("%s: Wrong format on user setting data\n", __func__);
> -		return -ENOEXEC;
> -	}
> -
>   	/* Get the next handoff section address */
>   	next_section_header = handoff->cntlr_handoff_base +
>   				handoff->cntlr_total_length;
> @@ -1789,13 +1783,6 @@ int populate_ddr_handoff(struct ddr_handoff *handoff)
>   		      __func__,
>   		     (u32)handoff->cntlr2_handoff_length);
>   
> -		/* Wrong format on user setting data */
> -		if (handoff->cntlr2_handoff_length < 0) {
> -			debug("%s: Wrong format on umctl2 user setting data\n",
> -			      __func__);
> -			return -ENOEXEC;
> -		}
> -
>   		/* Get the next handoff section address */
>   		next_section_header = handoff->cntlr2_handoff_base +
>   					handoff->cntlr2_total_length;
> @@ -1831,13 +1818,6 @@ int populate_ddr_handoff(struct ddr_handoff *handoff)
>   		debug("%s: PHY handoff length in word(32-bit) = 0x%x\n",
>   		      __func__, (u32)handoff->phy_handoff_length);
>   
> -		/* Wrong format on PHY user setting data */
> -		if (handoff->phy_handoff_length < 0) {
> -			debug("%s: Wrong format on PHY user setting data\n",
> -			      __func__);
> -			return -ENOEXEC;
> -		}
> -
>   		/* Get the next handoff section address */
>   		next_section_header = handoff->phy_handoff_base +
>   					handoff->phy_total_length;
> @@ -1875,11 +1855,11 @@ int populate_ddr_handoff(struct ddr_handoff *handoff)
>   		debug("%s: PHY engine handoff length in word(32-bit) = 0x%x\n",
>   		      __func__, (u32)handoff->phy_engine_handoff_length);
>   
> -		/* Wrong format on PHY init engine setting data */
> -		if (handoff->phy_engine_handoff_length < 0) {
> -			debug("%s: Wrong format on PHY init engine ",
> -			      __func__);
> -			debug("user setting data\n");
> +		/* Checking DDR handoff is overflow? */
> +		if (SOC64_OCRAM_PHY_BACKUP_BASE <=
> +		    (handoff->phy_engine_handoff_base +
> +		    handoff->phy_engine_total_length)) {
> +			printf("%s: DDR handoff is overflow\n ", __func__);
>   			return -ENOEXEC;
>   		}
>   


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 14/15] ddr: altera: n5x: reconfigure DDR when mem-clk handoff changes

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> Trigger a DDR reconfiguration when the memory clock values in the handoff
> data have changed, so that a retention restore is not attempted against a
> DDR configuration that no longer matches the running mem-clk settings.
>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   .../mach-socfpga/include/mach/handoff_soc64.h |  1 +
>   arch/arm/mach-socfpga/wrap_handoff_soc64.c    |  7 ++-
>   drivers/ddr/altera/sdram_n5x.c                | 59 ++++++++++++++++++-
>   3 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> index ae5af1f0100..0fb43acc98b 100644
> --- a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> +++ b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
> @@ -47,6 +47,7 @@
>   #define SOC64_HANDOFF_BASE		0x0007F000
>   #elif IS_ENABLED(CONFIG_ARCH_SOCFPGA_N5X)
>   #define SOC64_HANDOFF_BASE		0xFFE5F000
> +#define SOC64_HANDOFF_DDR_CLK_HANDOFF	(SOC64_HANDOFF_BASE + 0x610)
>   #define SOC64_HANDOFF_MISC		(SOC64_HANDOFF_BASE + 0x630)
>   
>   /* DDR handoff */
> diff --git a/arch/arm/mach-socfpga/wrap_handoff_soc64.c b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> index ecde90f76f4..72e98806f8d 100644
> --- a/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> +++ b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
> @@ -27,7 +27,6 @@ static enum endianness check_endianness(u32 handoff)
>   	case SOC64_HANDOFF_MAGIC_IOCTL:
>   	case SOC64_HANDOFF_MAGIC_FPGA:
>   	case SOC64_HANDOFF_MAGIC_DELAY:
> -	case SOC64_HANDOFF_MAGIC_CLOCK:
>   	case SOC64_HANDOFF_MAGIC_SDRAM:
>   #if IS_ENABLED(CONFIG_ARCH_SOCFPGA_AGILEX5)
>   	case SOC64_HANDOFF_MAGIC_PERI:
> @@ -35,6 +34,12 @@ static enum endianness check_endianness(u32 handoff)
>   	case SOC64_HANDOFF_MAGIC_MISC:
>   #endif
>   		return BIG_ENDIAN;
> +	case SOC64_HANDOFF_MAGIC_CLOCK:
> +		if (IS_ENABLED(CONFIG_ARCH_SOCFPGA_N5X)) {
> +			debug("%s: mem clk  handoff data\n", __func__);
> +			return LITTLE_ENDIAN;
> +		}
> +		return BIG_ENDIAN;
>   #if IS_ENABLED(CONFIG_ARCH_SOCFPGA_N5X)
>   	case SOC64_HANDOFF_DDR_UMCTL2_MAGIC:
>   		debug("%s: umctl2 handoff data\n", __func__);
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 558415d7931..11aa020d380 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -37,8 +37,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   
>   /* DDR handoff SHA384 attribute */
>   #define DDR_HANDOFF_IMG_ADDR	0xFFE44000
> -#define DDR_HANDOFF_IMG_LEN	0x1A000
> +#define DDR_HANDOFF_IMG_LEN	0x1A200
>   #define CHUNKSZ_PER_WD_RESET	(256 * 1024)
> +#define MEMCLK_HANDOFF_SIZE 0x2C
>   
>   /* MPFE NOC registers */
>   #define FPGA2SDRAM_MGR_MAIN_SIDEBANDMGR_FLAGOUTSET0	0xF8024050
> @@ -1105,6 +1106,28 @@ static void phy_ocram(phys_addr_t phy_base, phys_addr_t phy_offset,
>   	}
>   }
>   
> +static int populate_memclk_handoff(void *handoff_base, u32 *mclk_table, u32 size)
> +{
> +	u32 handoff_table[MEMCLK_HANDOFF_SIZE - 4];
> +	u32 length = ARRAY_SIZE(handoff_table);
> +	u32 offset = length - 8;
> +	u32 i;
> +	int ret = 0;
> +
> +	ret = socfpga_handoff_read((void *)handoff_base, handoff_table, length);
> +	if (ret) {
> +		debug("%s: handoff read failed. ret: %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < size; i++) {
> +		mclk_table[i] = handoff_table[offset + i];
> +		debug("mclk_table[%d]: 0x%x\n", i, mclk_table[i]);
> +	}
> +
> +	return ret;
> +}
> +
>   static int cal_data_ocram(phys_addr_t phy_base, u32 addr,
>   			  enum data_process proc)
>   {
> @@ -1390,6 +1413,11 @@ static int cal_data_ocram(phys_addr_t phy_base, u32 addr,
>   	size_t phybak_num;
>   	const u32 *phybak_p = phybak;
>   	u16 *data;
> +	u32 memclk_table[4];
> +	u32 size = ARRAY_SIZE(memclk_table);
> +	u8 *memclk_hash_offset = (u8 *)(SOC64_OCRAM_PHY_BACKUP_BASE - SZ_1K);
> +	int ret = 0;
> +
>   	struct bac_cal_data_t *cal = (struct bac_cal_data_t *)
>   					((uintptr_t)addr);
>   
> @@ -1424,6 +1452,17 @@ static int cal_data_ocram(phys_addr_t phy_base, u32 addr,
>   	};
>   
>   	if (proc == STORE) {
> +		ret = populate_memclk_handoff((void *)SOC64_HANDOFF_CLOCK,
> +					      memclk_table, size);
> +		if (ret) {
> +			debug("%s: Memclks config Magic Number miss match. ret :%d\n",
> +			      __func__, ret);
> +			return -EFAULT;
> +		}
> +
> +		sha384_csum_wd((u8 *)memclk_table, 4 * size,
> +			       memclk_hash_offset, CHUNKSZ_PER_WD_RESET);
> +
>   		/* Creating header */
>   		/* Generate HASH384 from the DDR config */
>   		sha384_csum_wd((u8 *)DDR_HANDOFF_IMG_ADDR,
> @@ -1454,10 +1493,28 @@ static bool is_ddrconfig_hash_match(const void *buffer)
>   {
>   	int ret;
>   	u8 hash[SHA384_SUM_LEN];
> +	u32 memclk_table[4];
> +	u32 size = ARRAY_SIZE(memclk_table);
>   
>   	/* Magic symbol in first 4 bytes of header */
>   	struct cal_header_t *header = (struct cal_header_t *)buffer;
>   
> +	/*
> +	 * Read and generate the hash for the Mem clk values and store before
> +	 * calcuting the HASH384 generation before Image.
> +	 */
> +	ret = populate_memclk_handoff((void *)SOC64_HANDOFF_CLOCK,
> +				      memclk_table, size);
> +	if (ret) {
> +		debug("%s: Memclks config Magic Number miss match. ret : %d\n",
> +		      __func__, ret);
> +		return false;
> +	}
> +
> +	sha384_csum_wd((u8 *)memclk_table, (4 * sizeof(u32)),
> +		       (u8 *)DDR_HANDOFF_IMG_ADDR + DDR_HANDOFF_IMG_LEN - SZ_512,
> +		       CHUNKSZ_PER_WD_RESET);
> +
>   	/* Generate HASH384 from the image */
>   	sha384_csum_wd((u8 *)DDR_HANDOFF_IMG_ADDR, DDR_HANDOFF_IMG_LEN,
>   		       hash, CHUNKSZ_PER_WD_RESET);


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong

Re: [PATCH v4 15/15] ddr: altera: n5x: Replay PIE handoff on DDR retention restore

[Chee, Tien Fong]

On 16/10/2026 12:40 pm, Chen Huei Lok wrote:
> On the DDR retention restore path the trained PHY results are reloaded
> from the calibration backup by cal_data_ocram() in init_phy(), which
> leaves the PHY microcontroller stalled. Running update_training_result()
> there reads back results from a PHY that was never freshly trained and
> fails the DFI handshake, so DRAM init returns -ENODEV.
>
> Instead, bring the PHY microcontroller out of reset/stall and replay the
> PHY Init Engine handoff to return the PHY to normal operating mode.
>
> The PIE handoff table shares memory with the memclk hash scratch area
> (DDR_HANDOFF_IMG_ADDR + DDR_HANDOFF_IMG_LEN - SZ_512) used by
> is_ddrconfig_hash_match(). Snapshot and restore those bytes around the
> hash computation so the replayed PIE table is not corrupted.
>
> Signed-off-by: Chen Huei Lok <chen.huei.lok@altera.com>
> ---
>   drivers/ddr/altera/sdram_n5x.c | 48 +++++++++++++++++++++++++++++-----
>   1 file changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/ddr/altera/sdram_n5x.c b/drivers/ddr/altera/sdram_n5x.c
> index 11aa020d380..fd90a895a80 100644
> --- a/drivers/ddr/altera/sdram_n5x.c
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -1495,6 +1495,9 @@ static bool is_ddrconfig_hash_match(const void *buffer)
>   	u8 hash[SHA384_SUM_LEN];
>   	u32 memclk_table[4];
>   	u32 size = ARRAY_SIZE(memclk_table);
> +	u8 *memclk_hash_dst = (u8 *)(DDR_HANDOFF_IMG_ADDR +
> +				     DDR_HANDOFF_IMG_LEN - SZ_512);
> +	u8 saved_handoff[SHA384_SUM_LEN];
>   
>   	/* Magic symbol in first 4 bytes of header */
>   	struct cal_header_t *header = (struct cal_header_t *)buffer;
> @@ -1511,14 +1514,26 @@ static bool is_ddrconfig_hash_match(const void *buffer)
>   		return false;
>   	}
>   
> +	/*
> +	 * The memclk hash is written into the tail of the hashed handoff image
> +	 * (DDR_HANDOFF_IMG_ADDR + DDR_HANDOFF_IMG_LEN - SZ_512) so that the
> +	 * whole-image hash covers it. That location overlaps the PHY init
> +	 * engine handoff table that phy_init_engine() replays on the retention
> +	 * restore path, so snapshot the original handoff bytes and restore them
> +	 * once the hash has been computed.
> +	 */
> +	memcpy(saved_handoff, memclk_hash_dst, SHA384_SUM_LEN);
> +
>   	sha384_csum_wd((u8 *)memclk_table, (4 * sizeof(u32)),
> -		       (u8 *)DDR_HANDOFF_IMG_ADDR + DDR_HANDOFF_IMG_LEN - SZ_512,
> -		       CHUNKSZ_PER_WD_RESET);
> +		       memclk_hash_dst, CHUNKSZ_PER_WD_RESET);
>   
>   	/* Generate HASH384 from the image */
>   	sha384_csum_wd((u8 *)DDR_HANDOFF_IMG_ADDR, DDR_HANDOFF_IMG_LEN,
>   		       hash, CHUNKSZ_PER_WD_RESET);
>   
> +	/* Restore the handoff bytes clobbered by the memclk hash above */
> +	memcpy(memclk_hash_dst, saved_handoff, SHA384_SUM_LEN);
> +
>   	ret = memcmp((void *)hash, (const void *)header->ddrconfig_hash,
>   		     SHA384_SUM_LEN);
>   	if (ret) {
> @@ -2961,10 +2976,31 @@ int sdram_mmr_init_full(struct udevice *dev)
>   			}
>   
>   		} else {
> -			/* Updating training result to DDR controller */
> -			ret = update_training_result(&ddr_handoff_info);
> -			if (ret)
> -				return ret;
> +			/*
> +			 * Retention restore path: the trained results were
> +			 * already reloaded into the PHY by cal_data_ocram()
> +			 * (LOADING) in init_phy(), so update_training_result()
> +			 * must not run here (it would read back results from a
> +			 * PHY that was never freshly trained and fail the DFI
> +			 * handshake). Bring the PHY microcontroller out of
> +			 * reset/stall, then replay the PIE handoff to return the
> +			 * PHY to normal operating mode.
> +			 */
> +			clrbits_le16(ddr_handoff_info.phy_base +
> +				     DDR_PHY_APBONLY0_OFFSET,
> +				     DDR_PHY_MICROCONTMUXSEL);
> +			setbits_le16(ddr_handoff_info.phy_base +
> +				     DDR_PHY_MICRORESET_OFFSET,
> +				     DDR_PHY_MICRORESET_RESET |
> +				     DDR_PHY_MICRORESET_STALL);
> +			clrbits_le16(ddr_handoff_info.phy_base +
> +				     DDR_PHY_MICRORESET_OFFSET,
> +				     DDR_PHY_MICRORESET_RESET);
> +			setbits_le16(ddr_handoff_info.phy_base +
> +				     DDR_PHY_APBONLY0_OFFSET,
> +				     DDR_PHY_MICROCONTMUXSEL);
> +
> +			phy_init_engine(&ddr_handoff_info);
>   		}
>   
>   		/*


Reviewed-by: Tien Fong Chee <tien.fong.chee@altera.com>

Best regards,
Tien Fong