[U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT

[Michal Simek <michal.simek@xilinx.com>]

On 9.4.2016 20:36, Simon Glass wrote:
> Hi Michal,
> 
> On 6 April 2016 at 12:28, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 6.4.2016 03:28, Masahiro Yamada wrote:
>>> Hi.
>>>
>>>
>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>>> Hi Michal,
>>>>
>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>> Hi Simon,
>>>>>
>>>>> On 5.4.2016 02:03, Simon Glass wrote:
>>>>>> Hi Michal,
>>>>>>
>>>>>> On 4 April 2016 at 11:50, Michal Simek <michal.simek@xilinx.com> wrote:
>>>>>>> Create CMD_FDT Kconfig entry to have an option to disable fdt command
>>>>>>> which is not required for small configuration which requires libfdt
>>>>>>> only.
>>>>>>> Enable it by default for all targets which enables OF_LIBFDT.
>>>>>>>
>>>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  cmd/Kconfig  | 7 +++++++
>>>>>>>  cmd/Makefile | 2 +-
>>>>>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>>>>>>> index fe8b4f0510da..8703cdb4a9be 100644
>>>>>>> --- a/cmd/Kconfig
>>>>>>> +++ b/cmd/Kconfig
>>>>>>> @@ -173,6 +173,13 @@ config CMD_ELF
>>>>>>>         help
>>>>>>>           Boot an ELF/vxWorks image from the memory.
>>>>>>>
>>>>>>> +config CMD_FDT
>>>>>>> +       bool "Flattened Device Tree utility commands"
>>>>>>> +       default y
>>>>>>
>>>>>> Should that be:
>>>>>>
>>>>>> default y if OF_LIBFDT
>>>>>>
>>>>>> ?
>>>>>>
>>>>>>> +       depends on OF_LIBFDT
>>>>>>> +       help
>>>>>>> +         Do FDT related setup before booting into the Operating System.
>>>>>>> +
>>>>>
>>>>>
>>>>> In recent commits to this file both formats are used.
>>>>>
>>>>> +config CMD_BLOCK_CACHE
>>>>> + bool "blkcache - control and stats for block cache"
>>>>> + depends on BLOCK_CACHE
>>>>> + default y if BLOCK_CACHE
>>>>>
>>>>> even looks non standard.
>>>>>
>>>>> +config CMD_BOOTEFI
>>>>> + bool "bootefi"
>>>>> + depends on EFI_LOADER
>>>>> + default y
>>>>>
>>>>> I am happy to change whatever style you prefer but I think it should be
>>>>> synchronized. The efi one was Reviewed by you. :-)
>>>>
>>>> I think Masahiro knows most about this. If it works it's fine with me.
>>>> The way you have it is more intuitive and I prefer it. But he did
>>>> point at a problem at some point.
>>>
>>>
>>> I think "depends on OF_LIBFDT"
>>> is correct in this case.
>>>
>>>
>>> do_fdt() calls fdt_fixup_memory(), which is defined in common/fdt_support.c,
>>> which is enabled by CONFIG_OF_LIBFDT.
>>>
>>> So, CMD_FDT should depend on OF_LIBFDT.
>>> Otherwise, "make menuconfig" would allow users
>>> to enable CMD_FDT without OF_LIBFDT,
>>> which would cause link error.
>>>
>>>
>>>> One other question - won't this disable the 'fdt' command for many boards?
>>>
>>>
>>> config CMD_FDT
>>>     bool "Flattened Device Tree utility commands"
>>>     default y
>>>     depends on OF_LIBFDT
>>>
>>>
>>> "default y" cares about it.
>>> So, if CONFIG_OF_LIBFDT is enabled in the defconfig,
>>> CONFIG_CMD_FDT should be enabled as well.
>>>
>>>
>>>
>>> But the following 6 boards opt out of Kconfig.
>>> They define CONFIG_OF_LIBFDT in their config headers,
>>> so this patch would disable "fdt" command for them.
>>>
>>> include/configs/legoev3.h
>>> include/configs/ma5d4evk.h
>>> include/configs/pic32mzdask.h
>>> include/configs/stm32f746-disco.h
>>> include/configs/xilinx-ppc.h
>>> include/configs/zipitz2.h
>>>
>>>
>>>
>>> Could you move them to defconfigs?
>>>
>>
>> I have sent v2 to address these. I have used buildman and there is up to
>> 2k difference when symbol is in Kconfig. Not sure why but it shouldn't
>> be big deal.
> 
> What does 2k difference mean?

hm. I wanted to use buildman to show me that but that looks weird.
It looks like that it is not running make legoev3_defconfig after new
patch because LIBFDT is not enabled when 3 commits are built.
But when 2 commits are built then it is enabled properly.
Isn't there any bug in buildman?

Thanks,
Michal

[u-boot]$ ./tools/buildman/buildman -b xnext/kconfig2 legoev3 -o /tmp/
-sSedKv
boards.cfg is up to date. Nothing to do.
Summary of 3 commits for 1 boards (1 thread, 8 jobs per thread)
01: doc: clarify openssl-based key and certificate generation process
02: kconfig: Move CONFIG_OF_LIBFDT to Kconfig
       arm: (for 1/1 boards)  all -23674.0  bss -1032.0  data -1920.0
rodata -3622.0  text -17100.0
            legoev3        :  all -23674  bss -1032  data -1920  rodata
-3622  text -17100
arm:
   - autoconf.mk: CONFIG_OF_LIBFDT=y
   - all: CONFIG_OF_LIBFDT=y
legoev3 :
   - autoconf.mk: CONFIG_OF_LIBFDT=y
   - all: CONFIG_OF_LIBFDT=y
03: cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT
(no errors to report)
[u-boot]$ git log --oneline | head -n 3
0440a5580b8a cmd: fdt: Use separate CMD_FDT Kconfig entry instead of
OF_LIBFDT
ef3a52ec50b0 kconfig: Move CONFIG_OF_LIBFDT to Kconfig
4c1d5c29b5de doc: clarify openssl-based key and certificate generation
process
[u-boot]$
[u-boot]$ ./tools/buildman/buildman -b xnext/kconfig2 legoev3 -o /tmp/ -c 2
boards.cfg is up to date. Nothing to do.
Building 2 commits for 1 boards (1 thread, 8 jobs per thread)
    2    0    0 /2      legoev3
[u-boot]$ ./tools/buildman/buildman -b xnext/kconfig2 legoev3 -o /tmp/
-c 2 -sSedKv
boards.cfg is up to date. Nothing to do.
Summary of 2 commits for 1 boards (1 thread, 8 jobs per thread)
01: kconfig: Move CONFIG_OF_LIBFDT to Kconfig
02: cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT
arm:
   + .config: CONFIG_CMD_FDT=y
   + autoconf.h: CONFIG_CMD_FDT=1
   + u-boot.cfg: CONFIG_CMD_FDT=1
   + all: CONFIG_CMD_FDT=1
legoev3 :
   + .config: CONFIG_CMD_FDT=y
   + autoconf.h: CONFIG_CMD_FDT=1
   + u-boot.cfg: CONFIG_CMD_FDT=1
   + all: CONFIG_CMD_FDT=1
(no errors to report)
[u-boot]$