mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: "Alvin Šipraga" <ALSI@bang-olufsen.dk>,
	"Ahmad Fatoum" <a.fatoum@pengutronix.de>
Subject: [PATCH] ddr: imx8m: fix broken sharing of DRAM timing with TF-A for DFS
Date: Mon, 29 Apr 2024 16:16:10 +0200	[thread overview]
Message-ID: <20240429141610.3556452-1-a.fatoum@pengutronix.de> (raw)

To facilitate dynamic frequency scaling, barebox will place the DRAM
parameters it has configured into a well-known location in DRAM
for consumption by TF-A.

This means that we need to be very picky about the struct layout for the
data we write there to maintain ABI-compatibility. Unfortunately, NXP
chose for the i.MX9 to add two fields into the middle, when it would
have been possible to cleanly embed the i.MX8 DRAM timings in the
i.MX9 DRAM timing struct. This is done now, so we need to arrange
ourselves with it and ensure that the i.MX8M DRAM timings we share with
TF-A don't have glimpses of the i.MX9 future.

As templating dram_config_save using the preprocessor would be
excessively ugly, we add a fixup that turns the common dram_timing_info
format into the format expected by TF-A on the i.MX8M family of SoCs.

Fixes: e6234f907416 ("ddr: Initial i.MX9 support")
Reported-by: Alvin Šipraga <ALSI@bang-olufsen.dk>
Tested-by: Alvin Šipraga <ALSI@bang-olufsen.dk>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/ddr/imx/imx8m_ddr_init.c |  7 ++++
 include/soc/imx/ddr.h            | 62 +++++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/drivers/ddr/imx/imx8m_ddr_init.c b/drivers/ddr/imx/imx8m_ddr_init.c
index d9a5d589f27b..c16e04d27483 100644
--- a/drivers/ddr/imx/imx8m_ddr_init.c
+++ b/drivers/ddr/imx/imx8m_ddr_init.c
@@ -481,6 +481,7 @@ static void ddrphy_init_set_dfi_clk(struct dram_controller *dram, unsigned int d
 
 int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_timing)
 {
+	struct dram_timing_info *saved_timing;
 	unsigned long src_ddrc_rcr = MX8M_SRC_DDRC_RCR_ADDR;
 	unsigned int tmp, initial_drate, target_freq;
 	int ret;
@@ -644,5 +645,11 @@ int imx8m_ddr_init(struct dram_controller *dram, struct dram_timing_info *dram_t
 	/* save the dram timing config into memory */
 	dram_config_save(dram, dram_timing, IMX8M_SAVED_DRAM_TIMING_BASE);
 
+	saved_timing = (struct dram_timing_info *)IMX8M_SAVED_DRAM_TIMING_BASE;
+
+	/* There's no fsp_cfg for i.MX8, so we must close the gap again */
+	memmove(&saved_timing->fsp_config, &saved_timing->common_config,
+		sizeof(saved_timing->common_config));
+
 	return 0;
 }
diff --git a/include/soc/imx/ddr.h b/include/soc/imx/ddr.h
index 0225ac0e033c..caf33d3e34b9 100644
--- a/include/soc/imx/ddr.h
+++ b/include/soc/imx/ddr.h
@@ -6,6 +6,8 @@
 #ifndef __SOC_IMX_DDR_H
 #define __SOC_IMX_DDR_H
 
+#include <linux/stddef.h>
+
 /* user data type */
 enum fw_type {
 	FW_1D_IMAGE,
@@ -58,27 +60,47 @@ struct dram_fsp_msg {
 	__attribute__((deprecated("board-specific data here is ignored in favor of the defaults." \
 				  " You can probably remove the array")))
 
+/*
+ * The DRAM timing values are machine-generated by an NXP tool.
+ * This is evaluated by barebox to initialize and train the DDR and
+ * is then shared with TF-A to enable dynamic frequency scaling.
+ *
+ * To minimize both duplicate code and manual fixups needed for the
+ * NXP tool output, we use a struct definition that's a superset
+ * of both of i.MX8M and i.MX9. This means that each SoC type making
+ * use of this struct needs to translate it to the appropriate format
+ * expected by TF-A.
+ *
+ * i.e: Do not change the struct definition without adjusting
+ * dram_config_save and its callers to maintain ABI-compatibility.
+ */
 struct dram_timing_info {
-	/* umctl2 config */
-	struct dram_cfg_param *ddrc_cfg;
-	unsigned int ddrc_cfg_num;
-	/* fsp config */
-	struct dram_fsp_cfg *fsp_cfg;
-	unsigned int fsp_cfg_num;
-	/* ddrphy config */
-	struct dram_cfg_param *ddrphy_cfg;
-	unsigned int ddrphy_cfg_num;
-	/* ddr fsp train info */
-	struct dram_fsp_msg *fsp_msg;
-	unsigned int fsp_msg_num;
-	/* ddr phy trained CSR */
-	struct dram_cfg_param *ddrphy_trained_csr __deprecated_dram_timing_info;
-	unsigned int ddrphy_trained_csr_num __deprecated_dram_timing_info;
-	/* ddr phy PIE */
-	struct dram_cfg_param *ddrphy_pie;
-	unsigned int ddrphy_pie_num;
-	/* initialized drate table */
-	unsigned int fsp_table[4];
+	struct_group(umctl2_config,
+		struct dram_cfg_param *ddrc_cfg;
+		unsigned int ddrc_cfg_num;
+	);
+
+	struct_group(fsp_config, /* i.MX9-only */
+		struct dram_fsp_cfg *fsp_cfg;
+		unsigned int fsp_cfg_num;
+	);
+
+	struct_group(common_config,
+		/* ddrphy config */
+		struct dram_cfg_param *ddrphy_cfg;
+		unsigned int ddrphy_cfg_num;
+		/* ddr fsp train info */
+		struct dram_fsp_msg *fsp_msg;
+		unsigned int fsp_msg_num;
+		/* ddr phy trained CSR */
+		struct dram_cfg_param *ddrphy_trained_csr __deprecated_dram_timing_info;
+		unsigned int ddrphy_trained_csr_num __deprecated_dram_timing_info;
+		/* ddr phy PIE */
+		struct dram_cfg_param *ddrphy_pie;
+		unsigned int ddrphy_pie_num;
+		/* initialized drate table */
+		unsigned int fsp_table[4];
+	);
 };
 
 struct dram_controller {
-- 
2.39.2




             reply	other threads:[~2024-04-29 14:17 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-29 14:16 Ahmad Fatoum [this message]
2024-05-03  7:46 ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240429141610.3556452-1-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=ALSI@bang-olufsen.dk \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox