[PATCH 0/4] Minor i.MX DDR init clean-up

[PATCH 0/4] Minor i.MX DDR init clean-up

[Philip Molloy]

This series makes minor clean-up changes to the i.MX DDR init code.

Unfortunately, I don't have access to any NXP hardware. I ran into LTO
issues with my Fedora toolchain building imx8mm_evk_defconfig. And
binman issues because of missing binary blobs with some imx9*
defconfigs. But compilation was successful.

To support an upcoming ADI SoC that also uses LPDDR4 and Synpsys IP I
would like to leverage the i.MX implementation. Therefore this clean-up
can also serve as a kind of pre-RFC. I would love to know if the
maintainers are open to transitioning to a more generic implementation.
If so, I'll submit a proper RFC. I'll also be at Embedded World next
week if anyone would like to discuss in person.

Some additional context and a comparison of DDR init implementations:

https://philipmolloy.com/ddr-init.html

I reviewed the NXP i.MX downstream repo on Github.[1] The changes in
this series are not fixed downstream. The only major difference with
downtream for the PHY driver appears to be "Quickboot" support, which is
specific to the i.MX 95.

[1]: https://github.com/nxp-imx/uboot-imx

Philip Molloy (4):
  ddr: imx: Remove duplicate includes
  ddr: imx: Remove undefined declarations
  ddr: imx: Remove unused macros
  ddr: imx: Remove unnecessary macro argument

 arch/arm/include/asm/arch-imx8m/ddr.h | 10 ++++------
 arch/arm/include/asm/arch-imx9/ddr.h  |  8 ++------
 drivers/ddr/imx/imx8m/ddr_init.c      |  8 ++++----
 drivers/ddr/imx/phy/ddrphy_utils.c    | 15 +++++++--------
 drivers/ddr/imx/phy/helper.c          |  4 +---
 5 files changed, 18 insertions(+), 27 deletions(-)

-- 
2.53.0

base-commit: 7995bf8dea2d5b3eb7fcb836636f4773924ec35d
branch: ddr-imx-clean-up

[PATCH 1/4] ddr: imx: Remove duplicate includes

[Philip Molloy]

From: Philip Molloy <philip.molloy@analog.com>

Signed-off-by: Philip Molloy <philip.molloy@analog.com>

---

 drivers/ddr/imx/phy/ddrphy_utils.c | 1 -
 drivers/ddr/imx/phy/helper.c       | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/ddr/imx/phy/ddrphy_utils.c b/drivers/ddr/imx/phy/ddrphy_utils.c
index 8e350de8315..b406418840c 100644
--- a/drivers/ddr/imx/phy/ddrphy_utils.c
+++ b/drivers/ddr/imx/phy/ddrphy_utils.c
@@ -6,7 +6,6 @@
 #include <errno.h>
 #include <log.h>
 #include <asm/io.h>
-#include <asm/arch/ddr.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/ddr.h>
 #include <asm/arch/sys_proto.h>
diff --git a/drivers/ddr/imx/phy/helper.c b/drivers/ddr/imx/phy/helper.c
index b0dfc3a0b4f..7b590ef397b 100644
--- a/drivers/ddr/imx/phy/helper.c
+++ b/drivers/ddr/imx/phy/helper.c
@@ -9,8 +9,6 @@
 #include <asm/global_data.h>
 #include <asm/io.h>
 #include <errno.h>
-#include <asm/io.h>
-#include <asm/arch/ddr.h>
 #include <asm/arch/ddr.h>
 #include <asm/sections.h>
 
-- 
2.53.0

[PATCH 2/4] ddr: imx: Remove undefined declarations

[Philip Molloy]

From: Philip Molloy <philip.molloy@analog.com>

Signed-off-by: Philip Molloy <philip.molloy@analog.com>
---

 arch/arm/include/asm/arch-imx8m/ddr.h | 1 -
 arch/arm/include/asm/arch-imx9/ddr.h  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h b/arch/arm/include/asm/arch-imx8m/ddr.h
