Re: [PATCH v2 23/25] rockchip: Support building the all output files in binman

Peter Geis
2022-03-02T22:16:26Z

On Wed, Feb 23, 2022 at 6:04 PM Simon Glass <sjg@chromium.org> wrote:
>

Good Evening,

> Add the required binman images to replace the Makefile rules which are
> currently used. This includes subsuming:
>
> - tpl/u-boot-tpl-rockchip.bin if TPL is enabled
> - idbloader.img if either or both of SPL and TPL are enabled
> - u-boot.itb if SPL_FIT is enabled
> - u-boot-rockchip.bin if SPL is used, either using u-boot.itb when
> SPL_FIT is enabled or u-boot.img when it isn't
>
> Note that the intermediate files are dropped with binman, since it
> producing everything in one pass. This means that
> tpl/u-boot-tpl-rockchip.bin is not created, for example.

I love this series, and look forward to it working.
I've tested this with rk3399, unfortunately it does not produce a
functional u-boot.itb.
I originally thought it was, but then I realized it was placing it at
an incorrect location.

>
> Note that for some 32-bit rk3288 boards, rockchip-optee.dtsi is included.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Rename op-tee to tee-os
> - Drop use of .itb2
>
> arch/arm/dts/rockchip-u-boot.dtsi | 84 ++++++++++++++++++++++++++++++-
> 1 file changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index eae3ee715d..64e4466489 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -17,13 +17,93 @@
> filename = "u-boot-rockchip.bin";
> pad-byte = <0xff>;
>
> - blob {
> - filename = "idbloader.img";
> +#ifdef CONFIG_TPL
> + mkimage {
> + args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> +
> + u-boot-tpl {
> + };
> + };
> +
> + u-boot-spl {
> };
> +#elif defined(CONFIG_SPL) /* SPL only */
> + mkimage {
> + args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> +
> + u-boot-spl {
> + };
> + };
> +#endif
> +#if defined(CONFIG_SPL_FIT) && defined(CONFIG_ARM64)
> + fit: fit {
> + description = "FIT image for U-Boot with bl31 (TF-A)";
> + #address-cells = <1>;
> + fit,fdt-list = "of-list";
> + fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;

You need:
offset = <CONFIG_SPL_PAD_TO>;
here or the image is located at the wrong location.

> + images {
> + u-boot {
> + description = "U-Boot (64-bit)";
> + type = "standalone";
> + os = "U-Boot";
> + arch = "arm64";
> + compression = "none";
> + load = <CONFIG_SYS_TEXT_BASE>;
> + u-boot-nodtb {
> + };
> + };
>
> + @atf-SEQ {
> + fit,operation = "split-elf";
> + description = "ARM Trusted Firmware";
> + type = "firmware";
> + arch = "arm64";
> + os = "arm-trusted-firmware";
> + compression = "none";
> + fit,load;
> + fit,entry;
> + fit,data;
> +
> + atf-bl31 {
> + };
> + };
> + @tee-SEQ {
> + fit,operation = "split-elf";
> + description = "TEE";
> + type = "tee";
> + arch = "arm64";
> + os = "tee";
> + compression = "none";
> + fit,load;
> + fit,entry;
> + fit,data;
> +
> + tee-os {
> + };
> + };
> +
> + @fdt-SEQ {
> + description = "fdt-NAME";
> + compression = "none";
> + type = "flat_dt";
> + };
> + };
> +
> + configurations {
> + default = "@config-DEFAULT-SEQ";
> + @config-SEQ {
> + description = "NAME.dtb";
> + fdt = "fdt-SEQ";
> + firmware = "u-boot";
> + fit,loadables;
> + };
> + };
> + };
> +#else
> u-boot-img {
> offset = <CONFIG_SPL_PAD_TO>;
> };
> +#endif /* CONFIG_ARM64 */
> };
> };
> #endif
> --
> 2.35.1.574.g5d30c73bfb-goog
>

The image produced is not functional however, because it swaps the
firmware node with the loadable-1 node.
Working image:
Configuration 0 (config_1)
Description: rk3399-pinephone-pro.dtb
Kernel: unavailable
Firmware: atf_1
FDT: fdt_1
Loadables: uboot
atf_2
atf_3

Non working image produced from this:
Configuration 0 (config-1)
Description: rk3399-rockpro64.dtb
Kernel: unavailable
Firmware: u-boot
FDT: fdt-1
Loadables: atf-1
atf-2
atf-3

Also, it still doesn't support passing two separate files to mkimage
at the same time, required for proper support of rk33xx_SPI generation
and for rk35xx in general.
Please see my standalone patch for what I hacked together to get that working:
http://patchwork.ozlabs.org/project/uboot/patch/20220301024826.1228290-1-pgwipeout@gmail.com/
Note: those images are not functional either due to this issue.

A wishlist item would be for it to produce the original idbloader.img
and u-boot.itb files, at least for now.

Thanks!
Very Respectfully,
Peter Geis