On 09.07.20 12:27, Ovidiu Panait wrote:
Hi,
On 09.07.2020 12:15, Heinrich Schuchardt wrote:
On 09.07.20 10:04, Ovidiu Panait wrote:
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd;
+ /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */
+#ifdef CONFIG_SYS_SRAM_BASE
We want to get rid of #ifdef where possible. So it is preferable to write:
if IS_ENABLED(CONFIG_SYS_SRAM_BASE) {
One benefit is that static code analysis will consider the code.
Best regards
Heinrich
My understanding is that IS_ENABLED() only works with with boolean and tristate options.
In this case, CONFIG_SYS_SRAM_BASE is a hex value:
include/configs/pic32mzdask.h:22:#define CONFIG_SYS_SRAM_BASE 0x80000000
Switching to IS_ENABLED() produces the following build errors for qemu mips:
You could add the following helper macro to include/linux/kconfig.h
#define config_defined(cfg) _config_defined(cfg, #cfg) #define _config_defined(a1, a2) !!__builtin_strcmp(#a1, a2)
config_defined(cfg) evaluates to true if: * cfg is defined and * cfg is not defined as cfg.
As long as optimization is switch on __builtin_strcmp() is evaluated at compile time.
Best regards
Heinrich
$ git diff diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index b3a48ce10f..aa047335ec 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -19,10 +19,10 @@ int arch_setup_bdinfo(void) bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif + if (IS_ENABLED(CONFIG_SYS_SRAM_BASE)) { + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ + }
return 0; }
$ make
...
arch/mips/lib/boot.c: In function 'arch_setup_bdinfo': CC common/init/board_init.o arch/mips/lib/boot.c:23:22: error: 'CONFIG_SYS_SRAM_BASE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 23 | bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE arch/mips/lib/boot.c:23:22: note: each undeclared identifier is reported only once for each function it appears in arch/mips/lib/boot.c:24:21: error: 'CONFIG_SYS_SRAM_SIZE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 24 | bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE
Thanks,
Ovidiu
+ bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif
+ return 0; +}
unsigned long do_go_exec(ulong (*entry)(int, char * const []), int argc, char * const argv[]) { diff --git a/common/board_f.c b/common/board_f.c index 9bfcd6b236..fd7e6a17ad 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,26 +602,6 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) -static int setup_board_part1(void) -{ - bd_t *bd = gd->bd;
- /* - * Save local variables to board info struct - */ - bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ - bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif
- return 0; -} -#endif
#ifdef CONFIG_POST static int init_post(void) { @@ -942,11 +922,8 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config, - arch_setup_bdinfo, -#if defined(CONFIG_MIPS) - setup_board_part1, INIT_FUNC_WATCHDOG_RESET -#endif + arch_setup_bdinfo, display_new_sp, #ifdef CONFIG_OF_BOARD_FIXUP fix_fdt,