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

Michal Simek michal.simek at xilinx.com
Mon Apr 11 16:48:45 CEST 2016


On 11.4.2016 16:43, Simon Glass wrote:
> 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.

Ah. Ok.

This is causing that 2k diff.
#define IMAGE_ENABLE_OF_LIBFDT  CONFIG_IS_ENABLED(OF_LIBFDT)

I will run it over all boards which are affected.

Thanks,
Michal

       arm: (for 1/1 boards)  all +2192.0  bss +28.0  rodata +64.0  text
+2100.0
            legoev3        :  all +2192  bss +28  rodata +64  text +2100
               u-boot: add: 7/-1, grow: 4/0 bytes: 2240/-356 (1884)
                 function                                   old     new
  delta
                 boot_get_fdt                                 -     780
   +780
                 boot_prep_linux                              -     400
   +400
                 image_setup_libfdt                           -     316
   +316
                 lmb_free                                     -     268
   +268
                 fdt_root                                     -     140
   +140
                 image_setup_linux                            -     136
   +136
                 bootm_find_images                           72     168
    +96
                 do_bootm_states                           1952    2004
    +52
                 genimg_get_format                           52      76
    +24
                 boot_jump_linux                            172     192
    +20
                 genimg_has_config                            -       8
     +8
                 static.boot_prep_linux                     356       -
   -356





More information about the U-Boot mailing list