[PATCH 1/4] ddr: imx: Reload the training firmware for every PHY configuration

[PATCH 1/4] ddr: imx: Reload the training firmware for every PHY configuration

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

ddr_load_train_firmware() remembers the type of the last loaded firmware
image in a static variable and returns early if the same type is
requested again. That is valid within a single ddr_cfg_phy() call, where
the 1D image is used for several frequency setpoints in a row, but the
state also survives across calls.

Boards that probe several DDR configurations call ddr_init() more than
once. The power up procedure at the beginning of ddr_init() resets the
DDR PHY, which leaves the image in the PHY memory in an undefined state.
The second call then skips the loading, starts the PMU on whatever is
left of it and the training firmware never reports a result. As the PHY
also stops answering on its APB interface at that point, the boot hangs
in a register read, which no software timeout can recover from.

On some specific Kontron SL i.MX8MM with 1GB or 2GB DDR, where the first
init is expected to fail and the second one uses an adjusted
configuration, this made the boot hang every few cycles.

Move the check into ddr_cfg_phy(), the only caller, so that the loading
is still skipped for repeated setpoints of one run, but never across
runs.

Fixes: b614ddb5d335 ("ddr: imx: Save the FW loading if it hasn't changed")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/ddr/imx/phy/ddrphy_train.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/ddr/imx/phy/ddrphy_train.c b/drivers/ddr/imx/phy/ddrphy_train.c
index 1a2d071d6f1..63a6ca800a3 100644
--- a/drivers/ddr/imx/phy/ddrphy_train.c
+++ b/drivers/ddr/imx/phy/ddrphy_train.c
@@ -12,6 +12,7 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing)
 {
 	struct dram_cfg_param *dram_cfg;
 	struct dram_fsp_msg *fsp_msg;
+	int last_fw_type = -1;
 	unsigned int num;
 	int i = 0;
 	int j = 0;
@@ -33,9 +34,18 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing)
 		/* set dram PHY input clocks to desired frequency */
 		ddrphy_init_set_dfi_clk(fsp_msg->drate);
 
-		/* load the dram training firmware image */
+		/*
+		 * Load the DRAM training firmware image, unless the same image has
+		 * already been loaded for an earlier frequency setpoint of this run.
+		 * It must be loaded again for every ddr_cfg_phy() call as the caller
+		 * resets the PHY before this, which leaves the image in the PHY memory
+		 * in an undefined state.
+		 */
 		dwc_ddrphy_apb_wr(0xd0000, 0x0);
-		ddr_load_train_firmware(fsp_msg->fw_type);
+		if (fsp_msg->fw_type != last_fw_type) {
+			ddr_load_train_firmware(fsp_msg->fw_type);
+			last_fw_type = fsp_msg->fw_type;
+		}
 
 		/* load the frequency set point message block parameter */
 		dram_cfg = fsp_msg->fsp_cfg;
-- 
2.55.0

[PATCH 2/4] ddr: imx: Move error message for failed training to calling side

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

Move the error message from wait_ddrphy_training_complete() to the
calling site in ddr_cfg_phy(). This is more robust to future changes
in wait_ddrphy_training_complete() and allows us to make the message
optional.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 drivers/ddr/imx/phy/ddrphy_train.c | 4 +++-
 drivers/ddr/imx/phy/ddrphy_utils.c | 1 -
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/ddr/imx/phy/ddrphy_train.c b/drivers/ddr/imx/phy/ddrphy_train.c
index 63a6ca800a3..68468610fd6 100644
--- a/drivers/ddr/imx/phy/ddrphy_train.c
+++ b/drivers/ddr/imx/phy/ddrphy_train.c
@@ -72,8 +72,10 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing)
 
 		/* Wait for the training firmware to complete */
 		ret = wait_ddrphy_training_complete();
-		if (ret)
+		if (ret) {
+			printf("Training FAILED\n");
 			return ret;
+		}
 
 		/* Halt the microcontroller. */
 		dwc_ddrphy_apb_wr(0xd0099, 0x1);
