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

[Michal Simek <monstr@monstr.eu>]

On 08/21/2015 10:16 AM, Marek Vasut wrote:
> 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@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@socionext.com>
>>>>>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>>> Acked-by: Linus Walleij <linus.walleij@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 :)

I want to look at moving microblaze DM and just use OF_CONTROL and
remove this ancient configuration method.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150821/739cd33e/attachment.sig>