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

[U-Boot] [PATCH v3 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 is required to let cx9020 boot again. Please include at
least this one. Without that patch u-boot on cx9020 is broken.

The second patch moves that code to a common place to be reused by
m53evk and mx53loco. Pick it only if you think this change is useful for
one of these boards and you are willing to include the third and/or last
patch.
Pick the third patch if you want to use the changed code for m53evk.
Pick the last patch if you want to use the changed code for mx53loco.


Changes in v3:
- rebase to v2018.01
- fix the cover-letter to make it more clear that the first patch is
  required to make the cx9020 boot again. All other patches are
  optional.

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 v3 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 v3:
- rebase to v2018.01
- fix the cover-letter to make it more clear that the first patch is
  required to make the cx9020 boot again. All other patches are
  optional.

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 v3 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 v3: None
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 v3 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 v3: None
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 v3 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 v3: None
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 v3 0/4] arm: imx53: remove usage of mx53_dram_size

[Stefano Babic]

Hi Patrick,

On 16/01/2018 07:59, linux-kernel-dev at beckhoff.com wrote:
> 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 is required to let cx9020 boot again. Please include at
> least this one. Without that patch u-boot on cx9020 is broken.
> 

I applied this.

> The second patch moves that code to a common place to be reused by
> m53evk and mx53loco. Pick it only if you think this change is useful for
> one of these boards and you are willing to include the third and/or last
> patch.

From discussion between you and Marek, I understood that mx53evk is fine
with changes. Applied, too.

> Pick the third patch if you want to use the changed code for m53evk.
> Pick the last patch if you want to use the changed code for mx53loco.

And if mx53evk is fine, I prefer that mx53 boards behave in the same
way. I pick up these, too.

Thanks,

Stefano Babic

> 
> 
> Changes in v3:
> - rebase to v2018.01
> - fix the cover-letter to make it more clear that the first patch is
>   required to make the cx9020 boot again. All other patches are
>   optional.
> 
> 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
> 


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================