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

[Simon Glass]

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.

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