[PATCH master 0/4] ARM: i.MX8M: fix various miscalculation of DRAM size

[PATCH master 0/4] ARM: i.MX8M: fix various miscalculation of DRAM size

[Ahmad Fatoum]

This fixes issues of wrong DRAM size calculation for imx8mq-evk,
phytec-som-imx8mq, zii-imx8mq-dev as well as an out-of-tree 
i.MX8MM board I have.

For i.MX8MN-EVK, DDR4 size is correctly calculated, but not LPDDR4.
I can't see where the issue is though, so I am disabling detection
there until it's figured out. Device Tree already contains correct
RAM size.

Ahmad Fatoum (4):
  ARM: i.MX8M: esdctl: ignore ADDRMAP8 for non-DDR4
  ARM: i.MX8MQ: initialize ADDRMAP7
  ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7
  ARM: i.MX8M: imx8mn-evk: disable DDRC memory detection

 arch/arm/boards/nxp-imx8mn-evk/board.c       |  7 +++++++
 arch/arm/boards/nxp-imx8mq-evk/ddr_init.c    |  3 ++-
 arch/arm/boards/phytec-som-imx8mq/ddr_init.c |  1 +
 arch/arm/boards/zii-imx8mq-dev/ddr_init.c    |  3 ++-
 arch/arm/mach-imx/esdctl.c                   |  8 +++++++-
 drivers/ddr/imx8m/ddr_init.c                 | 18 ++++++++++++++++++
 drivers/ddr/imx8m/helper.c                   |  6 ++++++
 include/soc/imx8m/ddr.h                      |  1 +
 8 files changed, 44 insertions(+), 3 deletions(-)

-- 
2.30.2

[PATCH master 1/4] ARM: i.MX8M: esdctl: ignore ADDRMAP8 for non-DDR4

[Ahmad Fatoum]

ADDRMAP8 handling was added for i.MX8MN DDR4 handling,
but as done currently has a few issues:

  - Bank groups are a DDR4 feature (well, borrowed from GDDR5),
    so we should just skip it for other RAM types

  - addrmap[8] == 0 is actually a valid value according
    to both reference manual and spreadsheet

  - Spreadsheet claims DDRC_ADDRMAP8_BG_B0 to be 6-bit, while
    reference manual claims 5-bit

  - Spreadsheet claims DDRC_ADDRMAP8_BG_B0 == 63 to be the
    neutral value. The code assumes 31 and the reference manual
    describes all values 0-31 to have an effect.

This commit fixes the first two issues. The calculation may still
be wrong, but at least for the i.MX8MN-DDR4-EVK it seems to return
a correct value of 2G.

Fixes: 42d45ef380c5 ("ARM: imx: Add imx8 support for SDRAM with two or more bank groups")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-imx/esdctl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index b070ebc62a45..d7802e2a4d16 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -317,6 +317,7 @@ static int vf610_ddrmc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
 #define DDRC_ADDRMAP0_CS_BIT0			GENMASK(4, 0)
 
 #define DDRC_MSTR				0x0000
+#define DDRC_MSTR_DDR4				BIT(4)
 #define DDRC_MSTR_LPDDR4			BIT(5)
 #define DDRC_MSTR_DATA_BUS_WIDTH		GENMASK(13, 12)
 #define DDRC_MSTR_ACTIVE_RANKS			GENMASK(27, 24)
@@ -423,7 +424,12 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[DDRC_ADDRMAP_LENGTH],
 	if (FIELD_GET(DDRC_ADDRMAP1_BANK_B2, addrmap[1]) != 0b11111)
 		banks++;
 
-	if (addrmap[8]) {
+	if (is_imx8 && (mstr & DDRC_MSTR_DDR4)) {
+		/* FIXME: DDR register spreasheet claims this to be
+		 * 6-bit and 63 meaning bank group address bit 0 is 0,
+		 * but reference manual claims 5-bit without 'neutral' value
+		 * See MX8M_Mini_DDR4_RPA_v17, MX8M_Nano_DDR4_RPA_v8
+		 */
 		if (FIELD_GET(DDRC_ADDRMAP8_BG_B0, addrmap[8]) != 0b11111)
 			banks++;
 		if (FIELD_GET(DDRC_ADDRMAP8_BG_B1, addrmap[8]) != 0b111111)
-- 
2.30.2

[PATCH master 2/4] ARM: i.MX8MQ: initialize ADDRMAP7

[Ahmad Fatoum]

Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving it at its
POR default of zero. Now that barebox looks at ADDRMAP7 to be able to
correctly detect bigger memory sizes, barebox proper on boards with
older spreadsheets may read back 4x times as much RAM as actually
fitted.

MNT Reform LPDDR4 setup already writes 0xf0f (the neutral ignore-me
value for the register) into ADDRMAP7. Follow suit for the other
i.MX8MQ boards that don't. In-tree Non-i.MX8MQ boards aren't affected.
Out of tree boards might and will get a common workaround in a follow-up
commit. No workaround for out of tree i.MX8MQ boards.

Tested on i.MX8M-EVK (i.MX8MQuad), where now 3G are correctly detected
instead of 12G.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/boards/nxp-imx8mq-evk/ddr_init.c    | 3 ++-
 arch/arm/boards/phytec-som-imx8mq/ddr_init.c | 1 +
 arch/arm/boards/zii-imx8mq-dev/ddr_init.c    | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boards/nxp-imx8mq-evk/ddr_init.c b/arch/arm/boards/nxp-imx8mq-evk/ddr_init.c
index 39addea97320..b1f752c4cb20 100644
--- a/arch/arm/boards/nxp-imx8mq-evk/ddr_init.c
+++ b/arch/arm/boards/nxp-imx8mq-evk/ddr_init.c
@@ -81,6 +81,7 @@ void ddr_init(void)
 	reg32_write(0x3d400200,0x15);
 	reg32_write(0x3d40020c,0x0);
 	reg32_write(0x3d400210,0x1f1f);
+	reg32_write(0x3d40021c,0xf0f);
 	reg32_write(0x3d400204,0x80808);
 	reg32_write(0x3d400214,0x7070707);
 	reg32_write(0x3d400218,0x48080707);
@@ -222,4 +223,4 @@ void ddr_init(void)
 	/* enable DDR auto-refresh mode */
 	tmp = reg32_read(DDRC_RFSHCTL3(0)) & ~0x1;
 	reg32_write(DDRC_RFSHCTL3(0), tmp);
-}
\ No newline at end of file
+}
diff --git a/arch/arm/boards/phytec-som-imx8mq/ddr_init.c b/arch/arm/boards/phytec-som-imx8mq/ddr_init.c
index aa327d3fb0cb..c6812e3efaec 100644
--- a/arch/arm/boards/phytec-som-imx8mq/ddr_init.c
+++ b/arch/arm/boards/phytec-som-imx8mq/ddr_init.c
@@ -84,6 +84,7 @@ void ddr_init(void)
 	reg32_write(0x3d400204,0x80808);
 	reg32_write(0x3d400214,0x7070707);
 	reg32_write(0x3d400218,0xf070707);
+	reg32_write(0x3d40021c,0xf0f);
 	reg32_write(0x3d402020,0x1);
 	reg32_write(0x3d402024,0x518b00);
 	reg32_write(0x3d402050,0x20d040);
diff --git a/arch/arm/boards/zii-imx8mq-dev/ddr_init.c b/arch/arm/boards/zii-imx8mq-dev/ddr_init.c
index 7a955193fd7c..902d0ee3cd6e 100644
--- a/arch/arm/boards/zii-imx8mq-dev/ddr_init.c
+++ b/arch/arm/boards/zii-imx8mq-dev/ddr_init.c
@@ -81,6 +81,7 @@ void ddr_init(void)
 	reg32_write(0x3d400200,0x17);
 	reg32_write(0x3d40020c,0x0);
 	reg32_write(0x3d400210,0x1f1f);
+	reg32_write(0x3d40021c,0xf0f);
 	reg32_write(0x3d400204,0x80808);
 	reg32_write(0x3d400214,0x7070707);
 	reg32_write(0x3d400218,0x7070707);
@@ -222,4 +223,4 @@ void ddr_init(void)
 	/* enable DDR auto-refresh mode */
 	tmp = reg32_read(DDRC_RFSHCTL3(0)) & ~0x1;
 	reg32_write(DDRC_RFSHCTL3(0), tmp);
-}
\ No newline at end of file
+}
-- 
2.30.2

[PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Ahmad Fatoum]

Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving it at its
POR default of zero. Now that barebox looks at ADDRMAP7 to be able to
correctly detect bigger memory sizes, barebox proper on out-of-tree
boards with older spreadsheets may read back 4x times as much RAM
as actually fitted.

Work around this by writing a trailing 0xf0f (the neutral ignore-me
value for the register) if the register wasn't written through
dram_timing_info::ddrc_cfg. We consider this safe to do, because
the DDRC is held in reset while these values are programmed.

Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit SDRAM row size handle")
Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for imx8m")
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
 drivers/ddr/imx8m/helper.c   |  6 ++++++
 include/soc/imx8m/ddr.h      |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/ddr/imx8m/ddr_init.c b/drivers/ddr/imx8m/ddr_init.c
index ae05b136229c..9a4b4e2ca88a 100644
--- a/drivers/ddr/imx8m/ddr_init.c
+++ b/drivers/ddr/imx8m/ddr_init.c
@@ -13,14 +13,32 @@
 #include <mach/imx8m-regs.h>
 #include <mach/imx8m-ccm-regs.h>
 
+bool imx8m_ddr_old_spreadsheet = true;
+
 static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg, int num)
 {
 	int i = 0;
 
 	for (i = 0; i < num; i++) {
+		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
+		    imx8m_ddr_old_spreadsheet = false;
 		reg32_write((unsigned long)ddrc_cfg->reg, ddrc_cfg->val);
 		ddrc_cfg++;
 	}
+
+	/*
+	 * Older NXP DDR configuration spreadsheets don't initialize ADDRMAP7,
+	 * which falsifies the memory size read back from the controller
+	 * in barebox proper.
+	 */
+	if (imx8m_ddr_old_spreadsheet) {
+		pr_warn("Working around old spreadsheet. Please regenerate\n");
+		/*
+		 * Alternatively, stick { DDRC_ADDRMAP7(0), 0xf0f } into
+		 * struct dram_timing_info::ddrc_cfg of your old timing file
+		 */
+		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
+	}
 }
 
 /*
diff --git a/drivers/ddr/imx8m/helper.c b/drivers/ddr/imx8m/helper.c
index 94bbb811576d..98e40849584b 100644
--- a/drivers/ddr/imx8m/helper.c
+++ b/drivers/ddr/imx8m/helper.c
@@ -62,6 +62,12 @@ void dram_config_save(struct dram_timing_info *timing_info,
 		cfg++;
 	}
 
+	if (imx8m_ddr_old_spreadsheet) {
+		cfg->reg = DDRC_ADDRMAP7(0);
+		cfg->val = 0xf0f;
+		cfg++;
+	}
+
 	/* save ddrphy config */
 	saved_timing->ddrphy_cfg = cfg;
 	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
index 9ae7cb877686..147a7d499aaf 100644
--- a/include/soc/imx8m/ddr.h
+++ b/include/soc/imx8m/ddr.h
@@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned long addr, u32 bit)
 #define dwc_ddrphy_apb_rd(addr) \
 	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 * (addr))
 
+extern bool imx8m_ddr_old_spreadsheet;
 extern struct dram_cfg_param ddrphy_trained_csr[];
 extern uint32_t ddrphy_trained_csr_num;
 
-- 
2.30.2

[PATCH master 4/4] ARM: i.MX8M: imx8mn-evk: disable DDRC memory detection

[Ahmad Fatoum]

While detection now seems to work for all other supported boards, it is
no longer correct for imx8mn-evk with LPDDR4. Until that's resolved,
disable the dynamic detection. Upstream device tree has a size of 2G
already.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/boards/nxp-imx8mn-evk/board.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boards/nxp-imx8mn-evk/board.c b/arch/arm/boards/nxp-imx8mn-evk/board.c
index 3c478d5f7088..07e197ccedd2 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/board.c
+++ b/arch/arm/boards/nxp-imx8mn-evk/board.c
@@ -57,6 +57,13 @@ static int nxp_imx8mn_evk_init(void)
 	phy_register_fixup_for_uid(PHY_ID_AR8031, AR_PHY_ID_MASK,
 				   ar8031_phy_fixup);
 
