[PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Stefan Roese
Since commit 86cf1c82850f ("configs: Migrate CONFIG_NR_DRAM_BANKS") &
commit 999a772d9f24 ("Kconfig: Migrate CONFIG_NR_DRAM_BANKS"),
CONFIG_NR_DRAM_BANKS is always defined with a value (4 is default).
It makes no sense to still carry code that is guarded with
"#ifndef CONFIG_NR_DRAM_BANKS" (and similar). This patch removes
all these unreferenced code paths.

Also complete remove bi_memstart & bi_memsize from the board-info
struct. As now bi_dram[] is always enabled and should be used instead.
This removes the redundant varriables.

Signed-off-by: Stefan Roese <[hidden email]>
Cc: Daniel Schwierzeck <[hidden email]>
Cc: Tom Rini <[hidden email]>
Cc: Ramon Fried <[hidden email]>
Cc: Simon Glass <[hidden email]>
Cc: Michal Simek <[hidden email]>
Cc: Pali Rohár <[hidden email]>
---
v3:
- Rebase on TOT
- Add check to not overwrite bi_dram[0] if already configured in
  setup_bdinfo(). This fixes the test issue with Nokia RX51.

Azure test report success:
https://dev.azure.com/sr0718/u-boot/_build/results?buildId=26&view=results
 
v2:
- Change all references to bi_memstart & bi_memsize to bi_dram[0].start/
  size and remove it from the bd_info struct completely, as suggested by
  Daniel

 api/api_platform-mips.c               |  4 ++--
 api/api_platform-powerpc.c            |  2 +-
 arch/mips/lib/boot.c                  |  2 +-
 arch/mips/lib/bootm.c                 |  2 +-
 arch/powerpc/cpu/mpc83xx/fdt.c        |  2 +-
 arch/powerpc/cpu/mpc83xx/traps.c      |  2 +-
 arch/powerpc/cpu/mpc85xx/fdt.c        |  4 ++--
 arch/powerpc/cpu/mpc85xx/traps.c      |  2 +-
 arch/powerpc/cpu/mpc86xx/fdt.c        |  2 +-
 arch/powerpc/cpu/mpc86xx/traps.c      |  2 +-
 arch/powerpc/cpu/mpc8xx/fdt.c         |  2 +-
 arch/powerpc/lib/bootm.c              |  4 ++--
 arch/x86/cpu/broadwell/cpu_from_spl.c |  2 --
 arch/xtensa/lib/bdinfo.c              |  4 ++--
 arch/xtensa/lib/bootm.c               |  4 ++--
 board/Arcturus/ucp1020/spl.c          |  4 ++--
 board/freescale/p1010rdb/spl.c        |  4 ++--
 board/freescale/p1_p2_rdb_pc/spl.c    |  4 ++--
 board/freescale/t102xrdb/spl.c        |  4 ++--
 board/freescale/t104xrdb/spl.c        |  4 ++--
 board/freescale/t208xqds/spl.c        |  4 ++--
 board/freescale/t208xrdb/spl.c        |  4 ++--
 board/freescale/t4rdb/spl.c           |  4 ++--
 board/xilinx/zynqmp/zynqmp.c          |  2 --
 cmd/bdinfo.c                          |  6 ++---
 cmd/bedbug.c                          |  2 +-
 common/board_f.c                      | 13 +++++------
 common/image.c                        | 10 +-------
 common/init/handoff.c                 | 33 +++++++++++----------------
 drivers/pci/pci-uclass.c              | 17 +-------------
 drivers/video/cfb_console.c           |  8 +------
 include/asm-generic/u-boot.h          |  4 ----
 include/handoff.h                     |  2 --
 lib/fdtdec.c                          |  5 ----
 lib/lmb.c                             |  7 ------
 35 files changed, 60 insertions(+), 121 deletions(-)

diff --git a/api/api_platform-mips.c b/api/api_platform-mips.c
index 51cd328b3d..f1721cef19 100644
--- a/api/api_platform-mips.c
+++ b/api/api_platform-mips.c
@@ -24,8 +24,8 @@ DECLARE_GLOBAL_DATA_PTR;
 int platform_sys_info(struct sys_info *si)
 {
 
- platform_set_mr(si, gd->bd->bi_memstart,
- gd->bd->bi_memsize, MR_ATTR_DRAM);
+ platform_set_mr(si, gd->bd->bi_dram[0].start,
+ gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
 
  return 1;
 }
diff --git a/api/api_platform-powerpc.c b/api/api_platform-powerpc.c
index 15930cfdb6..8fff6e4cae 100644
--- a/api/api_platform-powerpc.c
+++ b/api/api_platform-powerpc.c
@@ -42,7 +42,7 @@ int platform_sys_info(struct sys_info *si)
  si->bar = 0;
 #endif
 
- platform_set_mr(si, gd->bd->bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);
+ platform_set_mr(si, gd->bd->bi_dram[0].start, gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
  platform_set_mr(si, gd->bd->bi_flashstart, gd->bd->bi_flashsize, MR_ATTR_FLASH);
  platform_set_mr(si, gd->bd->bi_sramstart, gd->bd->bi_sramsize, MR_ATTR_SRAM);
 
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c
index db862f6379..3b960691c5 100644
--- a/arch/mips/lib/boot.c
+++ b/arch/mips/lib/boot.c
@@ -17,7 +17,7 @@ unsigned long do_go_exec(ulong (*entry)(int, char * const []),
  * whole SDRAM area, since we don't know the size of the image
  * that was loaded.
  */
- flush_cache(gd->bd->bi_memstart, gd->ram_top - gd->bd->bi_memstart);
+ flush_cache(gd->bd->bi_dram[0].start, gd->ram_top - gd->bd->bi_dram[0].start);
 
  return entry(argc, argv);
 }
diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
index 0a13f6edb7..b3ef15963e 100644
--- a/arch/mips/lib/bootm.c
+++ b/arch/mips/lib/bootm.c
@@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
 #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
 int arch_fixup_fdt(void *blob)
 {
- u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
+ u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
  u64 mem_size = gd->ram_size;
 
  return fdt_fixup_memory_banks(blob, &mem_start, &mem_size, 1);
diff --git a/arch/powerpc/cpu/mpc83xx/fdt.c b/arch/powerpc/cpu/mpc83xx/fdt.c
index ebdedb2888..05523ecc67 100644
--- a/arch/powerpc/cpu/mpc83xx/fdt.c
+++ b/arch/powerpc/cpu/mpc83xx/fdt.c
@@ -121,7 +121,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
                 "clock-frequency", get_serial_clock(), 1);
 #endif
 
- fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
+ fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
 
 #if defined(CONFIG_BOOTCOUNT_LIMIT) && \
  (defined(CONFIG_QE) && !defined(CONFIG_ARCH_MPC831X))
diff --git a/arch/powerpc/cpu/mpc83xx/traps.c b/arch/powerpc/cpu/mpc83xx/traps.c
index c3cc119d65..5a8cd36420 100644
--- a/arch/powerpc/cpu/mpc83xx/traps.c
+++ b/arch/powerpc/cpu/mpc83xx/traps.c
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
 /* Returns 0 if exception not found and fixup otherwise.  */
 extern unsigned long search_exception_table(unsigned long);
 
-#define END_OF_MEM (gd->bd->bi_memstart + gd->bd->bi_memsize)
+#define END_OF_MEM (gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size)
 
 /*
  * Trap & Exception support
diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
index 9569c1a64b..2cc567a5f4 100644
--- a/arch/powerpc/cpu/mpc85xx/fdt.c
+++ b/arch/powerpc/cpu/mpc85xx/fdt.c
@@ -672,10 +672,10 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
  "clock-frequency", get_bus_freq(0), 1);
 #endif
 
- fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
+ fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
 
 #ifdef CONFIG_MP
- ft_fixup_cpu(blob, (u64)bd->bi_memstart + (u64)bd->bi_memsize);
+ ft_fixup_cpu(blob, (u64)bd->bi_dram[0].start + (u64)bd->bi_dram[0].size);
  ft_fixup_num_cores(blob);
 #endif
 
diff --git a/arch/powerpc/cpu/mpc85xx/traps.c b/arch/powerpc/cpu/mpc85xx/traps.c
index f37a45e269..061b7b8f6e 100644
--- a/arch/powerpc/cpu/mpc85xx/traps.c
+++ b/arch/powerpc/cpu/mpc85xx/traps.c
@@ -37,7 +37,7 @@ extern unsigned long search_exception_table(unsigned long);
  * amount of memory on the system if we're unable to keep all
  * the memory mapped in.
  */
-#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
+#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
 
 static __inline__ void set_tsr(unsigned long val)
 {
diff --git a/arch/powerpc/cpu/mpc86xx/fdt.c b/arch/powerpc/cpu/mpc86xx/fdt.c
index 24e53115ec..143575befd 100644
--- a/arch/powerpc/cpu/mpc86xx/fdt.c
+++ b/arch/powerpc/cpu/mpc86xx/fdt.c
@@ -27,7 +27,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
  do_fixup_by_prop_u32(blob, "device_type", "soc", 4,
      "bus-frequency", bd->bi_busfreq, 1);
 
- fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
+ fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
 
 #ifdef CONFIG_SYS_NS16550
  do_fixup_by_compat_u32(blob, "ns16550",
diff --git a/arch/powerpc/cpu/mpc86xx/traps.c b/arch/powerpc/cpu/mpc86xx/traps.c
index c0161e3379..532df93c3c 100644
--- a/arch/powerpc/cpu/mpc86xx/traps.c
+++ b/arch/powerpc/cpu/mpc86xx/traps.c
@@ -30,7 +30,7 @@ extern unsigned long search_exception_table(unsigned long);
  * amount of memory on the system if we're unable to keep all
  * the memory mapped in.
  */
-#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
+#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
 
 /*
  * Trap & Exception support
diff --git a/arch/powerpc/cpu/mpc8xx/fdt.c b/arch/powerpc/cpu/mpc8xx/fdt.c
index 4d952a3882..e593242631 100644
--- a/arch/powerpc/cpu/mpc8xx/fdt.c
+++ b/arch/powerpc/cpu/mpc8xx/fdt.c
@@ -25,5 +25,5 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
  do_fixup_by_compat_u32(blob, "fsl,cpm-brg", "clock-frequency",
        gd->arch.brg_clk, 1);
 
- fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
+ fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
 }
diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
index 8c8ed99cd3..d8bf5a14f6 100644
--- a/arch/powerpc/lib/bootm.c
+++ b/arch/powerpc/lib/bootm.c
@@ -298,8 +298,8 @@ void boot_prep_vxworks(bootm_headers_t *images)
  if (!images->ft_addr)
  return;
 
- base = (u64)gd->bd->bi_memstart;
- size = (u64)gd->bd->bi_memsize;
+ base = (u64)gd->bd->bi_dram[0].start;
+ size = (u64)gd->bd->bi_dram[0].size;
 
  off = fdt_path_offset(images->ft_addr, "/memory");
  if (off < 0)
diff --git a/arch/x86/cpu/broadwell/cpu_from_spl.c b/arch/x86/cpu/broadwell/cpu_from_spl.c
index 6567d50653..4d4cdafa2b 100644
--- a/arch/x86/cpu/broadwell/cpu_from_spl.c
+++ b/arch/x86/cpu/broadwell/cpu_from_spl.c
@@ -53,14 +53,12 @@ void board_debug_uart_init(void)
 
 int dram_init_banksize(void)
 {
-#ifdef CONFIG_NR_DRAM_BANKS
  struct spl_handoff *ho;
 
  ho = bloblist_find(BLOBLISTT_SPL_HANDOFF, sizeof(*ho));
  if (!ho)
  return log_msg_ret("Missing SPL hand-off info", -ENOENT);
  handoff_load_dram_banks(ho);
-#endif
 
  return 0;
 }
diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
index 4ec8529521..1839edd9bb 100644
--- a/arch/xtensa/lib/bdinfo.c
+++ b/arch/xtensa/lib/bdinfo.c
@@ -15,8 +15,8 @@ int arch_setup_bdinfo(void)
 {
  struct bd_info *bd = gd->bd;
 
- bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
- bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
+ bd->bi_dram[0].start = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
+ bd->bi_dram[0].size = CONFIG_SYS_SDRAM_SIZE;
 
  return 0;
 }
diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
index 458eaf95c0..16ab51dbc9 100644
--- a/arch/xtensa/lib/bootm.c
+++ b/arch/xtensa/lib/bootm.c
@@ -48,8 +48,8 @@ static struct bp_tag *setup_memory_tag(struct bp_tag *params)
  params->size = sizeof(struct meminfo);
  mem = (struct meminfo *)params->data;
  mem->type = MEMORY_TYPE_CONVENTIONAL;
- mem->start = bd->bi_memstart;
- mem->end = bd->bi_memstart + bd->bi_memsize;
+ mem->start = bd->bi_dram[0].start;
+ mem->end = bd->bi_dram[0].start + bd->bi_dram[0].size;
 
  printf("   MEMORY:          tag:0x%04x, type:0X%lx, start:0X%lx, end:0X%lx\n",
        BP_TAG_MEMORY, mem->type, mem->start, mem->end);
diff --git a/board/Arcturus/ucp1020/spl.c b/board/Arcturus/ucp1020/spl.c
index 5416a5b663..8eb641ef8d 100644
--- a/board/Arcturus/ucp1020/spl.c
+++ b/board/Arcturus/ucp1020/spl.c
@@ -83,8 +83,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
- bd->bi_memsize = CONFIG_SYS_L2_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/p1010rdb/spl.c b/board/freescale/p1010rdb/spl.c
index 4ee4573d2b..ddc0af3faa 100644
--- a/board/freescale/p1010rdb/spl.c
+++ b/board/freescale/p1010rdb/spl.c
@@ -69,8 +69,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
- bd->bi_memsize = CONFIG_SYS_L2_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/p1_p2_rdb_pc/spl.c b/board/freescale/p1_p2_rdb_pc/spl.c
index e76c3e82c3..f67b0fcbb9 100644
--- a/board/freescale/p1_p2_rdb_pc/spl.c
+++ b/board/freescale/p1_p2_rdb_pc/spl.c
@@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
- bd->bi_memsize = CONFIG_SYS_L2_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/t102xrdb/spl.c b/board/freescale/t102xrdb/spl.c
index da442fcc18..ce7ccfe41d 100644
--- a/board/freescale/t102xrdb/spl.c
+++ b/board/freescale/t102xrdb/spl.c
@@ -103,8 +103,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(gd + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
- bd->bi_memsize = CONFIG_SYS_L3_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/t104xrdb/spl.c b/board/freescale/t104xrdb/spl.c
index f83d69ba15..4b5fb8b955 100644
--- a/board/freescale/t104xrdb/spl.c
+++ b/board/freescale/t104xrdb/spl.c
@@ -94,8 +94,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(gd + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
- bd->bi_memsize = CONFIG_SYS_L3_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/t208xqds/spl.c b/board/freescale/t208xqds/spl.c
index c197884421..dc0046d93c 100644
--- a/board/freescale/t208xqds/spl.c
+++ b/board/freescale/t208xqds/spl.c
@@ -102,8 +102,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(gd + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
- bd->bi_memsize = CONFIG_SYS_L3_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/t208xrdb/spl.c b/board/freescale/t208xrdb/spl.c
index 07aab6349c..f523622de9 100644
--- a/board/freescale/t208xrdb/spl.c
+++ b/board/freescale/t208xrdb/spl.c
@@ -72,8 +72,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(gd + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
- bd->bi_memsize = CONFIG_SYS_L3_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/freescale/t4rdb/spl.c b/board/freescale/t4rdb/spl.c
index 64d2753da8..a5351e813a 100644
--- a/board/freescale/t4rdb/spl.c
+++ b/board/freescale/t4rdb/spl.c
@@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
  bd = (struct bd_info *)(gd + sizeof(gd_t));
  memset(bd, 0, sizeof(struct bd_info));
  gd->bd = bd;
- bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
- bd->bi_memsize = CONFIG_SYS_L3_SIZE;
+ bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
+ bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
 
  arch_cpu_init();
  get_clocks();
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index ebb7172908..4cc5cb6fd7 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -467,10 +467,8 @@ int dram_init(void)
 #else
 int dram_init_banksize(void)
 {
-#if defined(CONFIG_NR_DRAM_BANKS)
  gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
  gd->bd->bi_dram[0].size = get_effective_memsize();
-#endif
 
  mem_map_fill();
 
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index 9593b345a3..117788dade 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -49,7 +49,6 @@ void bdinfo_print_mhz(const char *name, unsigned long hz)
 
 static void print_bi_dram(const struct bd_info *bd)
 {
-#ifdef CONFIG_NR_DRAM_BANKS
  int i;
 
  for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
@@ -59,7 +58,6 @@ static void print_bi_dram(const struct bd_info *bd)
  bdinfo_print_num("-> size", bd->bi_dram[i].size);
  }
  }
-#endif
 }
 
 __weak void arch_print_bdinfo(void)
@@ -75,8 +73,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 #endif
  bdinfo_print_num("boot_params", (ulong)bd->bi_boot_params);
  print_bi_dram(bd);
- bdinfo_print_num("memstart", (ulong)bd->bi_memstart);
- print_phys_addr("memsize", bd->bi_memsize);
+ bdinfo_print_num("memstart", (ulong)bd->bi_dram[0].start);
+ print_phys_addr("memsize", bd->bi_dram[0].size);
  if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
  bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
  bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
diff --git a/cmd/bedbug.c b/cmd/bedbug.c
index 81ce256480..0db42d7eed 100644
--- a/cmd/bedbug.c
+++ b/cmd/bedbug.c
@@ -348,7 +348,7 @@ int do_bedbug_stack(struct cmd_tbl *cmdtp, int flag, int argc,
  return 1;
  }
 
- top = gd->bd->bi_memstart + gd->bd->bi_memsize;
+ top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;
  depth = 0;
 
  printf ("Depth     PC\n");
diff --git a/common/board_f.c b/common/board_f.c
index 79532f4365..1d2f6a2545 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -216,7 +216,6 @@ static int show_dram_config(void)
 {
  unsigned long long size;
 
-#ifdef CONFIG_NR_DRAM_BANKS
  int i;
 
  debug("\nRAM Configuration:\n");
@@ -229,9 +228,6 @@ static int show_dram_config(void)
 #endif
  }
  debug("\nDRAM:  ");
-#else
- size = gd->ram_size;
-#endif
 
  print_size(size, "");
  board_add_ram_info(0);
@@ -242,7 +238,7 @@ static int show_dram_config(void)
 
 __weak int dram_init_banksize(void)
 {
-#if defined(CONFIG_NR_DRAM_BANKS) && defined(CONFIG_SYS_SDRAM_BASE)
+#if defined(CONFIG_SYS_SDRAM_BASE)
  gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
  gd->bd->bi_dram[0].size = get_effective_memsize();
 #endif
@@ -607,8 +603,11 @@ int setup_bdinfo(void)
 {
  struct bd_info *bd = gd->bd;
 
- bd->bi_memstart = gd->ram_base;  /* start of memory */
- bd->bi_memsize = gd->ram_size;   /* size in bytes */
+ /* Only overwrite bi_dram[0] values if not already set */
+ if (!bd->bi_dram[0].size) {
+ bd->bi_dram[0].start = gd->ram_base;  /* start of memory */
+ bd->bi_dram[0].size = gd->ram_size;   /* size in bytes */
+ }
 
  if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
  bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
diff --git a/common/image.c b/common/image.c
index 9d7d5c17d1..35db5c2fda 100644
--- a/common/image.c
+++ b/common/image.c
@@ -668,10 +668,8 @@ ulong env_get_bootm_low(void)
 
 #if defined(CONFIG_SYS_SDRAM_BASE)
  return CONFIG_SYS_SDRAM_BASE;
-#elif defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)
- return gd->bd->bi_dram[0].start;
 #else
- return 0;
+ return gd->bd->bi_dram[0].start;
 #endif
 }
 
@@ -685,14 +683,8 @@ phys_size_t env_get_bootm_size(void)
  return tmp;
  }
 
-#if (defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)) && \
-     defined(CONFIG_NR_DRAM_BANKS)
  start = gd->bd->bi_dram[0].start;
  size = gd->bd->bi_dram[0].size;
-#else
- start = gd->bd->bi_memstart;
- size = gd->bd->bi_memsize;
-#endif
 
  s = env_get("bootm_low");
  if (s)
diff --git a/common/init/handoff.c b/common/init/handoff.c
index e00b43e6a7..62071bd017 100644
--- a/common/init/handoff.c
+++ b/common/init/handoff.c
@@ -12,18 +12,15 @@ DECLARE_GLOBAL_DATA_PTR;
 
 void handoff_save_dram(struct spl_handoff *ho)
 {
+ struct bd_info *bd = gd->bd;
+ int i;
+
  ho->ram_size = gd->ram_size;
-#ifdef CONFIG_NR_DRAM_BANKS
- {
- struct bd_info *bd = gd->bd;
- int i;
-
- for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
- ho->ram_bank[i].start = bd->bi_dram[i].start;
- ho->ram_bank[i].size = bd->bi_dram[i].size;
- }
+
+ for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+ ho->ram_bank[i].start = bd->bi_dram[i].start;
+ ho->ram_bank[i].size = bd->bi_dram[i].size;
  }
-#endif
 }
 
 void handoff_load_dram_size(struct spl_handoff *ho)
@@ -33,15 +30,11 @@ void handoff_load_dram_size(struct spl_handoff *ho)
 
 void handoff_load_dram_banks(struct spl_handoff *ho)
 {
-#ifdef CONFIG_NR_DRAM_BANKS
- {
- struct bd_info *bd = gd->bd;
- int i;
-
- for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
- bd->bi_dram[i].start = ho->ram_bank[i].start;
- bd->bi_dram[i].size = ho->ram_bank[i].size;
- }
+ struct bd_info *bd = gd->bd;
+ int i;
+
+ for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+ bd->bi_dram[i].start = ho->ram_bank[i].start;
+ bd->bi_dram[i].size = ho->ram_bank[i].size;
  }
-#endif
 }
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 834526c5a4..69fb46d3f4 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -871,6 +871,7 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
    ofnode node)
 {
  int pci_addr_cells, addr_cells, size_cells;
+ struct bd_info *bd = gd->bd;
  int cells_per_record;
  const u32 *prop;
  int len;
@@ -938,9 +939,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
  }
 
  /* Add a region for our local memory */