index 5092ccae188..bf2d1f6bc40 100644
--- a/arch/arm/include/asm/arch-imx8m/ddr.h
+++ b/arch/arm/include/asm/arch-imx8m/ddr.h
@@ -707,7 +707,6 @@ extern struct dram_timing_info dram_timing;
 void ddr_load_train_firmware(enum fw_type type);
 int ddr_init(struct dram_timing_info *timing_info);
 int ddr_cfg_phy(struct dram_timing_info *timing_info);
-void load_lpddr4_phy_pie(void);
 void ddrphy_trained_csr_save(struct dram_cfg_param *param, unsigned int num);
 void *dram_config_save(struct dram_timing_info *info, unsigned long base);
 void board_dram_ecc_scrub(void);
diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
index a8e3f7354c7..f6ae3baf3b0 100644
--- a/arch/arm/include/asm/arch-imx9/ddr.h
+++ b/arch/arm/include/asm/arch-imx9/ddr.h
@@ -103,7 +103,6 @@ extern struct dram_timing_info dram_timing;
 void ddr_load_train_firmware(enum fw_type type);
 int ddr_init(struct dram_timing_info *timing_info);
 int ddr_cfg_phy(struct dram_timing_info *timing_info);
-void load_lpddr4_phy_pie(void);
 void ddrphy_trained_csr_save(struct dram_cfg_param *param, unsigned int num);
 void *dram_config_save(struct dram_timing_info *info, unsigned long base);
 void board_dram_ecc_scrub(void);
-- 
2.53.0

[PATCH 3/4] ddr: imx: Remove unused macros

[Philip Molloy]

From: Philip Molloy <philip.molloy@analog.com>

Signed-off-by: Philip Molloy <philip.molloy@analog.com>
---

 arch/arm/include/asm/arch-imx8m/ddr.h | 1 -
 arch/arm/include/asm/arch-imx9/ddr.h  | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h b/arch/arm/include/asm/arch-imx8m/ddr.h
index bf2d1f6bc40..47086d6b9f1 100644
--- a/arch/arm/include/asm/arch-imx8m/ddr.h
+++ b/arch/arm/include/asm/arch-imx8m/ddr.h
@@ -13,7 +13,6 @@
 #define DDRC_DDR_SS_GPR0		0x3d000000
 #define DDRC_IPS_BASE_ADDR_0		0x3f400000
 #define IP2APB_DDRPHY_IPS_BASE_ADDR(X)	(0x3c000000 + (X * 0x2000000))
