mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically
@ 2022-09-21 17:20 Marco Felsch
  2022-09-21 17:20 ` [PATCH 2/2] ARM: i.MX8M: remove fsp_table entry Marco Felsch
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Marco Felsch @ 2022-09-21 17:20 UTC (permalink / raw)
  To: barebox

Currently the fsp_table must be set manually within the 'struct
dram_timing_info'. Since the 'struct fsp_msg' already has all
information needed for the fsp_table we can use it to set it
automatically. This approach is less error-prone and avoids information
duplication.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/ddr/imx8m/ddrphy_train.c |  3 +++
 drivers/ddr/imx8m/helper.c       | 19 +++++++++++++++++++
 include/soc/imx8m/ddr.h          |  2 ++
 3 files changed, 24 insertions(+)

diff --git a/drivers/ddr/imx8m/ddrphy_train.c b/drivers/ddr/imx8m/ddrphy_train.c
index e9d35afdfb..09f1e77295 100644
--- a/drivers/ddr/imx8m/ddrphy_train.c
+++ b/drivers/ddr/imx8m/ddrphy_train.c
@@ -165,6 +165,9 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing, unsigned type)
 
 		dwc_ddrphy_apb_wr(0xd0000, 0x1);
 
+		/* Training done, add entry to fsp_table */
+		dram_write_fsp_table_entry(dram_timing, fsp_msg->drate);
+
 		fsp_msg++;
 	}
 
diff --git a/drivers/ddr/imx8m/helper.c b/drivers/ddr/imx8m/helper.c
index 98e4084958..e71401c1b2 100644
--- a/drivers/ddr/imx8m/helper.c
+++ b/drivers/ddr/imx8m/helper.c
@@ -18,6 +18,25 @@
 #define DMEM_OFFSET_ADDR 0x00054000
 #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
 
+void dram_write_fsp_table_entry(struct dram_timing_info *timing_info,
+				unsigned int data_rate)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(timing_info->fsp_table); i++) {
+		unsigned int entry = timing_info->fsp_table[i];
+
+		/* Skip already existing entries */
+		if (entry == data_rate)
+			return;
+
+		if (entry == 0) {
+			timing_info->fsp_table[i] = data_rate;
+			return;
+		}
+	}
+}
+
 void ddrphy_trained_csr_save(struct dram_cfg_param *ddrphy_csr,
 			     unsigned int num)
 {
diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
index 2149ae4325..222ab8e0f4 100644
--- a/include/soc/imx8m/ddr.h
+++ b/include/soc/imx8m/ddr.h
@@ -436,6 +436,8 @@ int ddr_cfg_phy(struct dram_timing_info *timing_info, enum ddrc_type ddrc_type);
 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 dram_write_fsp_table_entry(struct dram_timing_info *timing_info,
+				unsigned int data_rate);
 
 /* utils function for ddr phy training */
 int wait_ddrphy_training_complete(void);
-- 
2.30.2




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] ARM: i.MX8M: remove fsp_table entry
  2022-09-21 17:20 [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Marco Felsch
@ 2022-09-21 17:20 ` Marco Felsch
  2022-09-22  9:51 ` [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Sascha Hauer
  2022-09-26  6:01 ` Sascha Hauer
  2 siblings, 0 replies; 5+ messages in thread
From: Marco Felsch @ 2022-09-21 17:20 UTC (permalink / raw)
  To: barebox

Barebox now have the support to write the fsp_table entries
automatically based on fsp_msg.drata information. Remove the fsp_table
entry and let the common code do the rest for us.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/boards/mnt-reform/lpddr4-timing.c     | 1 -
 arch/arm/boards/nxp-imx8mn-evk/ddr4-timing.c   | 1 -
 arch/arm/boards/nxp-imx8mn-evk/lpddr4-timing.c | 1 -
 arch/arm/boards/nxp-imx8mp-evk/lpddr4-timing.c | 1 -
 4 files changed, 4 deletions(-)

