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

Mon Apr 11 16:43:19 CEST 2016

Hi Michal,

On 11 April 2016 at 08:41, Michal Simek <michal.simek at xilinx.com> wrote:
> On 9.4.2016 20:36, Simon Glass wrote:
>> Hi Michal,
>>
>> On 6 April 2016 at 12:28, Michal Simek <michal.simek at xilinx.com> wrote:
>>> On 6.4.2016 03:28, Masahiro Yamada wrote:
>>>> Hi.
>>>>
>>>>
>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg at chromium.org>:
>>>>> Hi Michal,
>>>>>
>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek at 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 at 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 at 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]$
>
>

You may need the -C flag to make it reconfigure on each commit, and
now you might need -f also since you already have a build.

Regards,
Simon