[U-Boot] [PATCH v2 0/4] arm: imx53: remove usage of mx53_dram_size

[U-Boot] [PATCH v2 0/4] arm: imx53: remove usage of mx53_dram_size

[linux-kernel-dev at beckhoff.com]

From: Patrick Bruenn <p.bruenn@beckhoff.com>

Global variables are not available during board_init_f().
The i.MX53 boards m53evk, mx53cx9020 and mx53loco are using the exact
same dram initialization code, which uses
'static uint32_t mx53_dram_size[2];' in dram_init(),
dram_init_banksize() and get_effective_memsize() to avoid multiple
calls to get_ram_size().

This series replaces the static variable with multiple calls to
get_ram_size() and moves the shared code into arch/arm/mach-imx/mx5/.

The first patch fixes cx9020. The next patch moves that code to a common
place to be reused by m53evk and mx53loco with the next patches.

Changes in v2:
- move dram initialization into common location
- reuse fixed dram initialization for m53evk and mx53loco

Patrick Bruenn (4):
  arm: imx: cx9020: remove usage of mx53_dram_size
  arm: imx: cx9020: move dram init into common place
  arm: imx: m53evk: remove usage of mx53_dram_size
  arm: imx: mx53loco: remove usage of mx53_dram_size

 arch/arm/mach-imx/mx5/Makefile         |  5 ++++
 arch/arm/mach-imx/mx5/mx53_dram.c      | 45 ++++++++++++++++++++++++++++++++++
 board/aries/m53evk/m53evk.c            | 39 -----------------------------
 board/beckhoff/mx53cx9020/mx53cx9020.c | 39 -----------------------------
 board/freescale/mx53loco/mx53loco.c    | 39 -----------------------------
 5 files changed, 50 insertions(+), 117 deletions(-)
 create mode 100644 arch/arm/mach-imx/mx5/mx53_dram.c

-- 
2.11.0

[U-Boot] [PATCH v2 1/4] arm: imx: cx9020: remove usage of mx53_dram_size

[linux-kernel-dev at beckhoff.com]

From: Patrick Bruenn <p.bruenn@beckhoff.com>

Static variables are not available during board_init_f().
'static uint32_t mx53_dram_size[2];' was used in board specific
dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
multiple calls to get_ram_size().

However multiple calls are better than undefined behavior...
This fixes:
https://lists.denx.de/pipermail/u-boot/2017-November/313214.html
https://lists.denx.de/pipermail/u-boot/2017-December/314480.html

Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

---

Changes in v2:
- move dram initialization into common location
- reuse fixed dram initialization for m53evk and mx53loco

 board/beckhoff/mx53cx9020/mx53cx9020.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/board/beckhoff/mx53cx9020/mx53cx9020.c b/board/beckhoff/mx53cx9020/mx53cx9020.c
index 021bd967c4..d8bdfc27bb 100644
--- a/board/beckhoff/mx53cx9020/mx53cx9020.c
+++ b/board/beckhoff/mx53cx9020/mx53cx9020.c
@@ -59,8 +59,6 @@ static const u32 CCAT_MODE_RUN = 0x0033DC8F;
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static uint32_t mx53_dram_size[2];
-
 phys_size_t get_effective_memsize(void)
 {
 	/*
@@ -74,15 +72,13 @@ phys_size_t get_effective_memsize(void)
 	 * U-Boot into invalid memory area close to the end of the first
 	 * DRAM bank.
 	 */
-	return mx53_dram_size[0];
+	return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
 }
 
 int dram_init(void)
 {
-	mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-	mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
-
-	gd->ram_size = mx53_dram_size[0] + mx53_dram_size[1];
+	gd->ram_size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
+	gd->ram_size += get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
 
 	return 0;
 }
@@ -90,10 +86,10 @@ int dram_init(void)
 int dram_init_banksize(void)
 {
 	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size = mx53_dram_size[0];
+	gd->bd->bi_dram[0].size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
 
 	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
-	gd->bd->bi_dram[1].size = mx53_dram_size[1];
+	gd->bd->bi_dram[1].size = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
 
 	return 0;
 }
-- 
2.11.0

[U-Boot] [PATCH v2 2/4] arm: imx: cx9020: move dram init into common place

[linux-kernel-dev at beckhoff.com]

From: Patrick Bruenn <p.bruenn@beckhoff.com>

Move dram_init(), dram_init_banksize() and get_effective_memsize() to
arch/arm/mach-imx/mx5/mx53_dram.c, where it can be reused by m53evk and
mx53loco.

Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

---

Changes in v2: None

Patch-Cc:  Fabio Estevam <fabio.estevam@nxp.com>
Patch-Cc:  Christian Gmeiner <christian.gmeiner@gmail.com>
Patch-Cc:  Jason Liu <r64343@freescale.com>
Patch-Cc:  Patrick Bruenn <p.bruenn@beckhoff.com>
Patch-Cc:  Stefano Babic <sbabic@denx.de>
Patch-Cc:  u-boot at lists.denx.de
Patch-Cc:  Marek Vasut <marex@denx.de>
Patch-Cc:  Albert Aribaud <albert.u.boot@aribaud.net>

---
 arch/arm/mach-imx/mx5/Makefile         |  3 +++
 arch/arm/mach-imx/mx5/mx53_dram.c      | 45 ++++++++++++++++++++++++++++++++++
 board/beckhoff/mx53cx9020/mx53cx9020.c | 35 --------------------------
 3 files changed, 48 insertions(+), 35 deletions(-)
 create mode 100644 arch/arm/mach-imx/mx5/mx53_dram.c

diff --git a/arch/arm/mach-imx/mx5/Makefile b/arch/arm/mach-imx/mx5/Makefile
index d021842f68..368cfde98b 100644
--- a/arch/arm/mach-imx/mx5/Makefile
+++ b/arch/arm/mach-imx/mx5/Makefile
@@ -9,3 +9,6 @@
 
 obj-y := soc.o clock.o
 obj-y += lowlevel_init.o
+
+# common files for mx53 dram initialization
+obj-$(CONFIG_TARGET_MX53CX9020) += mx53_dram.o
diff --git a/arch/arm/mach-imx/mx5/mx53_dram.c b/arch/arm/mach-imx/mx5/mx53_dram.c
new file mode 100644
index 0000000000..7e5fc42d1f
--- /dev/null
+++ b/arch/arm/mach-imx/mx5/mx53_dram.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2017  Beckhoff Automation GmbH & Co. KG
+ * Patrick Bruenn <p.bruenn@beckhoff.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+phys_size_t get_effective_memsize(void)
+{
+	/*
+	 * WARNING: We must override get_effective_memsize() function here
+	 * to report only the size of the first DRAM bank. This is to make
+	 * U-Boot relocator place U-Boot into valid memory, that is, at the
+	 * end of the first DRAM bank. If we did not override this function
+	 * like so, U-Boot would be placed at the address of the first DRAM
+	 * bank + total DRAM size - sizeof(uboot), which in the setup where
+	 * each DRAM bank contains 512MiB of DRAM would result in placing
+	 * U-Boot into invalid memory area close to the end of the first
+	 * DRAM bank.
+	 */
+	return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
+}
+
+int dram_init(void)
+{
+	gd->ram_size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
+	gd->ram_size += get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
+
+	return 0;
+}
+
+int dram_init_banksize(void)
+{
+	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
+	gd->bd->bi_dram[0].size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
+
+	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
+	gd->bd->bi_dram[1].size = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
+
+	return 0;
+}
diff --git a/board/beckhoff/mx53cx9020/mx53cx9020.c b/board/beckhoff/mx53cx9020/mx53cx9020.c
index d8bdfc27bb..f9df3604cd 100644
--- a/board/beckhoff/mx53cx9020/mx53cx9020.c
+++ b/board/beckhoff/mx53cx9020/mx53cx9020.c
@@ -59,41 +59,6 @@ static const u32 CCAT_MODE_RUN = 0x0033DC8F;
 
 DECLARE_GLOBAL_DATA_PTR;
 
