[U-Boot] [PATCH 1/7] tegra: Drop generation of -nodtb file with OF_CONTROL

[Simon Glass]

Hi Stephen,

On 25 January 2016 at 14:26, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 01/25/2016 02:18 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 25 January 2016 at 14:08, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>
>>>
>>> On 01/25/2016 01:30 PM, Simon Glass wrote:
>>>>
>>>>
>>>> It seems clear that the intent is to generate u-boot-nodtb-tegra.bin
>>>> only
>>>> when OF_CONTROL is not set. But due to the way the rules are set up,
>>>> this
>>>> file is always generated. Fix this.
>>>
>>>
>>>
>>> Nak, this file is used by our flashing tools, so needs to be kept around.
>>>
>>> (As background, we take the separate u-boot-nodtb-tegra.bin and
>>> u-boot.dtb, modify u-boot.dtb to inject some changes to the environment such
>>> as over-writing bootcmd with flashing instructions etc., and then blend the
>>> two back together for download into RAM and subsequent execution).
>>
>>
>> OK. well in that case It think the code at the top is wrong:
>>
>> # enable combined SPL/u-boot/dtb rules for tegra
>> ifneq ($(CONFIG_TEGRA),)
>> ifeq ($(CONFIG_SPL),y)
>> ifeq ($(CONFIG_OF_SEPARATE),y)
>> ALL-y += u-boot-dtb-tegra.bin
>> else
>> ALL-y += u-boot-nodtb-tegra.bin
>> endif
>> endif
>> endif
>>
>> It should be this, right?
>>
>> # enable combined SPL/u-boot/dtb rules for tegra
>> ifneq ($(CONFIG_TEGRA),)
>> ifeq ($(CONFIG_SPL),y)
>> ALL-y += u-boot-dtb-tegra.bin u-boot-nodtb-tegra.bin
>> endif
>> endif
>
>
> That change looks almost fine, yes. I'd phrase it as follows:
>
> ifneq ($(CONFIG_TEGRA),)
> ifeq ($(CONFIG_SPL),y)
> ALL-y += u-boot-nodtb-tegra.bin
> ifeq ($(CONFIG_OF_SEPARATE),y)
> ALL-y += u-boot-dtb-tegra.bin
> endif
> endif
> endif
>
> ... so that u-boot-dtb-tegra.bin is only built when there's a DTB in use.
>
> (or we could drop the most nested ifdef as you wrote, if we assume the value
> of CONFIG_OF_SEPARATE on Tegra)
>
> Everything works and is safe right now since later Makefile says:
>
> ifeq ($(CONFIG_OF_SEPARATE),y)
> u-boot-dtb-tegra.bin: u-boot-nodtb-tegra.bin dts/dt.dtb FORCE
>         $(call if_changed,cat)
> endif
>
> So the if statement you're quoting is ensuring that the "final" or "most
> complete" file is built, and then the dependency chain ends up ensuring that
> all the others are built. As you say though, it would be good if ALL-y
> listed everything that users expect to use.

OK thanks for the info.

Regards,
Simon