diff --git a/drivers/ddr/imx/phy/ddrphy_utils.c b/drivers/ddr/imx/phy/ddrphy_utils.c
index 8e350de8315..8779db01607 100644
--- a/drivers/ddr/imx/phy/ddrphy_utils.c
+++ b/drivers/ddr/imx/phy/ddrphy_utils.c
@@ -97,7 +97,6 @@ int wait_ddrphy_training_complete(void)
 			debug("Training PASS\n");
 			return 0;
 		} else if (mail == 0xff) {
-			printf("Training FAILED\n");
 			return -1;
 		}
 	}
-- 
2.55.0

[PATCH 3/4] ddr: imx: Allow to call ddr init without logging failure

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

In some cases we need to call ddr_init() multiple times to try
different configurations. In this case failures are expected and
error messages cause confusion. Introduce ddr_init_silent_fail()
for those cases.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 arch/arm/include/asm/arch-imx8m/ddr.h |  4 +++-
 arch/arm/include/asm/arch-imx9/ddr.h  |  4 +++-
 drivers/ddr/imx/imx8m/ddr_init.c      | 14 ++++++++++++--
 drivers/ddr/imx/imx9/ddr_init.c       | 14 ++++++++++++--
 drivers/ddr/imx/phy/ddrphy_train.c    |  5 +++--
 5 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/arch/arm/include/asm/arch-imx8m/ddr.h b/arch/arm/include/asm/arch-imx8m/ddr.h
index 5092ccae188..442c45237f9 100644
--- a/arch/arm/include/asm/arch-imx8m/ddr.h
+++ b/arch/arm/include/asm/arch-imx8m/ddr.h
@@ -9,6 +9,7 @@
 #include <asm/io.h>
 #include <asm/types.h>
 #include <asm/arch/imx-regs.h>
+#include <linux/types.h>
 
 #define DDRC_DDR_SS_GPR0		0x3d000000
 #define DDRC_IPS_BASE_ADDR_0		0x3f400000
@@ -705,8 +706,9 @@ struct dram_timing_info {
 extern struct dram_timing_info dram_timing;
 
 void ddr_load_train_firmware(enum fw_type type);
+int ddr_init_silent_fail(struct dram_timing_info *timing_info);
 int ddr_init(struct dram_timing_info *timing_info);
-int ddr_cfg_phy(struct dram_timing_info *timing_info);
+int ddr_cfg_phy(struct dram_timing_info *timing_info, bool log_fail);
 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);
diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
index b0f90b53f64..c85f4cc7810 100644
--- a/arch/arm/include/asm/arch-imx9/ddr.h
+++ b/arch/arm/include/asm/arch-imx9/ddr.h
@@ -8,6 +8,7 @@
 
 #include <asm/io.h>
 #include <asm/types.h>
+#include <linux/types.h>
 
 #define DDR_CTL_BASE			0x4E300000
 #define DDR_PHY_BASE			0x4E100000
@@ -147,8 +148,9 @@ struct ddrphy_qb_state {
 };
 
 void ddr_load_train_firmware(enum fw_type type);
+int ddr_init_silent_fail(struct dram_timing_info *timing_info);
 int ddr_init(struct dram_timing_info *timing_info);
-int ddr_cfg_phy(struct dram_timing_info *timing_info);
+int ddr_cfg_phy(struct dram_timing_info *timing_info, bool log_fail);
 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);
diff --git a/drivers/ddr/imx/imx8m/ddr_init.c b/drivers/ddr/imx/imx8m/ddr_init.c
index e9209ce8b61..537c36acc8d 100644
--- a/drivers/ddr/imx/imx8m/ddr_init.c
+++ b/drivers/ddr/imx/imx8m/ddr_init.c
@@ -310,7 +310,7 @@ void update_umctl2_rank_space_setting(unsigned int pstat_num)
 	}
 }
 
-int ddr_init(struct dram_timing_info *dram_timing)
+static int __ddr_init(struct dram_timing_info *dram_timing, bool log_fail)
 {
 	unsigned int tmp, initial_drate, target_freq;
 	int ret;
@@ -390,7 +390,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	 */
 	debug("DDRINFO:ddrphy config start\n");
 
-	ret = ddr_cfg_phy(dram_timing);
+	ret = ddr_cfg_phy(dram_timing, log_fail);
 	if (ret)
 		return ret;
 
@@ -470,6 +470,16 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	return 0;
 }
 