-phys_size_t get_effective_memsize(void)
-{
-	/*
-	 * WARNING: We must override get_effective_memsize() function here
-	 * to report only the size of the first DRAM bank. This is to make
-	 * U-Boot relocator place U-Boot into valid memory, that is, at the
-	 * end of the first DRAM bank. If we did not override this function
-	 * like so, U-Boot would be placed@the address of the first DRAM
-	 * bank + total DRAM size - sizeof(uboot), which in the setup where
-	 * each DRAM bank contains 512MiB of DRAM would result in placing
-	 * U-Boot into invalid memory area close to the end of the first
-	 * DRAM bank.
-	 */
-	return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-}
-
-int dram_init(void)
-{
-	gd->ram_size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-	gd->ram_size += get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
-
-	return 0;
-}
-
-int dram_init_banksize(void)
-{
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-
-	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
-	gd->bd->bi_dram[1].size = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
-
-	return 0;
-}
-
 u32 get_board_rev(void)
 {
 	struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
-- 
2.11.0

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[linux-kernel-dev at beckhoff.com]

From: Patrick Bruenn <p.bruenn@beckhoff.com>

Static variables are not available during board_init_f().
'static uint32_t mx53_dram_size[2];' was used in board specific
dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
multiple calls to get_ram_size().

Reused dram initialization functions from arch/arm/mach-imx/mx5/mx53_dram.c

Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

---

Changes in v2: None


Only compile tested with make m53evk_defconfig

---
 arch/arm/mach-imx/mx5/Makefile |  1 +
 board/aries/m53evk/m53evk.c    | 39 ---------------------------------------
 2 files changed, 1 insertion(+), 39 deletions(-)

diff --git a/arch/arm/mach-imx/mx5/Makefile b/arch/arm/mach-imx/mx5/Makefile
index 368cfde98b..2cc2cbc07a 100644
--- a/arch/arm/mach-imx/mx5/Makefile
+++ b/arch/arm/mach-imx/mx5/Makefile
@@ -11,4 +11,5 @@ obj-y := soc.o clock.o
 obj-y += lowlevel_init.o
 
 # common files for mx53 dram initialization
+obj-$(CONFIG_TARGET_M53EVK)     += mx53_dram.o
 obj-$(CONFIG_TARGET_MX53CX9020) += mx53_dram.o
diff --git a/board/aries/m53evk/m53evk.c b/board/aries/m53evk/m53evk.c
index ece8957aaf..a798d83376 100644
--- a/board/aries/m53evk/m53evk.c
+++ b/board/aries/m53evk/m53evk.c
@@ -31,45 +31,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static uint32_t mx53_dram_size[2];
-
-phys_size_t get_effective_memsize(void)
-{
-	/*
-	 * WARNING: We must override get_effective_memsize() function here
-	 * to report only the size of the first DRAM bank. This is to make
-	 * U-Boot relocator place U-Boot into valid memory, that is, at the
-	 * end of the first DRAM bank. If we did not override this function
-	 * like so, U-Boot would be placed at the address of the first DRAM
-	 * bank + total DRAM size - sizeof(uboot), which in the setup where
-	 * each DRAM bank contains 512MiB of DRAM would result in placing
-	 * U-Boot into invalid memory area close to the end of the first
-	 * DRAM bank.
-	 */
-	return mx53_dram_size[0];
-}
-
-int dram_init(void)
-{
-	mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-	mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
-
-	gd->ram_size = mx53_dram_size[0] + mx53_dram_size[1];
-
-	return 0;
-}
-
-int dram_init_banksize(void)
-{
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size = mx53_dram_size[0];
-
-	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
-	gd->bd->bi_dram[1].size = mx53_dram_size[1];
-
-	return 0;
-}
-
 static void setup_iomux_uart(void)
 {
 	static const iomux_v3_cfg_t uart_pads[] = {
-- 
2.11.0

[U-Boot] [PATCH v2 4/4] arm: imx: mx53loco: remove usage of mx53_dram_size

[linux-kernel-dev at beckhoff.com]

From: Patrick Bruenn <p.bruenn@beckhoff.com>

Static variables are not available during board_init_f().
'static uint32_t mx53_dram_size[2];' was used in board specific
dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
multiple calls to get_ram_size().

Reused dram initialization functions from arch/arm/mach-imx/mx5/mx53_dram.c

Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>

---

Changes in v2: None


Only compile tested with make mx53loco_defconfig

---
 arch/arm/mach-imx/mx5/Makefile      |  1 +
 board/freescale/mx53loco/mx53loco.c | 39 -------------------------------------
 2 files changed, 1 insertion(+), 39 deletions(-)

diff --git a/arch/arm/mach-imx/mx5/Makefile b/arch/arm/mach-imx/mx5/Makefile
index 2cc2cbc07a..4e305e92cf 100644
--- a/arch/arm/mach-imx/mx5/Makefile
+++ b/arch/arm/mach-imx/mx5/Makefile
@@ -13,3 +13,4 @@ obj-y += lowlevel_init.o
 # common files for mx53 dram initialization
 obj-$(CONFIG_TARGET_M53EVK)     += mx53_dram.o
 obj-$(CONFIG_TARGET_MX53CX9020) += mx53_dram.o
+obj-$(CONFIG_TARGET_MX53LOCO)   += mx53_dram.o
diff --git a/board/freescale/mx53loco/mx53loco.c b/board/freescale/mx53loco/mx53loco.c
index db0e2fbdd6..0beecf3459 100644
--- a/board/freescale/mx53loco/mx53loco.c
+++ b/board/freescale/mx53loco/mx53loco.c
@@ -31,45 +31,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-static uint32_t mx53_dram_size[2];
-
-phys_size_t get_effective_memsize(void)
-{
-	/*
-	 * WARNING: We must override get_effective_memsize() function here
-	 * to report only the size of the first DRAM bank. This is to make
-	 * U-Boot relocator place U-Boot into valid memory, that is, at the
-	 * end of the first DRAM bank. If we did not override this function
-	 * like so, U-Boot would be placed at the address of the first DRAM
-	 * bank + total DRAM size - sizeof(uboot), which in the setup where
-	 * each DRAM bank contains 512MiB of DRAM would result in placing
-	 * U-Boot into invalid memory area close to the end of the first
-	 * DRAM bank.
-	 */
-	return mx53_dram_size[0];
-}
-
-int dram_init(void)
-{
-	mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-	mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
-
-	gd->ram_size = mx53_dram_size[0] + mx53_dram_size[1];
-
-	return 0;
-}
-
-int dram_init_banksize(void)
-{
-	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size = mx53_dram_size[0];
-
-	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
-	gd->bd->bi_dram[1].size = mx53_dram_size[1];
-
-	return 0;
-}
-
 u32 get_board_rev(void)
 {
 	struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
-- 
2.11.0

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Marek Vasut]

On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
> From: Patrick Bruenn <p.bruenn@beckhoff.com>
> 
> Static variables are not available during board_init_f().

They are, since the board runs from RAM at that point already.

> 'static uint32_t mx53_dram_size[2];' was used in board specific
> dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
> multiple calls to get_ram_size().
> 
> Reused dram initialization functions from arch/arm/mach-imx/mx5/mx53_dram.c
> 
> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
> 
> ---
> 
> Changes in v2: None
> 
> 
> Only compile tested with make m53evk_defconfig

Are you sure this patch retains the original functionality ? Note the
warning ...

> ---
>  arch/arm/mach-imx/mx5/Makefile |  1 +
>  board/aries/m53evk/m53evk.c    | 39 ---------------------------------------
>  2 files changed, 1 insertion(+), 39 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mx5/Makefile b/arch/arm/mach-imx/mx5/Makefile
> index 368cfde98b..2cc2cbc07a 100644
> --- a/arch/arm/mach-imx/mx5/Makefile
> +++ b/arch/arm/mach-imx/mx5/Makefile
> @@ -11,4 +11,5 @@ obj-y := soc.o clock.o
>  obj-y += lowlevel_init.o
>  
>  # common files for mx53 dram initialization
> +obj-$(CONFIG_TARGET_M53EVK)     += mx53_dram.o
>  obj-$(CONFIG_TARGET_MX53CX9020) += mx53_dram.o
> diff --git a/board/aries/m53evk/m53evk.c b/board/aries/m53evk/m53evk.c
> index ece8957aaf..a798d83376 100644
> --- a/board/aries/m53evk/m53evk.c
> +++ b/board/aries/m53evk/m53evk.c
> @@ -31,45 +31,6 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -static uint32_t mx53_dram_size[2];
> -
> -phys_size_t get_effective_memsize(void)
> -{
> -	/*
> -	 * WARNING: We must override get_effective_memsize() function here
> -	 * to report only the size of the first DRAM bank. This is to make
> -	 * U-Boot relocator place U-Boot into valid memory, that is, at the
> -	 * end of the first DRAM bank. If we did not override this function
> -	 * like so, U-Boot would be placed at the address of the first DRAM
> -	 * bank + total DRAM size - sizeof(uboot), which in the setup where
> -	 * each DRAM bank contains 512MiB of DRAM would result in placing
> -	 * U-Boot into invalid memory area close to the end of the first
> -	 * DRAM bank.
> -	 */
> -	return mx53_dram_size[0];
> -}
> -
> -int dram_init(void)
> -{
> -	mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
> -	mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
> -
> -	gd->ram_size = mx53_dram_size[0] + mx53_dram_size[1];
> -
> -	return 0;
> -}
> -
> -int dram_init_banksize(void)
> -{
> -	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> -	gd->bd->bi_dram[0].size = mx53_dram_size[0];
> -
> -	gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
> -	gd->bd->bi_dram[1].size = mx53_dram_size[1];
> -
> -	return 0;
> -}
> -
>  static void setup_iomux_uart(void)
>  {
>  	static const iomux_v3_cfg_t uart_pads[] = {
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Montag, 18. Dezember 2017 10:17
>On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>
>> Static variables are not available during board_init_f().
>
>They are, since the board runs from RAM at that point already.
>
From reading the README and common/board_f.c I got the impression,
that dram_init() is called before it's save to access mx53_dram_size.
And as that change seemed to cure the strange behavior I described
in [1] and [2], I prepared a patch for my board, which ended up to be
requested for m53evk and mx53loco, too.

>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>> dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
>> multiple calls to get_ram_size().
>>
>> Reused dram initialization functions from arch/arm/mach-
>imx/mx5/mx53_dram.c
>>
>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>
>> ---
>>
>> Changes in v2: None
>>
>>
>> Only compile tested with make m53evk_defconfig
>
>Are you sure this patch retains the original functionality ? Note the
>warning ...

Effectively it changes:
 -      return mx53_dram_size[0];
+       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);

So, yes I am convinced that get_effective_memsize() still returns only the size
of the first dram bank.
However, I would be fine with just keeping the changes to my board (cx9020).
And if anyone has a better idea of what might be the root cause of [1] and [2],
I would absolute appreciate any input to that.

Best regards and thanks in advance for any feedback,
Patrick

[1] https://lists.denx.de/pipermail/u-boot/2017-November/313214.html
[2] https://lists.denx.de/pipermail/u-boot/2017-December/314480.html

---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Marek Vasut]

On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Montag, 18. Dezember 2017 10:17
>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>
>>> Static variables are not available during board_init_f().
>>
>> They are, since the board runs from RAM at that point already.
>>
> From reading the README and common/board_f.c I got the impression,
> that dram_init() is called before it's save to access mx53_dram_size.
> And as that change seemed to cure the strange behavior I described
> in [1] and [2], I prepared a patch for my board, which ended up to be
> requested for m53evk and mx53loco, too.

Technically yes, board_init_f means it runs from FLASH and has no or
minimal storage available. On the MX53 with SPL, it's running from RAM
so it's safe to use static variables too.

>>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>>> dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
>>> multiple calls to get_ram_size().
>>>
>>> Reused dram initialization functions from arch/arm/mach-
>> imx/mx5/mx53_dram.c
>>>
>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>
>>> ---
>>>
>>> Changes in v2: None
>>>
>>>
>>> Only compile tested with make m53evk_defconfig
>>
>> Are you sure this patch retains the original functionality ? Note the
>> warning ...
> 
> Effectively it changes:
>  -      return mx53_dram_size[0];
> +       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
> 
> So, yes I am convinced that get_effective_memsize() still returns only the size
> of the first dram bank.

I suspect that will fails on M53 due to it's split-bank configuration.
The board has two DRAM banks with a hole between them.

> However, I would be fine with just keeping the changes to my board (cx9020).
> And if anyone has a better idea of what might be the root cause of [1] and [2],
> I would absolute appreciate any input to that.

If your board also has two DRAM banks with a hole between them and you
can test if this works fine, then I'm OK with this change.

> Best regards and thanks in advance for any feedback,
> Patrick
> 
> [1] https://lists.denx.de/pipermail/u-boot/2017-November/313214.html
> [2] https://lists.denx.de/pipermail/u-boot/2017-December/314480.html
> 
> ---
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
> 
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Montag, 18. Dezember 2017 11:57
>On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Montag, 18. Dezember 2017 10:17
>>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>
>>>> Static variables are not available during board_init_f().
>>>
>>> They are, since the board runs from RAM at that point already.
>>>
>> From reading the README and common/board_f.c I got the impression,
>> that dram_init() is called before it's save to access mx53_dram_size.
>> And as that change seemed to cure the strange behavior I described
>> in [1] and [2], I prepared a patch for my board, which ended up to be
>> requested for m53evk and mx53loco, too.
>
>Technically yes, board_init_f means it runs from FLASH and has no or
>minimal storage available. On the MX53 with SPL, it's running from RAM
>so it's safe to use static variables too.
>
Thank's for your explanation.

>>>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>>>> dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
>>>> multiple calls to get_ram_size().
>>>>
>>>> Reused dram initialization functions from arch/arm/mach-
>>> imx/mx5/mx53_dram.c
>>>>
>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v2: None
>>>>
>>>>
>>>> Only compile tested with make m53evk_defconfig
>>>
>>> Are you sure this patch retains the original functionality ? Note the
>>> warning ...
>>
>> Effectively it changes:
>>  -      return mx53_dram_size[0];
>> +       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>>
>> So, yes I am convinced that get_effective_memsize() still returns only the
>size
>> of the first dram bank.
>
>I suspect that will fails on M53 due to it's split-bank configuration.
>The board has two DRAM banks with a hole between them.
>
This is how mx53_dram_size was initialized on m53 before that patch:
-       mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
-       mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);

So to me that's, absolutely the same. With the only difference, that get_ram_size(bank0)
is called multiple times, now.
>> However, I would be fine with just keeping the changes to my board
>(cx9020).
>> And if anyone has a better idea of what might be the root cause of [1] and
>[2],
>> I would absolute appreciate any input to that.
>
>If your board also has two DRAM banks with a hole between them and you
>can test if this works fine, then I'm OK with this change.
>
Yes, cx9020 uses two 512MB banks with 1GB between PHYS_SDRAM_1/2, too.

>> Best regards and thanks in advance for any feedback,
>> Patrick
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2017-November/313214.html
>> [2] https://lists.denx.de/pipermail/u-boot/2017-December/314480.html
>>
>> ---
>> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans
>Beckhoff
>> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
>>
>>
>
>
>--
>Best regards,
>Marek Vasut
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Marek Vasut]

On 12/18/2017 12:40 PM, Patrick Brünn wrote:
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Montag, 18. Dezember 2017 11:57
>> On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Montag, 18. Dezember 2017 10:17
>>>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>
>>>>> Static variables are not available during board_init_f().
>>>>
>>>> They are, since the board runs from RAM at that point already.
>>>>
>>> From reading the README and common/board_f.c I got the impression,
>>> that dram_init() is called before it's save to access mx53_dram_size.
>>> And as that change seemed to cure the strange behavior I described
>>> in [1] and [2], I prepared a patch for my board, which ended up to be
>>> requested for m53evk and mx53loco, too.
>>
>> Technically yes, board_init_f means it runs from FLASH and has no or
>> minimal storage available. On the MX53 with SPL, it's running from RAM
>> so it's safe to use static variables too.
>>
> Thank's for your explanation.

No problem, it's a bit weird.

>>>>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>>>>> dram_init(), dram_init_banksize() and get_effective_memsize() to avoid
>>>>> multiple calls to get_ram_size().
>>>>>
>>>>> Reused dram initialization functions from arch/arm/mach-
>>>> imx/mx5/mx53_dram.c
>>>>>
>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes in v2: None
>>>>>
>>>>>
>>>>> Only compile tested with make m53evk_defconfig
>>>>
>>>> Are you sure this patch retains the original functionality ? Note the
>>>> warning ...
>>>
>>> Effectively it changes:
>>>  -      return mx53_dram_size[0];
>>> +       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>>>
>>> So, yes I am convinced that get_effective_memsize() still returns only the
>> size
>>> of the first dram bank.
>>
>> I suspect that will fails on M53 due to it's split-bank configuration.
>> The board has two DRAM banks with a hole between them.
>>
> This is how mx53_dram_size was initialized on m53 before that patch:
> -       mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
> -       mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
> 
> So to me that's, absolutely the same. With the only difference, that get_ram_size(bank0)
> is called multiple times, now.

The get_ram_size() above is called for two different bank (addresses),
not for bank0 twice. Maybe I'm missing something.

btw where's the patch adding mx53_dram.c ? I don't see it in mainline yet.

>>> However, I would be fine with just keeping the changes to my board
>> (cx9020).
>>> And if anyone has a better idea of what might be the root cause of [1] and
>> [2],
>>> I would absolute appreciate any input to that.
>>
>> If your board also has two DRAM banks with a hole between them and you
>> can test if this works fine, then I'm OK with this change.
>>
> Yes, cx9020 uses two 512MB banks with 1GB between PHYS_SDRAM_1/2, too.

Then that should be the same situation. Can you share "bdinfo" from that
board of yours?

btw do you use SPL ? If not, you should ...

[...]

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Montag, 18. Dezember 2017 12:52
>On 12/18/2017 12:40 PM, Patrick Brünn wrote:
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Montag, 18. Dezember 2017 11:57
>>> On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>> Sent: Montag, 18. Dezember 2017 10:17
>>>>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>
>>>>>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>>>>>> dram_init(), dram_init_banksize() and get_effective_memsize() to
>avoid
>>>>>> multiple calls to get_ram_size().
>>>>>>
>>>>>> Reused dram initialization functions from arch/arm/mach-
>>>>> imx/mx5/mx53_dram.c
>>>>>>
>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>
>>>>>> ---
>>>>>>
>>>>>> Changes in v2: None
>>>>>>
>>>>>>
>>>>>> Only compile tested with make m53evk_defconfig
>>>>>
>>>>> Are you sure this patch retains the original functionality ? Note the
>>>>> warning ...
>>>>
>>>> Effectively it changes:
>>>>  -      return mx53_dram_size[0];
>>>> +       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>>>>
>>>> So, yes I am convinced that get_effective_memsize() still returns only the
>>> size
>>>> of the first dram bank.
>>>
>>> I suspect that will fails on M53 due to it's split-bank configuration.
>>> The board has two DRAM banks with a hole between them.
>>>
>> This is how mx53_dram_size was initialized on m53 before that patch:
>> -       mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>> -       mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
>>
>> So to me that's, absolutely the same. With the only difference, that
>get_ram_size(bank0)
>> is called multiple times, now.
>
>The get_ram_size() above is called for two different bank (addresses),
>not for bank0 twice. Maybe I'm missing something.
>
>btw where's the patch adding mx53_dram.c ? I don't see it in mainline yet.
>
It's #2 of this series:
https://lists.denx.de/pipermail/u-boot/2017-December/314742.html

>>>> However, I would be fine with just keeping the changes to my board
>>> (cx9020).
>>>> And if anyone has a better idea of what might be the root cause of [1]
>and
>>> [2],
>>>> I would absolute appreciate any input to that.
>>>
>>> If your board also has two DRAM banks with a hole between them and you
>>> can test if this works fine, then I'm OK with this change.
>>>
>> Yes, cx9020 uses two 512MB banks with 1GB between PHYS_SDRAM_1/2,
>too.
>
>Then that should be the same situation. Can you share "bdinfo" from that
>board of yours?
>
=> bdinfo
arch_number = 0x00000000
boot_params = 0x70000100
DRAM bank   = 0x00000000
-> start    = 0x70000000
-> size     = 0x20000000
DRAM bank   = 0x00000001
-> start    = 0xB0000000
-> size     = 0x20000000
eth0name    = FEC
ethaddr     = 00:01:05:23:87:85
current eth = FEC
ip_addr     = <NULL>
baudrate    = 115200 bps
TLB addr    = 0x8FFF0000
relocaddr   = 0x8FF8B000
reloc off   = 0x1878B000
irq_sp      = 0x8F586960
sp start    = 0x8F586950
FB base     = 0x8F58C7C0
Early malloc usage: 134 / 400
fdt_blob = 8f586978

>btw do you use SPL ? If not, you should ...
I will read more about SPL. Until now, I thought I will only benefit,
if my initial boot media is limited in size. As we boot from sdcard,
that wasn't a problem to store 360k u-boot.

Regards,
Patrick
---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Marek Vasut]