-#define DDRPHY_MEM(X)			(0x3c000000 + (X * 0x2000000) + 0x50000)
 
 struct ddrc_freq {
 	u32 res0[8];
diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
index f6ae3baf3b0..6c2069b9707 100644
--- a/arch/arm/include/asm/arch-imx9/ddr.h
+++ b/arch/arm/include/asm/arch-imx9/ddr.h
@@ -35,7 +35,6 @@
 #define REG_SRC_DPHY_SINGLE_RESET_SW_CTRL	(SRC_DPHY_BASE_ADDR + 0x24)
 
 #define IP2APB_DDRPHY_IPS_BASE_ADDR(X)	(DDR_PHY_BASE + ((X) * 0x2000000))
-#define DDRPHY_MEM(X)			(DDR_PHY_BASE + ((X) * 0x2000000) + 0x50000)
 
 /* PHY State */
 enum pstate {
-- 
2.53.0

[PATCH 4/4] ddr: imx: Remove unnecessary macro argument

[Philip Molloy]

From: Philip Molloy <philip.molloy@analog.com>

IP2APB_DDRPHY_IPS_BASE_ADDR() is always passed 0, which is then
multiplied by 0x2000000. Since that results in adding 0 to the base
address, remove 0x2000000 and the argument entirely. Instead use
DDR_PHY_BASE, which was already defined in imx9. Add the missing
definition to imx8m.

Signed-off-by: Philip Molloy <philip.molloy@analog.com>
---

 arch/arm/include/asm/arch-imx8m/ddr.h |  8 ++++----
 arch/arm/include/asm/arch-imx9/ddr.h  |  6 ++----
 drivers/ddr/imx/imx8m/ddr_init.c      |  8 ++++----
 drivers/ddr/imx/phy/ddrphy_utils.c    | 14 +++++++-------
 drivers/ddr/imx/phy/helper.c          |  2 +-
 5 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h b/arch/arm/include/asm/arch-imx8m/ddr.h
index 47086d6b9f1..63c3aa682aa 100644
--- a/arch/arm/include/asm/arch-imx8m/ddr.h
+++ b/arch/arm/include/asm/arch-imx8m/ddr.h
@@ -10,9 +10,9 @@
 #include <asm/types.h>
 #include <asm/arch/imx-regs.h>
 
+#define DDR_PHY_BASE			0x3c000000
 #define DDRC_DDR_SS_GPR0		0x3d000000
 #define DDRC_IPS_BASE_ADDR_0		0x3f400000
-#define IP2APB_DDRPHY_IPS_BASE_ADDR(X)	(0x3c000000 + (X * 0x2000000))
 
 struct ddrc_freq {
 	u32 res0[8];
@@ -635,7 +635,7 @@ enum msg_response {
 #define DDRC_DFITMG3_SHADOW(X)         (DDRC_IPS_BASE_ADDR(X) + 0x21b8)
 #define DDRC_ODTCFG_SHADOW(X)          (DDRC_IPS_BASE_ADDR(X) + 0x2240)
 
-#define DDRPHY_CalBusy(X) (IP2APB_DDRPHY_IPS_BASE_ADDR(X) + 4 * 0x020097)
+#define DDRPHY_CalBusy (DDR_PHY_BASE + 4 * 0x020097)
 
 #define DRC_PERF_MON_BASE_ADDR(X)            (0x3d800000 + ((X) * 0x2000000))
 #define DRC_PERF_MON_CNT0_CTL(X)             (DRC_PERF_MON_BASE_ADDR(X) + 0x0)
@@ -741,8 +741,8 @@ static inline void reg32setbit(unsigned long addr, u32 bit)
 }
 
 #define dwc_ddrphy_apb_wr(addr, data) \
-	reg32_write(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(addr), data)
+	reg32_write(DDR_PHY_BASE + ddrphy_addr_remap(addr), data)
 #define dwc_ddrphy_apb_rd(addr) \
-	reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(addr))
+	reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(addr))
 
 #endif
diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
index 6c2069b9707..b6090a20a4d 100644
--- a/arch/arm/include/asm/arch-imx9/ddr.h
+++ b/arch/arm/include/asm/arch-imx9/ddr.h
@@ -34,8 +34,6 @@
 #define REG_SRC_DPHY_SW_CTRL		(SRC_DPHY_BASE_ADDR + 0x20)
 #define REG_SRC_DPHY_SINGLE_RESET_SW_CTRL	(SRC_DPHY_BASE_ADDR + 0x24)
 
-#define IP2APB_DDRPHY_IPS_BASE_ADDR(X)	(DDR_PHY_BASE + ((X) * 0x2000000))
-
 /* PHY State */
 enum pstate {
 	PS0,
@@ -136,8 +134,8 @@ static inline void reg32setbit(unsigned long addr, u32 bit)
 }
 
 #define dwc_ddrphy_apb_wr(addr, data) \
-	reg32_write(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(addr), data)
+	reg32_write(DDR_PHY_BASE + ddrphy_addr_remap(addr), data)
 #define dwc_ddrphy_apb_rd(addr) \
-	reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(addr))
+	reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(addr))
 
 #endif
diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index e9209ce8b61..39434334205 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -168,13 +168,13 @@ void get_trained_CDD(u32 fsp)
 	ddr_type = reg32_read(DDRC_MSTR(0)) & 0x3f;
 	if (ddr_type == 0x20) {
 		for (i = 0; i < 6; i++) {
-			tmp = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + (0x54013 + i) * 4);
+			tmp = reg32_read(DDR_PHY_BASE + (0x54013 + i) * 4);
 			cdd_cha[i * 2] = tmp & 0xff;
 			cdd_cha[i * 2 + 1] = (tmp >> 8) & 0xff;
 		}
 
 		for (i = 0; i < 7; i++) {
-			tmp = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + (0x5402c + i) * 4);
+			tmp = reg32_read(DDR_PHY_BASE + (0x5402c + i) * 4);
 			if (i == 0) {
 				cdd_cha[0] = (tmp >> 8) & 0xff;
 			} else if (i == 6) {
@@ -205,7 +205,7 @@ void get_trained_CDD(u32 fsp)
 		unsigned int ddr4_cdd[64];
 
 		for (i = 0; i < 29; i++) {
-			tmp = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + (0x54012 + i) * 4);
+			tmp = reg32_read(DDR_PHY_BASE + (0x54012 + i) * 4);
 			ddr4_cdd[i * 2] = tmp & 0xff;
 			ddr4_cdd[i * 2 + 1] = (tmp >> 8) & 0xff;
 		}
@@ -401,7 +401,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	 * calibrating. Wait Calibrating done.
 	 */
 	do {
-		tmp = reg32_read(DDRPHY_CalBusy(0));
+		tmp = reg32_read(DDRPHY_CalBusy);
 	} while ((tmp & 0x1));
 
 	debug("DDRINFO:ddrphy calibration done\n");
diff --git a/drivers/ddr/imx/phy/ddrphy_utils.c b/drivers/ddr/imx/phy/ddrphy_utils.c
index b406418840c..22cd091f6ee 100644
--- a/drivers/ddr/imx/phy/ddrphy_utils.c
+++ b/drivers/ddr/imx/phy/ddrphy_utils.c
@@ -15,7 +15,7 @@ static inline void poll_pmu_message_ready(void)
 	unsigned int reg;
 
 	do {
-		reg = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0004));
+		reg = reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(0xd0004));
 	} while (reg & 0x1);
 }
 
@@ -23,13 +23,13 @@ static inline void ack_pmu_message_receive(void)
 {
 	unsigned int reg;
 
-	reg32_write(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0031), 0x0);
+	reg32_write(DDR_PHY_BASE + ddrphy_addr_remap(0xd0031), 0x0);
 
 	do {
-		reg = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0004));
+		reg = reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(0xd0004));
 	} while (!(reg & 0x1));
 
-	reg32_write(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0031), 0x1);
+	reg32_write(DDR_PHY_BASE + ddrphy_addr_remap(0xd0031), 0x1);
 }
 
 static inline unsigned int get_mail(void)
@@ -38,7 +38,7 @@ static inline unsigned int get_mail(void)
 
 	poll_pmu_message_ready();
 
-	reg = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0032));
+	reg = reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(0xd0032));
 
 	ack_pmu_message_receive();
 
@@ -51,9 +51,9 @@ static inline unsigned int get_stream_message(void)
 
 	poll_pmu_message_ready();
 
-	reg = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0032));
+	reg = reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(0xd0032));
 
-	reg2 = reg32_read(IP2APB_DDRPHY_IPS_BASE_ADDR(0) + ddrphy_addr_remap(0xd0034));
+	reg2 = reg32_read(DDR_PHY_BASE + ddrphy_addr_remap(0xd0034));
 
 	reg2 = (reg2 << 16) | reg;
 
diff --git a/drivers/ddr/imx/phy/helper.c b/drivers/ddr/imx/phy/helper.c
index 7b590ef397b..080a6fa1c57 100644
--- a/drivers/ddr/imx/phy/helper.c
+++ b/drivers/ddr/imx/phy/helper.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 #define IMEM_OFFSET_ADDR 0x00050000
 #define DMEM_OFFSET_ADDR 0x00054000
-#define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
+#define DDR_TRAIN_CODE_BASE_ADDR DDR_PHY_BASE
 
 binman_sym_declare(ulong, ddr_1d_imem_fw, image_pos);
 binman_sym_declare(ulong, ddr_1d_imem_fw, size);
-- 
2.53.0

Re: [PATCH 1/4] ddr: imx: Remove duplicate includes

[Francesco Dolcini]

On Fri, Mar 06, 2026 at 08:58:31AM +0000, Philip Molloy wrote:
> From: Philip Molloy <philip.molloy@analog.com>
> 
> Signed-off-by: Philip Molloy <philip.molloy@analog.com>

