Re: [PATCH v2 3/7] arm: dts: st: Import optee node from u-boot device trees

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] The `bootph-pre-ram` property is missing from the `optee@fe000000` node and its parent `reserved-memory` node in `stm32mp157c-ed1.dts`, which will crash U-Boot on this board.
- [High] The patch unconditionally reserves 32MB of RAM for OP-TEE in the base board device trees, permanently removing this memory for users booting without OP-TEE.
- [High] The OP-TEE nodes were added to the DK1 base DT but omitted from the DK2 base DT, which will silently break OP-TEE support for DK2 when U-Boot drops its local overrides.
- [Low] The newly added `optee@de000000` and `optee@fe000000` nodes duplicate definitions that already exist in the corresponding `-scmi.dts` variants.
--

--- Patch [3]: [PATCH v2 3/7] arm: dts: st: Import optee node from u-boot device trees ---
commit 455c0e25ccd33494a8e1d7d65c591a7e9158f5c7
Author: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>

arm: dts: st: Import optee node from u-boot device trees

This commit imports OP-TEE firmware nodes and reserved-memory regions from
U-Boot device trees into the upstream kernel device trees. The goal is to
allow U-Boot to drop its local additions and use the kernel device trees
directly for these platforms.

> diff --git a/arch/arm/boot/dts/st/stm32mp157a-dk1.dts b/arch/arm/boot/dts/st/stm32mp157a-dk1.dts
> index 0da3667ab1e0e..236da589eb909 100644
> --- a/arch/arm/boot/dts/st/stm32mp157a-dk1.dts
> +++ b/arch/arm/boot/dts/st/stm32mp157a-dk1.dts

[ ... ]

> @@ -22,4 +22,21 @@ aliases {
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	reserved-memory {
> +		bootph-pre-ram;
> +
> +		optee@de000000 {

[Severity: High]
Since stm32mp157c-dk2.dts does not include stm32mp157a-dk1.dts, won't adding
this only to the dk1-specific file silently break OP-TEE on the DK2 board once
U-Boot drops its local overrides? Should these nodes be placed in the shared
stm32mp15xx-dkx.dtsi instead?

[Severity: High]
Does this unconditionally consume 32MB of RAM for users booting the standard
stm32mp157a-dk1.dtb without OP-TEE? 

ST provides dedicated *-scmi.dts variants for OP-TEE environments that already
contain these exact memory reservations. By placing this in the base device
tree, it looks like standard non-secure users permanently lose this memory to
the memblock allocator.

[Severity: Low]
Since stm32mp157a-dk1-scmi.dts includes this base file, does adding
optee@de000000 here duplicate the node already existing in the -scmi.dts
variant?

> +			reg = <0xde000000 0x02000000>;
> +			no-map;
> +			bootph-pre-ram;
> +		};
> +	};

[ ... ]

> diff --git a/arch/arm/boot/dts/st/stm32mp157c-ed1.dts b/arch/arm/boot/dts/st/stm32mp157c-ed1.dts
> index 49dd555cc228c..7da7260350869 100644
> --- a/arch/arm/boot/dts/st/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/st/stm32mp157c-ed1.dts

[ ... ]

> @@ -70,6 +70,18 @@ retram: retram@38000000 {
>  			reg = <0x38000000 0x10000>;
>  			no-map;
>  		};
> +
> +		optee@fe000000 {

[Severity: Critical]
Is the bootph-pre-ram property missing here and in the parent reserved-memory
node? 

During pre-relocation, U-Boot allocates memory downwards from the top of RAM
where OP-TEE also resides. Without bootph-pre-ram, U-Boot's DT parser will
ignore the reservation. Won't this cause U-Boot to relocate its code and stack
directly into OP-TEE's secure memory space and trigger a synchronous abort?

[Severity: Low]
Similar to the DK1 file, does adding optee@fe000000 to the base ED1 device
tree duplicate the reservation that already exists in the
stm32mp157c-ed1-scmi.dts variant?

> +			reg = <0xfe000000 0x02000000>;
> +			no-map;
> +		};

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-mathieu-uboot-dts-import-v2-0-bc931417bd0e@bootlin.com?part=3