Re: [PATCH v33 6/7] arm64: dts: imx8ulp: add secure enclave node

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The commit message instructs boards to include `imx8ulp-firmware.dtsi` to provide the `memory-region` and enable the secure enclave, but this file is completely missing from the patch series and source tree.
- [Low] The newly added `hsm0: secure-enclave` node lacks the `memory-region` property, which will cause `dtbs_check` validation errors for all boards that include this SoC dtsi but do not explicitly provide it.
--

--- Patch [6]: [PATCH v33 6/7] arm64: dts: imx8ulp: add secure enclave node ---
commit 16318bc9e2bce734d6b1cf3cd835e37f3629b093
Author: Pankaj Gupta <[email protected]>

arm64: dts: imx8ulp: add secure enclave node

This commit adds the EdgeLock Enclave (ELE) secure-enclave node to the
i.MX8ULP SoC dtsi. The node is kept disabled by default to avoid imposing
reserved-memory requirements on all boards.

> Keep the node disabled in the SoC dtsi so it does not impose a
> reserved-memory requirement on every board. Boards enable the enclave and
> provide its memory-region by including imx8ulp-firmware.dtsi.

[Severity: Medium]
The commit message states that boards should include imx8ulp-firmware.dtsi
to provide the memory-region and enable the enclave, but is this file
missing from the patch series and the source tree?

It doesn't seem to be present, which might prevent boards from actually
following this procedure to enable the enclave.

> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index c6d1bb9edf388..38233dd74ee33 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi

[ ... ]

> @@ -185,6 +185,14 @@ scmi_sensor: protocol@15 {
>  				#thermal-sensor-cells = <1>;
>  			};
>  		};
> +
> +		hsm0: secure-enclave {
> +			compatible = "fsl,imx8ulp-se-ele-hsm";
> +			mbox-names = "tx", "rx";
> +			mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
> +			sram = <&sram0>;
> +			status = "disabled";
> +		};
>  	};
>  
>  	cm33: remoteproc-cm33 {

[Severity: Low]
Will this incomplete hsm0 node trigger dtbs_check validation errors?

The fsl,imx-se.yaml schema strictly requires a memory-region property for
the fsl,imx8ulp-se-ele-hsm compatible.

Since dt-validate evaluates all nodes, including those with
status = "disabled", it appears any board including imx8ulp.dtsi that does
not explicitly supply a memory-region for this node will fail schema
validation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6