[PATCH 3/3] rockchip: avoid rebuilding most binaries for u-boot-rockchip-spi.bin

[Quentin Schulz]

Hi Jonas,

On 4/8/25 7:48 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2025-02-11 12:43, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 2/11/25 12:35 PM, Jonas Karlman wrote:
>>> Hi Quentin,
>>>
>>> On 2025-02-11 11:29, Quentin Schulz wrote:
>>>> From: Quentin Schulz <quentin.schulz at cherry.de>
>>>>
>>>> Essentially the only differences between u-boot-rockchip.bin and
>>>> u-boot-rockchip-spi.bin are the formatting of idbloader.img which is
>>>> handled by mkimage (via -T rkspi/rksd) and the offset at which U-Boot
>>>> proper is flashed, the content of the binaries are identical otherwise.
>>>>
>>>> This fixes some issues[1] where binman tries to find the symbols defined
>>>> in the proper binary to install them in an xPL binary. However, because
>>>> we use the binary for proper (on Aarch64) generated in simple-bin image
>>>> node and not simple-bin-spi image node, binman doesn't have access to
>>>> that symbol anymore. Therefore, let's depend entirely on binaries built
>>>> by simple-bin in simple-bin-spi so those issues do not arise anymore as
>>>> nothing is compiled essentially, just assembled.
>>>>
>>>> [1] https://lore.kernel.org/u-boot/20250129132529.807031-3-naoki@radxa.com/
>>>> Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
>>>> ---
>>>>    arch/arm/dts/rockchip-u-boot.dtsi | 10 ++++++++++
>>>>    1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> index c8c928c7e5089db3a2239f2564e6dee1d82aad95..2e8a3bd09e49ed73a8cea3fadfb6d2b592082365 100644
>>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>>> @@ -187,18 +187,28 @@
>>>>    			};
>>>>    #elif defined(CONFIG_TPL)
>>>>    			u-boot-tpl {
>>>> +				type = "blob";
>>>> +				/* sync with /binman/simple-bin/mkimage/u-boot-tpl */
>>>> +				filename = "tpl/u-boot-tpl.bin";
>>>>    			};
>>>>    #endif
>>>>    			u-boot-spl {
>>>> +				type = "blob";
>>>> +				/* sync with /binman/simple-bin/mkimage/u-boot-spl */
>>>> +				filename = "spl/u-boot-spl.bin";
>>>>    			};
>>>>    		};
>>>>    
>>>>    #if defined(CONFIG_ARM64) || defined(CONFIG_SPL_OPTEE_IMAGE)
>>>>    		fit {
>>>>    			type = "blob";
>>>> +			/* sync with /binman/simple-bin/fit */
>>>>    			filename = "u-boot.itb";
>>>>    #else
>>>>    		u-boot-img {
>>>> +			type = "blob";
>>>> +			/* sync with /binman/simple-bin/u-boot-img */
>>>> +			filename = "u-boot.img";
>>>>    #endif
>>>>    			/* Sync with u-boot,spl-payload-offset if present */
>>>>    			offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
>>>>
>>>
>>> I think changing to using a template may suit binman better, however it
>>> will be at the expense of having to re-generate the FIT, not sure what
>>> option is best.
>>>
>>
>> Unnecessarily rebuilding images is not great for build times :) If I'm
>> not mistaken, binman still compiles sequentially so doubly not great.
>>
>> At the same time, this is relying on undefined behavior (according to
>> binman spec; i.e. using a binary from another image node) but we've been
>> doing this for a few years now and it seems to be doing ok so far?
>>
>>> It would be great if Simon can split out the rockchip-u-boot.dtsi
>>> improvement commits in a separate series as I suggested in [2].
>>>
>>
>> That's fine but we need something soon, this is already preventing
>> people from contributing stuff for their boards. I don't mind either
>> (haven't checked the template stuff so far though), just no need to wait
>> too long for the perfect implementation. Also this can be merged as an
>> intermediary fix and templating done afterwards.
> 
> The fit-template patch (and others) was extracted and are included in
> the "rockchip: binman: Use a template for FIT and other improvements" v4
> series, see [3].
> 
> I suggest we skip this last patch in this series, and instead apply the
> change to use a template from that series. If not, a v5 of that series
> would just end up revering the changes made in this patch any way.
> 
> The added time for building two (or three with ram boot) times seem to
> be negligible on my build machine.
> 

Yes but then there's no guarantee it's the same binary that is part of 
the eMMC or the SPI image since they are built separately.

It wouldn't be a revert but a rebase for the migration to the fit 
template which absorbs the changes made in that patch. I personally 
don't care.

Note that patches 1 and 2 should be merged anyway, they aren't impacted 
by that decision.

Cheers,
Quentin