+int ddr_init(struct dram_timing_info *dram_timing)
+{
+	return __ddr_init(dram_timing, true);
+}
+
+int ddr_init_silent_fail(struct dram_timing_info *dram_timing)
+{
+	return __ddr_init(dram_timing, false);
+}
+
 ulong ddrphy_addr_remap(uint32_t paddr_apb_from_ctlr)
 {
 	return 4 * paddr_apb_from_ctlr;
diff --git a/drivers/ddr/imx/imx9/ddr_init.c b/drivers/ddr/imx/imx9/ddr_init.c
index 5b0ad773875..f2600c65703 100644
--- a/drivers/ddr/imx/imx9/ddr_init.c
+++ b/drivers/ddr/imx/imx9/ddr_init.c
@@ -335,7 +335,7 @@ void save_trained_mr12_14(struct dram_cfg_param *cfg, u32 cfg_num, u32 mr12, u32
 	}
 }
 
-int ddr_init(struct dram_timing_info *dram_timing)
+static int __ddr_init(struct dram_timing_info *dram_timing, bool log_fail)
 {
 	unsigned int initial_drate;
 	struct dram_timing_info *saved_timing;
@@ -361,7 +361,7 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	 */
 	debug("DDRINFO:ddrphy config start\n");
 
-	ret = ddr_cfg_phy(dram_timing);
+	ret = ddr_cfg_phy(dram_timing, log_fail);
 	if (ret)
 		return ret;
 
@@ -412,6 +412,16 @@ int ddr_init(struct dram_timing_info *dram_timing)
 	return 0;
 }
 
+int ddr_init(struct dram_timing_info *dram_timing)
+{
+	return __ddr_init(dram_timing, true);
+}
+
+int ddr_init_silent_fail(struct dram_timing_info *dram_timing)
+{
+	return __ddr_init(dram_timing, false);
+}
+
 ulong ddrphy_addr_remap(u32 paddr_apb_from_ctlr)
 {
 	u32 paddr_apb_qual;
diff --git a/drivers/ddr/imx/phy/ddrphy_train.c b/drivers/ddr/imx/phy/ddrphy_train.c
index 68468610fd6..770a568d8f2 100644
--- a/drivers/ddr/imx/phy/ddrphy_train.c
+++ b/drivers/ddr/imx/phy/ddrphy_train.c
@@ -8,7 +8,7 @@
 #include <asm/arch/ddr.h>
 #include <asm/arch/sys_proto.h>
 
-int ddr_cfg_phy(struct dram_timing_info *dram_timing)
+int ddr_cfg_phy(struct dram_timing_info *dram_timing, bool log_fail)
 {
 	struct dram_cfg_param *dram_cfg;
 	struct dram_fsp_msg *fsp_msg;
@@ -73,7 +73,8 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing)
 		/* Wait for the training firmware to complete */
 		ret = wait_ddrphy_training_complete();
 		if (ret) {
-			printf("Training FAILED\n");
+			if (log_fail || _DEBUG)
+				printf("Training FAILED\n");
 			return ret;
 		}
 
-- 
2.55.0

[PATCH 4/4] imx: kontron-sl-mx8mm: Stop printing error message on first DDR init

[Frieder Schrempf]

From: Frieder Schrempf <frieder.schrempf@kontron.de>

The first try might fail and we are retrying for smaller DDR sizes
after that. Don't print an error message to not confuse the users
of 1GB and 2GB variants.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
---
 board/kontron/sl-mx8mm/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/kontron/sl-mx8mm/spl.c b/board/kontron/sl-mx8mm/spl.c
index a5cdd2ab6d1..ebb883f861a 100644
--- a/board/kontron/sl-mx8mm/spl.c
+++ b/board/kontron/sl-mx8mm/spl.c
@@ -77,7 +77,7 @@ static void spl_dram_init(void)
 	 * Try the default DDR settings in lpddr4_timing.c to
 	 * comply with the Micron 4GB DDR.
 	 */
-	if (!ddr_init(&dram_timing) && check_ram_available(SZ_4G)) {
+	if (!ddr_init_silent_fail(&dram_timing) && check_ram_available(SZ_4G)) {
 		size = 4;
 	} else {
 		/*
-- 
2.55.0