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

[Simon Glass]

Hi Andrew,

On Fri, 22 Sept 2023 at 12:44, Andrew Davis <afd at ti.com> wrote:
>
> 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?

I think you understand my concern, which is really around overwriting
an existing file with Binman. So long as you can resolve that in a
deterministic way (i.e. Kconfig) then I am happy.

Regards,
Simon

>
> 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