+	/* FIXME: DDRC driver detects 4G instead of 2G on LPDDR4 board,
+	 * perhaps because we leave something uninitialized in the DDRC
+	 * setup.  Until that's resolved, disable dynamic detection and
+	 * use only memory size specified in DT
+	 */
+	imx_esdctl_disable(1);
+
 	return 0;
 }
 coredevice_initcall(nxp_imx8mn_evk_init);
-- 
2.30.2

[PATCH] fixup! ARM: i.MX8M: imx8mn-evk: disable DDRC memory detection

[Ahmad Fatoum]

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Masked by patch not destined for master, that will change it
to board driver... Please squash.
---
 arch/arm/boards/nxp-imx8mn-evk/board.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boards/nxp-imx8mn-evk/board.c b/arch/arm/boards/nxp-imx8mn-evk/board.c
index 07e197ccedd2..cd58a5a397e3 100644
--- a/arch/arm/boards/nxp-imx8mn-evk/board.c
+++ b/arch/arm/boards/nxp-imx8mn-evk/board.c
@@ -9,6 +9,7 @@
 #include <linux/phy.h>
 #include <linux/sizes.h>
 #include <mach/bbu.h>
+#include <mach/esdctl.h>
 #include <envfs.h>
 
 #define PHY_ID_AR8031	0x004dd074
@@ -62,7 +63,7 @@ static int nxp_imx8mn_evk_init(void)
 	 * setup.  Until that's resolved, disable dynamic detection and
 	 * use only memory size specified in DT
 	 */
-	imx_esdctl_disable(1);
+	imx_esdctl_disable();
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH master 0/4] ARM: i.MX8M: fix various miscalculation of DRAM size

[Ahmad Fatoum]

On 23.06.22 12:30, Ahmad Fatoum wrote:
> This fixes issues of wrong DRAM size calculation for imx8mq-evk,
> phytec-som-imx8mq, zii-imx8mq-dev as well as an out-of-tree 
> i.MX8MM board I have.
> 
> For i.MX8MN-EVK, DDR4 size is correctly calculated, but not LPDDR4.
> I can't see where the issue is though, so I am disabling detection
> there until it's figured out. Device Tree already contains correct
> RAM size.

i.MX8MN has only 16-bit wide bus, which would explain the discrepancy.
v2 incoming..

> 
> Ahmad Fatoum (4):
>   ARM: i.MX8M: esdctl: ignore ADDRMAP8 for non-DDR4
>   ARM: i.MX8MQ: initialize ADDRMAP7
>   ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7
>   ARM: i.MX8M: imx8mn-evk: disable DDRC memory detection
> 
>  arch/arm/boards/nxp-imx8mn-evk/board.c       |  7 +++++++
>  arch/arm/boards/nxp-imx8mq-evk/ddr_init.c    |  3 ++-
>  arch/arm/boards/phytec-som-imx8mq/ddr_init.c |  1 +
>  arch/arm/boards/zii-imx8mq-dev/ddr_init.c    |  3 ++-
>  arch/arm/mach-imx/esdctl.c                   |  8 +++++++-
>  drivers/ddr/imx8m/ddr_init.c                 | 18 ++++++++++++++++++
>  drivers/ddr/imx8m/helper.c                   |  6 ++++++
>  include/soc/imx8m/ddr.h                      |  1 +
>  8 files changed, 44 insertions(+), 3 deletions(-)
> 


-- 
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 |

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Teresa Remmet]

Hello Ahmad,

Am Donnerstag, dem 23.06.2022 um 12:30 +0200 schrieb Ahmad Fatoum:
> Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving it at
> its
> POR default of zero. Now that barebox looks at ADDRMAP7 to be able to
> correctly detect bigger memory sizes, barebox proper on out-of-tree
> boards with older spreadsheets may read back 4x times as much RAM
> as actually fitted.
> 
> Work around this by writing a trailing 0xf0f (the neutral ignore-me
> value for the register) if the register wasn't written through
> dram_timing_info::ddrc_cfg. We consider this safe to do, because
> the DDRC is held in reset while these values are programmed.

have you tried this patch without actually updating the RAM Timings (
so patch 2/4)?
I have just played around also setting ADDRMAP7 right before 
ddr_cfg_umctl2() without fixing the timings and the register was 0
again after startup and so the RAM size wrong.
So it seems to me that the value is overwritten/resetted at some other
point again ... or maybe I just did something wrong.

Regards,
Teresa

> 
> Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit SDRAM row
> size handle")
> Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
> imx8m")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
>  drivers/ddr/imx8m/helper.c   |  6 ++++++
>  include/soc/imx8m/ddr.h      |  1 +
>  3 files changed, 25 insertions(+)
> 
> diff --git a/drivers/ddr/imx8m/ddr_init.c
> b/drivers/ddr/imx8m/ddr_init.c
> index ae05b136229c..9a4b4e2ca88a 100644
> --- a/drivers/ddr/imx8m/ddr_init.c
> +++ b/drivers/ddr/imx8m/ddr_init.c
> @@ -13,14 +13,32 @@
>  #include <mach/imx8m-regs.h>
>  #include <mach/imx8m-ccm-regs.h>
>  
> +bool imx8m_ddr_old_spreadsheet = true;
> +
>  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg, int num)
>  {
>  	int i = 0;
>  
>  	for (i = 0; i < num; i++) {
> +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
> +		    imx8m_ddr_old_spreadsheet = false;
>  		reg32_write((unsigned long)ddrc_cfg->reg, ddrc_cfg-
> >val);
>  		ddrc_cfg++;
>  	}
> +
> +	/*
> +	 * Older NXP DDR configuration spreadsheets don't initialize
> ADDRMAP7,
> +	 * which falsifies the memory size read back from the
> controller
> +	 * in barebox proper.
> +	 */
> +	if (imx8m_ddr_old_spreadsheet) {
> +		pr_warn("Working around old spreadsheet. Please
> regenerate\n");
> +		/*
> +		 * Alternatively, stick { DDRC_ADDRMAP7(0), 0xf0f }
> into
> +		 * struct dram_timing_info::ddrc_cfg of your old timing
> file
> +		 */
> +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
> +	}
>  }
>  
>  /*
> diff --git a/drivers/ddr/imx8m/helper.c b/drivers/ddr/imx8m/helper.c
> index 94bbb811576d..98e40849584b 100644
> --- a/drivers/ddr/imx8m/helper.c
> +++ b/drivers/ddr/imx8m/helper.c
> @@ -62,6 +62,12 @@ void dram_config_save(struct dram_timing_info
> *timing_info,
>  		cfg++;
>  	}
>  
> +	if (imx8m_ddr_old_spreadsheet) {
> +		cfg->reg = DDRC_ADDRMAP7(0);
> +		cfg->val = 0xf0f;
> +		cfg++;
> +	}
> +
>  	/* save ddrphy config */
>  	saved_timing->ddrphy_cfg = cfg;
>  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> index 9ae7cb877686..147a7d499aaf 100644
> --- a/include/soc/imx8m/ddr.h
> +++ b/include/soc/imx8m/ddr.h
> @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned long
> addr, u32 bit)
>  #define dwc_ddrphy_apb_rd(addr) \
>  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 * (addr))
>  
> +extern bool imx8m_ddr_old_spreadsheet;
>  extern struct dram_cfg_param ddrphy_trained_csr[];
>  extern uint32_t ddrphy_trained_csr_num;
>  
-- 
PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber |
Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE
149059855

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Ahmad Fatoum]