On 12/18/2017 01:16 PM, Patrick Brünn wrote:
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Montag, 18. Dezember 2017 12:52
>> On 12/18/2017 12:40 PM, Patrick Brünn wrote:
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Montag, 18. Dezember 2017 11:57
>>>> On 12/18/2017 11:47 AM, Patrick Brünn wrote:
>>>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>>>> Sent: Montag, 18. Dezember 2017 10:17
>>>>>> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
>>>>>>> From: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>>
>>>>>>> 'static uint32_t mx53_dram_size[2];' was used in board specific
>>>>>>> dram_init(), dram_init_banksize() and get_effective_memsize() to
>> avoid
>>>>>>> multiple calls to get_ram_size().
>>>>>>>
>>>>>>> Reused dram initialization functions from arch/arm/mach-
>>>>>> imx/mx5/mx53_dram.c
>>>>>>>
>>>>>>> Signed-off-by: Patrick Bruenn <p.bruenn@beckhoff.com>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v2: None
>>>>>>>
>>>>>>>
>>>>>>> Only compile tested with make m53evk_defconfig
>>>>>>
>>>>>> Are you sure this patch retains the original functionality ? Note the
>>>>>> warning ...
>>>>>
>>>>> Effectively it changes:
>>>>>  -      return mx53_dram_size[0];
>>>>> +       return get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>>>>>
>>>>> So, yes I am convinced that get_effective_memsize() still returns only the
>>>> size
>>>>> of the first dram bank.
>>>>
>>>> I suspect that will fails on M53 due to it's split-bank configuration.
>>>> The board has two DRAM banks with a hole between them.
>>>>
>>> This is how mx53_dram_size was initialized on m53 before that patch:
>>> -       mx53_dram_size[0] = get_ram_size((void *)PHYS_SDRAM_1, 1 << 30);
>>> -       mx53_dram_size[1] = get_ram_size((void *)PHYS_SDRAM_2, 1 << 30);
>>>
>>> So to me that's, absolutely the same. With the only difference, that
>> get_ram_size(bank0)
>>> is called multiple times, now.
>>
>> The get_ram_size() above is called for two different bank (addresses),
>> not for bank0 twice. Maybe I'm missing something.
>>
>> btw where's the patch adding mx53_dram.c ? I don't see it in mainline yet.
>>
> It's #2 of this series:
> https://lists.denx.de/pipermail/u-boot/2017-December/314742.html

