[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:59:48 CEST 2016
On 11.4.2016 16:50, Simon Glass wrote:
> 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...
I know that you have converted others.
I have checked and others boards are just the same (except xilinx_ppc
which is not affected) +2k because of that macro. But is this a problem?
I mean my goal is to separate CMD_FDT command because it is just too
huge if you have just small amount of memory.
I don't know these boards but only Xilinx PPC guy has responded that it
is fine and none else. It means that they are fine with this change and
they can fix it if +2k is problem for them.
I fully understand that this is the problem from conversion point of
view if simple move is causing it but at the end of the day I don't
think it is worth to invest time to look these 5.
What do you think?
Thanks,
Michal
[u-boot]$ ./tools/buildman/buildman -b xnext/kconfig2 legoev3 xilinx-ppc
ma5d4evk zipitz2 stm32f746-disco pic32mzdask -o /tmp/ -CsSedv
boards.cfg is up to date. Nothing to do.
Summary of 3 commits for 7 boards (7 threads, 2 jobs per thread)
01: Merge branch 'master' of git://git.denx.de/u-boot-fsl-qoriq
02: kconfig: Move CONFIG_OF_LIBFDT to Kconfig
arm: (for 4/4 boards) all +2033.0 bss +32.0 rodata +66.0 text
+1935.0
ma5d4evk : all +2328 bss +12 rodata +68 text +2248
zipitz2 : all +2256 bss +88 rodata +68 text +2100
legoev3 : all +2192 bss +28 rodata +64 text +2100
stm32f746-disco: all +1356 rodata +64 text +1292
03: cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT
More information about the U-Boot
mailing list