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

[Simon Glass]

Hi,

On Thu, 9 Jul 2020 at 06:28, Heinrich Schuchardt <xypron.debian at gmx.de> wrote:
>
> 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.

I think we should have a new CONFIG_SYS_HAS_SRAM.

Regards,
Simon