Ah, sorry, I missed that.

>>>>> However, I would be fine with just keeping the changes to my board
>>>> (cx9020).
>>>>> And if anyone has a better idea of what might be the root cause of [1]
>> and
>>>> [2],
>>>>> I would absolute appreciate any input to that.
>>>>
>>>> If your board also has two DRAM banks with a hole between them and you
>>>> can test if this works fine, then I'm OK with this change.
>>>>
>>> Yes, cx9020 uses two 512MB banks with 1GB between PHYS_SDRAM_1/2,
>> too.
>>
>> Then that should be the same situation. Can you share "bdinfo" from that
>> board of yours?
>>
> => bdinfo
> arch_number = 0x00000000
> boot_params = 0x70000100
> DRAM bank   = 0x00000000
> -> start    = 0x70000000
> -> size     = 0x20000000
> DRAM bank   = 0x00000001
> -> start    = 0xB0000000
> -> size     = 0x20000000
> eth0name    = FEC
> ethaddr     = 00:01:05:23:87:85
> current eth = FEC
> ip_addr     = <NULL>
> baudrate    = 115200 bps
> TLB addr    = 0x8FFF0000
> relocaddr   = 0x8FF8B000
> reloc off   = 0x1878B000
> irq_sp      = 0x8F586960
> sp start    = 0x8F586950
> FB base     = 0x8F58C7C0
> Early malloc usage: 134 / 400
> fdt_blob = 8f586978

