On 11 April 2016 at 08:59, Michal Simek <michal.si...@xilinx.com> wrote: > On 11.4.2016 16:50, Simon Glass wrote: >> 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... > > 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?
Looks OK to me. Reviewed-by: Simon Glass <s...@chromium.org> > > 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 > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
