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

Stefan Roese Thu, 30 Jul 2020 00:31:24 -0700

Hi Daniel,

On 24.07.20 22:12, Daniel Schwierzeck 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.


I wasn't aware of this change but this likely broke some boards or some
functionality for archs like MIPS or PPC at the time. Unconditionally
enabling CONFIG_NR_DRAM_BANKS was actually incorrect without migrating
all users of bd->bi_memstart and bd->bi_memsize to bd->bi_dram[0].
Maybe most stuff kept working because board_f.c
executed setup_board_part1() and dram_init_banksize() in parallel on
archs like MIPS or PPC and redundant DRAM information had been carried
around since then.


Thanks for catching this.

To make this patch complete, we should finally remove the redundant
bi_memstart and bi_memsize. The replacement with bi_dram[0].start and
bi_dram[0].size should be rather trivial.


I'll try to work on a v2 patch for this.

Thanks,
Stefan


$ git grep -n bi_memstart
api/api_platform-mips.c:27:     platform_set_mr(si, gd->bd-

bi_memstart,

api/api_platform-powerpc.c:45:  platform_set_mr(si, gd->bd-

bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);

arch/arc/lib/cpu.c:30:  gd->bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;
arch/mips/lib/boot.c:20:        flush_cache(gd->bd->bi_memstart, gd-

ram_top - gd->bd->bi_memstart);

arch/mips/lib/bootm.c:245:      u64 mem_start = virt_to_phys((void
*)gd->bd->bi_memstart);
arch/powerpc/cpu/mpc83xx/fdt.c:124:     fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc83xx/traps.c:26:#define END_OF_MEM  (gd->bd-

bi_memstart + gd->bd->bi_memsize)

arch/powerpc/cpu/mpc85xx/fdt.c:675:     fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc85xx/fdt.c:678:     ft_fixup_cpu(blob, (u64)bd-

bi_memstart + (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc85xx/traps.c:40:#define END_OF_MEM (gd->bd-

bi_memstart + get_effective_memsize())

arch/powerpc/cpu/mpc86xx/fdt.c:30:      fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc86xx/traps.c:33:#define END_OF_MEM (gd->bd-

bi_memstart + get_effective_memsize())

arch/powerpc/cpu/mpc8xx/fdt.c:28:       fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/lib/bootm.c:301:   base = (u64)gd->bd->bi_memstart;
arch/xtensa/lib/bootm.c:51:     mem->start = bd->bi_memstart;
arch/xtensa/lib/bootm.c:52:     mem->end = bd->bi_memstart + bd-

bi_memsize;

board/Arcturus/ucp1020/spl.c:86:        bd->bi_memstart =
CONFIG_SYS_INIT_L2_ADDR;
board/cadence/xtfpga/xtfpga.c:54:       gd->bd->bi_memstart =
PHYSADDR(CONFIG_SYS_SDRAM_BASE);
board/freescale/p1010rdb/spl.c:72:      bd->bi_memstart =
CONFIG_SYS_INIT_L2_ADDR;
board/freescale/p1_p2_rdb_pc/spl.c:78:  bd->bi_memstart =
CONFIG_SYS_INIT_L2_ADDR;
board/freescale/t102xrdb/spl.c:106:     bd->bi_memstart =
CONFIG_SYS_INIT_L3_ADDR;
board/freescale/t104xrdb/spl.c:97:      bd->bi_memstart =
CONFIG_SYS_INIT_L3_ADDR;
board/freescale/t208xqds/spl.c:105:     bd->bi_memstart =
CONFIG_SYS_INIT_L3_ADDR;
board/freescale/t208xrdb/spl.c:75:      bd->bi_memstart =
CONFIG_SYS_INIT_L3_ADDR;
board/freescale/t4rdb/spl.c:78: bd->bi_memstart =
CONFIG_SYS_INIT_L3_ADDR;
cmd/bdinfo.c:76:        bdinfo_print_num("memstart", (ulong)bd-

bi_memstart);

cmd/bedbug.c:351:       top = gd->bd->bi_memstart + gd->bd->bi_memsize;
common/board_f.c:610:   bd->bi_memstart =
CONFIG_SYS_SDRAM_BASE;        /* start of memory */
common/image.c:693:     start = gd->bd->bi_memstart;
drivers/video/cfb_console.c:1999:       if ((ulong)video_fb_address >=
bd->bi_memstart &&
drivers/video/cfb_console.c:2000:           (ulong)video_fb_address <
bd->bi_memstart + bd->bi_memsize)
include/asm-generic/u-boot.h:30:        unsigned
long   bi_memstart;    /* start of DRAM memory */
lib/lmb.c:130:          lmb_add(lmb, bd->bi_memstart, bd->bi_memsize);


$ git grep -n bi_memsize
api/api_platform-mips.c:28:                     gd->bd->bi_memsize,
MR_ATTR_DRAM);
api/api_platform-powerpc.c:45:  platform_set_mr(si, gd->bd-

bi_memstart, gd->bd->bi_memsize, MR_ATTR_DRAM);

arch/arc/lib/cpu.c:31:  gd->bd->bi_memsize = CONFIG_SYS_SDRAM_SIZE;
arch/powerpc/cpu/mpc83xx/fdt.c:124:     fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc83xx/traps.c:26:#define END_OF_MEM  (gd->bd-

bi_memstart + gd->bd->bi_memsize)

