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

Simon Glass sjg at chromium.org
Wed Sep 2 04:48:56 CEST 2015


Hi Michal,

On 1 September 2015 at 07:12, Michal Simek <monstr at monstr.eu> wrote:
> Hi Simon,
>
> On 09/01/2015 01:12 AM, Simon Glass wrote:
>> Hi Michal,
>>
>> On 31 August 2015 at 08:07, Michal Simek <monstr at 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 at 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 related problem is that in your commit message you are using correct
> line for bif if you use FSBL. But the problem is with the all software
> around it and users around.
> u-boot-dtb.bin is binary file and you need to keep record where to load
> it. If you have elf file you have entry point there.
> It means that this change breaks all current users which is something
> what is pretty problematic.
> I can live without disabling OF_EMBED because I will use it in xilinx
> tree but it is just pain which you should be aware of.
>
> And still I can't see the problem to have OF_EMBED enabled for these
> zynq boards because all of them are not recommended for production.

Mainline boards should follow best practice, since downstream boards
(and potentially other mainline boards) always copy from them.

It seems like this is painful for Zynq....

>
>>
>>>
>>> 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.
>
> The problem is more with using bin formats which lack information about
> entry point.
>
> You are using this line and it is completely valid.
> [load = 0x04000000,startup=0x04000000]/path/to/u-boot.bin
> but most of users are just using
> u-boot.elf
> and they don't need to care about addresses at all.
>
> I am fully aware that this is around for a long time but it will be the
> best to have something like u-boot-dtb.elf or u-boot.elf with DTB
> inside. As we discussed above u-boot.elf with DTB inside is when
> OF_EMBED is enabled.

I see. Well adding u-boot-dtb.elf seems reasonable to me although I'm
not sure how you would do it in practice. It seems a bit tricky to
tack a binary file onto an ELF. With the efi-x86 target it does this
by putting a u-boot-dtb.bin payload inside an ELF image. I'm not sure
if that would work for you.

>
>
>>>
>>> ...
>>>
>>>>>> 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?
>
> What I am saying is that we patched config to use just one name and
> currently you have started to use two again.
> OF_CONTROL will be enabled by default that's why only one name should be
> enough.
> Or even better just move this name to Kconfig and setup one default value.

OK so really we should just change it to u-boot-dtb.img.

Regards,
Simon


More information about the U-Boot mailing list