[PATCH 05/10] board_f: mips: Factor out mips-specific bdinfo setup

[Ovidiu Panait]

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 at 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:

$ 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,
>>