Hi Teresa,

On 23.06.22 12:59, Teresa Remmet wrote:
> Hello Ahmad,
> 
> Am Donnerstag, dem 23.06.2022 um 12:30 +0200 schrieb Ahmad Fatoum:
>> Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving it at
>> its
>> POR default of zero. Now that barebox looks at ADDRMAP7 to be able to
>> correctly detect bigger memory sizes, barebox proper on out-of-tree
>> boards with older spreadsheets may read back 4x times as much RAM
>> as actually fitted.
>>
>> Work around this by writing a trailing 0xf0f (the neutral ignore-me
>> value for the register) if the register wasn't written through
>> dram_timing_info::ddrc_cfg. We consider this safe to do, because
>> the DDRC is held in reset while these values are programmed.
> 
> have you tried this patch without actually updating the RAM Timings (
> so patch 2/4)?

Yes. I've an out-of-tree i.MX8MM board with 1G of LPDDR4 and old RAM setup.

With upstream/next: 4G
With Workaround: 1G + Warning
With fixed ADDRMAP7: 1G, no warning

Reading back 0x3d40021c gives me 0xf0f as expected.

> I have just played around also setting ADDRMAP7 right before 
> ddr_cfg_umctl2() without fixing the timings and the register was 0
> again after startup and so the RAM size wrong.
> So it seems to me that the value is overwritten/resetted at some other
> point again ... or maybe I just did something wrong.

Strange. What SoC/board is this on?

Cheers,
Ahmad

> 
> Regards,
> Teresa
> 
>>
>> Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit SDRAM row
>> size handle")
>> Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
>> imx8m")
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
>>  drivers/ddr/imx8m/helper.c   |  6 ++++++
>>  include/soc/imx8m/ddr.h      |  1 +
>>  3 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/ddr/imx8m/ddr_init.c
>> b/drivers/ddr/imx8m/ddr_init.c
>> index ae05b136229c..9a4b4e2ca88a 100644
>> --- a/drivers/ddr/imx8m/ddr_init.c
>> +++ b/drivers/ddr/imx8m/ddr_init.c
>> @@ -13,14 +13,32 @@
>>  #include <mach/imx8m-regs.h>
>>  #include <mach/imx8m-ccm-regs.h>
>>  
>> +bool imx8m_ddr_old_spreadsheet = true;
>> +
>>  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg, int num)
>>  {
>>  	int i = 0;
>>  
>>  	for (i = 0; i < num; i++) {
>> +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
>> +		    imx8m_ddr_old_spreadsheet = false;
>>  		reg32_write((unsigned long)ddrc_cfg->reg, ddrc_cfg-
>>> val);
>>  		ddrc_cfg++;
>>  	}
>> +
>> +	/*
>> +	 * Older NXP DDR configuration spreadsheets don't initialize
>> ADDRMAP7,
>> +	 * which falsifies the memory size read back from the
>> controller
>> +	 * in barebox proper.
>> +	 */
>> +	if (imx8m_ddr_old_spreadsheet) {
>> +		pr_warn("Working around old spreadsheet. Please
>> regenerate\n");
>> +		/*
>> +		 * Alternatively, stick { DDRC_ADDRMAP7(0), 0xf0f }
>> into
>> +		 * struct dram_timing_info::ddrc_cfg of your old timing
>> file
>> +		 */
>> +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
>> +	}
>>  }
>>  
>>  /*
>> diff --git a/drivers/ddr/imx8m/helper.c b/drivers/ddr/imx8m/helper.c
>> index 94bbb811576d..98e40849584b 100644
>> --- a/drivers/ddr/imx8m/helper.c
>> +++ b/drivers/ddr/imx8m/helper.c
>> @@ -62,6 +62,12 @@ void dram_config_save(struct dram_timing_info
>> *timing_info,
>>  		cfg++;
>>  	}
>>  
>> +	if (imx8m_ddr_old_spreadsheet) {
>> +		cfg->reg = DDRC_ADDRMAP7(0);
>> +		cfg->val = 0xf0f;
>> +		cfg++;
>> +	}
>> +
>>  	/* save ddrphy config */
>>  	saved_timing->ddrphy_cfg = cfg;
>>  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
>> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
>> index 9ae7cb877686..147a7d499aaf 100644
>> --- a/include/soc/imx8m/ddr.h
>> +++ b/include/soc/imx8m/ddr.h
>> @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned long
>> addr, u32 bit)
>>  #define dwc_ddrphy_apb_rd(addr) \
>>  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 * (addr))
>>  
>> +extern bool imx8m_ddr_old_spreadsheet;
>>  extern struct dram_cfg_param ddrphy_trained_csr[];
>>  extern uint32_t ddrphy_trained_csr_num;
>>  


-- 
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 |

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Teresa Remmet]

