[SPAM] [PATCH v2 0/7] migrate u-boot-rockchip.bin to binman and generate an image for SPI

[Quentin Schulz]

Hi Xavier,

On 7/24/22 09:46, Xavier Drudis Ferran wrote:
> El Fri, Jul 22, 2022 at 01:34:58PM +0200, Quentin Schulz deia:
>> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>>
>> This migrates the generation of u-boot-rockchip.bin from Makefile to binman
>> completely. There is therefore no idbloader.img anymore as it is created on
>> the fly by binman.
>>
> 
> Thanks a lot.
> 
> I've tested this series and it worked for me and my Rock Pi 4B. That's
> a similar board, RK3399 too. Maybe it needs more refinements for other
> rockchip boards.
> 
> I haven't spent a lot of time testing yet, and I've tested it with
> two or three dozen patches on top of master, so not quite the real thing.
> 
> One thing that comes to mind is that master has CONFIG_ENV_IS_IN_MMC=y
> and I have it disabled and CONFIG_ENV_IS_NOWHERE=y. So maybe we should
> check in dtsi whether we need binman to call mkenvimage (and
> scripts/get_default_envs.sh or from what source?) and put the output
> at CONFIG_ENV_OFFSET=0x3F8000 for the SD image.
> 

It's not currently done for u-boot-rockchip.bin in upstream master, so 
it's not part of this patch series.

You'd need a new binman entry I assume for calling mkenvimage.

It's not a super safe assumption that CONFIG_ENV_OFFSET will be used for 
declaring where the environment is stored. E.g., CONFIG_ENV_OFFSET for 
Puma declares where the env is located on SPI-NOR, however for MMC 
devices, it is specified with u-boot,mmc-env-offset DT property (though, 
if it is not defined, it defaults to CONFIG_ENV_OFFSET, c.f. env/mmc.c). 
But maybe the same comment as I did for CONFIG_SYS_SPI_U_BOOT_OFFS would 
be enough? e.g. states that this should be in sync with the DT property 
if there's one for the board in question. See: 
https://lore.kernel.org/u-boot/20220722160655.3904213-13-foss+uboot@0leil.net/ 
for what I needed to do for Puma to get u-boot-rockchip-spi.bin support.

But that overall seems like a reasonable feature to add.

> Or maybe we should just disable CONFIG_ENV_IS_IN_MMC in the board ? (I
> think it conflicts with trust settings anyway).
> 

Yes, anything but ENV_IS_NOWHERE depends on !CHAIN_OF_TRUST. So I guess 
a check on #ifndef CONFIG_CHAIN_OF_TRUST for that section in binman 
would do the trick?

> Or maybe just leave as it is and let env load non-fatally fail unless
> the user provides their own custom environment ?
> 

If there's no environment where U-Boot is looking for it, it'll not 
fatally fail right now. The next time you save the environment 
(saveenv), it'll be written in the correct location and read during next 
boot. Again, this patch series does not modify u-boot-rockchip.bin 
current behavior :)

> Now that we have Quentin's patches maybe we can remove the Makefile
> warning about CONFIG_USE_SPL_FIT_GENERATOR and move the task that
> arch/arm/mach-rockchip/make_fit_atf.py is doing to binman.

I wasn't aware of this entry, maybe it is actually possible. That'd be 
nice to have :)

> But I don't seem to find any dts using "split-elf"

There are some test dts files in tools/binman directory for split-elf 
though.

> I even wonder, am I right to assume make_fit_atf.py is deprecated and
> split-elf prefered ? Because split-elf seems to be unused and make_fit_atf.py
> seems to be default ???
>   

My understanding is that there's a will to replace make_fit_atf.py 
output generated by binman instead, if binman supports everything needed 
to do this migration, I don't know.

Cheers,
Quentin