[U-Boot] [U-Boot, v3, 15/15] of: clean up OF_CONTROL ifdef conditionals

[Marek Vasut]

On Friday, August 21, 2015 at 09:55:25 AM, Masahiro Yamada wrote:
> Hi Marek,

Hi Masahiro,

> 2015-08-20 6:15 GMT+09:00 Marek Vasut <marex at denx.de>:
> > On Wednesday, August 19, 2015 at 02:02:18 PM, Tom Rini wrote:
> >> On Wed, Aug 19, 2015 at 06:39:29AM +0200, Marek Vasut wrote:
> >> > On Tuesday, August 18, 2015 at 07:53:23 PM, Tom Rini wrote:
> >> > > On Wed, Aug 12, 2015 at 07:31:55AM +0900, Masahiro Yamada wrote:
> >> > > > We have flipped CONFIG_SPL_DISABLE_OF_CONTROL.  We have cleansing
> >> > > > devices, $(SPL_) and CONFIG_IS_ENABLED(), so we are ready to clear
> >> > > > 
> >> > > > away the ugly logic in include/fdtdec.h:
> >> > > >  #ifdef CONFIG_OF_CONTROL
> >> > > >  # if defined(CONFIG_SPL_BUILD) && !defined(SPL_OF_CONTROL)
> >> > > >  #  define OF_CONTROL 0
> >> > > >  # else
> >> > > >  #  define OF_CONTROL 1
> >> > > >  # endif
> >> > > >  #else
> >> > > >  # define OF_CONTROL 0
> >> > > >  #endif
> >> > > > 
> >> > > > Now CONFIG_IS_ENABLED(OF_CONTROL) is the substitute.  It refers to
> >> > > > CONFIG_OF_CONTROL for U-boot proper and CONFIG_SPL_OF_CONTROL for
> >> > > > SPL.
> >> > > > 
> >> > > > Also, we no longer have to cancel CONFIG_OF_CONTROL in
> >> > > > include/config_uncmd_spl.h and scripts/Makefile.spl.
> >> > > > 
> >> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> >> > > > Reviewed-by: Tom Rini <trini at konsulko.com>
> >> > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> >> > > > Acked-by: Linus Walleij <linus.walleij at linaro.org>
> >> > > 
> >> > > (With a slight change due to samsun PR, but applied cleanly after
> >> > > rename)
> >> > > Applied to u-boot/master, thanks!
> >> > 
> >> > And this just broke just about every SoCFPGA platform (try ie.
> >> > socfpga_cyclone5) ;-) I poked into this a bit and noticed that adding
> >> > #include <linux/kconfig.h> into include/configs/socfpga_common.h fixes
> >> > the issue, but I'd like to check with you if this is the correct way
> >> > of fixing it.
> >> 
> >> Broke it how?
> >> $ ./tools/buildman/buildman socfpga_cyclone5
> >> boards.cfg is up to date. Nothing to do.
> >> Building current source for 1 boards (1 thread, 6 jobs per thread)
> >> 
> >>     1    0    0 /1      socfpga_cyclone5
> > 
> > Looks like I wasn't really accurate and mainline actually managed to
> > avoid this breakage. I only triggered it with a custom patch, which
> > enables env in SPI NOR (see the spit below).
> > 
> > I suspect the problem is caused by tools/envcrc.c, which does include the
> > board config file (which uses this CONFIG_IS_ENABLED()), but does not
> > include a header file which defines it.
> > 
> > The question therefore is, shall we not used CONFIG_IS_ENABLED() in board
> > config files or shall we include linux/kconfig.h in tools/envcrc.c ?
> 
> Our final goal is:
>     We should not use any CONFIG_* in host tools.
>     Actually, some host tools in U-boot depend on the board
> configuration, which is weird.
>   If we could rip off CONFIG_* from them, neither CONFIG_IS_ENABLED()
> nor linux/kconfig.h
>   is needed for host tools.  I believe this is the right thing to do.
>     Any volunteer?
> 
> 
> What I can suggest now is:
>     Do not use CONFIG_IS_ENABLED() in board config headers.

Yup, this is what I'll do :)

> include/config/microblaze-generic.h does this:
> 
> #if !defined(CONFIG_OF_CONTROL) || \
> (defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_OF_CONTROL))
> /* ddr sdram - main memory */
> # define CONFIG_SYS_SDRAM_BASE XILINX_RAM_START
> # define CONFIG_SYS_SDRAM_SIZE XILINX_RAM_SIZE
> #endif
> 
> 
> This is indeed ugly, but works...

I don't think I even need this :)

Best regards,
Marek Vasut