Hi Michal, On 11 April 2016 at 08:48, Michal Simek <michal.si...@xilinx.com> wrote: > On 11.4.2016 16:43, Simon Glass wrote: >> Hi Michal, >> >> On 11 April 2016 at 08:41, Michal Simek <michal.si...@xilinx.com> wrote: >>> On 9.4.2016 20:36, Simon Glass wrote: >>>> Hi Michal, >>>> >>>> On 6 April 2016 at 12:28, Michal Simek <michal.si...@xilinx.com> wrote: >>>>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>>>> Hi. >>>>>> >>>>>> >>>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <s...@chromium.org>: >>>>>>> Hi Michal, >>>>>>> >>>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.si...@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.si...@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.si...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
