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

Stephen Warren
2016-01-25T21:26:31Z

On 01/25/2016 02:18 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 25 January 2016 at 14:08, Stephen Warren <swarren@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.