On 16/05/2022 14:07, Andrew Abbott wrote: > Currently, building for Rockchip targets produces: > > - idbloader.img > - rksd-formatted TPL with SPL appended; or > - rksd-formatted SPL > - u-boot.itb > - U-Boot Proper FIT image > - u-boot-rockchip.bin > - idbloader.img + u-boot.itb, padded the correct amount for SD/MMC > usage. > > For RK3399 targets: > > - u-boot.rom > - SPI image specific to the bob Chromebook target (see > c4cea2bbf995764f325a907061c22ecd6768cf7b).
I'm not sure we need anything special for the rk3399 chromebooks. I think it's meant to be your 'u-boot-rockchip-spi.bin', but is just incomplete (and partially broken, e.g. it should use u-boot.itb). I'll try to test things on my chromebook_kevin later on, but probably not before you can send a v3... > This commit adds binman definitions to produce these images: > > - idbloader.img > - rksd-formatted [TPL + ] SPL, as before. Do we need the 'idbloader.img' as a build output, assuming we have a working 'u-boot-rockchip.bin'? I'm asking because Simon was trying to drop it in a similar patch [1]. [1] rockchip: Support building the all output files in binman https://lore.kernel.org/u-boot/20220223230040.159317-24-...@chromium.org/ > - u-boot-rockchip.bin > - [TPL + ] SPL all rksd-formatted + u-boot.itb padded for SD/MMC > usage, as before. > - u-boot-rockchip-spi.bin > - [TPL + ] SPL all rkspi-formatted + u-boot.itb padded for SPI > usage. > > This commit also generalizes the CONFIG_ROCKCHIP_SPI_IMAGE config > setting - it now means to generate a generic SPI flash image, in > addition to the generic SD/MMC image. With what I said above, I think you should rename this to 'u-boot.rom' and remove the definitions in {rk3288,rk3399}-u-boot.dtsi. > > Signed-off-by: Andrew Abbott <and...@mirx.dev> > --- > Question: Does this break/not play nicely with rockchip-optee > generation? It creates u-boot.itb for rk3288 targets. That would need to > run before what I've implemented here? The 'u-boot.itb' entries you add here are guarded with CONFIG_ARM64, so don't apply to RK3288. I think that's fine until we can convert these 'u-boot.itb's to be completely binman-built as well. OTOH, things appear work fine if I: - Replace CONFIG_ARM64 with CONFIG_SPL_FIT below - Include rockchip-optee before rockchip-u-boot in rk3288-u-boot.dtsi - Move the empty binman node to rk3288-u-boot.dtsi above both includes at least, until the patch that forces Makefile to build 'u-boot.itb'. > > Changes in v2: > - Revert u-boot-rockchip-sdmmc.bin name to u-boot-rockchip.bin, to > keep the name the same as before. > - Fix whitespace issues. > > arch/arm/dts/rockchip-u-boot.dtsi | 53 +++++++++++++++++++++++++++++-- > arch/arm/mach-rockchip/Kconfig | 7 ++-- > 2 files changed, 53 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/dts/rockchip-u-boot.dtsi > b/arch/arm/dts/rockchip-u-boot.dtsi > index eae3ee715d..9354b0f5a7 100644 > --- a/arch/arm/dts/rockchip-u-boot.dtsi > +++ b/arch/arm/dts/rockchip-u-boot.dtsi > @@ -13,17 +13,64 @@ > > #ifdef CONFIG_SPL > &binman { > - simple-bin { > + sdmmc-idbloader { > + filename = "idbloader.img"; > + > + mkimage { > + args = "-n", CONFIG_SYS_SOC, "-T", "rksd"; > + > +#ifdef CONFIG_TPL > + u-boot-tpl {}; > +#endif > + u-boot-spl {}; The closing braces should be on their own line, more instances below as well. > + }; > + }; > + > + sdmmc-image { > filename = "u-boot-rockchip.bin"; > pad-byte = <0xff>; > > - blob { > + idbloader { > filename = "idbloader.img"; > + type = "blob"; > }; > > - u-boot-img { > +#ifdef CONFIG_ARM64 Probably "#if defined(CONFIG_SPL_FIT) && defined(CONFIG_ARM64)" is better, like in Simon's patch. Likewise in the SPI image below. > + u-boot-fit { > + filename = "u-boot.itb"; > + type = "blob"; > offset = <CONFIG_SPL_PAD_TO>; > }; > +#else > + u-boot-img {}; I think this still needs the "offset = <CONFIG_SPL_PAD_TO>;" which shouldn't be removed. > +#endif > }; > }; > + > +#ifdef CONFIG_ROCKCHIP_SPI_IMAGE > +&binman { > + spi-image { > + filename = "u-boot-rockchip-spi.bin"; > + pad-byte = <0xff>; > + > + mkimage { > + args = "-n", CONFIG_SYS_SOC, "-T", "rkspi"; > + > +#ifdef CONFIG_TPL > + u-boot-tpl {}; > +#endif > + u-boot-spl {}; > + }; > + > +#ifdef CONFIG_ARM64 > + blob { This should have the same name & type as the one in the other image. > + filename = "u-boot.itb"; > + offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>; > + }; > +#else > + u-boot-img {}; I think this also needs "offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;". > #endif > + }; > +}; Also consider adding a 'fdtmap' entry to both SPI and SD/MMC images, it's useful for some interactive binman features. > +#endif // CONFIG_ROCKCHIP_SPI_IMAGE > +#endif // CONFIG_SPL > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig > index 18aff5480b..7149b9a530 100644 > --- a/arch/arm/mach-rockchip/Kconfig > +++ b/arch/arm/mach-rockchip/Kconfig > @@ -415,12 +415,11 @@ config SPL_MMC > > config ROCKCHIP_SPI_IMAGE > bool "Build a SPI image for rockchip" > - depends on HAS_ROM > help > Some Rockchip SoCs support booting from SPI flash. Enable this > - option to produce a 4MB SPI-flash image (called u-boot.rom) > - containing U-Boot. The image is built by binman. U-Boot sits near > - the start of the image. > + option to produce an SPI-flash image (called u-boot-rockchip-spi.bin) > + containing TPL (if enabled) and SPL, and U-Boot proper at the offset > + CONFIG_SYS_SPI_U_BOOT_OFFS. The image is built by binman. While renaming 'u-boot-rockchip-spi.bin' to 'u-boot.rom' you should drop these Kconfig changes as well, and select HAS_ROM wherever you select ROCKCHIP_SPI_IMAGE (i.e. in the last patch). > > config LNX_KRNL_IMG_TEXT_OFFSET_BASE > default SYS_TEXT_BASE