-#ifdef CONFIG_NR_DRAM_BANKS
- struct bd_info *bd = gd->bd;
-
  if (!bd)
  return;
 
@@ -958,19 +956,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
        PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
  }
  }
-#else
- phys_addr_t base = 0, size;
-
- size = gd->ram_size;
-#ifdef CONFIG_SYS_SDRAM_BASE
- base = CONFIG_SYS_SDRAM_BASE;
-#endif
- if (gd->pci_ram_top && gd->pci_ram_top < base + size)
- size = gd->pci_ram_top - base;
- if (size)
- pci_set_region(hose->regions + hose->region_count++, base,
- base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
-#endif
 
  return;
 }
diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
index badade353e..3f07f4eb29 100644
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -1983,8 +1983,6 @@ static void *video_logo(void)
 static int cfb_fb_is_in_dram(void)
 {
  struct bd_info *bd = gd->bd;
-#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || \
-defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
  ulong start, end;
  int i;
 
@@ -1995,11 +1993,7 @@ defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
     (ulong)video_fb_address < end)
  return 1;
  }
-#else
- if ((ulong)video_fb_address >= bd->bi_memstart &&
-    (ulong)video_fb_address < bd->bi_memstart + bd->bi_memsize)
- return 1;
-#endif
+
  return 0;
 }
 
diff --git a/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
index 62e61d41cc..637de0c455 100644
--- a/include/asm-generic/u-boot.h
+++ b/include/asm-generic/u-boot.h
@@ -27,8 +27,6 @@
 #include <linux/types.h>
 
 struct bd_info {
- unsigned long bi_memstart; /* start of DRAM memory */
- phys_size_t bi_memsize; /* size of DRAM memory in bytes */
  unsigned long bi_flashstart; /* start of FLASH memory */
  unsigned long bi_flashsize; /* size of FLASH memory */
  unsigned long bi_flashoffset; /* reserved area for startup monitor */
@@ -70,12 +68,10 @@ struct bd_info {
 #endif
  ulong        bi_arch_number; /* unique id for this board */
  ulong        bi_boot_params; /* where this board expects params */
-#ifdef CONFIG_NR_DRAM_BANKS
  struct { /* RAM configuration */
  phys_addr_t start;
  phys_size_t size;
  } bi_dram[CONFIG_NR_DRAM_BANKS];
-#endif /* CONFIG_NR_DRAM_BANKS */
 };
 
 #endif /* __ASSEMBLY__ */
diff --git a/include/handoff.h b/include/handoff.h
index 75d19b1f6e..070a79c1b9 100644
--- a/include/handoff.h
+++ b/include/handoff.h
@@ -20,12 +20,10 @@
 struct spl_handoff {
  struct arch_spl_handoff arch;
  u64 ram_size;
-#ifdef CONFIG_NR_DRAM_BANKS
  struct {
  u64 start;
  u64 size;
  } ram_bank[CONFIG_NR_DRAM_BANKS];
-#endif
 };
 
 void handoff_save_dram(struct spl_handoff *ho);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 78576b530f..ecbf10121d 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1055,8 +1055,6 @@ int fdtdec_setup_mem_size_base(void)
  return 0;
 }
 
-#if defined(CONFIG_NR_DRAM_BANKS)
-
 static int get_next_memory_node(const void *blob, int mem)
 {
  do {
@@ -1106,7 +1104,6 @@ int fdtdec_setup_memory_banksize(void)
 
  return 0;
 }
-#endif
 
 #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
 # if CONFIG_IS_ENABLED(MULTI_DTB_FIT_GZIP) ||\
@@ -1569,7 +1566,6 @@ int fdtdec_resetup(int *rescan)
 }
 #endif
 
-#ifdef CONFIG_NR_DRAM_BANKS
 int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
    phys_addr_t *basep, phys_size_t *sizep,
    struct bd_info *bd)
@@ -1675,6 +1671,5 @@ int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
 
  return 0;
 }
-#endif /* CONFIG_NR_DRAM_BANKS */
 
 #endif /* !USE_HOSTCC */
diff --git a/lib/lmb.c b/lib/lmb.c
index 75082f3559..600f55051c 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -117,22 +117,15 @@ static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
 /* Initialize the struct, add memory and call arch/board reserve functions */
 void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void *fdt_blob)
 {
-#ifdef CONFIG_NR_DRAM_BANKS
  int i;
-#endif
 
  lmb_init(lmb);
-#ifdef CONFIG_NR_DRAM_BANKS
  for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
  if (bd->bi_dram[i].size) {
  lmb_add(lmb, bd->bi_dram[i].start,
  bd->bi_dram[i].size);
  }
  }
-#else
- if (bd->bi_memsize)
- lmb_add(lmb, bd->bi_memstart, bd->bi_memsize);
-#endif
  lmb_reserve_common(lmb, fdt_blob);
 }
 
--
2.28.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Pali Rohár-2
On Wednesday 12 August 2020 09:44:17 Stefan Roese wrote:

