Hi Simon,
On 7/31/22 03:27, Simon Glass wrote:
Hi Quentin,
On Wed, 27 Jul 2022 at 04:34, Quentin Schulz
<quentin.sch...@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.sch...@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://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_u-2Dboot_20220722113505.3875669-2D1-2Dfoss-2Buboot-400leil.net_&d=DwIBaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=OevHoqt3vOXsWmXfEPFnvZ2KSNns-ZKBiiZBUovrynOXVCUZtFfK9jsk3C1r4Y_J&s=nXnNRN3hfa_mkkt_suH6wLGbwj06I7HmQKcagZ6JPE0&e=
,
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)?
Do they?
What (I think) I want is basically the following:
&binman {
multiple-images;
u_boot_itb: u-boot-itb {
fit {};
};
simple-bin {
[...]
collection {
content = <&u_boot_itb>;
};
};
simple-bin-spi {
[...]
collection {
content = <&u_boot_itb>;
};
};
};
or I guess something like:
&binman {
multiple-images;
u-boot-itb {
filename = "u-boot.itb";
collection {
content = <&u_boot_itb>;
};
};
simple-bin {
[...]
u_boot_itb: fit {};
};
simple-bin-spi {
[...]
collection {
content = <&u_boot_itb>;
};
};
};
However, I tried with:
// SPDX-License-Identifier: GPL-2.0+
/dts-v1/;
/ {
#address-cells = <1>;
#size-cells = <1>;
binman {
multiple-images;
text2: foo {
text1: text {
text = "bl31.bin";
};
};
bar {
collection {
content = <&text2>;
};
};
};
};
and:
// SPDX-License-Identifier: GPL-2.0+
/dts-v1/;
/ {
#address-cells = <1>;
#size-cells = <1>;
binman {
multiple-images;
text2: foo {
text1: text {
text = "bl31.bin";
};
};
bar {
collection {
content = <&text1>;
};
};
};
};
but tools/binman/binman build -d test.dts returns for both:
binman: Node '/binman/bar/collection': Cannot find entry for node 'text'
(or 'foo')
I guess the issue is that this collection would be crossing the image
boundary and this is not supported?
I'm not sure to really understand the point of the collection section? I
would assume wanting to get multiple times the same entries in the same
image should be pretty rare?
I guess I could simply have a huge constant defining the u-boot-itb fit
node and add it to the binman DT node in the correct places to avoid
having to maintain multiple copies of the DT node, but it's still
inefficient IMO since the image would be created as many times as it's
used. Ideally it'd be a dependency on an image being created and one
uses blob-ext or something similar to use this newly generated image. I
don't know, throwing ideas right now.
Am I completely lost or does what I want to do make some kind of sense?
Cheers,
Quentin
