Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI
Quentin Schulz
quentin.schulz at theobroma-systems.com
Mon Aug 1 19:04:54 CEST 2022
Hi Simon,
On 7/31/22 03:27, Simon Glass wrote:
> 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://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
More information about the U-Boot
mailing list