diff --git a/arch/arm/boards/mnt-reform/lpddr4-timing.c b/arch/arm/boards/mnt-reform/lpddr4-timing.c
index 0b5853000d..38fa2c50f1 100644
--- a/arch/arm/boards/mnt-reform/lpddr4-timing.c
+++ b/arch/arm/boards/mnt-reform/lpddr4-timing.c
@@ -1008,5 +1008,4 @@ struct dram_timing_info mnt_reform_dram_timing = {
 	.fsp_msg_num = ARRAY_SIZE(mnt_reform_lpddr4_dram_fsp_msg),
 	.ddrphy_pie = mnt_reform_lpddr4_phy_pie,
 	.ddrphy_pie_num = ARRAY_SIZE(mnt_reform_lpddr4_phy_pie),
-	.fsp_table = { 3200, 667, },
 };
diff --git a/arch/arm/boards/nxp-imx8mn-evk/ddr4-timing.c b/arch/arm/boards/nxp-imx8mn-evk/ddr4-timing.c
index 131a63156e..df10bdcc9d 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/ddr4-timing.c
+++ b/arch/arm/boards/nxp-imx8mn-evk/ddr4-timing.c
@@ -1050,5 +1050,4 @@ struct dram_timing_info imx8mn_evk_ddr4_timing = {
 	.ddrphy_trained_csr_num = ARRAY_SIZE(ddr_ddrphy_trained_csr),
 	.ddrphy_pie = ddr_phy_pie,
 	.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
-	.fsp_table = { 2400, 1066, },
 };
diff --git a/arch/arm/boards/nxp-imx8mn-evk/lpddr4-timing.c b/arch/arm/boards/nxp-imx8mn-evk/lpddr4-timing.c
index 940b21cedb..63851c87d4 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/lpddr4-timing.c
+++ b/arch/arm/boards/nxp-imx8mn-evk/lpddr4-timing.c
@@ -1586,5 +1586,4 @@ struct dram_timing_info imx8mn_evk_lpddr4_timing = {
 	.ddrphy_trained_csr_num = ARRAY_SIZE(ddr_ddrphy_trained_csr),
 	.ddrphy_pie = ddr_phy_pie,
 	.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
-	.fsp_table = { 3200, 400, 100, },
 };
diff --git a/arch/arm/boards/nxp-imx8mp-evk/lpddr4-timing.c b/arch/arm/boards/nxp-imx8mp-evk/lpddr4-timing.c
index 3028bc084c..b8c6b78017 100644
--- a/arch/arm/boards/nxp-imx8mp-evk/lpddr4-timing.c
+++ b/arch/arm/boards/nxp-imx8mp-evk/lpddr4-timing.c
@@ -1844,5 +1844,4 @@ struct dram_timing_info imx8mp_evk_dram_timing = {
 	.ddrphy_trained_csr_num = ARRAY_SIZE(ddr_ddrphy_trained_csr),
 	.ddrphy_pie = ddr_phy_pie,
 	.ddrphy_pie_num = ARRAY_SIZE(ddr_phy_pie),
-	.fsp_table = { 4000, 400, 100, },
 };