Am Donnerstag, dem 23.06.2022 um 13:14 +0200 schrieb Ahmad Fatoum:
> Hi Teresa,
> 
> On 23.06.22 12:59, Teresa Remmet wrote:
> > Hello Ahmad,
> > 
> > Am Donnerstag, dem 23.06.2022 um 12:30 +0200 schrieb Ahmad Fatoum:
> > > Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving it
> > > at
> > > its
> > > POR default of zero. Now that barebox looks at ADDRMAP7 to be
> > > able to
> > > correctly detect bigger memory sizes, barebox proper on out-of-
> > > tree
> > > boards with older spreadsheets may read back 4x times as much RAM
> > > as actually fitted.
> > > 
> > > Work around this by writing a trailing 0xf0f (the neutral ignore-
> > > me
> > > value for the register) if the register wasn't written through
> > > dram_timing_info::ddrc_cfg. We consider this safe to do, because
> > > the DDRC is held in reset while these values are programmed.
> > 
> > have you tried this patch without actually updating the RAM Timings
> > (
> > so patch 2/4)?
> 
> Yes. I've an out-of-tree i.MX8MM board with 1G of LPDDR4 and old RAM
> setup.
> 
> With upstream/next: 4G
> With Workaround: 1G + Warning
> With fixed ADDRMAP7: 1G, no warning
> 
> Reading back 0x3d40021c gives me 0xf0f as expected.
> 
> > I have just played around also setting ADDRMAP7 right before 
> > ddr_cfg_umctl2() without fixing the timings and the register was 0
> > again after startup and so the RAM size wrong.
> > So it seems to me that the value is overwritten/resetted at some
> > other
> > point again ... or maybe I just did something wrong.
> 
> Strange. What SoC/board is this on?

phyCORE-i.MX8MQ but with a not upstreamed 2GB RAM configuration.

I applied this patch now with the same result. Wrong RAM size. 

Regards,
Teresa

> 
> Cheers,
> Ahmad
> 
> > Regards,
> > Teresa
> > 
> > > Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit SDRAM
> > > row
> > > size handle")
> > > Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
> > > imx8m")
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > ---
> > >  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
> > >  drivers/ddr/imx8m/helper.c   |  6 ++++++
> > >  include/soc/imx8m/ddr.h      |  1 +
> > >  3 files changed, 25 insertions(+)
> > > 
> > > diff --git a/drivers/ddr/imx8m/ddr_init.c
> > > b/drivers/ddr/imx8m/ddr_init.c
> > > index ae05b136229c..9a4b4e2ca88a 100644
> > > --- a/drivers/ddr/imx8m/ddr_init.c
> > > +++ b/drivers/ddr/imx8m/ddr_init.c
> > > @@ -13,14 +13,32 @@
> > >  #include <mach/imx8m-regs.h>
> > >  #include <mach/imx8m-ccm-regs.h>
> > >  
> > > +bool imx8m_ddr_old_spreadsheet = true;
> > > +
> > >  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg, int
> > > num)
> > >  {
> > >  	int i = 0;
> > >  
> > >  	for (i = 0; i < num; i++) {
> > > +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
> > > +		    imx8m_ddr_old_spreadsheet = false;
> > >  		reg32_write((unsigned long)ddrc_cfg->reg, ddrc_cfg-
> > > > val);
> > >  		ddrc_cfg++;
> > >  	}
> > > +
> > > +	/*
> > > +	 * Older NXP DDR configuration spreadsheets don't initialize
> > > ADDRMAP7,
> > > +	 * which falsifies the memory size read back from the
> > > controller
> > > +	 * in barebox proper.
> > > +	 */
> > > +	if (imx8m_ddr_old_spreadsheet) {
> > > +		pr_warn("Working around old spreadsheet. Please
> > > regenerate\n");
> > > +		/*
> > > +		 * Alternatively, stick { DDRC_ADDRMAP7(0), 0xf0f }
> > > into
> > > +		 * struct dram_timing_info::ddrc_cfg of your old timing
> > > file
> > > +		 */
> > > +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
> > > +	}
> > >  }
> > >  
> > >  /*
> > > diff --git a/drivers/ddr/imx8m/helper.c
> > > b/drivers/ddr/imx8m/helper.c
> > > index 94bbb811576d..98e40849584b 100644
> > > --- a/drivers/ddr/imx8m/helper.c
> > > +++ b/drivers/ddr/imx8m/helper.c
> > > @@ -62,6 +62,12 @@ void dram_config_save(struct dram_timing_info
> > > *timing_info,
> > >  		cfg++;
> > >  	}
> > >  
> > > +	if (imx8m_ddr_old_spreadsheet) {
> > > +		cfg->reg = DDRC_ADDRMAP7(0);
> > > +		cfg->val = 0xf0f;
> > > +		cfg++;
> > > +	}
> > > +
> > >  	/* save ddrphy config */
> > >  	saved_timing->ddrphy_cfg = cfg;
> > >  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
> > > diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> > > index 9ae7cb877686..147a7d499aaf 100644
> > > --- a/include/soc/imx8m/ddr.h
> > > +++ b/include/soc/imx8m/ddr.h
> > > @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned long
> > > addr, u32 bit)
> > >  #define dwc_ddrphy_apb_rd(addr) \
> > >  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 * (addr))
> > >  
> > > +extern bool imx8m_ddr_old_spreadsheet;
> > >  extern struct dram_cfg_param ddrphy_trained_csr[];
> > >  extern uint32_t ddrphy_trained_csr_num;
> > >  
> 
> 
-- 
PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber |
Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE
149059855

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Teresa Remmet]

Hello Ahmad,

Am Donnerstag, dem 23.06.2022 um 13:26 +0200 schrieb Teresa Remmet:
> Am Donnerstag, dem 23.06.2022 um 13:14 +0200 schrieb Ahmad Fatoum:
> > Hi Teresa,
> > 
> > On 23.06.22 12:59, Teresa Remmet wrote:
> > > Hello Ahmad,
> > > 
> > > Am Donnerstag, dem 23.06.2022 um 12:30 +0200 schrieb Ahmad
> > > Fatoum:
> > > > Older NXP DDR spreadsheets don't initialize ADDRMAP7, leaving
> > > > it
> > > > at
> > > > its
> > > > POR default of zero. Now that barebox looks at ADDRMAP7 to be
> > > > able to
> > > > correctly detect bigger memory sizes, barebox proper on out-of-
> > > > tree
> > > > boards with older spreadsheets may read back 4x times as much
> > > > RAM
> > > > as actually fitted.
> > > > 
> > > > Work around this by writing a trailing 0xf0f (the neutral
> > > > ignore-
> > > > me
> > > > value for the register) if the register wasn't written through
> > > > dram_timing_info::ddrc_cfg. We consider this safe to do,
> > > > because
> > > > the DDRC is held in reset while these values are programmed.
> > > 
> > > have you tried this patch without actually updating the RAM
> > > Timings
> > > (
> > > so patch 2/4)?
> > 
> > Yes. I've an out-of-tree i.MX8MM board with 1G of LPDDR4 and old
> > RAM
> > setup.
> > 
> > With upstream/next: 4G
> > With Workaround: 1G + Warning
> > With fixed ADDRMAP7: 1G, no warning
> > 
> > Reading back 0x3d40021c gives me 0xf0f as expected.
> > 
> > > I have just played around also setting ADDRMAP7 right before 
> > > ddr_cfg_umctl2() without fixing the timings and the register was
> > > 0
> > > again after startup and so the RAM size wrong.
> > > So it seems to me that the value is overwritten/resetted at some
> > > other
> > > point again ... or maybe I just did something wrong.
> > 
> > Strange. What SoC/board is this on?
> 
> phyCORE-i.MX8MQ but with a not upstreamed 2GB RAM configuration.
> 
> I applied this patch now with the same result. Wrong RAM size. 

I now have also tried some out of tree phyCORE-i.MX8MM implementation
without and with this patch and the patch works for our MM. too. RAM
size is corrected.
So maybe the MQ is somehow different here?

Teresa




> Regards,
> Teresa
> 
> > Cheers,
> > Ahmad
> > 
> > > Regards,
> > > Teresa
> > > 
> > > > Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit
> > > > SDRAM
> > > > row
> > > > size handle")
> > > > Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
> > > > imx8m")
> > > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > ---
> > > >  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
> > > >  drivers/ddr/imx8m/helper.c   |  6 ++++++
> > > >  include/soc/imx8m/ddr.h      |  1 +
> > > >  3 files changed, 25 insertions(+)
> > > > 
> > > > diff --git a/drivers/ddr/imx8m/ddr_init.c
> > > > b/drivers/ddr/imx8m/ddr_init.c
> > > > index ae05b136229c..9a4b4e2ca88a 100644
> > > > --- a/drivers/ddr/imx8m/ddr_init.c
> > > > +++ b/drivers/ddr/imx8m/ddr_init.c
> > > > @@ -13,14 +13,32 @@
> > > >  #include <mach/imx8m-regs.h>
> > > >  #include <mach/imx8m-ccm-regs.h>
> > > >  
> > > > +bool imx8m_ddr_old_spreadsheet = true;
> > > > +
> > > >  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg,
> > > > int
> > > > num)
> > > >  {
> > > >  	int i = 0;
> > > >  
> > > >  	for (i = 0; i < num; i++) {
> > > > +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
> > > > +		    imx8m_ddr_old_spreadsheet = false;
> > > >  		reg32_write((unsigned long)ddrc_cfg->reg,
> > > > ddrc_cfg-
> > > > > val);
> > > >  		ddrc_cfg++;
> > > >  	}
> > > > +
> > > > +	/*
> > > > +	 * Older NXP DDR configuration spreadsheets don't
> > > > initialize
> > > > ADDRMAP7,
> > > > +	 * which falsifies the memory size read back from the
> > > > controller
> > > > +	 * in barebox proper.
> > > > +	 */
> > > > +	if (imx8m_ddr_old_spreadsheet) {
> > > > +		pr_warn("Working around old spreadsheet. Please
> > > > regenerate\n");
> > > > +		/*
> > > > +		 * Alternatively, stick { DDRC_ADDRMAP7(0),
> > > > 0xf0f }
> > > > into
> > > > +		 * struct dram_timing_info::ddrc_cfg of your
> > > > old timing
> > > > file
> > > > +		 */
> > > > +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
> > > > +	}
> > > >  }
> > > >  
> > > >  /*
> > > > diff --git a/drivers/ddr/imx8m/helper.c
> > > > b/drivers/ddr/imx8m/helper.c
> > > > index 94bbb811576d..98e40849584b 100644
> > > > --- a/drivers/ddr/imx8m/helper.c
> > > > +++ b/drivers/ddr/imx8m/helper.c
> > > > @@ -62,6 +62,12 @@ void dram_config_save(struct
> > > > dram_timing_info
> > > > *timing_info,
> > > >  		cfg++;
> > > >  	}
> > > >  
> > > > +	if (imx8m_ddr_old_spreadsheet) {
> > > > +		cfg->reg = DDRC_ADDRMAP7(0);
> > > > +		cfg->val = 0xf0f;
> > > > +		cfg++;
> > > > +	}
> > > > +
> > > >  	/* save ddrphy config */
> > > >  	saved_timing->ddrphy_cfg = cfg;
> > > >  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
> > > > diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
> > > > index 9ae7cb877686..147a7d499aaf 100644
> > > > --- a/include/soc/imx8m/ddr.h
> > > > +++ b/include/soc/imx8m/ddr.h
> > > > @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned
> > > > long
> > > > addr, u32 bit)
> > > >  #define dwc_ddrphy_apb_rd(addr) \
> > > >  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 *
> > > > (addr))
> > > >  
> > > > +extern bool imx8m_ddr_old_spreadsheet;
> > > >  extern struct dram_cfg_param ddrphy_trained_csr[];
> > > >  extern uint32_t ddrphy_trained_csr_num;
> > > >  
-- 
PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber |
Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE
149059855

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Ahmad Fatoum]

On 23.06.22 13:47, Teresa Remmet wrote:
> Am Donnerstag, dem 23.06.2022 um 13:26 +0200 schrieb Teresa Remmet:
>> Am Donnerstag, dem 23.06.2022 um 13:14 +0200 schrieb Ahmad Fatoum:
>>>> I have just played around also setting ADDRMAP7 right before 
>>>> ddr_cfg_umctl2() without fixing the timings and the register was
>>>> 0
>>>> again after startup and so the RAM size wrong.
>>>> So it seems to me that the value is overwritten/resetted at some
>>>> other
>>>> point again ... or maybe I just did something wrong.
>>>
>>> Strange. What SoC/board is this on?
>>
>> phyCORE-i.MX8MQ but with a not upstreamed 2GB RAM configuration.
>>
>> I applied this patch now with the same result. Wrong RAM size. 
> 
> I now have also tried some out of tree phyCORE-i.MX8MM implementation
> without and with this patch and the patch works for our MM. too. RAM
> size is corrected.
> So maybe the MQ is somehow different here?

The PhyCORE with i.MX8MQ upstream doesn't use the generic DRAM register
setup in drivers/ddr/imx8m, so you wouldn't get a warning there and we
can't add one easily, because there is no suitable hook point.

Could this be your issue?

For i.MX8MM, the generic setup is used, so you get the fixup and the
warning.


> 
> Teresa
> 
> 
> 
> 
>> Regards,
>> Teresa
>>
>>> Cheers,
>>> Ahmad
>>>
>>>> Regards,
>>>> Teresa
>>>>
>>>>> Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit
>>>>> SDRAM
>>>>> row
>>>>> size handle")
>>>>> Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max for
>>>>> imx8m")
>>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>> ---
>>>>>  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
>>>>>  drivers/ddr/imx8m/helper.c   |  6 ++++++
>>>>>  include/soc/imx8m/ddr.h      |  1 +
>>>>>  3 files changed, 25 insertions(+)
>>>>>
>>>>> diff --git a/drivers/ddr/imx8m/ddr_init.c
>>>>> b/drivers/ddr/imx8m/ddr_init.c
>>>>> index ae05b136229c..9a4b4e2ca88a 100644
>>>>> --- a/drivers/ddr/imx8m/ddr_init.c
>>>>> +++ b/drivers/ddr/imx8m/ddr_init.c
>>>>> @@ -13,14 +13,32 @@
>>>>>  #include <mach/imx8m-regs.h>
>>>>>  #include <mach/imx8m-ccm-regs.h>
>>>>>  
>>>>> +bool imx8m_ddr_old_spreadsheet = true;
>>>>> +
>>>>>  static void ddr_cfg_umctl2(struct dram_cfg_param *ddrc_cfg,
>>>>> int
>>>>> num)
>>>>>  {
>>>>>  	int i = 0;
>>>>>  
>>>>>  	for (i = 0; i < num; i++) {
>>>>> +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
>>>>> +		    imx8m_ddr_old_spreadsheet = false;
>>>>>  		reg32_write((unsigned long)ddrc_cfg->reg,
>>>>> ddrc_cfg-
>>>>>> val);
>>>>>  		ddrc_cfg++;
>>>>>  	}
>>>>> +
>>>>> +	/*
>>>>> +	 * Older NXP DDR configuration spreadsheets don't
>>>>> initialize
>>>>> ADDRMAP7,
>>>>> +	 * which falsifies the memory size read back from the
>>>>> controller
>>>>> +	 * in barebox proper.
>>>>> +	 */
>>>>> +	if (imx8m_ddr_old_spreadsheet) {
>>>>> +		pr_warn("Working around old spreadsheet. Please
>>>>> regenerate\n");
>>>>> +		/*
>>>>> +		 * Alternatively, stick { DDRC_ADDRMAP7(0),
>>>>> 0xf0f }
>>>>> into
>>>>> +		 * struct dram_timing_info::ddrc_cfg of your
>>>>> old timing
>>>>> file
>>>>> +		 */
>>>>> +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
>>>>> +	}
>>>>>  }
>>>>>  
>>>>>  /*
>>>>> diff --git a/drivers/ddr/imx8m/helper.c
>>>>> b/drivers/ddr/imx8m/helper.c
>>>>> index 94bbb811576d..98e40849584b 100644
>>>>> --- a/drivers/ddr/imx8m/helper.c
>>>>> +++ b/drivers/ddr/imx8m/helper.c
>>>>> @@ -62,6 +62,12 @@ void dram_config_save(struct
>>>>> dram_timing_info
>>>>> *timing_info,
>>>>>  		cfg++;
>>>>>  	}
>>>>>  
>>>>> +	if (imx8m_ddr_old_spreadsheet) {
>>>>> +		cfg->reg = DDRC_ADDRMAP7(0);
>>>>> +		cfg->val = 0xf0f;
>>>>> +		cfg++;
>>>>> +	}
>>>>> +
>>>>>  	/* save ddrphy config */
>>>>>  	saved_timing->ddrphy_cfg = cfg;
>>>>>  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
>>>>> diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
>>>>> index 9ae7cb877686..147a7d499aaf 100644
>>>>> --- a/include/soc/imx8m/ddr.h
>>>>> +++ b/include/soc/imx8m/ddr.h
>>>>> @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned
>>>>> long
>>>>> addr, u32 bit)
>>>>>  #define dwc_ddrphy_apb_rd(addr) \
>>>>>  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 *
>>>>> (addr))
>>>>>  
>>>>> +extern bool imx8m_ddr_old_spreadsheet;
>>>>>  extern struct dram_cfg_param ddrphy_trained_csr[];
>>>>>  extern uint32_t ddrphy_trained_csr_num;
>>>>>  


-- 
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 |

Re: [PATCH master 3/4] ddr: imx8m: workaround old spreadsheets not initializing ADDRMAP7

[Teresa Remmet]

Am Donnerstag, dem 23.06.2022 um 14:04 +0200 schrieb Ahmad Fatoum:
> On 23.06.22 13:47, Teresa Remmet wrote:
> > Am Donnerstag, dem 23.06.2022 um 13:26 +0200 schrieb Teresa Remmet:
> > > Am Donnerstag, dem 23.06.2022 um 13:14 +0200 schrieb Ahmad
> > > Fatoum:
> > > > > I have just played around also setting ADDRMAP7 right before 
> > > > > ddr_cfg_umctl2() without fixing the timings and the register
> > > > > was
> > > > > 0
> > > > > again after startup and so the RAM size wrong.
> > > > > So it seems to me that the value is overwritten/resetted at
> > > > > some
> > > > > other
> > > > > point again ... or maybe I just did something wrong.
> > > > 
> > > > Strange. What SoC/board is this on?
> > > 
> > > phyCORE-i.MX8MQ but with a not upstreamed 2GB RAM configuration.
> > > 
> > > I applied this patch now with the same result. Wrong RAM size. 
> > 
> > I now have also tried some out of tree phyCORE-i.MX8MM
> > implementation
> > without and with this patch and the patch works for our MM. too.
> > RAM
> > size is corrected.
> > So maybe the MQ is somehow different here?
> 
> The PhyCORE with i.MX8MQ upstream doesn't use the generic DRAM
> register
> setup in drivers/ddr/imx8m, so you wouldn't get a warning there and
> we
> can't add one easily, because there is no suitable hook point.

