[PATCH v2 2/3] arm: dts: rockchip: Separate rockchip-binman.dtsi

Quentin Schulz quentin.schulz at theobroma-systems.com
Thu Nov 3 11:02:53 CET 2022


Hi Jagan,

On 11/3/22 07:19, Jagan Teki wrote:
> rockchip-u-boot.dtsi has the FIT image for the final stage of
> binman image creation.
> 
> If the actual binman node is part of this dtsi then there are
> build issues to use optee as input to this final stage binman
> image since optee is built via another binman image creation
> unlike ATF built via tools like make_fit_atf.py.
> 
>     binman: Filename 'u-boot.itb' not found in input path
> 
> Fix this by separating binman FIT image in rockchip-binman.dtsi
> 

My understanding is that this is a work-around for something that should 
be implemented in binman instead (e.g. dependency between images). If 
i'm not mistaken, what you're suggesting is to not build 
u-boot-rockchip.bin for some platforms? IIRC the plan for this binary 
was that it would apply to all Rockchip platforms, and this patch makes 
this "promise" go away.

> rockchip-u-boot.dtsi: binman node
> rockchip-binman.dtsi: binman FIT image node
> 
> The inclusion of rockchip-binman.dtsi is always to be last in
> included files as it has a FIT image node for final image creation.
> 

You are not respecting this in your patch. Please update or remove this 
section if not required. (I assume you have this limitation because you 
use a binman phandle, meaning the node needs to be defined before).

Also, rockchip-u-boot.dtsi content is now literally:
/ {
	binman: binman {
		multiple-images;
	};
};

which is pretty much useless.

Since you want to work around your build issue, why just not include 
rockchip-u-boot.dtsi instead of moving part of it to another file 
without any added benefit (at least at first glance, I may be missing 
some context).

BTW, we were discussing some months ago on moving away from 
make_fit_atf.py to binman for all Rockchip platforms, c.f. the long 
discussion here: 
https://lore.kernel.org/u-boot/20220725172953.GD2029@begut/ So maybe we 
should just do this and that might fix the problem you're trying to 
work-around?

In any case, can you provide a bit more context on the failing platform(s)?

Cheers,
Quentin