> Since commit 86cf1c82850f ("configs: Migrate CONFIG_NR_DRAM_BANKS") &
> commit 999a772d9f24 ("Kconfig: Migrate CONFIG_NR_DRAM_BANKS"),
> CONFIG_NR_DRAM_BANKS is always defined with a value (4 is default).
> It makes no sense to still carry code that is guarded with
> "#ifndef CONFIG_NR_DRAM_BANKS" (and similar). This patch removes
> all these unreferenced code paths.
>
> Also complete remove bi_memstart & bi_memsize from the board-info
> struct. As now bi_dram[] is always enabled and should be used instead.
> This removes the redundant varriables.
>
> Signed-off-by: Stefan Roese <[hidden email]>
> Cc: Daniel Schwierzeck <[hidden email]>
> Cc: Tom Rini <[hidden email]>
> Cc: Ramon Fried <[hidden email]>
> Cc: Simon Glass <[hidden email]>
> Cc: Michal Simek <[hidden email]>
> Cc: Pali Rohár <[hidden email]>
> ---
> v3:
> - Rebase on TOT
> - Add check to not overwrite bi_dram[0] if already configured in
>   setup_bdinfo(). This fixes the test issue with Nokia RX51.
>
> Azure test report success:
> https://dev.azure.com/sr0718/u-boot/_build/results?buildId=26&view=results
>  
> v2:
> - Change all references to bi_memstart & bi_memsize to bi_dram[0].start/
>   size and remove it from the bd_info struct completely, as suggested by
>   Daniel
>
>  api/api_platform-mips.c               |  4 ++--
>  api/api_platform-powerpc.c            |  2 +-
>  arch/mips/lib/boot.c                  |  2 +-
>  arch/mips/lib/bootm.c                 |  2 +-
>  arch/powerpc/cpu/mpc83xx/fdt.c        |  2 +-
>  arch/powerpc/cpu/mpc83xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc85xx/fdt.c        |  4 ++--
>  arch/powerpc/cpu/mpc85xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc86xx/fdt.c        |  2 +-
>  arch/powerpc/cpu/mpc86xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc8xx/fdt.c         |  2 +-
>  arch/powerpc/lib/bootm.c              |  4 ++--
>  arch/x86/cpu/broadwell/cpu_from_spl.c |  2 --
>  arch/xtensa/lib/bdinfo.c              |  4 ++--
>  arch/xtensa/lib/bootm.c               |  4 ++--
>  board/Arcturus/ucp1020/spl.c          |  4 ++--
>  board/freescale/p1010rdb/spl.c        |  4 ++--
>  board/freescale/p1_p2_rdb_pc/spl.c    |  4 ++--
>  board/freescale/t102xrdb/spl.c        |  4 ++--
>  board/freescale/t104xrdb/spl.c        |  4 ++--
>  board/freescale/t208xqds/spl.c        |  4 ++--
>  board/freescale/t208xrdb/spl.c        |  4 ++--
>  board/freescale/t4rdb/spl.c           |  4 ++--
>  board/xilinx/zynqmp/zynqmp.c          |  2 --
>  cmd/bdinfo.c                          |  6 ++---
>  cmd/bedbug.c                          |  2 +-
>  common/board_f.c                      | 13 +++++------
>  common/image.c                        | 10 +-------
>  common/init/handoff.c                 | 33 +++++++++++----------------
>  drivers/pci/pci-uclass.c              | 17 +-------------
>  drivers/video/cfb_console.c           |  8 +------
>  include/asm-generic/u-boot.h          |  4 ----
>  include/handoff.h                     |  2 --
>  lib/fdtdec.c                          |  5 ----
>  lib/lmb.c                             |  7 ------
>  35 files changed, 60 insertions(+), 121 deletions(-)
>
> diff --git a/api/api_platform-mips.c b/api/api_platform-mips.c
> index 51cd328b3d..f1721cef19 100644
> --- a/api/api_platform-mips.c
> +++ b/api/api_platform-mips.c
> @@ -24,8 +24,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  int platform_sys_info(struct sys_info *si)
>  {
>  
> - platform_set_mr(si, gd->bd->bi_memstart,
> - gd->bd->bi_memsize, MR_ATTR_DRAM);
> + platform_set_mr(si, gd->bd->bi_dram[0].start,
> + gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>  
>   return 1;
>  }
> diff --git a/api/api_platform-powerpc.c b/api/api_platform-powerpc.c
> index 15930cfdb6..8fff6e4cae 100644
> --- a/api/api_platform-powerpc.c
> +++ b/api/api_platform-powerpc.c
> @@ -42,7 +42,7 @@ int platform_sys_info(struct sys_info *si)
>   si->bar = 0;
>  #endif
>  
> - platform_set_mr(si, gd->bd->bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);
> + platform_set_mr(si, gd->bd->bi_dram[0].start, gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>   platform_set_mr(si, gd->bd->bi_flashstart, gd->bd->bi_flashsize, MR_ATTR_FLASH);
>   platform_set_mr(si, gd->bd->bi_sramstart, gd->bd->bi_sramsize, MR_ATTR_SRAM);
>  
> diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c
> index db862f6379..3b960691c5 100644
> --- a/arch/mips/lib/boot.c
> +++ b/arch/mips/lib/boot.c
> @@ -17,7 +17,7 @@ unsigned long do_go_exec(ulong (*entry)(int, char * const []),
>   * whole SDRAM area, since we don't know the size of the image
>   * that was loaded.
>   */
> - flush_cache(gd->bd->bi_memstart, gd->ram_top - gd->bd->bi_memstart);
> + flush_cache(gd->bd->bi_dram[0].start, gd->ram_top - gd->bd->bi_dram[0].start);
>  
>   return entry(argc, argv);
>  }
> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> index 0a13f6edb7..b3ef15963e 100644
> --- a/arch/mips/lib/bootm.c
> +++ b/arch/mips/lib/bootm.c
> @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
>  #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
>  int arch_fixup_fdt(void *blob)
>  {
> - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
> + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
>   u64 mem_size = gd->ram_size;

I do not fully understand this change. Why on some places you are suing
bi_dram[0].size and on some places gd->ram_size?

>  
>   return fdt_fixup_memory_banks(blob, &mem_start, &mem_size, 1);
> diff --git a/arch/powerpc/cpu/mpc83xx/fdt.c b/arch/powerpc/cpu/mpc83xx/fdt.c
> index ebdedb2888..05523ecc67 100644
> --- a/arch/powerpc/cpu/mpc83xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
> @@ -121,7 +121,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>                  "clock-frequency", get_serial_clock(), 1);
>  #endif
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #if defined(CONFIG_BOOTCOUNT_LIMIT) && \
>   (defined(CONFIG_QE) && !defined(CONFIG_ARCH_MPC831X))
> diff --git a/arch/powerpc/cpu/mpc83xx/traps.c b/arch/powerpc/cpu/mpc83xx/traps.c
> index c3cc119d65..5a8cd36420 100644
> --- a/arch/powerpc/cpu/mpc83xx/traps.c
> +++ b/arch/powerpc/cpu/mpc83xx/traps.c
> @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  /* Returns 0 if exception not found and fixup otherwise.  */
>  extern unsigned long search_exception_table(unsigned long);
>  
> -#define END_OF_MEM (gd->bd->bi_memstart + gd->bd->bi_memsize)
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size)
>  
>  /*
>   * Trap & Exception support
> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
> index 9569c1a64b..2cc567a5f4 100644
> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
> @@ -672,10 +672,10 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   "clock-frequency", get_bus_freq(0), 1);
>  #endif
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #ifdef CONFIG_MP
> - ft_fixup_cpu(blob, (u64)bd->bi_memstart + (u64)bd->bi_memsize);
> + ft_fixup_cpu(blob, (u64)bd->bi_dram[0].start + (u64)bd->bi_dram[0].size);
>   ft_fixup_num_cores(blob);
>  #endif
>  
> diff --git a/arch/powerpc/cpu/mpc85xx/traps.c b/arch/powerpc/cpu/mpc85xx/traps.c
> index f37a45e269..061b7b8f6e 100644
> --- a/arch/powerpc/cpu/mpc85xx/traps.c
> +++ b/arch/powerpc/cpu/mpc85xx/traps.c
> @@ -37,7 +37,7 @@ extern unsigned long search_exception_table(unsigned long);
>   * amount of memory on the system if we're unable to keep all
>   * the memory mapped in.
>   */
> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())

And on other place is get_effective_memsize()?

