Re: [PATCH v1 01/12] imx9: scmi: Get DDR size through SM SCMI API

[Peng Fan <peng.fan@oss.nxp.com>]

On Thu, Feb 05, 2026 at 09:55:07PM +0800, alice.guo@oss.nxp.com wrote:
>From: Ye Li <ye.li@nxp.com>
>
>SM has implemented MISC protocol for get DDR info. Using this API, u-boot
>could get DDR size instead of static configs. This will facilitate the DDR
>ECC enabled case which has 1/8 DDR size reserved by ECC data. SM get DDR info
>API provides the reduced DDR size.
>To be compatible with old SM, if calling this API failed, will fall back
>to static configs.
>
>Signed-off-by: Ye Li <ye.li@nxp.com>
>Signed-off-by: Alice Guo <alice.guo@nxp.com>
>---
> arch/arm/include/asm/mach-imx/sys_proto.h | 10 ++++
> arch/arm/mach-imx/imx9/scmi/soc.c         | 76 +++++++++++++++++++++++++------
> include/scmi_protocols.h                  |  3 +-
> 3 files changed, 74 insertions(+), 15 deletions(-)
>
>diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h
>index 46da7a1eff5..6c8bd6c9085 100644
>--- a/arch/arm/include/asm/mach-imx/sys_proto.h
>+++ b/arch/arm/include/asm/mach-imx/sys_proto.h
>@@ -254,6 +254,16 @@ struct scmi_rom_passover_get_out {
> 	u32 passover[(sizeof(rom_passover_t) + 8) / 4];
> };
> 
>+struct scmi_ddr_info_out {
>+	s32 status;
>+	u32 attributes;
>+	u32 mts;
>+	u32 startlow;
>+	u32 starthigh;
>+	u32 endlow;
>+	u32 endhigh;
>+};

Add comment to explain the fields of the structure.

>+
> #endif
> 
> /* For i.MX ULP */
>diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
>index c1458ccca3c..e573736825c 100644
>--- a/arch/arm/mach-imx/imx9/scmi/soc.c
>+++ b/arch/arm/mach-imx/imx9/scmi/soc.c
>@@ -58,6 +58,35 @@ uint32_t scmi_get_rom_data(rom_passover_t *rom_data)
> 	return 0;
> }
> 
>+int scmi_misc_ddrinfo(u32 ddrc_id, struct scmi_ddr_info_out *out)
>+{
>+	u32 in = ddrc_id;
>+	struct scmi_msg msg = {
>+		.protocol_id = SCMI_PROTOCOL_ID_IMX_MISC,
>+		.message_id = SCMI_MISC_DDR_INFO_GET,
>+		.in_msg = (u8 *)&in,
>+		.in_msg_sz = sizeof(in),
>+		.out_msg = (u8 *)out,
>+		.out_msg_sz = sizeof(*out),
>+	};
>+	int ret;
>+	struct udevice *dev;
>+
>+	memset(out, 0, sizeof(*out));

This could be done in caller by set an initialization value to 0.

>+	ret = uclass_get_device_by_name(UCLASS_CLK, "protocol@14", &dev);
>+	if (ret)
>+		return ret;
>+
>+	ret = devm_scmi_process_msg(dev, &msg);
>+	if (ret != 0 || out->status != 0) {
>+		printf("Failed to get ddr cfg, scmi_err = %d\n",
>+		       out->status);
>+		return -EINVAL;
>+	}
>+
>+	return 0;
>+}
>+
> #if IS_ENABLED(CONFIG_ENV_IS_IN_MMC)
> __weak int board_mmc_get_env_dev(int devno)
> {
>@@ -335,25 +364,44 @@ void enable_caches(void)
> 
> __weak int board_phys_sdram_size(phys_size_t *size)
> {
>+	struct scmi_ddr_info_out ddr_info;

Init with "= { 0 }"

>+	int ret;
>+	u32 ddrc_id = 0, ddrc_num = 1;
> 	phys_size_t start, end;
>-	phys_size_t val;
> 
> 	if (!size)
> 		return -EINVAL;
> 
>-	val = readl(REG_DDR_CS0_BNDS);
>-	start = (val >> 16) << 24;
>-	end   = (val & 0xFFFF);
>-	end   = end ? end + 1 : 0;
>-	end   = end << 24;
>-	*size = end - start;
>-
>-	val = readl(REG_DDR_CS1_BNDS);
>-	start = (val >> 16) << 24;
>-	end   = (val & 0xFFFF);
>-	end   = end ? end + 1 : 0;
>-	end   = end << 24;
>-	*size += end - start;
>+	*size = 0;
>+	do {
>+		ret = scmi_misc_ddrinfo(ddrc_id++, &ddr_info);
>+		if (ret) {
>+			/* if get DDR info failed, fall to default config */
>+			*size = PHYS_SDRAM_SIZE;
>+#ifdef PHYS_SDRAM_2_SIZE
>+			*size += PHYS_SDRAM_2_SIZE;
>+#endif

You dropped the method of directly reading REG_DDR_CS[0,1]_BNDS but switch
to using PHYS_SDRAM[x]_SIZE, and commit log does not mention it.

>+			return 0;
>+		} else {
>+			ddrc_num = ((ddr_info.attributes >> 16) & 0x3);
>+			start = ddr_info.starthigh;
>+			start <<= 32;
>+			start += ddr_info.startlow;
>+
>+			end = ddr_info.endhigh;
>+			end <<= 32;
>+			end += ddr_info.endlow;
>+
>+			*size += end + 1 - start;
>+
>+			debug("ddr info attr 0x%x, start 0x%x 0x%x, end 0x%x 0x%x, mts %u\n",
>+			      ddr_info.attributes, ddr_info.starthigh, ddr_info.startlow,
>+			      ddr_info.endhigh, ddr_info.endlow, ddr_info.mts);
>+		}
>+	} while (ddrc_id < ddrc_num);
>+
>+	/* SM reports total DDR size, need remove secure memory */
>+	*size -= PHYS_SDRAM - 0x80000000;

secure memory size is 2GB? Need to mention in commit log or at the place
of struct scmi_ddr_info_out definition about this behavior.

Regards
Peng