you need some commit message to explain the change, the title is not
enough

Re: [PATCH 0/4] Minor i.MX DDR init clean-up

[Peng Fan]

Hi Philip,

Thanks for your patches!

On Fri, Mar 06, 2026 at 08:58:27AM +0000, Philip Molloy wrote:
>This series makes minor clean-up changes to the i.MX DDR init code.
>
>Unfortunately, I don't have access to any NXP hardware. I ran into LTO
>issues with my Fedora toolchain building imx8mm_evk_defconfig. And

Would you please share the error logs?

>binman issues because of missing binary blobs with some imx9*
>defconfigs. But compilation was successful.

Right, the ddr binaries are out of tree.

>
>To support an upcoming ADI SoC that also uses LPDDR4 and Synpsys IP I
>would like to leverage the i.MX implementation. Therefore this clean-up
>can also serve as a kind of pre-RFC. I would love to know if the
>maintainers are open to transitioning to a more generic implementation.

If most code could be reused by ADI SoC, that is good to have a generic
implementation to avoid code duplication.

>If so, I'll submit a proper RFC. I'll also be at Embedded World next
>week if anyone would like to discuss in person.
>
>Some additional context and a comparison of DDR init implementations:
>
>https://philipmolloy.com/ddr-init.html
>
>I reviewed the NXP i.MX downstream repo on Github.[1] The changes in
>this series are not fixed downstream. The only major difference with
>downtream for the PHY driver appears to be "Quickboot" support, which is

Yes, this is being prepared in internal reviewing stage before posting out,
should be soon.

>specific to the i.MX 95.

i.MX9[5,4,52] as of now.

Regards
Peng

>
>[1]: https://github.com/nxp-imx/uboot-imx
>
>Philip Molloy (4):
>  ddr: imx: Remove duplicate includes
>  ddr: imx: Remove undefined declarations
>  ddr: imx: Remove unused macros
>  ddr: imx: Remove unnecessary macro argument
>
> arch/arm/include/asm/arch-imx8m/ddr.h | 10 ++++------
> arch/arm/include/asm/arch-imx9/ddr.h  |  8 ++------
> drivers/ddr/imx/imx8m/ddr_init.c      |  8 ++++----
> drivers/ddr/imx/phy/ddrphy_utils.c    | 15 +++++++--------
> drivers/ddr/imx/phy/helper.c          |  4 +---
> 5 files changed, 18 insertions(+), 27 deletions(-)
>
>-- 
>2.53.0
>
>base-commit: 7995bf8dea2d5b3eb7fcb836636f4773924ec35d
>branch: ddr-imx-clean-up
>

Re: [PATCH 0/4] Minor i.MX DDR init clean-up

[Philip Molloy]

Hi Peng,

Thanks so much for the quick reply!

On Monday, March 9th, 2026 at 6:22 AM, Peng Fan <peng.fan@oss.nxp.com> wrote:
> Hi Philip,
> 
> Thanks for your patches!
> 
> On Fri, Mar 06, 2026 at 08:58:27AM +0000, Philip Molloy wrote:
> >This series makes minor clean-up changes to the i.MX DDR init code.
> >
> >Unfortunately, I don't have access to any NXP hardware. I ran into LTO
> >issues with my Fedora toolchain building imx8mm_evk_defconfig. And
> 
> Would you please share the error logs?

Digging a bit deeper now I came across the following Fedora issue:

https://bugzilla.redhat.com/show_bug.cgi?id=2357465

> >binman issues because of missing binary blobs with some imx9*
> >defconfigs. But compilation was successful.
> 
> Right, the ddr binaries are out of tree.
> 
> >To support an upcoming ADI SoC that also uses LPDDR4 and Synpsys IP I
> >would like to leverage the i.MX implementation. Therefore this clean-up
> >can also serve as a kind of pre-RFC. I would love to know if the
> >maintainers are open to transitioning to a more generic implementation.
> 
> If most code could be reused by ADI SoC, that is good to have a generic
> implementation to avoid code duplication.
 
Great to hear! Hopefully I can submit something before the end of April.