* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT @ 2016-04-04 17:50 Michal Simek 2016-04-05 0:03 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-04 17:50 UTC (permalink / raw) To: u-boot 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@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 + depends on OF_LIBFDT + help + Do FDT related setup before booting into the Operating System. + config CMD_GO bool "go" default y diff --git a/cmd/Makefile b/cmd/Makefile index ba041973079c..f95759e67044 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -54,7 +54,7 @@ obj-$(CONFIG_CMD_EXT4) += ext4.o obj-$(CONFIG_CMD_EXT2) += ext2.o obj-$(CONFIG_CMD_FAT) += fat.o obj-$(CONFIG_CMD_FDC) += fdc.o -obj-$(CONFIG_OF_LIBFDT) += fdt.o +obj-$(CONFIG_CMD_FDT) += fdt.o obj-$(CONFIG_CMD_FITUPD) += fitupd.o obj-$(CONFIG_CMD_FLASH) += flash.o ifdef CONFIG_FPGA -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-04 17:50 [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT Michal Simek @ 2016-04-05 0:03 ` Simon Glass 2016-04-05 10:15 ` Michal Simek 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-04-05 0:03 UTC (permalink / raw) To: u-boot Hi Michal, On 4 April 2016 at 11:50, Michal Simek <michal.simek@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@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. > + > config CMD_GO > bool "go" > default y > diff --git a/cmd/Makefile b/cmd/Makefile > index ba041973079c..f95759e67044 100644 > --- a/cmd/Makefile > +++ b/cmd/Makefile > @@ -54,7 +54,7 @@ obj-$(CONFIG_CMD_EXT4) += ext4.o > obj-$(CONFIG_CMD_EXT2) += ext2.o > obj-$(CONFIG_CMD_FAT) += fat.o > obj-$(CONFIG_CMD_FDC) += fdc.o > -obj-$(CONFIG_OF_LIBFDT) += fdt.o > +obj-$(CONFIG_CMD_FDT) += fdt.o > obj-$(CONFIG_CMD_FITUPD) += fitupd.o > obj-$(CONFIG_CMD_FLASH) += flash.o > ifdef CONFIG_FPGA > -- > 1.9.1 > Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-05 0:03 ` Simon Glass @ 2016-04-05 10:15 ` Michal Simek 2016-04-05 19:09 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-05 10:15 UTC (permalink / raw) To: u-boot Hi Simon, On 5.4.2016 02:03, Simon Glass wrote: > Hi Michal, > > On 4 April 2016 at 11:50, Michal Simek <michal.simek@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@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. :-) Thanks, Michal ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-05 10:15 ` Michal Simek @ 2016-04-05 19:09 ` Simon Glass 2016-04-06 1:28 ` Masahiro Yamada 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-04-05 19:09 UTC (permalink / raw) To: u-boot Hi Michal, On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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. One other question - won't this disable the 'fdt' command for many boards? Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-05 19:09 ` Simon Glass @ 2016-04-06 1:28 ` Masahiro Yamada 2016-04-06 18:28 ` Michal Simek 0 siblings, 1 reply; 13+ messages in thread From: Masahiro Yamada @ 2016-04-06 1:28 UTC (permalink / raw) To: u-boot Hi. 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: > Hi Michal, > > On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-06 1:28 ` Masahiro Yamada @ 2016-04-06 18:28 ` Michal Simek 2016-04-09 18:36 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-06 18:28 UTC (permalink / raw) To: u-boot On 6.4.2016 03:28, Masahiro Yamada wrote: > Hi. > > > 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >> Hi Michal, >> >> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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. Thanks, Michal ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-06 18:28 ` Michal Simek @ 2016-04-09 18:36 ` Simon Glass 2016-04-11 14:41 ` Michal Simek 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-04-09 18:36 UTC (permalink / raw) To: u-boot Hi Michal, On 6 April 2016 at 12:28, Michal Simek <michal.simek@xilinx.com> wrote: > On 6.4.2016 03:28, Masahiro Yamada wrote: >> Hi. >> >> >> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>> Hi Michal, >>> >>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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? Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-09 18:36 ` Simon Glass @ 2016-04-11 14:41 ` Michal Simek 2016-04-11 14:43 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-11 14:41 UTC (permalink / raw) To: u-boot On 9.4.2016 20:36, Simon Glass wrote: > Hi Michal, > > On 6 April 2016 at 12:28, Michal Simek <michal.simek@xilinx.com> wrote: >> On 6.4.2016 03:28, Masahiro Yamada wrote: >>> Hi. >>> >>> >>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>> Hi Michal, >>>> >>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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]$ ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-11 14:41 ` Michal Simek @ 2016-04-11 14:43 ` Simon Glass 2016-04-11 14:48 ` Michal Simek 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-04-11 14:43 UTC (permalink / raw) To: u-boot Hi Michal, On 11 April 2016 at 08:41, Michal Simek <michal.simek@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@xilinx.com> wrote: >>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>> Hi. >>>> >>>> >>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>>> Hi Michal, >>>>> >>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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. Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-11 14:43 ` Simon Glass @ 2016-04-11 14:48 ` Michal Simek 2016-04-11 14:50 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-11 14:48 UTC (permalink / raw) To: u-boot On 11.4.2016 16:43, Simon Glass wrote: > Hi Michal, > > On 11 April 2016 at 08:41, Michal Simek <michal.simek@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@xilinx.com> wrote: >>>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>>> Hi. >>>>> >>>>> >>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>>>> Hi Michal, >>>>>> >>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-11 14:48 ` Michal Simek @ 2016-04-11 14:50 ` Simon Glass 2016-04-11 14:59 ` Michal Simek 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-04-11 14:50 UTC (permalink / raw) To: u-boot Hi Michal, On 11 April 2016 at 08:48, Michal Simek <michal.simek@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@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@xilinx.com> wrote: >>>>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>>>> Hi. >>>>>> >>>>>> >>>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>>>>> Hi Michal, >>>>>>> >>>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-11 14:50 ` Simon Glass @ 2016-04-11 14:59 ` Michal Simek 2016-04-20 14:40 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Michal Simek @ 2016-04-11 14:59 UTC (permalink / raw) To: u-boot On 11.4.2016 16:50, Simon Glass wrote: > Hi Michal, > > On 11 April 2016 at 08:48, Michal Simek <michal.simek@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@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@xilinx.com> wrote: >>>>>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>>>>> Hi. >>>>>>> >>>>>>> >>>>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>>>>>> Hi Michal, >>>>>>>> >>>>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT 2016-04-11 14:59 ` Michal Simek @ 2016-04-20 14:40 ` Simon Glass 0 siblings, 0 replies; 13+ messages in thread From: Simon Glass @ 2016-04-20 14:40 UTC (permalink / raw) To: u-boot On 11 April 2016 at 08:59, Michal Simek <michal.simek@xilinx.com> wrote: > On 11.4.2016 16:50, Simon Glass wrote: >> Hi Michal, >> >> On 11 April 2016 at 08:48, Michal Simek <michal.simek@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@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@xilinx.com> wrote: >>>>>>> On 6.4.2016 03:28, Masahiro Yamada wrote: >>>>>>>> Hi. >>>>>>>> >>>>>>>> >>>>>>>> 2016-04-06 4:09 GMT+09:00 Simon Glass <sjg@chromium.org>: >>>>>>>>> Hi Michal, >>>>>>>>> >>>>>>>>> On 5 April 2016 at 04:15, Michal Simek <michal.simek@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@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@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 <sjg@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 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-04-20 14:40 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-04 17:50 [U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT Michal Simek 2016-04-05 0:03 ` Simon Glass 2016-04-05 10:15 ` Michal Simek 2016-04-05 19:09 ` Simon Glass 2016-04-06 1:28 ` Masahiro Yamada 2016-04-06 18:28 ` Michal Simek 2016-04-09 18:36 ` Simon Glass 2016-04-11 14:41 ` Michal Simek 2016-04-11 14:43 ` Simon Glass 2016-04-11 14:48 ` Michal Simek 2016-04-11 14:50 ` Simon Glass 2016-04-11 14:59 ` Michal Simek 2016-04-20 14:40 ` Simon Glass
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox