[U-Boot] [PATCH] cmd: fdt: Use separate CMD_FDT Kconfig entry instead of OF_LIBFDT

[Michal Simek]

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.

Thanks,
Michal