[Question] TI's u-boot.img is built twice

[Andrew Davis]

On 9/22/23 1:29 PM, Simon Glass wrote:
> Hi Andrew,
> 
> On Fri, 22 Sept 2023 at 12:14, Andrew Davis <afd at ti.com> wrote:
>>
>> On 9/22/23 11:40 AM, Simon Glass wrote:
>>> Hi Andrew,
>>>
>>> On Fri, 22 Sept 2023 at 10:35, Andrew Davis <afd at ti.com> wrote:
>>>>
>>>> On 9/22/23 10:01 AM, Simon Glass wrote:
>>>>> Hi Masahiro,
>>>>>
>>>>> On Thu, 21 Sept 2023 at 09:36, Masahiro Yamada <masahiroy at kernel.org> wrote:
>>>>>>
>>>>>> Hi.
>>>>>>
>>>>>> Since the TI platform migrated to binman,
>>>>>> u-boot.img is built twice.
>>>>>>
>>>>>> It is created by "mkimage -E",
>>>>>> then overwritten by binman.
>>>>>>
>>>>>>
>>>>>> So, the data are embedded in the FIT structure
>>>>>> instead of being appended.
>>>>>>
>>>>>> Is this intentional?
>>>>>>
>>>>>> To me, it looks weird.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> To confirm it, apply the following hack.
>>>>>>
>>>>>> Since u-boot.img is overwritten by binman,
>>>>>> copy it to u-boot.img.backup.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> diff --git a/Makefile b/Makefile
>>>>>> index 87f9fc786e..4cffa8a061 100644
>>>>>> --- a/Makefile
>>>>>> +++ b/Makefile
>>>>>> @@ -1112,6 +1112,7 @@ endef
>>>>>>     # Timestamp file to make sure that binman always runs
>>>>>>     .binman_stamp: $(INPUTS-y) FORCE
>>>>>>     ifeq ($(CONFIG_BINMAN),y)
>>>>>> +       cp u-boot.img u-boot.img.backup
>>>>>>            $(call if_changed,binman)
>>>>>>     endif
>>>>>>            @touch $@
>>>>>>
>>>>>>
>>>>>>
>>>>>> Then, build it for the main core.
>>>>>>
>>>>>>
>>>>>> make -j$(nproc) CROSS_COMPILE=aarch64-linux-gnu-
>>>>>>       am64x_evm_a53_defconfig all
>>>>>>       TEE=~/ref/OP-TEE/optee_os/out/arm-plat-k3/core/tee-raw.bin
>>>>>>       BL31=~/ref/trusted-firmware-a/build/k3/lite/release/bl31.bin
>>>>>>       BINMAN_INDIRS=~/ref/ti-linux-firmware
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Compare the two files.
>>>>>> Run fdtdump to see what happened to them.
>>>>>>
>>>>>>
>>>>>> $ diff -u u-boot.img  u-boot.img.backup
>>>>>> Binary files u-boot.img and u-boot.img.backup differ
>>>>>>
>>>>>>
>>>>>> $ fdtdump u-boot.img
>>>>>>        => u-boot and dt are embedded.
>>>>>>
>>>>>> $ fdtdump u-boot.img.backup
>>>>>>        => u-boot and dt are appended after the
>>>>>>           FIT structure
>>>>>
>>>>> That seems like a bug to me...at some point we might consider building
>>>>> u-boot.img with Binman, but for now it is built by the Makefile.
>>>>>
>>>>
>>>> This is not true for K3 as we now use Binman for u-boot.img generation
>>>> (we need this as we have a signing step injected at this point).
>>>
>>> No, no.
>>>
>>> You must not mess with the outputs of Makefile - create a new file
>>> with what you need, e.g. u-boot-ti.img
>>>
>>> While we could use Binman to produce the .img we are quite a way from
>>> doing that as not that many boards have been converted to binman.
>>>
>>
>> Too late, we already use Binman to generate u-boot.img on K3, why
>> would we want to go back to using Makefile?
> 
> I don't mean that, I mean give it another name. The u-boot.img file is
> intended to be a certain format, and it seems this is being changed.
> 

Well we need u-boot.img, and it needs to be called that, and
it needs to be the one generated by Binman (the one generated by
Makefile can only boot on GP devices, but not HS-SE).

The format produced by Makefile and Binman will be identical after
we fix Binman to put the data at the end. At which point why not
drop the Makefile made one?

Andrew

> Really, this is a problem with binman, as it should have refused to
> creating u-boot.img...?
> 
>>
>> Let's go with what you suggest below and add a Kconfig that can
>> be used for boards that have already switched to binman that disables
>> the Makefile generator for u-boot.img.
>>
>> Once all boards have switched to Binman, then we could refactor
>> the implementations into something more common.
> 
> That seems OK to me.
> 
> Regards,
> Simon
> 
>>
>> Andrew
>>
>>>>
>>>> So as Masahiro suggested in the other branch of this thread, we need
>>>> to disable the Makefile generation of u-boot.img for K3.
>>>>
>>>> Neha,
>>>>
>>>> Can you take this action? I assume you can add it as part of your
>>>> work in making the data external again, which would make the u-boot.img
>>>> identical to before and hence Makefile version could be disabled at
>>>> the same time/series.
>>>
>>> If you are suggesting that we slowly migrate boards away from
>>> generating the .img using Makefile...perhaps that makes sense? I'm not
>>> really sure. But in that case we need another Kconfig option.
>>>
>>> Regards,
>>> Simon