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

[Heinrich Schuchardt]

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

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