[U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT
Simon Glass
sjg at chromium.org
Mon Apr 11 16:50:48 CEST 2016
Hi Michal,
On 11 April 2016 at 08:48, Michal Simek <michal.simek at xilinx.com> wrote:
> 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
That seems bad. I've been in this code quite a bit - it is tricky to
keep the options the same - e.g. my recent conversion of CONFIG_FIT to
Kconfig was a bit of a mission. Worth digging into...
Regards,
Simon
More information about the U-Boot
mailing list