arch/powerpc/cpu/mpc85xx/fdt.c:675:     fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc85xx/fdt.c:678:     ft_fixup_cpu(blob, (u64)bd-

bi_memstart + (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc86xx/fdt.c:30:      fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/cpu/mpc8xx/fdt.c:28:       fdt_fixup_memory(blob, (u64)bd-

bi_memstart, (u64)bd->bi_memsize);

arch/powerpc/lib/bootm.c:302:   size = (u64)gd->bd->bi_memsize;
arch/xtensa/lib/bootm.c:52:     mem->end = bd->bi_memstart + bd-

bi_memsize;

board/Arcturus/ucp1020/spl.c:87:        bd->bi_memsize =
CONFIG_SYS_L2_SIZE;
board/cadence/xtfpga/xtfpga.c:55:       gd->bd->bi_memsize =
CONFIG_SYS_SDRAM_SIZE;
board/freescale/p1010rdb/spl.c:73:      bd->bi_memsize =
CONFIG_SYS_L2_SIZE;
board/freescale/p1_p2_rdb_pc/spl.c:79:  bd->bi_memsize =
CONFIG_SYS_L2_SIZE;
board/freescale/t102xrdb/spl.c:107:     bd->bi_memsize =
CONFIG_SYS_L3_SIZE;
board/freescale/t104xrdb/spl.c:98:      bd->bi_memsize =
CONFIG_SYS_L3_SIZE;
board/freescale/t208xqds/spl.c:106:     bd->bi_memsize =
CONFIG_SYS_L3_SIZE;
board/freescale/t208xrdb/spl.c:76:      bd->bi_memsize =
CONFIG_SYS_L3_SIZE;
board/freescale/t4rdb/spl.c:79: bd->bi_memsize = CONFIG_SYS_L3_SIZE;
cmd/bdinfo.c:77:        print_lnum("memsize", (u64)bd->bi_memsize);
cmd/bedbug.c:351:       top = gd->bd->bi_memstart + gd->bd->bi_memsize;
common/board_f.c:611:   bd->bi_memsize = gd-

ram_size;                  /* size in bytes */

common/image.c:694:     size = gd->bd->bi_memsize;
drivers/video/cfb_console.c:2000:           (ulong)video_fb_address <
bd->bi_memstart + bd->bi_memsize)
include/asm-generic/u-
boot.h:31:        phys_size_t     bi_memsize;     /* size  of DRAM
memory in bytes */
lib/lmb.c:129:  if (bd->bi_memsize)
lib/lmb.c:130:          lmb_add(lmb, bd->bi_memstart, bd->bi_memsize);


Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Tom Rini <tr...@konsulko.com>
Cc: Ramon Fried <ramon.fr...@gmail.com>
Cc: Simon Glass <s...@chromium.org>
Cc: Michal Simek <michal.si...@xilinx.com>
---
  arch/x86/cpu/broadwell/cpu_from_spl.c |  2 --
  board/xilinx/zynqmp/zynqmp.c          |  2 --
  cmd/bdinfo.c                          |  2 --
  common/board_f.c                      |  6 +-----
  common/image.c                        |  3 +--
  common/init/handoff.c                 |  4 ----
  drivers/pci/pci-uclass.c              | 17 +----------------
  include/asm-generic/u-boot.h          |  2 --
  include/handoff.h                     |  2 --
  lib/fdtdec.c                          |  5 -----
  lib/lmb.c                             |  7 -------
  11 files changed, 3 insertions(+), 49 deletions(-)

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/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 8b2c105e77..61daef4214 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -47,7 +47,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) {

@@ -57,7 +56,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)

diff --git a/common/board_f.c b/common/board_f.c
index 88ff0424a7..d7c13f63c9 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
diff --git a/common/image.c b/common/image.c
index 9d7d5c17d1..2ed46f7685 100644
--- a/common/image.c
+++ b/common/image.c
@@ -685,8 +685,7 @@ phys_size_t env_get_bootm_size(void)
                return tmp;
        }

-#if (defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)) && \

-     defined(CONFIG_NR_DRAM_BANKS)
+#if defined(CONFIG_ARM) || defined(CONFIG_MICROBLAZE)
        start = gd->bd->bi_dram[0].start;
        size = gd->bd->bi_dram[0].size;
  #else
diff --git a/common/init/handoff.c b/common/init/handoff.c
index e00b43e6a7..c20fbf78b8 100644
--- a/common/init/handoff.c
+++ b/common/init/handoff.c
@@ -13,7 +13,6 @@ DECLARE_GLOBAL_DATA_PTR;
  void handoff_save_dram(struct spl_handoff *ho)
  {
        ho->ram_size = gd->ram_size;
-#ifdef CONFIG_NR_DRAM_BANKS
        {
                struct bd_info *bd = gd->bd;
                int i;
@@ -23,7 +22,6 @@ void handoff_save_dram(struct spl_handoff *ho)
                        ho->ram_bank[i].size = bd->bi_dram[i].size;
                }
        }
-#endif
  }

void handoff_load_dram_size(struct spl_handoff *ho)

@@ -33,7 +31,6 @@ 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;
@@ -43,5 +40,4 @@ void handoff_load_dram_banks(struct spl_handoff *ho)
                        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/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
index 62e61d41cc..99d3fe33ad 100644
--- a/include/asm-generic/u-boot.h
+++ b/include/asm-generic/u-boot.h
@@ -70,12 +70,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 2d680d8d02..f0adc9592e 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -113,22 +113,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);
  }



Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

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

Reply via email to