>  static __inline__ void set_tsr(unsigned long val)
>  {
> diff --git a/arch/powerpc/cpu/mpc86xx/fdt.c b/arch/powerpc/cpu/mpc86xx/fdt.c
> index 24e53115ec..143575befd 100644
> --- a/arch/powerpc/cpu/mpc86xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc86xx/fdt.c
> @@ -27,7 +27,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   do_fixup_by_prop_u32(blob, "device_type", "soc", 4,
>       "bus-frequency", bd->bi_busfreq, 1);
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #ifdef CONFIG_SYS_NS16550
>   do_fixup_by_compat_u32(blob, "ns16550",
> diff --git a/arch/powerpc/cpu/mpc86xx/traps.c b/arch/powerpc/cpu/mpc86xx/traps.c
> index c0161e3379..532df93c3c 100644
> --- a/arch/powerpc/cpu/mpc86xx/traps.c
> +++ b/arch/powerpc/cpu/mpc86xx/traps.c
> @@ -30,7 +30,7 @@ extern unsigned long search_exception_table(unsigned long);
>   * amount of memory on the system if we're unable to keep all
>   * the memory mapped in.
>   */
> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
>  
>  /*
>   * Trap & Exception support
> diff --git a/arch/powerpc/cpu/mpc8xx/fdt.c b/arch/powerpc/cpu/mpc8xx/fdt.c
> index 4d952a3882..e593242631 100644
> --- a/arch/powerpc/cpu/mpc8xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc8xx/fdt.c
> @@ -25,5 +25,5 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   do_fixup_by_compat_u32(blob, "fsl,cpm-brg", "clock-frequency",
>         gd->arch.brg_clk, 1);
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  }
> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
> index 8c8ed99cd3..d8bf5a14f6 100644
> --- a/arch/powerpc/lib/bootm.c
> +++ b/arch/powerpc/lib/bootm.c
> @@ -298,8 +298,8 @@ void boot_prep_vxworks(bootm_headers_t *images)
>   if (!images->ft_addr)
>   return;
>  
> - base = (u64)gd->bd->bi_memstart;
> - size = (u64)gd->bd->bi_memsize;
> + base = (u64)gd->bd->bi_dram[0].start;
> + size = (u64)gd->bd->bi_dram[0].size;
>  
>   off = fdt_path_offset(images->ft_addr, "/memory");
>   if (off < 0)
> diff --git a/arch/x86/cpu/broadwell/cpu_from_spl.c b/arch/x86/cpu/broadwell/cpu_from_spl.c
> index 6567d50653..4d4cdafa2b 100644
> --- a/arch/x86/cpu/broadwell/cpu_from_spl.c
> +++ b/arch/x86/cpu/broadwell/cpu_from_spl.c
> @@ -53,14 +53,12 @@ void board_debug_uart_init(void)
>  
>  int dram_init_banksize(void)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
>   struct spl_handoff *ho;
>  
>   ho = bloblist_find(BLOBLISTT_SPL_HANDOFF, sizeof(*ho));
>   if (!ho)
>   return log_msg_ret("Missing SPL hand-off info", -ENOENT);
>   handoff_load_dram_banks(ho);
> -#endif
>  
>   return 0;
>  }
> diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
> index 4ec8529521..1839edd9bb 100644
> --- a/arch/xtensa/lib/bdinfo.c
> +++ b/arch/xtensa/lib/bdinfo.c
> @@ -15,8 +15,8 @@ int arch_setup_bdinfo(void)
>  {
>   struct bd_info *bd = gd->bd;
>  
> - bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> - bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
> + bd->bi_dram[0].start = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> + bd->bi_dram[0].size = CONFIG_SYS_SDRAM_SIZE;
>  
>   return 0;
>  }
> diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
> index 458eaf95c0..16ab51dbc9 100644
> --- a/arch/xtensa/lib/bootm.c
> +++ b/arch/xtensa/lib/bootm.c
> @@ -48,8 +48,8 @@ static struct bp_tag *setup_memory_tag(struct bp_tag *params)
>   params->size = sizeof(struct meminfo);
>   mem = (struct meminfo *)params->data;
>   mem->type = MEMORY_TYPE_CONVENTIONAL;
> - mem->start = bd->bi_memstart;
> - mem->end = bd->bi_memstart + bd->bi_memsize;
> + mem->start = bd->bi_dram[0].start;
> + mem->end = bd->bi_dram[0].start + bd->bi_dram[0].size;
>  
>   printf("   MEMORY:          tag:0x%04x, type:0X%lx, start:0X%lx, end:0X%lx\n",
>         BP_TAG_MEMORY, mem->type, mem->start, mem->end);
> diff --git a/board/Arcturus/ucp1020/spl.c b/board/Arcturus/ucp1020/spl.c
> index 5416a5b663..8eb641ef8d 100644
> --- a/board/Arcturus/ucp1020/spl.c
> +++ b/board/Arcturus/ucp1020/spl.c
> @@ -83,8 +83,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/p1010rdb/spl.c b/board/freescale/p1010rdb/spl.c
> index 4ee4573d2b..ddc0af3faa 100644
> --- a/board/freescale/p1010rdb/spl.c
> +++ b/board/freescale/p1010rdb/spl.c
> @@ -69,8 +69,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/p1_p2_rdb_pc/spl.c b/board/freescale/p1_p2_rdb_pc/spl.c
> index e76c3e82c3..f67b0fcbb9 100644
> --- a/board/freescale/p1_p2_rdb_pc/spl.c
> +++ b/board/freescale/p1_p2_rdb_pc/spl.c
> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t102xrdb/spl.c b/board/freescale/t102xrdb/spl.c
> index da442fcc18..ce7ccfe41d 100644
> --- a/board/freescale/t102xrdb/spl.c
> +++ b/board/freescale/t102xrdb/spl.c
> @@ -103,8 +103,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t104xrdb/spl.c b/board/freescale/t104xrdb/spl.c
> index f83d69ba15..4b5fb8b955 100644
> --- a/board/freescale/t104xrdb/spl.c
> +++ b/board/freescale/t104xrdb/spl.c
> @@ -94,8 +94,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t208xqds/spl.c b/board/freescale/t208xqds/spl.c
> index c197884421..dc0046d93c 100644
> --- a/board/freescale/t208xqds/spl.c
> +++ b/board/freescale/t208xqds/spl.c
> @@ -102,8 +102,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t208xrdb/spl.c b/board/freescale/t208xrdb/spl.c
> index 07aab6349c..f523622de9 100644
> --- a/board/freescale/t208xrdb/spl.c
> +++ b/board/freescale/t208xrdb/spl.c
> @@ -72,8 +72,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t4rdb/spl.c b/board/freescale/t4rdb/spl.c
> index 64d2753da8..a5351e813a 100644
> --- a/board/freescale/t4rdb/spl.c
> +++ b/board/freescale/t4rdb/spl.c
> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index ebb7172908..4cc5cb6fd7 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -467,10 +467,8 @@ int dram_init(void)
>  #else
>  int dram_init_banksize(void)
>  {
> -#if defined(CONFIG_NR_DRAM_BANKS)
>   gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>   gd->bd->bi_dram[0].size = get_effective_memsize();
> -#endif
>  
>   mem_map_fill();
>  
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index 9593b345a3..117788dade 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -49,7 +49,6 @@ void bdinfo_print_mhz(const char *name, unsigned long hz)
>  
>  static void print_bi_dram(const struct bd_info *bd)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
>   int i;
>  
>   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> @@ -59,7 +58,6 @@ static void print_bi_dram(const struct bd_info *bd)
>   bdinfo_print_num("-> size", bd->bi_dram[i].size);
>   }
>   }
> -#endif
>  }
>  
>  __weak void arch_print_bdinfo(void)
> @@ -75,8 +73,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  #endif
>   bdinfo_print_num("boot_params", (ulong)bd->bi_boot_params);
>   print_bi_dram(bd);
> - bdinfo_print_num("memstart", (ulong)bd->bi_memstart);
> - print_phys_addr("memsize", bd->bi_memsize);
> + bdinfo_print_num("memstart", (ulong)bd->bi_dram[0].start);
> + print_phys_addr("memsize", bd->bi_dram[0].size);
>   if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>   bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
>   bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
> diff --git a/cmd/bedbug.c b/cmd/bedbug.c
> index 81ce256480..0db42d7eed 100644
> --- a/cmd/bedbug.c
> +++ b/cmd/bedbug.c
> @@ -348,7 +348,7 @@ int do_bedbug_stack(struct cmd_tbl *cmdtp, int flag, int argc,
>   return 1;
>   }
>  
> - top = gd->bd->bi_memstart + gd->bd->bi_memsize;
> + top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;
>   depth = 0;
>  
>   printf ("Depth     PC\n");
> diff --git a/common/board_f.c b/common/board_f.c
> index 79532f4365..1d2f6a2545 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -216,7 +216,6 @@ static int show_dram_config(void)
>  {
>   unsigned long long size;
>  
> -#ifdef CONFIG_NR_DRAM_BANKS
>   int i;
>  
>   debug("\nRAM Configuration:\n");
> @@ -229,9 +228,6 @@ static int show_dram_config(void)
>  #endif
>   }
>   debug("\nDRAM:  ");
> -#else
> - size = gd->ram_size;
> -#endif
>  
>   print_size(size, "");
>   board_add_ram_info(0);
> @@ -242,7 +238,7 @@ static int show_dram_config(void)
>  
>  __weak int dram_init_banksize(void)
>  {
> -#if defined(CONFIG_NR_DRAM_BANKS) && defined(CONFIG_SYS_SDRAM_BASE)
> +#if defined(CONFIG_SYS_SDRAM_BASE)
>   gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>   gd->bd->bi_dram[0].size = get_effective_memsize();
>  #endif
> @@ -607,8 +603,11 @@ int setup_bdinfo(void)
>  {
>   struct bd_info *bd = gd->bd;
>  
> - bd->bi_memstart = gd->ram_base;  /* start of memory */
> - bd->bi_memsize = gd->ram_size;   /* size in bytes */
> + /* Only overwrite bi_dram[0] values if not already set */
> + if (!bd->bi_dram[0].size) {

This still look suspicious. When is bi_dram[0].size zero? I tried to
trace source code. dram_init_banksize() is always called before
setup_bdinfo() which fills bi_dram[], so when can be dram size zero?
Or I'm missing something?

> + bd->bi_dram[0].start = gd->ram_base;  /* start of memory */
> + bd->bi_dram[0].size = gd->ram_size;   /* size in bytes */
> + }
>  
>   if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>   bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
> diff --git a/common/image.c b/common/image.c
> index 9d7d5c17d1..35db5c2fda 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -668,10 +668,8 @@ ulong env_get_bootm_low(void)
>  
>  #if defined(CONFIG_SYS_SDRAM_BASE)
>   return CONFIG_SYS_SDRAM_BASE;
> -#elif defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)
> - return gd->bd->bi_dram[0].start;
>  #else
> - return 0;
> + return gd->bd->bi_dram[0].start;
>  #endif
>  }
>  
> @@ -685,14 +683,8 @@ phys_size_t env_get_bootm_size(void)
>   return tmp;
>   }
>  
> -#if (defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)) && \
> -     defined(CONFIG_NR_DRAM_BANKS)
>   start = gd->bd->bi_dram[0].start;
>   size = gd->bd->bi_dram[0].size;
> -#else
> - start = gd->bd->bi_memstart;
> - size = gd->bd->bi_memsize;
> -#endif
>  
>   s = env_get("bootm_low");
>   if (s)
> diff --git a/common/init/handoff.c b/common/init/handoff.c
> index e00b43e6a7..62071bd017 100644
> --- a/common/init/handoff.c
> +++ b/common/init/handoff.c
> @@ -12,18 +12,15 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  void handoff_save_dram(struct spl_handoff *ho)
>  {
> + struct bd_info *bd = gd->bd;
> + int i;
> +
>   ho->ram_size = gd->ram_size;
> -#ifdef CONFIG_NR_DRAM_BANKS
> - {
> - struct bd_info *bd = gd->bd;
> - int i;
> -
> - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> - ho->ram_bank[i].start = bd->bi_dram[i].start;
> - ho->ram_bank[i].size = bd->bi_dram[i].size;
> - }
> +
> + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> + ho->ram_bank[i].start = bd->bi_dram[i].start;
> + ho->ram_bank[i].size = bd->bi_dram[i].size;
>   }
> -#endif
>  }
>  
>  void handoff_load_dram_size(struct spl_handoff *ho)
> @@ -33,15 +30,11 @@ void handoff_load_dram_size(struct spl_handoff *ho)
>  
>  void handoff_load_dram_banks(struct spl_handoff *ho)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
> - {
> - struct bd_info *bd = gd->bd;
> - int i;
> -
> - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> - bd->bi_dram[i].start = ho->ram_bank[i].start;
> - bd->bi_dram[i].size = ho->ram_bank[i].size;
> - }
> + struct bd_info *bd = gd->bd;
> + int i;
> +
> + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> + bd->bi_dram[i].start = ho->ram_bank[i].start;
> + bd->bi_dram[i].size = ho->ram_bank[i].size;
>   }
> -#endif
>  }
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 834526c5a4..69fb46d3f4 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -871,6 +871,7 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>     ofnode node)
>  {
>   int pci_addr_cells, addr_cells, size_cells;
> + struct bd_info *bd = gd->bd;
>   int cells_per_record;
>   const u32 *prop;
>   int len;
> @@ -938,9 +939,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>   }
>  
>   /* Add a region for our local memory */
> -#ifdef CONFIG_NR_DRAM_BANKS
> - struct bd_info *bd = gd->bd;
> -
>   if (!bd)
>   return;
>  
> @@ -958,19 +956,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>         PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>   }
>   }
> -#else
> - phys_addr_t base = 0, size;
> -
> - size = gd->ram_size;
> -#ifdef CONFIG_SYS_SDRAM_BASE
> - base = CONFIG_SYS_SDRAM_BASE;
> -#endif
> - if (gd->pci_ram_top && gd->pci_ram_top < base + size)
> - size = gd->pci_ram_top - base;
> - if (size)
> - pci_set_region(hose->regions + hose->region_count++, base,
> - base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
> -#endif
>  
>   return;
>  }
> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
> index badade353e..3f07f4eb29 100644
> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
> @@ -1983,8 +1983,6 @@ static void *video_logo(void)
>  static int cfb_fb_is_in_dram(void)
>  {
>   struct bd_info *bd = gd->bd;
> -#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || \
> -defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
>   ulong start, end;
>   int i;
>  
> @@ -1995,11 +1993,7 @@ defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
>      (ulong)video_fb_address < end)
>   return 1;
>   }
> -#else
> - if ((ulong)video_fb_address >= bd->bi_memstart &&
> -    (ulong)video_fb_address < bd->bi_memstart + bd->bi_memsize)
> - return 1;
> -#endif
> +
>   return 0;
>  }
>  
> diff --git a/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
> index 62e61d41cc..637de0c455 100644
> --- a/include/asm-generic/u-boot.h
> +++ b/include/asm-generic/u-boot.h
> @@ -27,8 +27,6 @@
>  #include <linux/types.h>
>  
>  struct bd_info {
> - unsigned long bi_memstart; /* start of DRAM memory */
> - phys_size_t bi_memsize; /* size of DRAM memory in bytes */
>   unsigned long bi_flashstart; /* start of FLASH memory */
>   unsigned long bi_flashsize; /* size of FLASH memory */
>   unsigned long bi_flashoffset; /* reserved area for startup monitor */
> @@ -70,12 +68,10 @@ struct bd_info {
>  #endif
>   ulong        bi_arch_number; /* unique id for this board */
>   ulong        bi_boot_params; /* where this board expects params */
> -#ifdef CONFIG_NR_DRAM_BANKS
>   struct { /* RAM configuration */
>   phys_addr_t start;
>   phys_size_t size;
>   } bi_dram[CONFIG_NR_DRAM_BANKS];
> -#endif /* CONFIG_NR_DRAM_BANKS */
>  };
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/include/handoff.h b/include/handoff.h
> index 75d19b1f6e..070a79c1b9 100644
> --- a/include/handoff.h
> +++ b/include/handoff.h
> @@ -20,12 +20,10 @@
>  struct spl_handoff {
>   struct arch_spl_handoff arch;
>   u64 ram_size;
> -#ifdef CONFIG_NR_DRAM_BANKS
>   struct {
>   u64 start;
>   u64 size;
>   } ram_bank[CONFIG_NR_DRAM_BANKS];
> -#endif
>  };
>  
>  void handoff_save_dram(struct spl_handoff *ho);
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 78576b530f..ecbf10121d 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -1055,8 +1055,6 @@ int fdtdec_setup_mem_size_base(void)
>   return 0;
>  }
>  
> -#if defined(CONFIG_NR_DRAM_BANKS)
> -
>  static int get_next_memory_node(const void *blob, int mem)
>  {
>   do {
> @@ -1106,7 +1104,6 @@ int fdtdec_setup_memory_banksize(void)
>  
>   return 0;
>  }
> -#endif
>  
>  #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>  # if CONFIG_IS_ENABLED(MULTI_DTB_FIT_GZIP) ||\
> @@ -1569,7 +1566,6 @@ int fdtdec_resetup(int *rescan)
>  }
>  #endif
>  
> -#ifdef CONFIG_NR_DRAM_BANKS
>  int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
>     phys_addr_t *basep, phys_size_t *sizep,
>     struct bd_info *bd)
> @@ -1675,6 +1671,5 @@ int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
>  
>   return 0;
>  }
> -#endif /* CONFIG_NR_DRAM_BANKS */
>  
>  #endif /* !USE_HOSTCC */
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 75082f3559..600f55051c 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -117,22 +117,15 @@ static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
>  /* Initialize the struct, add memory and call arch/board reserve functions */
>  void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void *fdt_blob)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
>   int i;
> -#endif
>  
>   lmb_init(lmb);
> -#ifdef CONFIG_NR_DRAM_BANKS
>   for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>   if (bd->bi_dram[i].size) {
>   lmb_add(lmb, bd->bi_dram[i].start,
>   bd->bi_dram[i].size);
>   }
>   }
> -#else
> - if (bd->bi_memsize)
> - lmb_add(lmb, bd->bi_memstart, bd->bi_memsize);
> -#endif
>   lmb_reserve_common(lmb, fdt_blob);
>  }
>  
> --
> 2.28.0
>

For me it looks like that this patch is mixing different structures for
ram size from different sources. It is really correct? Does not it break
other boards?

I think that there is missing explanation of these changes.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Stefan Roese
Hi Pali,

On 12.08.20 10:05, Pali Rohár wrote:

