Re: [PATCH v2 3/4] arm64: dts: imx943-evk: Add CM7 and CM33S nodes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing reserved memory regions for remote processor firmware
--

--- Patch [3]: [PATCH v2 3/4] arm64: dts: imx943-evk: Add CM7 and CM33S nodes ---
commit 348fa25c5d73cddade66c1646cc1a5d82686eae7
Author: Peng Fan <[email protected]>

arm64: dts: imx943-evk: Add CM7 and CM33S nodes

This commit adds remoteproc nodes for the Cortex-M7 (CM7-0, CM7-1) and
Cortex-M33S cores on the i.MX943 EVK. It configures the mailbox units and
various reserved memory regions for inter-processor communication.

> Add remoteproc nodes for the two Cortex-M7 cores (CM7-0, CM7-1) and
> the Cortex-M33S core, along with their respective reserved memory
> regions for firmware, virtio device vrings, resource tables and
> shared DMA pools.

[Severity: Medium]
The commit message states that reserved memory regions for firmware are added,
but they appear to be entirely missing from the device tree changes.

If the remote processors are intended to execute their firmware from system DDR
RAM, the omission means Linux will not carve out this memory. If preloaded by a
bootloader, Linux might map and clobber the live firmware.

Are these firmware reservations accidentally omitted, or does the firmware run
exclusively from TCM memory for this specific EVK?

> diff --git a/arch/arm64/boot/dts/freescale/imx943-evk.dts b/arch/arm64/boot/dts/freescale/imx943-evk.dts
> index 674410e541cba..da7bfbd52e312 100644
> --- a/arch/arm64/boot/dts/freescale/imx943-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx943-evk.dts
> @@ -133,6 +133,36 @@ reg_audio_pwr: regulator-wm8962-pwr {
>  		enable-active-high;
>  	};
>  
> +	remoteproc-cm70 {
> +		compatible = "fsl,imx94-cm70";
> +		mbox-names = "tx", "rx", "rxdb";
> +		mboxes = <&mu11 0 1
> +			  &mu11 1 1
> +			  &mu11 3 1>;
> +		memory-region = <&cm70_vdevbuffer>, <&cm70_vdev0vring0>, <&cm70_vdev0vring1>,
> +				<&cm70_vdev1vring0>, <&cm70_vdev1vring1>, <&cm70_rsc_table>;

[Severity: Medium]
Does this memory-region list need a reference to the firmware reserved memory
(e.g., cm70_reserved), or is it intentionally omitted here?

[ ... ]

> @@ -145,6 +175,99 @@ linux,cma {
>  			size = <0 0x10000000>;
>  			linux,cma-default;
>  		};
> +
> +		cm33s_vdev0vring0: vdev0vring0@20800000 {
> +			reg = <0 0x20800000 0 0x8000>;
> +			no-map;
> +		};

[Severity: Medium]
The added reserved-memory nodes fail to define firmware memory regions for the
CM7-0, CM7-1, and CM33S cores, which contradicts the commit message.

If the firmware is loaded into system DDR by the remoteproc driver,
imx_rproc_da_to_va will fail to find the carveout and the load will fail.
Will these firmware regions need to be added?

[ ... ]

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