Looks fine then.

>> btw do you use SPL ? If not, you should ...
> I will read more about SPL. Until now, I thought I will only benefit,
> if my initial boot media is limited in size. As we boot from sdcard,
> that wasn't a problem to store 360k u-boot.

The other is that you can ie. skip the whole u-boot altogether and boot
linux directly.

I wonder if it would be better to keep the static variable and avoid
calling the get_ram_size() twice, it can save some CPU cycles. Besides
that, thanks for the explanation/discussion !1

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Marek Vasut [mailto:marex at denx.de]
>Sent: Montag, 18. Dezember 2017 13:30
>On 12/18/2017 01:16 PM, Patrick Brünn wrote:
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Montag, 18. Dezember 2017 12:52
>>> On 12/18/2017 12:40 PM, Patrick Brünn wrote:
[...]
>>> btw do you use SPL ? If not, you should ...
>> I will read more about SPL. Until now, I thought I will only benefit,
>> if my initial boot media is limited in size. As we boot from sdcard,
>> that wasn't a problem to store 360k u-boot.
>
>The other is that you can ie. skip the whole u-boot altogether and boot
>linux directly.
>
Okay, I will try that.
>I wonder if it would be better to keep the static variable and avoid
>calling the get_ram_size() twice, it can save some CPU cycles. Besides
>that, thanks for the explanation/discussion !1
>
The pleasure was all mine. I will test SPL on my board and in case that's working I think it still makes sense to move our common code into one file. Is arch/arm/mach-imx/mx53_dram.c the best location for it?
At first I tried board/freescale/common/ but in that case I needed to use ugly relative pathes in the Makefiles "../../"