Ah yes, this is the only place where we still have the non generic
setup. So I did not thought of it. Enabling debug would have helped.
So thank you. 

At least I can give my:

Tested-by: Teresa Remmet <t.remmet@phytec.de>

Thanks!
Teresa


> 
> Could this be your issue?
> 
> For i.MX8MM, the generic setup is used, so you get the fixup and the
> warning.
> 
> 
> > Teresa
> > 
> > 
> > 
> > 
> > > Regards,
> > > Teresa
> > > 
> > > > Cheers,
> > > > Ahmad
> > > > 
> > > > > Regards,
> > > > > Teresa
> > > > > 
> > > > > > Fixes: dad2b5636bd8 ("ARM: imx: Add imx8 support for 18 bit
> > > > > > SDRAM
> > > > > > row
> > > > > > size handle")
> > > > > > Fixes: 6cf197fa61f9 ("arm: imx: mmdc_size: Increase row_max
> > > > > > for
> > > > > > imx8m")
> > > > > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > > > > ---
> > > > > >  drivers/ddr/imx8m/ddr_init.c | 18 ++++++++++++++++++
> > > > > >  drivers/ddr/imx8m/helper.c   |  6 ++++++
> > > > > >  include/soc/imx8m/ddr.h      |  1 +
> > > > > >  3 files changed, 25 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/ddr/imx8m/ddr_init.c
> > > > > > b/drivers/ddr/imx8m/ddr_init.c
> > > > > > index ae05b136229c..9a4b4e2ca88a 100644
> > > > > > --- a/drivers/ddr/imx8m/ddr_init.c
> > > > > > +++ b/drivers/ddr/imx8m/ddr_init.c
> > > > > > @@ -13,14 +13,32 @@
> > > > > >  #include <mach/imx8m-regs.h>
> > > > > >  #include <mach/imx8m-ccm-regs.h>
> > > > > >  
> > > > > > +bool imx8m_ddr_old_spreadsheet = true;
> > > > > > +
> > > > > >  static void ddr_cfg_umctl2(struct dram_cfg_param
> > > > > > *ddrc_cfg,
> > > > > > int
> > > > > > num)
> > > > > >  {
> > > > > >  	int i = 0;
> > > > > >  
> > > > > >  	for (i = 0; i < num; i++) {
> > > > > > +		if (ddrc_cfg->reg == DDRC_ADDRMAP7(0))
> > > > > > +		    imx8m_ddr_old_spreadsheet = false;
> > > > > >  		reg32_write((unsigned long)ddrc_cfg->reg,
> > > > > > ddrc_cfg-
> > > > > > > val);
> > > > > >  		ddrc_cfg++;
> > > > > >  	}
> > > > > > +
> > > > > > +	/*
> > > > > > +	 * Older NXP DDR configuration spreadsheets don't
> > > > > > initialize
> > > > > > ADDRMAP7,
> > > > > > +	 * which falsifies the memory size read back from the
> > > > > > controller
> > > > > > +	 * in barebox proper.
> > > > > > +	 */
> > > > > > +	if (imx8m_ddr_old_spreadsheet) {
> > > > > > +		pr_warn("Working around old spreadsheet. Please
> > > > > > regenerate\n");
> > > > > > +		/*
> > > > > > +		 * Alternatively, stick { DDRC_ADDRMAP7(0),
> > > > > > 0xf0f }
> > > > > > into
> > > > > > +		 * struct dram_timing_info::ddrc_cfg of your
> > > > > > old timing
> > > > > > file
> > > > > > +		 */
> > > > > > +		reg32_write(DDRC_ADDRMAP7(0), 0xf0f);
> > > > > > +	}
> > > > > >  }
> > > > > >  
> > > > > >  /*
> > > > > > diff --git a/drivers/ddr/imx8m/helper.c
> > > > > > b/drivers/ddr/imx8m/helper.c
> > > > > > index 94bbb811576d..98e40849584b 100644
> > > > > > --- a/drivers/ddr/imx8m/helper.c
> > > > > > +++ b/drivers/ddr/imx8m/helper.c
> > > > > > @@ -62,6 +62,12 @@ void dram_config_save(struct
> > > > > > dram_timing_info
> > > > > > *timing_info,
> > > > > >  		cfg++;
> > > > > >  	}
> > > > > >  
> > > > > > +	if (imx8m_ddr_old_spreadsheet) {
> > > > > > +		cfg->reg = DDRC_ADDRMAP7(0);
> > > > > > +		cfg->val = 0xf0f;
> > > > > > +		cfg++;
> > > > > > +	}
> > > > > > +
> > > > > >  	/* save ddrphy config */
> > > > > >  	saved_timing->ddrphy_cfg = cfg;
> > > > > >  	for (i = 0; i < timing_info->ddrphy_cfg_num; i++) {
> > > > > > diff --git a/include/soc/imx8m/ddr.h
> > > > > > b/include/soc/imx8m/ddr.h
> > > > > > index 9ae7cb877686..147a7d499aaf 100644
> > > > > > --- a/include/soc/imx8m/ddr.h
> > > > > > +++ b/include/soc/imx8m/ddr.h
> > > > > > @@ -407,6 +407,7 @@ static inline void reg32setbit(unsigned
> > > > > > long
> > > > > > addr, u32 bit)
> > > > > >  #define dwc_ddrphy_apb_rd(addr) \
> > > > > >  	reg32_read(IOMEM(IP2APB_DDRPHY_IPS_BASE_ADDR(0)) + 4 *
> > > > > > (addr))
> > > > > >  
> > > > > > +extern bool imx8m_ddr_old_spreadsheet;
> > > > > >  extern struct dram_cfg_param ddrphy_trained_csr[];
> > > > > >  extern uint32_t ddrphy_trained_csr_num;
> > > > > >  
> 
> 
-- 
PHYTEC Messtechnik GmbH | Robert-Koch-Str. 39 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber |
Handelsregister Mainz HRB 4656 | Finanzamt Mainz | St.Nr. 266500608, DE
149059855