Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI

[Simon Glass]

Hi Quentin,

On Wed, 27 Jul 2022 at 04:34, Quentin Schulz
<quentin.schulz at theobroma-systems.com> wrote:
>
> Hi Simon,
>
> On 7/26/22 21:58, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Tue, 26 Jul 2022 at 03:08, Quentin Schulz
> > <quentin.schulz at theobroma-systems.com> wrote:
> >>
> >> Hi Xavier,
> >>
> >> On 7/25/22 19:33, Xavier Drudis Ferran wrote:
> >>> El Mon, Jul 25, 2022 at 07:29:53PM +0200, Xavier Drudis Ferran deia:
> >>>>
> >>>> I copy here the rockchip-u-boot.dtsi file and then 2 patches on top of yours.
> >>>>
> >>>
> >>> Sorry I copied a dirty version that din't work. The patches were correct, the dtsi wasn't.
> >>>
> >
> > [..]
> >
> >>
> >>>> +                            };
> >>>> +                    };
> >>>> +            };
> >>>> +    };
> >>>>       simple-bin {
> >>>>               filename = "u-boot-rockchip.bin";
> >>>>               pad-byte = <0xff>;
> >>>> @@ -62,6 +131,7 @@
> >>>>    #ifdef CONFIG_ARM64
> >>>>               blob {
> >>>>                       filename = "u-boot.itb";
> >>>> +
> >>
> >> This is unfortunately not possible since binman parallelizes the build
> >> of images in the binman DT node. This means there is no guarantee the
> >> u-boot.itb file is generated before this section is worked on by binman.
> >> Or maybe I misunderstood the docs.
> >
> > You are correct, but this is something that has bothered me.
> >
> > At the moment we handle this by embedding the definition for one file
> > into the one that uses it. This is on the theory that there is no need
> > to actually write the embedded file, since it is a temporary file. In
> > fact binman does generate temporary files though.
> >
> > Is that good enough? If not I'd like to understand the need better,
> > before we make any changes.
> >
>
> For Rockchip, with the patch series from
> https://lore.kernel.org/u-boot/20220722113505.3875669-1-foss+uboot@0leil.net/,
> we now have two binaries:
> u-boot-rockchip.bin and u-boot-rockchip-spi.bin
>
> Both share the exact same u-boot.itb file (though at a different offset
> in the storage medium) embedded.
>
> This u-boot.itb is currently externally created via the Makefile +
> make_fit_atf.py prior to binman being called. This allows us to have a
> blob { filename = "u-boot.itb"; } in the binman DT node and that's
> enough for it to work fine.
>
> There's a desire to get rid of make_fit_atf.py in favor of binman. This
> means that we need to create this file in binman now.
>
> There's been some resistance to making binman not create idbloader.img
> image in the patch series mentioned above (it is *technically* created
> by binman but only in temporary files and then embedded in
> u-boot-rockchip*.bin). I assume the same resistance will be met for
> u-boot.itb.
>
> With how binman works currently and since we have u-boot.itb content
> embedded in at least two different images created by binman (might be
> even more once there's USB/NAND support?), we'd need to have the fit
> device tree node appear thrice (or more). One for u-boot.itb creation
> (because of people not wanting it disappear from binaries generated by
> U-Boot), one for embedding into u-boot-rockchip.bin and one for
> embedding into u-boot-rockchip-spi.bin.
> This increases the risk of not updating all fit device tree nodes at once.
> This is suboptimal in terms of build time because the image will
> effectively be created thrice (or more).
> This is also not the best for easy check of reproducibility since the
> content of the fit device tree node is technically not the same as
> u-boot.itb file (because it is the result of a different image built by
> binman).
>
> So I think the minimal implementation would be to allow to define an
> image/section (u-boot.itb) and to "#include" it inline in another
> section (where blob { filename = "u-boot.itb"; } currently is for
> u-boot-rockchip.bin and u-boot-rockchip-spi.bin). From reading the docs,
> I expected collection entry to be exactly this? But I couldn't make it work.
>
> Ideally, allowing binman to define a dependency from one image on
> another would mean we could still use the blob image/section, but
> safely, because the dependency is explicit so binman will know which
> image to build first. Phandles is what we're after on the Device Tree
> side I would assume. We could have something like: blob { image =
> <&u-boot-itb>; } for example. No idea how binman would create this
> dependency tree though :)
>
> Straight from my brain, lemme know if something needs to be clarified or
> rephrased.

Collections should allow this. Can you try running with threading
disabled (-T0)?

Another thought is that we could provide a way for binman to write a
section to a file in an official way, i.e. give it a filename
property.

Regards,
Simon