[PATCH v1 4/5] ddr: altera: agilex5: LPDDRs in-line ECC support

Chee, Tien Fong tien.fong.chee at altera.com
Mon Apr 21 08:36:51 CEST 2025



> -----Original Message-----
> From: Meng, Tingting <tingting.meng at altera.com>
> Sent: Monday, April 21, 2025 1:15 PM
> To: u-boot at lists.denx.de
> Cc: Marek Vasut <marex at denx.de>; Chee, Tien Fong
> <tien.fong.chee at altera.com>; Meng, Tingting <tingting.meng at altera.com>;
> Hea, Kok Kiang <kok.kiang.hea at altera.com>; Maniyam, Dinesh
> <dinesh.maniyam at altera.com>; Ng, Boon Khai <boon.khai.ng at altera.com>;
> Yuslaimi, Alif Zakuan <alif.zakuan.yuslaimi at altera.com>; Rosdi, Danish
> Ahmad <danish.ahmad.rosdi at altera.com>; Zamri, Muhammad Hazim Izzat
> <muhammad.hazim.izzat.zamri at altera.com>; Lim, Jit Loon
> <jit.loon.lim at altera.com>
> Subject: [PATCH v1 4/5] ddr: altera: agilex5: LPDDRs in-line ECC support
> 
> From: Tingting Meng <tingting.meng at altera.com>
> 
> In-line ECC support was added for LPDDR by reserving the last one-eighth of
> the memory space for ECC data. Full memory initialization using the BIST
> MEM INIT mailbox command, based on address and size, is required to
> correctly generate ECC data and enable proper ECC logic verification.
> 
> Signed-off-by: Tingting Meng <tingting.meng at altera.com>
> ---
>  .../arm/dts/socfpga_agilex5_socdk-u-boot.dtsi |  26 ++-
>  drivers/ddr/altera/iossm_mailbox.c            | 161 ++++++++++++++++--
>  drivers/ddr/altera/iossm_mailbox.h            |  11 +-
>  drivers/ddr/altera/sdram_agilex5.c            |  19 ++-
>  drivers/ddr/altera/sdram_soc64.c              |  52 ++++--
>  5 files changed, 220 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/arm/dts/socfpga_agilex5_socdk-u-boot.dtsi
> b/arch/arm/dts/socfpga_agilex5_socdk-u-boot.dtsi
> index d7ab58267eb..8d7dc0945ab 100644
> --- a/arch/arm/dts/socfpga_agilex5_socdk-u-boot.dtsi
> +++ b/arch/arm/dts/socfpga_agilex5_socdk-u-boot.dtsi
> @@ -25,34 +25,44 @@
>  	/*
>  	 * Both Memory base address and size default info is retrieved from
> HW setting.
>  	 * Reconfiguration / Overwrite these info can be done with examples
> below.
> -	 */
> -	/*
> +	 *
> +	 * When LPDDR ECC is enabled, the last 1/8 of the memory region
> must
> +	 * be reserved for the Inline ECC buffer.
> +	 *
>  	 * Example for memory size with 2GB:
>  	 * memory {
>  	 *	reg = <0x0 0x80000000 0x0 0x80000000>;
>  	 * };
> -	 */
> -	/*
> +	 *
>  	 * Example for memory size with 8GB:
>  	 * memory {
>  	 *	reg = <0x0 0x80000000 0x0 0x80000000>,
>  	 *	      <0x8 0x80000000 0x1 0x80000000>;
>  	 * };
> -	 */
> -	/*
> +	 *
>  	 * Example for memory size with 32GB:
>  	 * memory {
>  	 *	reg = <0x0 0x80000000 0x0 0x80000000>,
>  	 *	      <0x8 0x80000000 0x7 0x80000000>;
>  	 * };
> -	 */
> -	/*
> +	 *
>  	 * Example for memory size with 512GB:
>  	 * memory {
>  	 *	reg = <0x0 0x80000000 0x0 0x80000000>,
>  	 *	      <0x8 0x80000000 0x7 0x80000000>,
>  	 *	      <0x88 0x00000000 0x78 0x00000000>;
>  	 * };
> +	 *
> +	 * Example for memory size with 2GB with LPDDR Inline ECC ON:
> +	 * memory {
> +	 *	reg = <0x0 0x80000000 0x0 0x70000000>;
> +	 * };
> +	 *
> +	 * Example for memory size with 8GB with LPDDR Inline ECC ON:
> +	 * memory {
> +	 *	reg = <0x0 0x80000000 0x0 0x80000000>,
> +	 *	      <0x8 0x80000000 0x1 0x40000000>;
> +	 * };
>  	 */
> 
>  	chosen {
> diff --git a/drivers/ddr/altera/iossm_mailbox.c
> b/drivers/ddr/altera/iossm_mailbox.c
> index db9435db657..26602858777 100644
> --- a/drivers/ddr/altera/iossm_mailbox.c
> +++ b/drivers/ddr/altera/iossm_mailbox.c
> @@ -10,6 +10,7 @@
>  #include <asm/arch/base_addr_soc64.h>
>  #include <asm/io.h>
>  #include <linux/bitfield.h>
> +#include <linux/sizes.h>
>  #include "iossm_mailbox.h"
> 
>  #define TIMEOUT_120000MS			120000
> @@ -87,6 +88,7 @@
> 
>  /* offset info of ECC_ENABLE_INTF */
>  #define INTF_ECC_ENABLE_TYPE_MASK	GENMASK(1, 0)
> +#define INTF_ECC_TYPE_MASK	BIT(8)
> 
>  /* cmd opcode BIST_MEM_INIT_START, BIST performed on full memory
> address range */
>  #define BIST_FULL_MEM			BIT(6)
> @@ -107,6 +109,10 @@
> 
>  #define MAX_ECC_ERR_INFO_COUNT 16
> 
> +#define BIST_START_ADDR_SPACE_MASK GENMASK(5, 0)
> +#define BIST_START_ADDR_LOW_MASK   GENMASK(31, 0)
> +#define BIST_START_ADDR_HIGH_MASK  GENMASK(37, 32)
> +
>  #define IO96B_MB_REQ_SETUP(v, w, x, y, z) \
>  	usr_req.ip_type = v; \
>  	usr_req.ip_id = w; \
> @@ -512,7 +518,7 @@ int get_mem_width_info(struct io96b_info
> *io96b_ctrl)  {
>  	int i, j, ret = 0;
>  	u32 mem_width_info;
> -	u16 memory_size, total_memory_size = 0;
> +	phys_size_t memory_size, total_memory_size = 0;
> 
>  	u32
> mem_total_capacity_intf_offset[MAX_MEM_INTERFACE_SUPPORTED] = {
>  		IOSSM_MEM_TOTAL_CAPACITY_INTF0_OFFSET,
> @@ -526,8 +532,11 @@ int get_mem_width_info(struct io96b_info
> *io96b_ctrl)
>  			mem_width_info = readl(io96b_ctrl-
> >io96b[i].io96b_csr_addr +
> 
> mem_total_capacity_intf_offset[j]);
> 
> -			memory_size = memory_size +
> -
> 	FIELD_GET(INTF_CAPACITY_GBITS_MASK, mem_width_info);
> +			io96b_ctrl->io96b[i].mb_ctrl.memory_size[j] =
> +				FIELD_GET(INTF_CAPACITY_GBITS_MASK,
> mem_width_info) * SZ_1G / SZ_8;
> +
> +			if (io96b_ctrl->io96b[i].mb_ctrl.memory_size[j] != 0)
> +				memory_size += io96b_ctrl-
> >io96b[i].mb_ctrl.memory_size[j];
>  		}
> 
>  		if (!memory_size) {
> @@ -536,8 +545,6 @@ int get_mem_width_info(struct io96b_info
> *io96b_ctrl)
>  			goto err;
>  		}
> 
> -		io96b_ctrl->io96b[i].size = memory_size;
> -
>  		total_memory_size = total_memory_size + memory_size;
>  	}
> 
> @@ -556,7 +563,7 @@ int ecc_enable_status(struct io96b_info *io96b_ctrl)  {
>  	int i, j, ret = 0;
>  	u32 ecc_enable_intf;
> -	bool ecc_stat, ecc_stat_set = false;
> +	bool ecc_status, ecc_status_set = false, inline_ecc = false;
> 
>  	u32 ecc_enable_intf_offset[MAX_MEM_INTERFACE_SUPPORTED] =
> {
>  		IOSSM_ECC_ENABLE_INTF0_OFFSET,
> @@ -565,6 +572,7 @@ int ecc_enable_status(struct io96b_info *io96b_ctrl)
> 
>  	/* Initialize ECC status */
>  	io96b_ctrl->ecc_status = false;
> +	io96b_ctrl->inline_ecc = false;
> 
>  	/* Get and ensure all memory interface(s) same ECC status */
>  	for (i = 0; i < io96b_ctrl->num_instance; i++) { @@ -572,15 +580,21
> @@ int ecc_enable_status(struct io96b_info *io96b_ctrl)
>  			ecc_enable_intf = readl(io96b_ctrl-
> >io96b[i].io96b_csr_addr +
>  						ecc_enable_intf_offset[j]);
> 
> -			ecc_stat =
> (FIELD_GET(INTF_ECC_ENABLE_TYPE_MASK, ecc_enable_intf)
> +			ecc_status =
> (FIELD_GET(INTF_ECC_ENABLE_TYPE_MASK, ecc_enable_intf)
>  				    == 0) ? false : true;
> +			inline_ecc = FIELD_GET(INTF_ECC_TYPE_MASK,
> ecc_enable_intf);
> +
> +			if (!ecc_status_set) {
> +				io96b_ctrl->ecc_status = ecc_status;
> 
> -			if (!ecc_stat_set) {
> -				io96b_ctrl->ecc_status = ecc_stat;
> -				ecc_stat_set = true;
> +				if (io96b_ctrl->ecc_status)
> +					io96b_ctrl->inline_ecc = inline_ecc;
> +
> +				ecc_status_set = true;
>  			}
> 
> -			if (ecc_stat != io96b_ctrl->ecc_status) {
> +			if (ecc_status != io96b_ctrl->ecc_status ||
> +			    (io96b_ctrl->ecc_status && inline_ecc !=
> +io96b_ctrl->inline_ecc)) {
>  				printf("%s: Mismatch DDR ECC status on
> IO96B_%d\n", __func__, i);
> 
>  				ret = -EINVAL;
> @@ -673,7 +687,7 @@ bool ecc_interrupt_status(struct io96b_info
> *io96b_ctrl)
>  	return ecc_error_flag;
>  }
> 
> -int bist_mem_init_start(struct io96b_info *io96b_ctrl)
> +int out_of_band_bist_mem_init_start(struct io96b_info *io96b_ctrl)
>  {
>  	struct io96b_mb_req usr_req;
>  	struct io96b_mb_resp usr_resp;
> @@ -746,3 +760,126 @@ int bist_mem_init_start(struct io96b_info
> *io96b_ctrl)
>  err:
>  	return ret;
>  }
> +
> +int bist_mem_init_by_addr(struct io96b_info *io96b_ctrl, int inst_id, int
> intf_id,
> +			  phys_addr_t base_addr, phys_size_t size) {
> +	struct io96b_mb_req usr_req;
> +	struct io96b_mb_resp usr_resp;
> +	int n, ret = 0;
> +	bool bist_start, bist_success;
> +	u32 mem_exp, mem_init_status_intf, start;
> +	phys_size_t chunk_size;
> +
> +	u32 mem_init_status_offset[MAX_MEM_INTERFACE_SUPPORTED]
> = {
> +		IOSSM_MEM_INIT_STATUS_INTF0_OFFSET,
> +		IOSSM_MEM_INIT_STATUS_INTF1_OFFSET
> +	};
> +
> +	/* Check if size is a power of 2 */
> +	if (size == 0 || (size & (size - 1)) != 0) {
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	mem_exp = 0;
> +	chunk_size = size;
> +
> +	while (chunk_size >>= 1)
> +		mem_exp++;
> +
> +	/* Start memory initialization BIST on the specified address range */
> +	IO96B_MB_REQ_SETUP(io96b_ctrl-
> >io96b[inst_id].mb_ctrl.ip_type[intf_id],
> +			   io96b_ctrl->io96b[inst_id].mb_ctrl.ip_id[intf_id],
> +			   CMD_TRIG_CONTROLLER_OP,
> BIST_MEM_INIT_START, 0);
> +
> +	/* CMD_PARAM_0 bit[5:0] = mem_exp */
> +	/* CMD_PARAM_0 bit[6]: 0 - on the specified address range */
> +	usr_req.cmd_param[0] =
> FIELD_PREP(BIST_START_ADDR_SPACE_MASK, mem_exp);
> +	/* Extract address fields START_ADDR[31:0] */
> +	usr_req.cmd_param[1] =
> FIELD_GET(BIST_START_ADDR_LOW_MASK, base_addr);
> +	/* Extract address fields START_ADDR[37:32] */
> +	usr_req.cmd_param[2] =
> FIELD_GET(BIST_START_ADDR_HIGH_MASK, base_addr);
> +	/* Initialize memory to all zeros */
> +	usr_req.cmd_param[3] = 0;
> +
> +	bist_start = false;
> +	bist_success = false;
> +
> +	/* Send request to DDR controller */
> +	debug("%s:Initializing memory: Addr=0x%llx, Size=2^%u\n",
> __func__,
> +	      base_addr, mem_exp);
> +	ret = io96b_mb_req(io96b_ctrl->io96b[inst_id].io96b_csr_addr,
> +			   usr_req, 0, &usr_resp);
> +	if (ret)
> +		goto err;
> +
> +	bist_start =
> IOSSM_CMD_RESPONSE_DATA_SHORT(usr_resp.cmd_resp_status)
> +		& BIT(0);
> +
> +	if (!bist_start) {
> +		printf("%s: Failed to initialize memory on IO96B_%d\n",
> __func__,
> +		       inst_id);
> +		printf("%s: BIST_MEM_INIT_START Error code 0x%lx\n",
> __func__,
> +
> IOSSM_STATUS_CMD_RESPONSE_ERROR(usr_resp.cmd_resp_status));
> +
> +		ret = -EINVAL;
> +		goto err;
> +	}
> +
> +	/* Polling for the initiated memory initialization BIST status */
> +	start = get_timer(0);
> +	while (!bist_success) {
> +		udelay(1);
> +
> +		mem_init_status_intf = readl(io96b_ctrl-
> >io96b[inst_id].io96b_csr_addr +
> +					     mem_init_status_offset[intf_id]);
> +
> +		bist_success = FIELD_GET(INTF_BIST_STATUS_MASK,
> +mem_init_status_intf);
> +
> +		if (!bist_success && (get_timer(start) > TIMEOUT)) {
> +			printf("%s: Timeout initialize memory on
> IO96B_%d\n",
> +			       __func__, inst_id);
> +			printf("%s: BIST_MEM_INIT_STATUS Error code
> 0x%lx\n",
> +			       __func__,
> +
> 	IOSSM_STATUS_CMD_RESPONSE_ERROR(usr_resp.cmd_resp_statu
> s));
> +
> +			ret = -ETIMEDOUT;
> +			goto err;
> +		}
> +	}
> +
> +	debug("%s:DDR memory initializationat 0x%llx completed.\n",
> __func__,
> +base_addr);
> +
> +err:
> +	return ret;
> +}
> +
> +int inline_ecc_bist_mem_init(struct io96b_info *io96b_ctrl) {
> +	int i, j, ret = 0;
> +
> +	/* Memory initialization BIST performed on all memory interfaces */
> +	for (i = 0; i < io96b_ctrl->num_instance; i++) {
> +		for (j = 0; j < io96b_ctrl-
> >io96b[i].mb_ctrl.num_mem_interface; j++) {
> +			ret = bist_mem_init_by_addr(io96b_ctrl, i, j, 0,
> +						    io96b_ctrl-
> >io96b[i].mb_ctrl.memory_size[j]);
> +			if (ret) {
> +				printf("Error: Memory init failed at
> Instance %d, Interface %d\n",
> +				       i, j);
> +				goto err;
> +			}
> +		}
> +	}
> +
> +err:
> +	return ret;
> +}
> +
> +int bist_mem_init_start(struct io96b_info *io96b_ctrl) {
> +	if (io96b_ctrl->inline_ecc)
> +		return inline_ecc_bist_mem_init(io96b_ctrl);
> +	else
> +		return out_of_band_bist_mem_init_start(io96b_ctrl);
> +}
> diff --git a/drivers/ddr/altera/iossm_mailbox.h
> b/drivers/ddr/altera/iossm_mailbox.h
> index 6f794781d30..02d1db28e20 100644
> --- a/drivers/ddr/altera/iossm_mailbox.h
> +++ b/drivers/ddr/altera/iossm_mailbox.h
> @@ -40,11 +40,13 @@ enum iossm_mailbox_cmd_opcode  {
>   * @num_mem_interface:	Number of memory interfaces instantiated
>   * @ip_type:		IP type implemented on the IO96B
>   * @ip_instance_id:	IP identifier for every IP instance implemented on
> the IO96B
> + * @memory_size[2]:	Memory size for every IP instance implemented on
> the IO96B
>   */
>  struct io96b_mb_ctrl {
>  	u32 num_mem_interface;
>  	u32 ip_type[2];
>  	u32 ip_id[2];
> +	phys_size_t memory_size[2];
>  };
> 
>  /* CMD_REQ Register Definition */
> @@ -53,6 +55,9 @@ struct io96b_mb_ctrl {
>  #define CMD_TYPE_MASK			GENMASK(23, 16)
>  #define CMD_OPCODE_MASK			GENMASK(15, 0)
> 
> +/* Computes the Inline ECC data region size */ #define
> +CALC_INLINE_ECC_HW_SIZE(size) (((size) * 7) / 8)
> +
>  /*
>   * IOSSM mailbox request
>   * @ip_type:	    IP type for the specified memory interface
> @@ -83,13 +88,11 @@ struct io96b_mb_resp {
>  /*
>   * IO96B instance specific information
>   *
> - * @size:		Memory size
>   * @io96b_csr_addr:	IO96B instance CSR address
>   * @cal_status:		IO96B instance calibration status
>   * @mb_ctrl:		IOSSM mailbox required information
>   */
>  struct io96b_instance {
> -	u16 size;
>  	phys_addr_t io96b_csr_addr;
>  	bool cal_status;
>  	struct io96b_mb_ctrl mb_ctrl;
> @@ -102,6 +105,7 @@ struct io96b_instance {
>   * @overall_cal_status: Overall calibration status for all IO96B instance(s)
>   * @ddr_type:		DDR memory type
>   * @ecc_status:		ECC enable status (false = disabled, true =
> enabled)
> + * @inline_ecc:		Inline ECC or Out of Band ECC (false = Out of
> Band ECC, true = Inline ECC)
>   * @overall_size:	Total DDR memory size
>   * @io96b[]:		IO96B instance specific information
>   * @ckgen_lock:		IO96B GEN PLL lock (false = not locked, true =
> locked)
> @@ -115,7 +119,8 @@ struct io96b_info {
>  	bool overall_cal_status;
>  	const char *ddr_type;
>  	bool ecc_status;
> -	u16 overall_size;
> +	bool inline_ecc;
> +	phys_size_t overall_size;
>  	struct io96b_instance io96b[MAX_IO96B_SUPPORTED];
>  	bool ckgen_lock;
>  	u8 num_port;
> diff --git a/drivers/ddr/altera/sdram_agilex5.c
> b/drivers/ddr/altera/sdram_agilex5.c
> index 801a6bbab46..ee66c72157a 100644
> --- a/drivers/ddr/altera/sdram_agilex5.c
> +++ b/drivers/ddr/altera/sdram_agilex5.c
> @@ -291,7 +291,14 @@ int sdram_mmr_init_full(struct udevice *dev)
>  		goto err;
>  	}
> 
> -	hw_size = (phys_size_t)io96b_ctrl->overall_size * SZ_1G / SZ_8;
> +	ret = ecc_enable_status(io96b_ctrl);
> +	if (ret) {
> +		printf("DDR: Failed to get ECC enabled status\n");
> +
> +		goto err;
> +	}
> +
> +	hw_size = io96b_ctrl->overall_size;
> 
>  	/* Get bank configuration from devicetree */
>  	ret = fdtdec_decode_ram_size(gd->fdt_blob, NULL, 0, NULL, @@ -
> 303,6 +310,9 @@ int sdram_mmr_init_full(struct udevice *dev)
>  		goto err;
>  	}
> 
> +	if (io96b_ctrl->inline_ecc)
> +		hw_size = CALC_INLINE_ECC_HW_SIZE(hw_size);
> +
>  	if (gd->ram_size > hw_size) {
>  		printf("DDR: Warning: DRAM size from device tree (%lld MiB)
> exceeds\n",
>  		       gd->ram_size >> 20);
> @@ -355,13 +365,6 @@ int sdram_mmr_init_full(struct udevice *dev)
> 
>  	printf("%s: %lld MiB\n", io96b_ctrl->ddr_type, gd->ram_size >> 20);
> 
> -	ret = ecc_enable_status(io96b_ctrl);
> -	if (ret) {
> -		printf("DDR: Failed to get ECC enabled status\n");
> -
> -		goto err;
> -	}
> -
>  	/* Is HPS cold or warm reset? If yes, Skip full memory initialization if
> ECC
>  	 *  enabled to preserve memory content
>  	 */
> diff --git a/drivers/ddr/altera/sdram_soc64.c
> b/drivers/ddr/altera/sdram_soc64.c
> index 75c6de1e4ec..27fbe80ed41 100644
> --- a/drivers/ddr/altera/sdram_soc64.c
> +++ b/drivers/ddr/altera/sdram_soc64.c
> @@ -185,35 +185,51 @@ void sdram_init_ecc_bits(struct bd_info *bd)  void
> sdram_size_check(struct bd_info *bd)  {
>  	phys_size_t total_ram_check = 0;
> -	phys_size_t ram_check = 0;
> -	phys_addr_t start = 0;
> -	phys_size_t size, remaining_size;
>  	int bank;
> 
>  	/* Sanity check ensure correct SDRAM size specified */
>  	debug("DDR: Running SDRAM size sanity check\n");
> 
>  	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +		phys_size_t ram_check = 0;
> +		phys_addr_t start = 0;
> +		phys_size_t remaining_size;
> +
>  		start = bd->bi_dram[bank].start;
>  		remaining_size = bd->bi_dram[bank].size;
> +		debug("Checking bank %d: start=0x%llx, size=0x%llx\n",
> +		      bank, start, remaining_size);
> +
>  		while (ram_check < bd->bi_dram[bank].size) {
> -			size = min((phys_addr_t)SZ_1G,
> -				   (phys_addr_t)remaining_size);
> -
> -			/*
> -			 * Ensure the size is power of two, this is requirement
> -			 * to run get_ram_size() / memory test
> -			 */
> -			if (size != 0 && ((size & (size - 1)) == 0)) {
> -				ram_check += get_ram_size((void *)
> -						(start + ram_check), size);
> -				remaining_size = bd->bi_dram[bank].size -
> -							ram_check;
> -			} else {
> -				puts("DDR: Memory test requires SDRAM
> size ");
> -				puts("in power of two!\n");
> +			phys_size_t size, test_size, detected_size;
> +
> +			size = min((phys_addr_t)SZ_1G,
> (phys_addr_t)remaining_size);
> +
> +			if (size < SZ_8) {
> +				puts("Invalid size: Memory size required to
> be multiple\n");
> +				puts("of 64-Bit word!\n");
>  				hang();
>  			}
> +
> +			/* Adjust size to the nearest power of two to support
> get_ram_size() */
> +			test_size = SZ_8;
> +
> +			while (test_size * 2 <= size)
> +				test_size *= 2;
> +
> +			debug("Testing memory at 0x%llx with size 0x%llx\n",
> +			      start + ram_check, test_size);
> +			detected_size = get_ram_size((void *)(start +
> ram_check),
> +test_size);
> +
> +			if (detected_size != test_size) {
> +				debug("Detected size 0x%llx doesn’t match
> the test size 0x%llx!\n",
> +				      detected_size, test_size);
> +				puts("Memory testing failed!\n");
> +				hang();
> +			}
> +
> +			ram_check += detected_size;
> +			remaining_size = bd->bi_dram[bank].size -
> ram_check;
>  		}
> 
>  		total_ram_check += ram_check;
> --
> 2.25.1

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

Best regards,
Tien Fong


More information about the U-Boot mailing list