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

Simon Glass
2016-04-09T18:36:06Z

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