-- 
2.30.2




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically
  2022-09-21 17:20 [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Marco Felsch
  2022-09-21 17:20 ` [PATCH 2/2] ARM: i.MX8M: remove fsp_table entry Marco Felsch
@ 2022-09-22  9:51 ` Sascha Hauer
  2022-09-22  9:54   ` Marco Felsch
  2022-09-26  6:01 ` Sascha Hauer
  2 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2022-09-22  9:51 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

On Wed, Sep 21, 2022 at 07:20:51PM +0200, Marco Felsch wrote:
> Currently the fsp_table must be set manually within the 'struct
> dram_timing_info'. Since the 'struct fsp_msg' already has all
> information needed for the fsp_table we can use it to set it
> automatically. This approach is less error-prone and avoids information
> duplication.

Is this really worth it? The whole dram timing code comes directly from
the NXP DRAM tools, so the fsp_table comes back with each new board or
with a DRAM timing update.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically
  2022-09-22  9:51 ` [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Sascha Hauer
@ 2022-09-22  9:54   ` Marco Felsch
  0 siblings, 0 replies; 5+ messages in thread
From: Marco Felsch @ 2022-09-22  9:54 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 22-09-22, Sascha Hauer wrote:
> On Wed, Sep 21, 2022 at 07:20:51PM +0200, Marco Felsch wrote:
> > Currently the fsp_table must be set manually within the 'struct
> > dram_timing_info'. Since the 'struct fsp_msg' already has all
> > information needed for the fsp_table we can use it to set it
> > automatically. This approach is less error-prone and avoids information
> > duplication.
> 
> Is this really worth it? The whole dram timing code comes directly from
> the NXP DRAM tools, so the fsp_table comes back with each new board or
> with a DRAM timing update.

I had a problem with the missing fsp_table for the MX8MM-EVK and a buggy
TF-A. Therefore I would say yes, since it is not guaranteed that the
fsp_table is existing.

Regards,
  Marco



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically
  2022-09-21 17:20 [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Marco Felsch
  2022-09-21 17:20 ` [PATCH 2/2] ARM: i.MX8M: remove fsp_table entry Marco Felsch
  2022-09-22  9:51 ` [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Sascha Hauer
@ 2022-09-26  6:01 ` Sascha Hauer
  2 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2022-09-26  6:01 UTC (permalink / raw)
  To: Marco Felsch; +Cc: barebox

On Wed, Sep 21, 2022 at 07:20:51PM +0200, Marco Felsch wrote:
> Currently the fsp_table must be set manually within the 'struct
> dram_timing_info'. Since the 'struct fsp_msg' already has all
> information needed for the fsp_table we can use it to set it
> automatically. This approach is less error-prone and avoids information
> duplication.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  drivers/ddr/imx8m/ddrphy_train.c |  3 +++
>  drivers/ddr/imx8m/helper.c       | 19 +++++++++++++++++++
>  include/soc/imx8m/ddr.h          |  2 ++
>  3 files changed, 24 insertions(+)
> 
> diff --git a/drivers/ddr/imx8m/ddrphy_train.c b/drivers/ddr/imx8m/ddrphy_train.c
> index e9d35afdfb..09f1e77295 100644
> --- a/drivers/ddr/imx8m/ddrphy_train.c
> +++ b/drivers/ddr/imx8m/ddrphy_train.c
> @@ -165,6 +165,9 @@ int ddr_cfg_phy(struct dram_timing_info *dram_timing, unsigned type)
>  
>  		dwc_ddrphy_apb_wr(0xd0000, 0x1);
>  
> +		/* Training done, add entry to fsp_table */
> +		dram_write_fsp_table_entry(dram_timing, fsp_msg->drate);
> +
>  		fsp_msg++;
>  	}
>  
> diff --git a/drivers/ddr/imx8m/helper.c b/drivers/ddr/imx8m/helper.c
> index 98e4084958..e71401c1b2 100644
> --- a/drivers/ddr/imx8m/helper.c
> +++ b/drivers/ddr/imx8m/helper.c
> @@ -18,6 +18,25 @@
>  #define DMEM_OFFSET_ADDR 0x00054000
>  #define DDR_TRAIN_CODE_BASE_ADDR IP2APB_DDRPHY_IPS_BASE_ADDR(0)
>  
> +void dram_write_fsp_table_entry(struct dram_timing_info *timing_info,
> +				unsigned int data_rate)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(timing_info->fsp_table); i++) {
> +		unsigned int entry = timing_info->fsp_table[i];
> +
> +		/* Skip already existing entries */
> +		if (entry == data_rate)
> +			return;
> +
> +		if (entry == 0) {
> +			timing_info->fsp_table[i] = data_rate;
> +			return;
> +		}
> +	}
> +}

The input struct dram_timing_info * comes directly from the boards and
should be treated as const. If you go that route please parse the
.fsp_msg data directly in dram_config_save().

Another way could be to just check the .fsp_msg and .fsp_table for
consistency and warn about it if necessary.
In the end the input data from the boards is redundant here. We are
not going to change the input data format as that comes from the NXP
DRAM tools. Just ignoring the .fsp_table like you do here might be
confusing as well.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-26  6:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 17:20 [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Marco Felsch
2022-09-21 17:20 ` [PATCH 2/2] ARM: i.MX8M: remove fsp_table entry Marco Felsch
2022-09-22  9:51 ` [PATCH 1/2] ddr: imx8m: write fsp_table entry automatically Sascha Hauer
2022-09-22  9:54   ` Marco Felsch
2022-09-26  6:01 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox