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

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

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

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@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 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; -} - u32 get_board_rev(void) { struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;

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[] = {

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;

Hi Patrick,
On 16/01/2018 07:59, linux-kernel-dev@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
participants (2)
-
linux-kernel-dev@beckhoff.com
-
Stefano Babic