Regards, Patrick

---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Patrick Brünn]

>From: Patrick Brünn
>Sent: Dienstag, 19. Dezember 2017 05:29
>>From: Marek Vasut [mailto:marex at denx.de]
>>Sent: Montag, 18. Dezember 2017 13:30
>>On 12/18/2017 01:16 PM, Patrick Brünn wrote:
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Montag, 18. Dezember 2017 12:52
>>>> On 12/18/2017 12:40 PM, Patrick Brünn wrote:
>[...]
>>>> btw do you use SPL ? If not, you should ...
>>> I will read more about SPL. Until now, I thought I will only benefit,
>>> if my initial boot media is limited in size. As we boot from sdcard,
>>> that wasn't a problem to store 360k u-boot.
>>
>>The other is that you can ie. skip the whole u-boot altogether and boot
>>linux directly.
>>
>Okay, I will try that.
>>I wonder if it would be better to keep the static variable and avoid
>>calling the get_ram_size() twice, it can save some CPU cycles. Besides
>>that, thanks for the explanation/discussion !1
>>
>The pleasure was all mine. I will test SPL on my board and in case that's
>working I think it still makes sense to move our common code into one file. Is
>arch/arm/mach-imx/mx53_dram.c the best location for it?
>At first I tried board/freescale/common/ but in that case I needed to use ugly
>relative pathes in the Makefiles "../../"
I spend a few hours this morning to experiment with SPL. From what I saw it
would require some additional patches to enable SPL_MMC for imx53 and
a few more changes to our sdcard layout (FAT vs. EXT4). Seeing all these
additional #ifdefs to consider I just don't have a good feeling to port that adhoc
for our board.
Directly booting Linux still sounds tempting, but for the moment I cannot spend
more effort into SPL for mx53cx9020. I will put that on my todo list. But for now
I would like to concentrate to get mainline boot on mx53cx9020, again.

