[U-Boot] [PATCH 07/13] arm: zynq: Use separate device tree instead of embedded

Simon Glass
2015-08-31T23:12:33Z

Hi Michal,

On 31 August 2015 at 08:07, Michal Simek <monstr@monstr.eu> wrote:
> On 08/31/2015 03:54 PM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 05:24, Michal Simek <monstr@monstr.eu> wrote:
>>> On 08/29/2015 05:10 PM, Simon Glass wrote:
>>>> Production boards should not use CONFIG_OF_EMBED. Fix this for the Zybo
>>>> boards.
>>>
>>> Zynq boards?
>>>
>>> As you see I have enabled OF_EMBED some weeks ago.
>>> zynq: Make CONFIG_OF_EMBED default case
>>> 98b532b42079a7ffd617ce0330d6778288b7c535
>>>
>>> What's the reason not to use CONFIG_OF_EMBED for production boards?
>>> Strictly speaking none of these boards are production one.
>>> I would label them more as refence boards, development boards.
>>
>> Well I mean that we should not have boards in the tree that use this
>> option, It has been a long-standing convention since device tree
>> support was added - see README.fdt-control. I'll send a patch to add
>> this note to Kconfig.
>
> ok then do we want to support this option? If this shouldn't be enabled
> in any config in the tree? (Note: someone has to enable that and keep
> building u-boot to ensure that this feature still work)

It's a bit like disabling relocation. It is useful when you want to
load an elf image onto your board with a debugger, without having to
worry about getting the device tree loaded also. It is normally
possible to load u-boot-dtb.bin and then load the symbols from u-boot
separately, but that's the reasoning.

>
> The reason why I have enabled that was that using u-boot-dtb.img is
> breaking all users because they have to start to change a lot of things.
> That's why having OF_EMBED enabled was less painful.
> Users which use these boards just don't recognize any change when this
> feature is enabled.

I see. Well perhaps you could enable the debug UART / do something
else so that a message will be printed in this case for all boards?

We could perhaps move to a different scheme:

u-boot.bin - contains U-Boot and device tree(s)
u-boot-nodtb.bin - contains plain binary without device tree

and perhaps this behaviour could even be optional (thus you could
select it for Zynq). This has come up before. Tegra uses
u-boot-nodtb-tegra.bin although it is for a different purpose. See
commits 9972db5c and 984df4ec for the reasoning. I suspect that could
perhaps be dropped but I am not sure.

>
> ...
>
>>>> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
>>>> index e7ab50a..aa4785f 100644
>>>> --- a/include/configs/zynq-common.h
>>>> +++ b/include/configs/zynq-common.h
>>>> @@ -319,7 +319,11 @@
>>>> #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1
>>>> #define CONFIG_SPL_LIBDISK_SUPPORT
>>>> #define CONFIG_SPL_FAT_SUPPORT
>>>> -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img"
>>>> +#ifdef CONFIG_OF_CONTROL
>>>> +# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot-dtb.img"
>>>> +#else
>>>> +# define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME "u-boot.img"
>>>> +#endif
>>>> #endif
>>>>
>>>> /* Disable dcache for SPL just for sure */
>>>>
>>>
>>> this was removed by Masahiro long time ago.
>>> kconfig: move CONFIG_OF_* to Kconfig
>>> sha1: 783e6a72b8278d59854ced41a4696c9a14abbb0b
>>
>> What was moved?
>
> Using different names for u-boot image.

OK. So are you saying my patch needs to change in this area?

Regards,
Simon