> On Wednesday 12 August 2020 09:44:17 Stefan Roese wrote:
>> Since commit 86cf1c82850f ("configs: Migrate CONFIG_NR_DRAM_BANKS") &
>> commit 999a772d9f24 ("Kconfig: Migrate CONFIG_NR_DRAM_BANKS"),
>> CONFIG_NR_DRAM_BANKS is always defined with a value (4 is default).
>> It makes no sense to still carry code that is guarded with
>> "#ifndef CONFIG_NR_DRAM_BANKS" (and similar). This patch removes
>> all these unreferenced code paths.
>>
>> Also complete remove bi_memstart & bi_memsize from the board-info
>> struct. As now bi_dram[] is always enabled and should be used instead.
>> This removes the redundant varriables.
>>
>> Signed-off-by: Stefan Roese <[hidden email]>
>> Cc: Daniel Schwierzeck <[hidden email]>
>> Cc: Tom Rini <[hidden email]>
>> Cc: Ramon Fried <[hidden email]>
>> Cc: Simon Glass <[hidden email]>
>> Cc: Michal Simek <[hidden email]>
>> Cc: Pali Rohár <[hidden email]>
>> ---
>> v3:
>> - Rebase on TOT
>> - Add check to not overwrite bi_dram[0] if already configured in
>>    setup_bdinfo(). This fixes the test issue with Nokia RX51.
>>
>> Azure test report success:
>> https://dev.azure.com/sr0718/u-boot/_build/results?buildId=26&view=results
>>    
>> v2:
>> - Change all references to bi_memstart & bi_memsize to bi_dram[0].start/
>>    size and remove it from the bd_info struct completely, as suggested by
>>    Daniel
>>
>>   api/api_platform-mips.c               |  4 ++--
>>   api/api_platform-powerpc.c            |  2 +-
>>   arch/mips/lib/boot.c                  |  2 +-
>>   arch/mips/lib/bootm.c                 |  2 +-
>>   arch/powerpc/cpu/mpc83xx/fdt.c        |  2 +-
>>   arch/powerpc/cpu/mpc83xx/traps.c      |  2 +-
>>   arch/powerpc/cpu/mpc85xx/fdt.c        |  4 ++--
>>   arch/powerpc/cpu/mpc85xx/traps.c      |  2 +-
>>   arch/powerpc/cpu/mpc86xx/fdt.c        |  2 +-
>>   arch/powerpc/cpu/mpc86xx/traps.c      |  2 +-
>>   arch/powerpc/cpu/mpc8xx/fdt.c         |  2 +-
>>   arch/powerpc/lib/bootm.c              |  4 ++--
>>   arch/x86/cpu/broadwell/cpu_from_spl.c |  2 --
>>   arch/xtensa/lib/bdinfo.c              |  4 ++--
>>   arch/xtensa/lib/bootm.c               |  4 ++--
>>   board/Arcturus/ucp1020/spl.c          |  4 ++--
>>   board/freescale/p1010rdb/spl.c        |  4 ++--
>>   board/freescale/p1_p2_rdb_pc/spl.c    |  4 ++--
>>   board/freescale/t102xrdb/spl.c        |  4 ++--
>>   board/freescale/t104xrdb/spl.c        |  4 ++--
>>   board/freescale/t208xqds/spl.c        |  4 ++--
>>   board/freescale/t208xrdb/spl.c        |  4 ++--
>>   board/freescale/t4rdb/spl.c           |  4 ++--
>>   board/xilinx/zynqmp/zynqmp.c          |  2 --
>>   cmd/bdinfo.c                          |  6 ++---
>>   cmd/bedbug.c                          |  2 +-
>>   common/board_f.c                      | 13 +++++------
>>   common/image.c                        | 10 +-------
>>   common/init/handoff.c                 | 33 +++++++++++----------------
>>   drivers/pci/pci-uclass.c              | 17 +-------------
>>   drivers/video/cfb_console.c           |  8 +------
>>   include/asm-generic/u-boot.h          |  4 ----
>>   include/handoff.h                     |  2 --
>>   lib/fdtdec.c                          |  5 ----
>>   lib/lmb.c                             |  7 ------
>>   35 files changed, 60 insertions(+), 121 deletions(-)
>>
>> diff --git a/api/api_platform-mips.c b/api/api_platform-mips.c
>> index 51cd328b3d..f1721cef19 100644
>> --- a/api/api_platform-mips.c
>> +++ b/api/api_platform-mips.c
>> @@ -24,8 +24,8 @@ DECLARE_GLOBAL_DATA_PTR;
>>   int platform_sys_info(struct sys_info *si)
>>   {
>>  
>> - platform_set_mr(si, gd->bd->bi_memstart,
>> - gd->bd->bi_memsize, MR_ATTR_DRAM);
>> + platform_set_mr(si, gd->bd->bi_dram[0].start,
>> + gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>>  
>>   return 1;
>>   }
>> diff --git a/api/api_platform-powerpc.c b/api/api_platform-powerpc.c
>> index 15930cfdb6..8fff6e4cae 100644
>> --- a/api/api_platform-powerpc.c
>> +++ b/api/api_platform-powerpc.c
>> @@ -42,7 +42,7 @@ int platform_sys_info(struct sys_info *si)
>>   si->bar = 0;
>>   #endif
>>  
>> - platform_set_mr(si, gd->bd->bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);
>> + platform_set_mr(si, gd->bd->bi_dram[0].start, gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>>   platform_set_mr(si, gd->bd->bi_flashstart, gd->bd->bi_flashsize, MR_ATTR_FLASH);
>>   platform_set_mr(si, gd->bd->bi_sramstart, gd->bd->bi_sramsize, MR_ATTR_SRAM);
>>  
>> diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c
>> index db862f6379..3b960691c5 100644
>> --- a/arch/mips/lib/boot.c
>> +++ b/arch/mips/lib/boot.c
>> @@ -17,7 +17,7 @@ unsigned long do_go_exec(ulong (*entry)(int, char * const []),
>>   * whole SDRAM area, since we don't know the size of the image
>>   * that was loaded.
>>   */
>> - flush_cache(gd->bd->bi_memstart, gd->ram_top - gd->bd->bi_memstart);
>> + flush_cache(gd->bd->bi_dram[0].start, gd->ram_top - gd->bd->bi_dram[0].start);
>>  
>>   return entry(argc, argv);
>>   }
>> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
>> index 0a13f6edb7..b3ef15963e 100644
>> --- a/arch/mips/lib/bootm.c
>> +++ b/arch/mips/lib/bootm.c
>> @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
>>   #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
>>   int arch_fixup_fdt(void *blob)
>>   {
>> - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
>> + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
>>   u64 mem_size = gd->ram_size;
>
> I do not fully understand this change. Why on some places you are suing
> bi_dram[0].size and on some places gd->ram_size?

This patch is mainly a search and replace
s/bd->bi_memstart/bd->bi_dram[0].start. But as your test has shown, this
is not always
correct.

>>  
>>   return fdt_fixup_memory_banks(blob, &mem_start, &mem_size, 1);
>> diff --git a/arch/powerpc/cpu/mpc83xx/fdt.c b/arch/powerpc/cpu/mpc83xx/fdt.c
>> index ebdedb2888..05523ecc67 100644
>> --- a/arch/powerpc/cpu/mpc83xx/fdt.c
>> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
>> @@ -121,7 +121,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>>                   "clock-frequency", get_serial_clock(), 1);
>>   #endif
>>  
>> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
>> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>>  
>>   #if defined(CONFIG_BOOTCOUNT_LIMIT) && \
>>   (defined(CONFIG_QE) && !defined(CONFIG_ARCH_MPC831X))
>> diff --git a/arch/powerpc/cpu/mpc83xx/traps.c b/arch/powerpc/cpu/mpc83xx/traps.c
>> index c3cc119d65..5a8cd36420 100644
>> --- a/arch/powerpc/cpu/mpc83xx/traps.c
>> +++ b/arch/powerpc/cpu/mpc83xx/traps.c
>> @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   /* Returns 0 if exception not found and fixup otherwise.  */
>>   extern unsigned long search_exception_table(unsigned long);
>>  
>> -#define END_OF_MEM (gd->bd->bi_memstart + gd->bd->bi_memsize)
>> +#define END_OF_MEM (gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size)
>>  
>>   /*
>>    * Trap & Exception support
>> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
>> index 9569c1a64b..2cc567a5f4 100644
>> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
>> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
>> @@ -672,10 +672,10 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>>   "clock-frequency", get_bus_freq(0), 1);
>>   #endif
>>  
>> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
>> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>>  
>>   #ifdef CONFIG_MP
>> - ft_fixup_cpu(blob, (u64)bd->bi_memstart + (u64)bd->bi_memsize);
>> + ft_fixup_cpu(blob, (u64)bd->bi_dram[0].start + (u64)bd->bi_dram[0].size);
>>   ft_fixup_num_cores(blob);
>>   #endif
>>  
>> diff --git a/arch/powerpc/cpu/mpc85xx/traps.c b/arch/powerpc/cpu/mpc85xx/traps.c
>> index f37a45e269..061b7b8f6e 100644
>> --- a/arch/powerpc/cpu/mpc85xx/traps.c
>> +++ b/arch/powerpc/cpu/mpc85xx/traps.c
>> @@ -37,7 +37,7 @@ extern unsigned long search_exception_table(unsigned long);
>>    * amount of memory on the system if we're unable to keep all
>>    * the memory mapped in.
>>    */
>> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
>> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
>
> And on other place is get_effective_memsize()?

Again, its more a search/replace. The idea was to have no functional
change, which did not work out.

>>   static __inline__ void set_tsr(unsigned long val)
>>   {
>> diff --git a/arch/powerpc/cpu/mpc86xx/fdt.c b/arch/powerpc/cpu/mpc86xx/fdt.c
>> index 24e53115ec..143575befd 100644
>> --- a/arch/powerpc/cpu/mpc86xx/fdt.c
>> +++ b/arch/powerpc/cpu/mpc86xx/fdt.c
>> @@ -27,7 +27,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>>   do_fixup_by_prop_u32(blob, "device_type", "soc", 4,
>>       "bus-frequency", bd->bi_busfreq, 1);
>>  
>> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
>> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>>  
>>   #ifdef CONFIG_SYS_NS16550
>>   do_fixup_by_compat_u32(blob, "ns16550",
>> diff --git a/arch/powerpc/cpu/mpc86xx/traps.c b/arch/powerpc/cpu/mpc86xx/traps.c
>> index c0161e3379..532df93c3c 100644
>> --- a/arch/powerpc/cpu/mpc86xx/traps.c
>> +++ b/arch/powerpc/cpu/mpc86xx/traps.c
>> @@ -30,7 +30,7 @@ extern unsigned long search_exception_table(unsigned long);
>>    * amount of memory on the system if we're unable to keep all
>>    * the memory mapped in.
>>    */
>> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
>> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
>>  
>>   /*
>>    * Trap & Exception support
>> diff --git a/arch/powerpc/cpu/mpc8xx/fdt.c b/arch/powerpc/cpu/mpc8xx/fdt.c
>> index 4d952a3882..e593242631 100644
>> --- a/arch/powerpc/cpu/mpc8xx/fdt.c
>> +++ b/arch/powerpc/cpu/mpc8xx/fdt.c
>> @@ -25,5 +25,5 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>>   do_fixup_by_compat_u32(blob, "fsl,cpm-brg", "clock-frequency",
>>         gd->arch.brg_clk, 1);
>>  
>> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
>> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>>   }
>> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
>> index 8c8ed99cd3..d8bf5a14f6 100644
>> --- a/arch/powerpc/lib/bootm.c
>> +++ b/arch/powerpc/lib/bootm.c
>> @@ -298,8 +298,8 @@ void boot_prep_vxworks(bootm_headers_t *images)
>>   if (!images->ft_addr)
>>   return;
>>  
>> - base = (u64)gd->bd->bi_memstart;
>> - size = (u64)gd->bd->bi_memsize;
>> + base = (u64)gd->bd->bi_dram[0].start;
>> + size = (u64)gd->bd->bi_dram[0].size;
>>  
>>   off = fdt_path_offset(images->ft_addr, "/memory");
>>   if (off < 0)
>> diff --git a/arch/x86/cpu/broadwell/cpu_from_spl.c b/arch/x86/cpu/broadwell/cpu_from_spl.c
>> index 6567d50653..4d4cdafa2b 100644
>> --- a/arch/x86/cpu/broadwell/cpu_from_spl.c
>> +++ b/arch/x86/cpu/broadwell/cpu_from_spl.c
>> @@ -53,14 +53,12 @@ void board_debug_uart_init(void)
>>  
>>   int dram_init_banksize(void)
>>   {
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   struct spl_handoff *ho;
>>  
>>   ho = bloblist_find(BLOBLISTT_SPL_HANDOFF, sizeof(*ho));
>>   if (!ho)
>>   return log_msg_ret("Missing SPL hand-off info", -ENOENT);
>>   handoff_load_dram_banks(ho);
>> -#endif
>>  
>>   return 0;
>>   }
>> diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
>> index 4ec8529521..1839edd9bb 100644
>> --- a/arch/xtensa/lib/bdinfo.c
>> +++ b/arch/xtensa/lib/bdinfo.c
>> @@ -15,8 +15,8 @@ int arch_setup_bdinfo(void)
>>   {
>>   struct bd_info *bd = gd->bd;
>>  
>> - bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
>> - bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
>> + bd->bi_dram[0].start = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
>> + bd->bi_dram[0].size = CONFIG_SYS_SDRAM_SIZE;
>>  
>>   return 0;
>>   }
>> diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
>> index 458eaf95c0..16ab51dbc9 100644
>> --- a/arch/xtensa/lib/bootm.c
>> +++ b/arch/xtensa/lib/bootm.c
>> @@ -48,8 +48,8 @@ static struct bp_tag *setup_memory_tag(struct bp_tag *params)
>>   params->size = sizeof(struct meminfo);
>>   mem = (struct meminfo *)params->data;
>>   mem->type = MEMORY_TYPE_CONVENTIONAL;
>> - mem->start = bd->bi_memstart;
>> - mem->end = bd->bi_memstart + bd->bi_memsize;
>> + mem->start = bd->bi_dram[0].start;
>> + mem->end = bd->bi_dram[0].start + bd->bi_dram[0].size;
>>  
>>   printf("   MEMORY:          tag:0x%04x, type:0X%lx, start:0X%lx, end:0X%lx\n",
>>         BP_TAG_MEMORY, mem->type, mem->start, mem->end);
>> diff --git a/board/Arcturus/ucp1020/spl.c b/board/Arcturus/ucp1020/spl.c
>> index 5416a5b663..8eb641ef8d 100644
>> --- a/board/Arcturus/ucp1020/spl.c
>> +++ b/board/Arcturus/ucp1020/spl.c
>> @@ -83,8 +83,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/p1010rdb/spl.c b/board/freescale/p1010rdb/spl.c
>> index 4ee4573d2b..ddc0af3faa 100644
>> --- a/board/freescale/p1010rdb/spl.c
>> +++ b/board/freescale/p1010rdb/spl.c
>> @@ -69,8 +69,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/p1_p2_rdb_pc/spl.c b/board/freescale/p1_p2_rdb_pc/spl.c
>> index e76c3e82c3..f67b0fcbb9 100644
>> --- a/board/freescale/p1_p2_rdb_pc/spl.c
>> +++ b/board/freescale/p1_p2_rdb_pc/spl.c
>> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/t102xrdb/spl.c b/board/freescale/t102xrdb/spl.c
>> index da442fcc18..ce7ccfe41d 100644
>> --- a/board/freescale/t102xrdb/spl.c
>> +++ b/board/freescale/t102xrdb/spl.c
>> @@ -103,8 +103,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/t104xrdb/spl.c b/board/freescale/t104xrdb/spl.c
>> index f83d69ba15..4b5fb8b955 100644
>> --- a/board/freescale/t104xrdb/spl.c
>> +++ b/board/freescale/t104xrdb/spl.c
>> @@ -94,8 +94,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/t208xqds/spl.c b/board/freescale/t208xqds/spl.c
>> index c197884421..dc0046d93c 100644
>> --- a/board/freescale/t208xqds/spl.c
>> +++ b/board/freescale/t208xqds/spl.c
>> @@ -102,8 +102,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/t208xrdb/spl.c b/board/freescale/t208xrdb/spl.c
>> index 07aab6349c..f523622de9 100644
>> --- a/board/freescale/t208xrdb/spl.c
>> +++ b/board/freescale/t208xrdb/spl.c
>> @@ -72,8 +72,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/freescale/t4rdb/spl.c b/board/freescale/t4rdb/spl.c
>> index 64d2753da8..a5351e813a 100644
>> --- a/board/freescale/t4rdb/spl.c
>> +++ b/board/freescale/t4rdb/spl.c
>> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>>   memset(bd, 0, sizeof(struct bd_info));
>>   gd->bd = bd;
>> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
>> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
>> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
>> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>>  
>>   arch_cpu_init();
>>   get_clocks();
>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>> index ebb7172908..4cc5cb6fd7 100644
>> --- a/board/xilinx/zynqmp/zynqmp.c
>> +++ b/board/xilinx/zynqmp/zynqmp.c
>> @@ -467,10 +467,8 @@ int dram_init(void)
>>   #else
>>   int dram_init_banksize(void)
>>   {
>> -#if defined(CONFIG_NR_DRAM_BANKS)
>>   gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>>   gd->bd->bi_dram[0].size = get_effective_memsize();
>> -#endif
>>  
>>   mem_map_fill();
>>  
>> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
>> index 9593b345a3..117788dade 100644
>> --- a/cmd/bdinfo.c
>> +++ b/cmd/bdinfo.c
>> @@ -49,7 +49,6 @@ void bdinfo_print_mhz(const char *name, unsigned long hz)
>>  
>>   static void print_bi_dram(const struct bd_info *bd)
>>   {
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   int i;
>>  
>>   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
>> @@ -59,7 +58,6 @@ static void print_bi_dram(const struct bd_info *bd)
>>   bdinfo_print_num("-> size", bd->bi_dram[i].size);
>>   }
>>   }
>> -#endif
>>   }
>>  
>>   __weak void arch_print_bdinfo(void)
>> @@ -75,8 +73,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>>   #endif
>>   bdinfo_print_num("boot_params", (ulong)bd->bi_boot_params);
>>   print_bi_dram(bd);
>> - bdinfo_print_num("memstart", (ulong)bd->bi_memstart);
>> - print_phys_addr("memsize", bd->bi_memsize);
>> + bdinfo_print_num("memstart", (ulong)bd->bi_dram[0].start);
>> + print_phys_addr("memsize", bd->bi_dram[0].size);
>>   if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>>   bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
>>   bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
>> diff --git a/cmd/bedbug.c b/cmd/bedbug.c
>> index 81ce256480..0db42d7eed 100644
>> --- a/cmd/bedbug.c
>> +++ b/cmd/bedbug.c
>> @@ -348,7 +348,7 @@ int do_bedbug_stack(struct cmd_tbl *cmdtp, int flag, int argc,
>>   return 1;
>>   }
>>  
>> - top = gd->bd->bi_memstart + gd->bd->bi_memsize;
>> + top = gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size;
>>   depth = 0;
>>  
>>   printf ("Depth     PC\n");
>> diff --git a/common/board_f.c b/common/board_f.c
>> index 79532f4365..1d2f6a2545 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -216,7 +216,6 @@ static int show_dram_config(void)
>>   {
>>   unsigned long long size;
>>  
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   int i;
>>  
>>   debug("\nRAM Configuration:\n");
>> @@ -229,9 +228,6 @@ static int show_dram_config(void)
>>   #endif
>>   }
>>   debug("\nDRAM:  ");
>> -#else
>> - size = gd->ram_size;
>> -#endif
>>  
>>   print_size(size, "");
>>   board_add_ram_info(0);
>> @@ -242,7 +238,7 @@ static int show_dram_config(void)
>>  
>>   __weak int dram_init_banksize(void)
>>   {
>> -#if defined(CONFIG_NR_DRAM_BANKS) && defined(CONFIG_SYS_SDRAM_BASE)
>> +#if defined(CONFIG_SYS_SDRAM_BASE)
>>   gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>>   gd->bd->bi_dram[0].size = get_effective_memsize();
>>   #endif
>> @@ -607,8 +603,11 @@ int setup_bdinfo(void)
>>   {
>>   struct bd_info *bd = gd->bd;
>>  
>> - bd->bi_memstart = gd->ram_base;  /* start of memory */
>> - bd->bi_memsize = gd->ram_size;   /* size in bytes */
>> + /* Only overwrite bi_dram[0] values if not already set */
>> + if (!bd->bi_dram[0].size) {
>
> This still look suspicious. When is bi_dram[0].size zero? I tried to
> trace source code. dram_init_banksize() is always called before
> setup_bdinfo() which fills bi_dram[], so when can be dram size zero?
> Or I'm missing something?

dram_init_banksize() is always called earlier, yes. But its weak default
only sets the bi_dram[] values when CONFIG_SYS_SDRAM_BASE is configured.
I'm checking right now, if this might be a problem.

>> + bd->bi_dram[0].start = gd->ram_base;  /* start of memory */
>> + bd->bi_dram[0].size = gd->ram_size;   /* size in bytes */
>> + }
>>  
>>   if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>>   bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
>> diff --git a/common/image.c b/common/image.c
>> index 9d7d5c17d1..35db5c2fda 100644
>> --- a/common/image.c
>> +++ b/common/image.c
>> @@ -668,10 +668,8 @@ ulong env_get_bootm_low(void)
>>  
>>   #if defined(CONFIG_SYS_SDRAM_BASE)
>>   return CONFIG_SYS_SDRAM_BASE;
>> -#elif defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)
>> - return gd->bd->bi_dram[0].start;
>>   #else
>> - return 0;
>> + return gd->bd->bi_dram[0].start;
>>   #endif
>>   }
>>  
>> @@ -685,14 +683,8 @@ phys_size_t env_get_bootm_size(void)
>>   return tmp;
>>   }
>>  
>> -#if (defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)) && \
>> -     defined(CONFIG_NR_DRAM_BANKS)
>>   start = gd->bd->bi_dram[0].start;
>>   size = gd->bd->bi_dram[0].size;
>> -#else
>> - start = gd->bd->bi_memstart;
>> - size = gd->bd->bi_memsize;
>> -#endif
>>  
>>   s = env_get("bootm_low");
>>   if (s)
>> diff --git a/common/init/handoff.c b/common/init/handoff.c
>> index e00b43e6a7..62071bd017 100644
>> --- a/common/init/handoff.c
>> +++ b/common/init/handoff.c
>> @@ -12,18 +12,15 @@ DECLARE_GLOBAL_DATA_PTR;
>>  
>>   void handoff_save_dram(struct spl_handoff *ho)
>>   {
>> + struct bd_info *bd = gd->bd;
>> + int i;
>> +
>>   ho->ram_size = gd->ram_size;
>> -#ifdef CONFIG_NR_DRAM_BANKS
>> - {
>> - struct bd_info *bd = gd->bd;
>> - int i;
>> -
>> - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>> - ho->ram_bank[i].start = bd->bi_dram[i].start;
>> - ho->ram_bank[i].size = bd->bi_dram[i].size;
>> - }
>> +
>> + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>> + ho->ram_bank[i].start = bd->bi_dram[i].start;
>> + ho->ram_bank[i].size = bd->bi_dram[i].size;
>>   }
>> -#endif
>>   }
>>  
>>   void handoff_load_dram_size(struct spl_handoff *ho)
>> @@ -33,15 +30,11 @@ void handoff_load_dram_size(struct spl_handoff *ho)
>>  
>>   void handoff_load_dram_banks(struct spl_handoff *ho)
>>   {
>> -#ifdef CONFIG_NR_DRAM_BANKS
>> - {
>> - struct bd_info *bd = gd->bd;
>> - int i;
>> -
>> - for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>> - bd->bi_dram[i].start = ho->ram_bank[i].start;
>> - bd->bi_dram[i].size = ho->ram_bank[i].size;
>> - }
>> + struct bd_info *bd = gd->bd;
>> + int i;
>> +
>> + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>> + bd->bi_dram[i].start = ho->ram_bank[i].start;
>> + bd->bi_dram[i].size = ho->ram_bank[i].size;
>>   }
>> -#endif
>>   }
>> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
>> index 834526c5a4..69fb46d3f4 100644
>> --- a/drivers/pci/pci-uclass.c
>> +++ b/drivers/pci/pci-uclass.c
>> @@ -871,6 +871,7 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>>     ofnode node)
>>   {
>>   int pci_addr_cells, addr_cells, size_cells;
>> + struct bd_info *bd = gd->bd;
>>   int cells_per_record;
>>   const u32 *prop;
>>   int len;
>> @@ -938,9 +939,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>>   }
>>  
>>   /* Add a region for our local memory */
>> -#ifdef CONFIG_NR_DRAM_BANKS
>> - struct bd_info *bd = gd->bd;
>> -
>>   if (!bd)
>>   return;
>>  
>> @@ -958,19 +956,6 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
>>         PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>>   }
>>   }
>> -#else
>> - phys_addr_t base = 0, size;
>> -
>> - size = gd->ram_size;
>> -#ifdef CONFIG_SYS_SDRAM_BASE
>> - base = CONFIG_SYS_SDRAM_BASE;
>> -#endif
>> - if (gd->pci_ram_top && gd->pci_ram_top < base + size)
>> - size = gd->pci_ram_top - base;
>> - if (size)
>> - pci_set_region(hose->regions + hose->region_count++, base,
>> - base, size, PCI_REGION_MEM | PCI_REGION_SYS_MEMORY);
>> -#endif
>>  
>>   return;
>>   }
>> diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c
>> index badade353e..3f07f4eb29 100644
>> --- a/drivers/video/cfb_console.c
>> +++ b/drivers/video/cfb_console.c
>> @@ -1983,8 +1983,6 @@ static void *video_logo(void)
>>   static int cfb_fb_is_in_dram(void)
>>   {
>>   struct bd_info *bd = gd->bd;
>> -#if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || \
>> -defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
>>   ulong start, end;
>>   int i;
>>  
>> @@ -1995,11 +1993,7 @@ defined(CONFIG_SANDBOX) || defined(CONFIG_X86)
>>      (ulong)video_fb_address < end)
>>   return 1;
>>   }
>> -#else
>> - if ((ulong)video_fb_address >= bd->bi_memstart &&
>> -    (ulong)video_fb_address < bd->bi_memstart + bd->bi_memsize)
>> - return 1;
>> -#endif
>> +
>>   return 0;
>>   }
>>  
>> diff --git a/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
>> index 62e61d41cc..637de0c455 100644
>> --- a/include/asm-generic/u-boot.h
>> +++ b/include/asm-generic/u-boot.h
>> @@ -27,8 +27,6 @@
>>   #include <linux/types.h>
>>  
>>   struct bd_info {
>> - unsigned long bi_memstart; /* start of DRAM memory */
>> - phys_size_t bi_memsize; /* size of DRAM memory in bytes */
>>   unsigned long bi_flashstart; /* start of FLASH memory */
>>   unsigned long bi_flashsize; /* size of FLASH memory */
>>   unsigned long bi_flashoffset; /* reserved area for startup monitor */
>> @@ -70,12 +68,10 @@ struct bd_info {
>>   #endif
>>   ulong        bi_arch_number; /* unique id for this board */
>>   ulong        bi_boot_params; /* where this board expects params */
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   struct { /* RAM configuration */
>>   phys_addr_t start;
>>   phys_size_t size;
>>   } bi_dram[CONFIG_NR_DRAM_BANKS];
>> -#endif /* CONFIG_NR_DRAM_BANKS */
>>   };
>>  
>>   #endif /* __ASSEMBLY__ */
>> diff --git a/include/handoff.h b/include/handoff.h
>> index 75d19b1f6e..070a79c1b9 100644
>> --- a/include/handoff.h
>> +++ b/include/handoff.h
>> @@ -20,12 +20,10 @@
>>   struct spl_handoff {
>>   struct arch_spl_handoff arch;
>>   u64 ram_size;
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   struct {
>>   u64 start;
>>   u64 size;
>>   } ram_bank[CONFIG_NR_DRAM_BANKS];
>> -#endif
>>   };
>>  
>>   void handoff_save_dram(struct spl_handoff *ho);
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 78576b530f..ecbf10121d 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -1055,8 +1055,6 @@ int fdtdec_setup_mem_size_base(void)
>>   return 0;
>>   }
>>  
>> -#if defined(CONFIG_NR_DRAM_BANKS)
>> -
>>   static int get_next_memory_node(const void *blob, int mem)
>>   {
>>   do {
>> @@ -1106,7 +1104,6 @@ int fdtdec_setup_memory_banksize(void)
>>  
>>   return 0;
>>   }
>> -#endif
>>  
>>   #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>>   # if CONFIG_IS_ENABLED(MULTI_DTB_FIT_GZIP) ||\
>> @@ -1569,7 +1566,6 @@ int fdtdec_resetup(int *rescan)
>>   }
>>   #endif
>>  
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
>>     phys_addr_t *basep, phys_size_t *sizep,
>>     struct bd_info *bd)
>> @@ -1675,6 +1671,5 @@ int fdtdec_decode_ram_size(const void *blob, const char *area, int board_id,
>>  
>>   return 0;
>>   }
>> -#endif /* CONFIG_NR_DRAM_BANKS */
>>  
>>   #endif /* !USE_HOSTCC */
>> diff --git a/lib/lmb.c b/lib/lmb.c
>> index 75082f3559..600f55051c 100644
>> --- a/lib/lmb.c
>> +++ b/lib/lmb.c
>> @@ -117,22 +117,15 @@ static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
>>   /* Initialize the struct, add memory and call arch/board reserve functions */
>>   void lmb_init_and_reserve(struct lmb *lmb, struct bd_info *bd, void *fdt_blob)
>>   {
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   int i;
>> -#endif
>>  
>>   lmb_init(lmb);
>> -#ifdef CONFIG_NR_DRAM_BANKS
>>   for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
>>   if (bd->bi_dram[i].size) {
>>   lmb_add(lmb, bd->bi_dram[i].start,
>>   bd->bi_dram[i].size);
>>   }
>>   }
>> -#else
>> - if (bd->bi_memsize)
>> - lmb_add(lmb, bd->bi_memstart, bd->bi_memsize);
>> -#endif
>>   lmb_reserve_common(lmb, fdt_blob);
>>   }
>>  
>> --
>> 2.28.0
>>
>
> For me it looks like that this patch is mixing different structures for
> ram size from different sources.