I would suggest:
1. I keep m53evk.c untouched for now.
2a. Either patch both mx53cx9020 and mx53loco to avoid static mx53_dram_size,
   and have the shared code in arch/arm/mach-imx/mx53_dram.c
2b. Or patch only board/beckhoff/mx53cx9020/mx53cx9020.c

Fabio, what do you prefer?

Regards, Patrick

---
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Lothar Waßmann]

Hi,

On Mon, 18 Dec 2017 10:17:03 +0100 Marek Vasut wrote:
> On 12/18/2017 10:02 AM, linux-kernel-dev at beckhoff.com wrote:
> > From: Patrick Bruenn <p.bruenn@beckhoff.com>
> > 
> > Static variables are not available during board_init_f().
> 
> They are, since the board runs from RAM at that point already.
> 
That's not the point. Zero-initialized variables (static or not) are
located in the .bss section, which is overlayed by the .rel.dyn
section. Thus writing to such a variable before relocation will trash
the relocation data.


Lothar Waßmann

[U-Boot] [PATCH v2 3/4] arm: imx: m53evk: remove usage of mx53_dram_size

[Marek Vasut]

On 12/19/2017 05:28 AM, Patrick Brünn wrote:
>> From: Marek Vasut [mailto:marex at denx.de]
>> Sent: Montag, 18. Dezember 2017 13:30
>> On 12/18/2017 01:16 PM, Patrick Brünn wrote:
>>>> From: Marek Vasut [mailto:marex at denx.de]
>>>> Sent: Montag, 18. Dezember 2017 12:52
>>>> On 12/18/2017 12:40 PM, Patrick Brünn wrote:
> [...]
>>>> btw do you use SPL ? If not, you should ...
>>> I will read more about SPL. Until now, I thought I will only benefit,
>>> if my initial boot media is limited in size. As we boot from sdcard,
>>> that wasn't a problem to store 360k u-boot.
>>
>> The other is that you can ie. skip the whole u-boot altogether and boot
>> linux directly.
>>
> Okay, I will try that.
>> I wonder if it would be better to keep the static variable and avoid
>> calling the get_ram_size() twice, it can save some CPU cycles. Besides
>> that, thanks for the explanation/discussion !1
>>
> The pleasure was all mine. I will test SPL on my board and in case that's working I think it still makes sense to move our common code into one file. Is arch/arm/mach-imx/mx53_dram.c the best location for it?
> At first I tried board/freescale/common/ but in that case I needed to use ugly relative pathes in the Makefiles "../../"

I think arch/arm/mach-imx/ is good , yes . We already have some imx6 ddr
there too, maybe you can follow that scheme.

> Regards, Patrick
> 
> ---
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
> 
> 


-- 
Best regards,
Marek Vasut