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

Stefan Roese sr at denx.de
Thu Jul 30 09:30:56 CEST 2020


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 <sr at denx.de>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: Ramon Fried <ramon.fried at gmail.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Michal Simek <michal.simek at 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: sr at denx.de


More information about the U-Boot mailing list