> Sample example of optee used rk3288 looks like
>     b/arch/arm/dts/rk3288-u-boot.dtsi
>     #include "rockchip-u-boot.dtsi"
>     #include "rockchip-optee.dtsi"
>     #include "rockchip-binman.dtsi"
> 
> Signed-off-by: Jagan Teki <jagan at edgeble.ai>
> ---
> Changes for v2:
> - excluded rv1126
> 
>   arch/arm/dts/px30-u-boot.dtsi              |  1 +
>   arch/arm/dts/rk3036-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3066a-u-boot.dtsi           |  1 +
>   arch/arm/dts/rk3188-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk322x-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3288-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3308-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi |  1 +
>   arch/arm/dts/rk3328-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3368-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk3399-u-boot.dtsi            |  1 +
>   arch/arm/dts/rk356x-u-boot.dtsi            |  1 +
>   arch/arm/dts/rockchip-binman.dtsi          | 67 ++++++++++++++++++++++
>   arch/arm/dts/rockchip-u-boot.dtsi          | 61 --------------------
>   arch/arm/dts/rv1108-u-boot.dtsi            |  1 +
>   15 files changed, 80 insertions(+), 61 deletions(-)
>   create mode 100644 arch/arm/dts/rockchip-binman.dtsi
> 
> diff --git a/arch/arm/dts/px30-u-boot.dtsi b/arch/arm/dts/px30-u-boot.dtsi
> index 462eaf68f8..7cea48181c 100644
> --- a/arch/arm/dts/px30-u-boot.dtsi
> +++ b/arch/arm/dts/px30-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rk3036-u-boot.dtsi b/arch/arm/dts/rk3036-u-boot.dtsi
> index 41ac054b81..a49526c63a 100644
> --- a/arch/arm/dts/rk3036-u-boot.dtsi
> +++ b/arch/arm/dts/rk3036-u-boot.dtsi
> @@ -4,3 +4,4 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
> diff --git a/arch/arm/dts/rk3066a-u-boot.dtsi b/arch/arm/dts/rk3066a-u-boot.dtsi
> index bc6e609d02..a85f752477 100644
> --- a/arch/arm/dts/rk3066a-u-boot.dtsi
> +++ b/arch/arm/dts/rk3066a-u-boot.dtsi
> @@ -1,4 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   #include "rk3xxx-u-boot.dtsi"
> diff --git a/arch/arm/dts/rk3188-u-boot.dtsi b/arch/arm/dts/rk3188-u-boot.dtsi
> index 735776c16b..bc028a8c1b 100644
> --- a/arch/arm/dts/rk3188-u-boot.dtsi
> +++ b/arch/arm/dts/rk3188-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   #include "rk3xxx-u-boot.dtsi"
>   
>   &global_timer {
> diff --git a/arch/arm/dts/rk322x-u-boot.dtsi b/arch/arm/dts/rk322x-u-boot.dtsi
> index 79c41e481b..bd31c3bdc4 100644
> --- a/arch/arm/dts/rk322x-u-boot.dtsi
> +++ b/arch/arm/dts/rk322x-u-boot.dtsi
> @@ -1,6 +1,7 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	bus_intmem at 10080000 {
> diff --git a/arch/arm/dts/rk3288-u-boot.dtsi b/arch/arm/dts/rk3288-u-boot.dtsi
> index e411445ed6..a24d590e20 100644
> --- a/arch/arm/dts/rk3288-u-boot.dtsi
> +++ b/arch/arm/dts/rk3288-u-boot.dtsi
> @@ -5,6 +5,7 @@
>   
>   #include "rockchip-u-boot.dtsi"
>   #include "rockchip-optee.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rk3308-u-boot.dtsi b/arch/arm/dts/rk3308-u-boot.dtsi
> index ab5bfc2ce9..665582ddd3 100644
> --- a/arch/arm/dts/rk3308-u-boot.dtsi
> +++ b/arch/arm/dts/rk3308-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi b/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
> index 16c33735eb..cd5d43b3c4 100644
> --- a/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
> +++ b/arch/arm/dts/rk3326-odroid-go2-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	chosen {
> diff --git a/arch/arm/dts/rk3328-u-boot.dtsi b/arch/arm/dts/rk3328-u-boot.dtsi
> index d4a7540a92..407286b176 100644
> --- a/arch/arm/dts/rk3328-u-boot.dtsi
> +++ b/arch/arm/dts/rk3328-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rk3368-u-boot.dtsi b/arch/arm/dts/rk3368-u-boot.dtsi
> index 811d59ac34..c33604266a 100644
> --- a/arch/arm/dts/rk3368-u-boot.dtsi
> +++ b/arch/arm/dts/rk3368-u-boot.dtsi
> @@ -5,6 +5,7 @@
>   
>   #include <dt-bindings/memory/rk3368-dmc.h>
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	dmc: dmc at ff610000 {
> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
> index 3c1a15fe51..03a328b295 100644
> --- a/arch/arm/dts/rk3399-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
> @@ -5,6 +5,7 @@
>   #define USB_CLASS_HUB			9
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rk356x-u-boot.dtsi b/arch/arm/dts/rk356x-u-boot.dtsi
> index ccb8db0001..8265e1a0bd 100644
> --- a/arch/arm/dts/rk356x-u-boot.dtsi
> +++ b/arch/arm/dts/rk356x-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   / {
>   	aliases {
> diff --git a/arch/arm/dts/rockchip-binman.dtsi b/arch/arm/dts/rockchip-binman.dtsi
> new file mode 100644
> index 0000000000..8f7b3ee53b
> --- /dev/null
> +++ b/arch/arm/dts/rockchip-binman.dtsi
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Jagan Teki <jagan at amarulasolutions.com>
> + */
> +
> +#include <config.h>
> +
> +#ifdef CONFIG_SPL
> +&binman {
> +	simple-bin {
> +		filename = "u-boot-rockchip.bin";
> +		pad-byte = <0xff>;
> +
> +		mkimage {
> +			filename = "idbloader.img";
> +			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> +#ifdef CONFIG_TPL
> +			multiple-data-files;
> +
> +			u-boot-tpl {
> +			};
> +#endif
> +			u-boot-spl {
> +			};
> +		};
> +
> +#ifdef CONFIG_ARM64
> +		blob {
> +			filename = "u-boot.itb";
> +#else
> +		u-boot-img {
> +#endif
> +			offset = <CONFIG_SPL_PAD_TO>;
> +		};
> +	};
> +
> +#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
> +	simple-bin-spi {
> +		filename = "u-boot-rockchip-spi.bin";
> +		pad-byte = <0xff>;
> +
> +		mkimage {
> +			filename = "idbloader-spi.img";
> +			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> +#ifdef CONFIG_TPL
> +			multiple-data-files;
> +
> +			u-boot-tpl {
> +			};
> +#endif
> +			u-boot-spl {
> +			};
> +		};
> +
> +#ifdef CONFIG_ARM64
> +		blob {
> +			filename = "u-boot.itb";
> +#else
> +		u-boot-img {
> +#endif
> +			/* Sync with u-boot,spl-payload-offset if present */
> +			offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
> +		};
> +	};
> +#endif
> +};
> +#endif
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index 584f21eb5b..3d55553e44 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -10,64 +10,3 @@
>   		multiple-images;
>   	};
>   };
> -
> -#ifdef CONFIG_SPL
> -&binman {
> -	simple-bin {
> -		filename = "u-boot-rockchip.bin";
> -		pad-byte = <0xff>;
> -
> -		mkimage {
> -			filename = "idbloader.img";
> -			args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> -#ifdef CONFIG_TPL
> -			multiple-data-files;
> -
> -			u-boot-tpl {
> -			};
> -#endif
> -			u-boot-spl {
> -			};
> -		};
> -
> -#ifdef CONFIG_ARM64
> -		blob {
> -			filename = "u-boot.itb";
> -#else
> -		u-boot-img {
> -#endif
> -			offset = <CONFIG_SPL_PAD_TO>;
> -		};
> -	};
> -
> -#ifdef CONFIG_ROCKCHIP_SPI_IMAGE
> -	simple-bin-spi {
> -		filename = "u-boot-rockchip-spi.bin";
> -		pad-byte = <0xff>;
> -
> -		mkimage {
> -			filename = "idbloader-spi.img";
> -			args = "-n", CONFIG_SYS_SOC, "-T", "rkspi";
> -#ifdef CONFIG_TPL
> -			multiple-data-files;
> -
> -			u-boot-tpl {
> -			};
> -#endif
> -			u-boot-spl {
> -			};
> -		};
> -
> -#ifdef CONFIG_ARM64
> -		blob {
> -			filename = "u-boot.itb";
> -#else
> -		u-boot-img {
> -#endif
> -			/* Sync with u-boot,spl-payload-offset if present */
> -			offset = <CONFIG_SYS_SPI_U_BOOT_OFFS>;
> -		};
> -	};
> -#endif
> -};
> -#endif
> diff --git a/arch/arm/dts/rv1108-u-boot.dtsi b/arch/arm/dts/rv1108-u-boot.dtsi
> index 6a2098b8d4..e6cb7b2f24 100644
> --- a/arch/arm/dts/rv1108-u-boot.dtsi
> +++ b/arch/arm/dts/rv1108-u-boot.dtsi
> @@ -4,6 +4,7 @@
>    */
>   
>   #include "rockchip-u-boot.dtsi"
> +#include "rockchip-binman.dtsi"
>   
>   &grf {
>   	u-boot,dm-pre-reloc;


More information about the U-Boot mailing list