Again, it started as a "simple" search and replace. It seems, that I
need to change this patch, so that the "correct" value is used in
place of bi_memstart & bi_memsize instead of always using bi_dram[].

I'll rework this patch again.

> It is really correct? Does not it break
> other boards?
>
> I think that there is missing explanation of these changes.

Perhaps its also the patch history that you are missing. This patch
started with a simple remove of the unreferenced CONFIG_NR_DRAM_BANKS
code paths (as the patch subject still mentions). And its extended,
after Daniel pointed out, that with the move of CONFIG_NR_DRAM_BANKS
the usage of bi_memstart etc is now not 100% correct any more. He
suggested to replace those references with bi_dram[].

I agree that this is a bit confusing. I'll rework this patch into
multiple separate patches now instead. This will hopefully make it
easier to follow the intention and review these changes.

Thanks,
Stefan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Pali Rohár-2
Hello!

> > > diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> > > index 0a13f6edb7..b3ef15963e 100644
> > > --- a/arch/mips/lib/bootm.c
> > > +++ b/arch/mips/lib/bootm.c
> > > @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
> > >   #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
> > >   int arch_fixup_fdt(void *blob)
> > >   {
> > > - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
> > > + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
> > >   u64 mem_size = gd->ram_size;
> >
> > I do not fully understand this change. Why on some places you are suing
> > bi_dram[0].size and on some places gd->ram_size?
>
> This patch is mainly a search and replace
> s/bd->bi_memstart/bd->bi_dram[0].start. But as your test has shown, this is
> not always
> correct.

Yes, this search+replace needs to be better reviewed and tested. Such
big change has potential to break random stuff which nobody think about.

> > > @@ -607,8 +603,11 @@ int setup_bdinfo(void)
> > >   {
> > >   struct bd_info *bd = gd->bd;
> > > - bd->bi_memstart = gd->ram_base;  /* start of memory */
> > > - bd->bi_memsize = gd->ram_size;   /* size in bytes */
> > > + /* Only overwrite bi_dram[0] values if not already set */
> > > + if (!bd->bi_dram[0].size) {
> >
> > This still look suspicious. When is bi_dram[0].size zero? I tried to
> > trace source code. dram_init_banksize() is always called before
> > setup_bdinfo() which fills bi_dram[], so when can be dram size zero?
> > Or I'm missing something?
>
> dram_init_banksize() is always called earlier, yes. But its weak default
> only sets the bi_dram[] values when CONFIG_SYS_SDRAM_BASE is configured.
> I'm checking right now, if this might be a problem.

Should not be in this case extended dram_init_banksize() to always
properly fill bi_dram[] structure? Setting bi_dram[] on more places and
doing overwriting could complicate things for future debugging or
extending.

> > For me it looks like that this patch is mixing different structures for
> > ram size from different sources.
>
> Again, it started as a "simple" search and replace. It seems, that I
> need to change this patch, so that the "correct" value is used in
> place of bi_memstart & bi_memsize instead of always using bi_dram[].
>
> I'll rework this patch again.
>
> > It is really correct? Does not it break
> > other boards?
> >
> > I think that there is missing explanation of these changes.
>
> Perhaps its also the patch history that you are missing. This patch
> started with a simple remove of the unreferenced CONFIG_NR_DRAM_BANKS
> code paths (as the patch subject still mentions). And its extended,
> after Daniel pointed out, that with the move of CONFIG_NR_DRAM_BANKS
> the usage of bi_memstart etc is now not 100% correct any more. He
> suggested to replace those references with bi_dram[].

I have not seen patch history, only its current version which caused
u-boot failure on n900. So I was not sure what is intension of this
patch. Sorry for that.

I think 1:1 replacement is not such trivial as with more dram banks you
would need to join address space from bi_dram[0], bi_dram[1], ... as
previous code is expecting.

I would suggest that more people with different hardware affected by
this change would test if everything is working fine.

I do not know how many boards have full boot test scenarios (from
booting u-boot to booting kernel) like N900 and such tests are useful
when playing with patches which changes such big parts.

I would suggest to other board maintainers to write or provides test
suite as it can really help with catching such fatal errors.

I cannot believe that N900 was the only device on which u-boot stopped
booting kernel. From that code it looks like that all OMAP3 devices must
have been affected and possible any architecture with two or more dram
banks.

> I agree that this is a bit confusing. I'll rework this patch into
> multiple separate patches now instead. This will hopefully make it
> easier to follow the intention and review these changes.

Lokesh, do you have OMAP3 devices to test these future Stefan's patches?
For me it looks like that OMAP3 is good candidate which can be affected
by these changes if code is not properly reviewed.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Daniel Schwierzeck-2
Am Mittwoch, den 12.08.2020, 14:37 +0200 schrieb Pali Rohár:

> Hello!
>
> > > > diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> > > > index 0a13f6edb7..b3ef15963e 100644
> > > > --- a/arch/mips/lib/bootm.c
> > > > +++ b/arch/mips/lib/bootm.c
> > > > @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
> > > >   #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
> > > >   int arch_fixup_fdt(void *blob)
> > > >   {
> > > > - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
> > > > + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
> > > >   u64 mem_size = gd->ram_size;
> > >
> > > I do not fully understand this change. Why on some places you are suing
> > > bi_dram[0].size and on some places gd->ram_size?
> >
> > This patch is mainly a search and replace
> > s/bd->bi_memstart/bd->bi_dram[0].start. But as your test has shown, this is
> > not always
> > correct.
>
> Yes, this search+replace needs to be better reviewed and tested. Such
> big change has potential to break random stuff which nobody think about.
>
> > > > @@ -607,8 +603,11 @@ int setup_bdinfo(void)
> > > >   {
> > > >   struct bd_info *bd = gd->bd;
> > > > - bd->bi_memstart = gd->ram_base;  /* start of memory */
> > > > - bd->bi_memsize = gd->ram_size;   /* size in bytes */
> > > > + /* Only overwrite bi_dram[0] values if not already set */
> > > > + if (!bd->bi_dram[0].size) {
> > >
> > > This still look suspicious. When is bi_dram[0].size zero? I tried to
> > > trace source code. dram_init_banksize() is always called before
> > > setup_bdinfo() which fills bi_dram[], so when can be dram size zero?
> > > Or I'm missing something?
> >
> > dram_init_banksize() is always called earlier, yes. But its weak default
> > only sets the bi_dram[] values when CONFIG_SYS_SDRAM_BASE is configured.
> > I'm checking right now, if this might be a problem.
>
> Should not be in this case extended dram_init_banksize() to always
> properly fill bi_dram[] structure? Setting bi_dram[] on more places and
> doing overwriting could complicate things for future debugging or
> extending.

for all users of bi_memstart/bi_memsize the search+replace should be
enough. But in setup_bdinfo() it is wrong because it overrides the
initialization done in dram_init_banksize(). But setup_bdinfo() is a
new function due to some other bd_info refactoring merged two weeks ago
and was not available in v2 of this patch. I guess N900 was still
working with v2 and v3 now has this kind of merge conflict regression.

Just removing those lines without replacing with bi_dram should fix
this:

bd->bi_memstart = gd->ram_base;  /* start of memory */
bd->bi_memsize = gd->ram_size;   /* size in bytes */


BTW: looking deeper in the code and history I think bd_info.bi_dram is
heavily misused as storage for DRAM configuration. Such information
should be stored in global_data where generic code can access it if
needed (e.g. LMB, fdt_fixup_memory_banks()). bd_info should just be
initialized and used when needed to boot old kernels. Also there should
be a Kconfig option to completely disable the support of bd_info.

--
- Daniel

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Daniel Schwierzeck-2
In reply to this post by Stefan Roese
Am Mittwoch, den 12.08.2020, 09:44 +0200 schrieb Stefan Roese:

> Since commit 86cf1c82850f ("configs: Migrate CONFIG_NR_DRAM_BANKS") &
> commit 999a772d9f24 ("Kconfig: Migrate CONFIG_NR_DRAM_BANKS"),
> CONFIG_NR_DRAM_BANKS is always defined with a value (4 is default).
> It makes no sense to still carry code that is guarded with
> "#ifndef CONFIG_NR_DRAM_BANKS" (and similar). This patch removes
> all these unreferenced code paths.
>
> Also complete remove bi_memstart & bi_memsize from the board-info
> struct. As now bi_dram[] is always enabled and should be used instead.
> This removes the redundant varriables.
>
> Signed-off-by: Stefan Roese <[hidden email]>
> Cc: Daniel Schwierzeck <[hidden email]>
> Cc: Tom Rini <[hidden email]>
> Cc: Ramon Fried <[hidden email]>
> Cc: Simon Glass <[hidden email]>
> Cc: Michal Simek <[hidden email]>
> Cc: Pali Rohár <[hidden email]>
> ---
> v3:
> - Rebase on TOT
> - Add check to not overwrite bi_dram[0] if already configured in
>   setup_bdinfo(). This fixes the test issue with Nokia RX51.
>
> Azure test report success:
> https://dev.azure.com/sr0718/u-boot/_build/results?buildId=26&view=results
>  
> v2:
> - Change all references to bi_memstart & bi_memsize to bi_dram[0].start/
>   size and remove it from the bd_info struct completely, as suggested by
>   Daniel
>
>  api/api_platform-mips.c               |  4 ++--
>  api/api_platform-powerpc.c            |  2 +-
>  arch/mips/lib/boot.c                  |  2 +-
>  arch/mips/lib/bootm.c                 |  2 +-
>  arch/powerpc/cpu/mpc83xx/fdt.c        |  2 +-
>  arch/powerpc/cpu/mpc83xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc85xx/fdt.c        |  4 ++--
>  arch/powerpc/cpu/mpc85xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc86xx/fdt.c        |  2 +-
>  arch/powerpc/cpu/mpc86xx/traps.c      |  2 +-
>  arch/powerpc/cpu/mpc8xx/fdt.c         |  2 +-
>  arch/powerpc/lib/bootm.c              |  4 ++--
>  arch/x86/cpu/broadwell/cpu_from_spl.c |  2 --
>  arch/xtensa/lib/bdinfo.c              |  4 ++--
>  arch/xtensa/lib/bootm.c               |  4 ++--
>  board/Arcturus/ucp1020/spl.c          |  4 ++--
>  board/freescale/p1010rdb/spl.c        |  4 ++--
>  board/freescale/p1_p2_rdb_pc/spl.c    |  4 ++--
>  board/freescale/t102xrdb/spl.c        |  4 ++--
>  board/freescale/t104xrdb/spl.c        |  4 ++--
>  board/freescale/t208xqds/spl.c        |  4 ++--
>  board/freescale/t208xrdb/spl.c        |  4 ++--
>  board/freescale/t4rdb/spl.c           |  4 ++--
>  board/xilinx/zynqmp/zynqmp.c          |  2 --
>  cmd/bdinfo.c                          |  6 ++---
>  cmd/bedbug.c                          |  2 +-
>  common/board_f.c                      | 13 +++++------
>  common/image.c                        | 10 +-------
>  common/init/handoff.c                 | 33 +++++++++++----------------
>  drivers/pci/pci-uclass.c              | 17 +-------------
>  drivers/video/cfb_console.c           |  8 +------
>  include/asm-generic/u-boot.h          |  4 ----
>  include/handoff.h                     |  2 --
>  lib/fdtdec.c                          |  5 ----
>  lib/lmb.c                             |  7 ------
>  35 files changed, 60 insertions(+), 121 deletions(-)
>
> diff --git a/api/api_platform-mips.c b/api/api_platform-mips.c
> index 51cd328b3d..f1721cef19 100644
> --- a/api/api_platform-mips.c
> +++ b/api/api_platform-mips.c
> @@ -24,8 +24,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  int platform_sys_info(struct sys_info *si)
>  {
>  
> - platform_set_mr(si, gd->bd->bi_memstart,
> - gd->bd->bi_memsize, MR_ATTR_DRAM);
> + platform_set_mr(si, gd->bd->bi_dram[0].start,
> + gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>  
>   return 1;
>  }
> diff --git a/api/api_platform-powerpc.c b/api/api_platform-powerpc.c
> index 15930cfdb6..8fff6e4cae 100644
> --- a/api/api_platform-powerpc.c
> +++ b/api/api_platform-powerpc.c
> @@ -42,7 +42,7 @@ int platform_sys_info(struct sys_info *si)
>   si->bar = 0;
>  #endif
>  
> - platform_set_mr(si, gd->bd->bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);
> + platform_set_mr(si, gd->bd->bi_dram[0].start, gd->bd->bi_dram[0].size, MR_ATTR_DRAM);
>   platform_set_mr(si, gd->bd->bi_flashstart, gd->bd->bi_flashsize, MR_ATTR_FLASH);
>   platform_set_mr(si, gd->bd->bi_sramstart, gd->bd->bi_sramsize, MR_ATTR_SRAM);
>  
> diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c
> index db862f6379..3b960691c5 100644
> --- a/arch/mips/lib/boot.c
> +++ b/arch/mips/lib/boot.c
> @@ -17,7 +17,7 @@ unsigned long do_go_exec(ulong (*entry)(int, char * const []),
>   * whole SDRAM area, since we don't know the size of the image
>   * that was loaded.
>   */
> - flush_cache(gd->bd->bi_memstart, gd->ram_top - gd->bd->bi_memstart);
> + flush_cache(gd->bd->bi_dram[0].start, gd->ram_top - gd->bd->bi_dram[0].start);
>  
>   return entry(argc, argv);
>  }
> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
> index 0a13f6edb7..b3ef15963e 100644
> --- a/arch/mips/lib/bootm.c
> +++ b/arch/mips/lib/bootm.c
> @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
>  #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
>  int arch_fixup_fdt(void *blob)
>  {
> - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
> + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
>   u64 mem_size = gd->ram_size;
>  
>   return fdt_fixup_memory_banks(blob, &mem_start, &mem_size, 1);
> diff --git a/arch/powerpc/cpu/mpc83xx/fdt.c b/arch/powerpc/cpu/mpc83xx/fdt.c
> index ebdedb2888..05523ecc67 100644
> --- a/arch/powerpc/cpu/mpc83xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc83xx/fdt.c
> @@ -121,7 +121,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>                  "clock-frequency", get_serial_clock(), 1);
>  #endif
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #if defined(CONFIG_BOOTCOUNT_LIMIT) && \
>   (defined(CONFIG_QE) && !defined(CONFIG_ARCH_MPC831X))
> diff --git a/arch/powerpc/cpu/mpc83xx/traps.c b/arch/powerpc/cpu/mpc83xx/traps.c
> index c3cc119d65..5a8cd36420 100644
> --- a/arch/powerpc/cpu/mpc83xx/traps.c
> +++ b/arch/powerpc/cpu/mpc83xx/traps.c
> @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  /* Returns 0 if exception not found and fixup otherwise.  */
>  extern unsigned long search_exception_table(unsigned long);
>  
> -#define END_OF_MEM (gd->bd->bi_memstart + gd->bd->bi_memsize)
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + gd->bd->bi_dram[0].size)
>  
>  /*
>   * Trap & Exception support
> diff --git a/arch/powerpc/cpu/mpc85xx/fdt.c b/arch/powerpc/cpu/mpc85xx/fdt.c
> index 9569c1a64b..2cc567a5f4 100644
> --- a/arch/powerpc/cpu/mpc85xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc85xx/fdt.c
> @@ -672,10 +672,10 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   "clock-frequency", get_bus_freq(0), 1);
>  #endif
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #ifdef CONFIG_MP
> - ft_fixup_cpu(blob, (u64)bd->bi_memstart + (u64)bd->bi_memsize);
> + ft_fixup_cpu(blob, (u64)bd->bi_dram[0].start + (u64)bd->bi_dram[0].size);
>   ft_fixup_num_cores(blob);
>  #endif
>  
> diff --git a/arch/powerpc/cpu/mpc85xx/traps.c b/arch/powerpc/cpu/mpc85xx/traps.c
> index f37a45e269..061b7b8f6e 100644
> --- a/arch/powerpc/cpu/mpc85xx/traps.c
> +++ b/arch/powerpc/cpu/mpc85xx/traps.c
> @@ -37,7 +37,7 @@ extern unsigned long search_exception_table(unsigned long);
>   * amount of memory on the system if we're unable to keep all
>   * the memory mapped in.
>   */
> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
>  
>  static __inline__ void set_tsr(unsigned long val)
>  {
> diff --git a/arch/powerpc/cpu/mpc86xx/fdt.c b/arch/powerpc/cpu/mpc86xx/fdt.c
> index 24e53115ec..143575befd 100644
> --- a/arch/powerpc/cpu/mpc86xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc86xx/fdt.c
> @@ -27,7 +27,7 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   do_fixup_by_prop_u32(blob, "device_type", "soc", 4,
>       "bus-frequency", bd->bi_busfreq, 1);
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  
>  #ifdef CONFIG_SYS_NS16550
>   do_fixup_by_compat_u32(blob, "ns16550",
> diff --git a/arch/powerpc/cpu/mpc86xx/traps.c b/arch/powerpc/cpu/mpc86xx/traps.c
> index c0161e3379..532df93c3c 100644
> --- a/arch/powerpc/cpu/mpc86xx/traps.c
> +++ b/arch/powerpc/cpu/mpc86xx/traps.c
> @@ -30,7 +30,7 @@ extern unsigned long search_exception_table(unsigned long);
>   * amount of memory on the system if we're unable to keep all
>   * the memory mapped in.
>   */
> -#define END_OF_MEM (gd->bd->bi_memstart + get_effective_memsize())
> +#define END_OF_MEM (gd->bd->bi_dram[0].start + get_effective_memsize())
>  
>  /*
>   * Trap & Exception support
> diff --git a/arch/powerpc/cpu/mpc8xx/fdt.c b/arch/powerpc/cpu/mpc8xx/fdt.c
> index 4d952a3882..e593242631 100644
> --- a/arch/powerpc/cpu/mpc8xx/fdt.c
> +++ b/arch/powerpc/cpu/mpc8xx/fdt.c
> @@ -25,5 +25,5 @@ void ft_cpu_setup(void *blob, struct bd_info *bd)
>   do_fixup_by_compat_u32(blob, "fsl,cpm-brg", "clock-frequency",
>         gd->arch.brg_clk, 1);
>  
> - fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
> + fdt_fixup_memory(blob, (u64)bd->bi_dram[0].start, (u64)bd->bi_dram[0].size);
>  }
> diff --git a/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c
> index 8c8ed99cd3..d8bf5a14f6 100644
> --- a/arch/powerpc/lib/bootm.c
> +++ b/arch/powerpc/lib/bootm.c
> @@ -298,8 +298,8 @@ void boot_prep_vxworks(bootm_headers_t *images)
>   if (!images->ft_addr)
>   return;
>  
> - base = (u64)gd->bd->bi_memstart;
> - size = (u64)gd->bd->bi_memsize;
> + base = (u64)gd->bd->bi_dram[0].start;
> + size = (u64)gd->bd->bi_dram[0].size;
>  
>   off = fdt_path_offset(images->ft_addr, "/memory");
>   if (off < 0)
> diff --git a/arch/x86/cpu/broadwell/cpu_from_spl.c b/arch/x86/cpu/broadwell/cpu_from_spl.c
> index 6567d50653..4d4cdafa2b 100644
> --- a/arch/x86/cpu/broadwell/cpu_from_spl.c
> +++ b/arch/x86/cpu/broadwell/cpu_from_spl.c
> @@ -53,14 +53,12 @@ void board_debug_uart_init(void)
>  
>  int dram_init_banksize(void)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
>   struct spl_handoff *ho;
>  
>   ho = bloblist_find(BLOBLISTT_SPL_HANDOFF, sizeof(*ho));
>   if (!ho)
>   return log_msg_ret("Missing SPL hand-off info", -ENOENT);
>   handoff_load_dram_banks(ho);
> -#endif
>  
>   return 0;
>  }
> diff --git a/arch/xtensa/lib/bdinfo.c b/arch/xtensa/lib/bdinfo.c
> index 4ec8529521..1839edd9bb 100644
> --- a/arch/xtensa/lib/bdinfo.c
> +++ b/arch/xtensa/lib/bdinfo.c
> @@ -15,8 +15,8 @@ int arch_setup_bdinfo(void)
>  {
>   struct bd_info *bd = gd->bd;
>  
> - bd->bi_memstart = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> - bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
> + bd->bi_dram[0].start = PHYSADDR(CONFIG_SYS_SDRAM_BASE);
> + bd->bi_dram[0].size = CONFIG_SYS_SDRAM_SIZE;
>  
>   return 0;
>  }
> diff --git a/arch/xtensa/lib/bootm.c b/arch/xtensa/lib/bootm.c
> index 458eaf95c0..16ab51dbc9 100644
> --- a/arch/xtensa/lib/bootm.c
> +++ b/arch/xtensa/lib/bootm.c
> @@ -48,8 +48,8 @@ static struct bp_tag *setup_memory_tag(struct bp_tag *params)
>   params->size = sizeof(struct meminfo);
>   mem = (struct meminfo *)params->data;
>   mem->type = MEMORY_TYPE_CONVENTIONAL;
> - mem->start = bd->bi_memstart;
> - mem->end = bd->bi_memstart + bd->bi_memsize;
> + mem->start = bd->bi_dram[0].start;
> + mem->end = bd->bi_dram[0].start + bd->bi_dram[0].size;
>  
>   printf("   MEMORY:          tag:0x%04x, type:0X%lx, start:0X%lx, end:0X%lx\n",
>         BP_TAG_MEMORY, mem->type, mem->start, mem->end);
> diff --git a/board/Arcturus/ucp1020/spl.c b/board/Arcturus/ucp1020/spl.c
> index 5416a5b663..8eb641ef8d 100644
> --- a/board/Arcturus/ucp1020/spl.c
> +++ b/board/Arcturus/ucp1020/spl.c
> @@ -83,8 +83,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/p1010rdb/spl.c b/board/freescale/p1010rdb/spl.c
> index 4ee4573d2b..ddc0af3faa 100644
> --- a/board/freescale/p1010rdb/spl.c
> +++ b/board/freescale/p1010rdb/spl.c
> @@ -69,8 +69,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/p1_p2_rdb_pc/spl.c b/board/freescale/p1_p2_rdb_pc/spl.c
> index e76c3e82c3..f67b0fcbb9 100644
> --- a/board/freescale/p1_p2_rdb_pc/spl.c
> +++ b/board/freescale/p1_p2_rdb_pc/spl.c
> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(CONFIG_SPL_GD_ADDR + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L2_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L2_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L2_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L2_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t102xrdb/spl.c b/board/freescale/t102xrdb/spl.c
> index da442fcc18..ce7ccfe41d 100644
> --- a/board/freescale/t102xrdb/spl.c
> +++ b/board/freescale/t102xrdb/spl.c
> @@ -103,8 +103,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t104xrdb/spl.c b/board/freescale/t104xrdb/spl.c
> index f83d69ba15..4b5fb8b955 100644
> --- a/board/freescale/t104xrdb/spl.c
> +++ b/board/freescale/t104xrdb/spl.c
> @@ -94,8 +94,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t208xqds/spl.c b/board/freescale/t208xqds/spl.c
> index c197884421..dc0046d93c 100644
> --- a/board/freescale/t208xqds/spl.c
> +++ b/board/freescale/t208xqds/spl.c
> @@ -102,8 +102,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t208xrdb/spl.c b/board/freescale/t208xrdb/spl.c
> index 07aab6349c..f523622de9 100644
> --- a/board/freescale/t208xrdb/spl.c
> +++ b/board/freescale/t208xrdb/spl.c
> @@ -72,8 +72,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/freescale/t4rdb/spl.c b/board/freescale/t4rdb/spl.c
> index 64d2753da8..a5351e813a 100644
> --- a/board/freescale/t4rdb/spl.c
> +++ b/board/freescale/t4rdb/spl.c
> @@ -75,8 +75,8 @@ void board_init_r(gd_t *gd, ulong dest_addr)
>   bd = (struct bd_info *)(gd + sizeof(gd_t));
>   memset(bd, 0, sizeof(struct bd_info));
>   gd->bd = bd;
> - bd->bi_memstart = CONFIG_SYS_INIT_L3_ADDR;
> - bd->bi_memsize = CONFIG_SYS_L3_SIZE;
> + bd->bi_dram[0].start = CONFIG_SYS_INIT_L3_ADDR;
> + bd->bi_dram[0].size = CONFIG_SYS_L3_SIZE;
>  
>   arch_cpu_init();
>   get_clocks();
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index ebb7172908..4cc5cb6fd7 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -467,10 +467,8 @@ int dram_init(void)
>  #else
>  int dram_init_banksize(void)
>  {
> -#if defined(CONFIG_NR_DRAM_BANKS)
>   gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
>   gd->bd->bi_dram[0].size = get_effective_memsize();
> -#endif
>  
>   mem_map_fill();
>  
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index 9593b345a3..117788dade 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -49,7 +49,6 @@ void bdinfo_print_mhz(const char *name, unsigned long hz)
>  
>  static void print_bi_dram(const struct bd_info *bd)
>  {
> -#ifdef CONFIG_NR_DRAM_BANKS
>   int i;
>  
>   for (i = 0; i < CONFIG_NR_DRAM_BANKS; ++i) {
> @@ -59,7 +58,6 @@ static void print_bi_dram(const struct bd_info *bd)
>   bdinfo_print_num("-> size", bd->bi_dram[i].size);
>   }
>   }
> -#endif
>  }
>  
>  __weak void arch_print_bdinfo(void)
> @@ -75,8 +73,8 @@ int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  #endif
>   bdinfo_print_num("boot_params", (ulong)bd->bi_boot_params);
>   print_bi_dram(bd);
> - bdinfo_print_num("memstart", (ulong)bd->bi_memstart);
> - print_phys_addr("memsize", bd->bi_memsize);
> + bdinfo_print_num("memstart", (ulong)bd->bi_dram[0].start);
> + print_phys_addr("memsize", bd->bi_dram[0].size);

this would be redundant because the DRAM banks are already printed
in print_bi_dram() above. You could just remove these two lines.

--
- Daniel

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3] CONFIG_NR_DRAM_BANKS: Remove unreferenced code as its always defined

Stefan Roese
In reply to this post by Daniel Schwierzeck-2
On 12.08.20 16:21, Daniel Schwierzeck wrote:

> Am Mittwoch, den 12.08.2020, 14:37 +0200 schrieb Pali Rohár:
>> Hello!
>>
>>>>> diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
>>>>> index 0a13f6edb7..b3ef15963e 100644
>>>>> --- a/arch/mips/lib/bootm.c
>>>>> +++ b/arch/mips/lib/bootm.c
>>>>> @@ -242,7 +242,7 @@ static int boot_reloc_fdt(bootm_headers_t *images)
>>>>>    #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT)
>>>>>    int arch_fixup_fdt(void *blob)
>>>>>    {
>>>>> - u64 mem_start = virt_to_phys((void *)gd->bd->bi_memstart);
>>>>> + u64 mem_start = virt_to_phys((void *)gd->bd->bi_dram[0].start);
>>>>>     u64 mem_size = gd->ram_size;
>>>>
>>>> I do not fully understand this change. Why on some places you are suing
>>>> bi_dram[0].size and on some places gd->ram_size?
>>>
>>> This patch is mainly a search and replace
>>> s/bd->bi_memstart/bd->bi_dram[0].start. But as your test has shown, this is
>>> not always
>>> correct.
>>
>> Yes, this search+replace needs to be better reviewed and tested. Such
>> big change has potential to break random stuff which nobody think about.
>>
>>>>> @@ -607,8 +603,11 @@ int setup_bdinfo(void)
>>>>>    {
>>>>>     struct bd_info *bd = gd->bd;
>>>>> - bd->bi_memstart = gd->ram_base;  /* start of memory */
>>>>> - bd->bi_memsize = gd->ram_size;   /* size in bytes */
>>>>> + /* Only overwrite bi_dram[0] values if not already set */
>>>>> + if (!bd->bi_dram[0].size) {
>>>>
>>>> This still look suspicious. When is bi_dram[0].size zero? I tried to
>>>> trace source code. dram_init_banksize() is always called before
>>>> setup_bdinfo() which fills bi_dram[], so when can be dram size zero?
>>>> Or I'm missing something?
>>>
>>> dram_init_banksize() is always called earlier, yes. But its weak default
>>> only sets the bi_dram[] values when CONFIG_SYS_SDRAM_BASE is configured.
>>> I'm checking right now, if this might be a problem.
>>
>> Should not be in this case extended dram_init_banksize() to always
>> properly fill bi_dram[] structure? Setting bi_dram[] on more places and
>> doing overwriting could complicate things for future debugging or
>> extending.
>
> for all users of bi_memstart/bi_memsize the search+replace should be
> enough. But in setup_bdinfo() it is wrong because it overrides the
> initialization done in dram_init_banksize(). But setup_bdinfo() is a
> new function due to some other bd_info refactoring merged two weeks ago
> and was not available in v2 of this patch. I guess N900 was still
> working with v2 and v3 now has this kind of merge conflict regression.
>
> Just removing those lines without replacing with bi_dram should fix
> this:
>
> bd->bi_memstart = gd->ram_base;  /* start of memory */
> bd->bi_memsize = gd->ram_size;   /* size in bytes */

Yes. A new patchset will follow soon, which removes these assignments
in setup_bdinfo() but also adds a default assignment to
dram_init_banksize().

> BTW: looking deeper in the code and history I think bd_info.bi_dram is
> heavily misused as storage for DRAM configuration. Such information
> should be stored in global_data where generic code can access it if
> needed (e.g. LMB, fdt_fixup_memory_banks()).

I agree. Some of this is addressed in the upcoming patchset.

> bd_info should just be
> initialized and used when needed to boot old kernels. Also there should
> be a Kconfig option to completely disable the support of bd_info.

I agree in general, but:

If we want to support booting of the old kernels (at least on PowerPC),
then we can't remove bi_memstart/memsize from bd_info. This is a
decision that we need to make. Currently my patches remove it and
therefore at least booting of old PowerPC kernels using bd_info to
transfer the memory infos to the kernel will not work any more. This
could easily be changed by not removing those values from the struct
and adding one assignment of them to some common place. I'm not sure
if its worth it though. And I'm not even sure, if I can test this
somehow, as I would need a PowerPC board that is supported in U-Boot &
Linux for a very long time.

Thanks,
Stefan