Re: [PATCH v4 02/10] rockchip: binman: Factor out arch and compression

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Jonas,

On 4/9/25 1:56 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2025-04-09 11:28, Quentin Schulz wrote:
>> Hi Jonas, Simon,
>>
>> On 3/29/25 4:06 PM, Jonas Karlman wrote:
>>> From: Simon Glass <sjg@chromium.org>
>>>
>>> Declare arch and compression at the top of the file to avoid needing
>>> ifdefs in every usage.
>>>
>>> Add a few comments to help with the remaining #ifdefs.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> Changes in v4:
>>> - Split from "VBE serial part H: Implement VBE on Rockchip RK3399"
>>> ---
>>>    arch/arm/dts/rockchip-u-boot.dtsi | 44 +++++++++++++++----------------
>>>    1 file changed, 22 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
>>> index e9ed1d4b5738..2b01dc660562 100644
>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
>>> @@ -5,6 +5,20 @@
>>>    
>>>    #include <config.h>
>>>    
>>> +#ifdef CONFIG_ARM64
>>> +#define ARCH	"arm64"
>>> +#else
>>> +#define ARCH	"arm"
>>> +#endif
>>> +
>>
>> I would refrain from using ARCH here as it's something we already use to
>> specify the architecture to build (e.g. make ARCH=arm64 CROSS_COMPILE=...).
>>

Actually you don't need (or even shouldn't?) provide ARCH to the make 
command, got confused because I'm compiling the kernel right now :)

>> Maybe FIT_ARCH?
> 
> sunxi-u-boot.dtsi is also using ARCH so I figured it was also safe here,
> we can change to FIT_ARCH for a v5.
> 

Indeed. I would prefer something not clashing with other 
environment/make variables :)

[...]

>>> @@ -84,7 +84,7 @@
>>>    					fit,operation = "split-elf";
>>>    					description = "ARM Trusted Firmware";
>>>    					type = "firmware";
>>> -					arch = "arm64";
>>> +					arch = ARCH;
>>>    					os = "arm-trusted-firmware";
>>>    					compression = "none";
>>>    					fit,load;
>>> @@ -103,7 +103,7 @@
>>>    					fit,operation = "split-elf";
>>>    					description = "TEE";
>>>    					type = "tee";
>>> -					arch = "arm64";
>>> +					arch = ARCH;
>>>    					os = "tee";
>>>    					compression = "none";
>>>    					fit,load;
>>> @@ -119,11 +119,11 @@
>>>    					};
>>>    #endif
>>>    				};
>>> -#else
>>> +#else /* !CONFIG_ARM64 */
>>>    				op-tee {
>>>    					description = "OP-TEE";
>>>    					type = "tee";
>>> -					arch = "arm";
>>> +					arch = ARCH;
>>>    					os = "tee";
>>>    					compression = "none";
>>>    					load = <(CFG_SYS_SDRAM_BASE + 0x8400000)>;
>>
>> Wondering if we couldn't put some of the Aarch32 and Aarch64 OP-TEE OS
>> node(s) in common?
> 
> Sounds like a good idea to maybe put op-tee in a template, personally I
> never use op-tee so typically try to minimize any change/impact related
> to op-tee.
> 
> The RK3506 does use op-tee so I may need to dig more into the op-tee
> parts in a future RK3506 enablement series, initially [1] was enough.
> Could look more into using a op-tee template in such future series.
> 
> [1] https://source.denx.de/u-boot/contributors/kwiboo/u-boot/-/commit/3d683f3b717de010fffeece8712373892a599905
> 

Interesting, is it required for RK3506? Do they do things in secure 
world and since it's Aarch32, no TF-A loaded by U-Boot?

You can play with OP-TEE on RK3588 from master, c.f. 
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/plat-rockchip/platform_rk3588.c

I haven't even built it but there's been some work on it since it was 
merged early December last year, so possibly people are using it.

Cheers,
Quentin