Re: [PATCH v2 6/7] arm64: dts: freescale: imx8mm-verdin: Add Cortex-M4F remoteproc

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional enablement of the Cortex-M4 co-processor and its memory reservations in a base SoM device tree file.
--

--- Patch [6]: [PATCH v2 6/7] arm64: dts: freescale: imx8mm-verdin: Add Cortex-M4F remoteproc ---
commit 5627e466b821e2ae1d5d44438ab0c467605d30f9
Author: Ernest Van Hoecke <[email protected]>

arm64: dts: freescale: imx8mm-verdin: Add Cortex-M4F remoteproc

This patch describes the Cortex-M4F remote processor available on the
Verdin iMX8M Mini SoM.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> index 5fc177f589cb2..ac450fc6cbfb3 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> @@ -52,6 +52,18 @@ hdmi_connector: hdmi-connector {
>  		status = "disabled";
>  	};
>  
> +	imx8mm-cm4 {
> +		compatible = "fsl,imx8mm-cm4";
> +		clocks = <&clk IMX8MM_CLK_M4_DIV>;

[Severity: Medium]
Should this node default to disabled?

Because it is added unconditionally to the base SoM dtsi, it defaults
to "okay". This causes the imx_rproc driver to bind and call
devm_clk_get_enabled() during probe, which keeps the IMX8MM_CLK_M4_DIV
clock permanently running even if the M4 firmware is never booted.

> +		mbox-names = "tx", "rx", "rxdb";
> +		mboxes = <&mu 0 1
> +			  &mu 1 1
> +			  &mu 3 1>;
> +		memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>,
> +				<&rsc_table>;
> +		syscon = <&src>;
> +	};
> +

[ ... ]

> @@ -164,6 +176,27 @@ reserved-memory {
>  
>  		/* Use the kernel configuration settings instead */
>  		/delete-node/ linux,cma;
> +
> +		vdev0vring0: vdev0vring0@55000000 {
> +			reg = <0 0x55000000 0 0x8000>;
> +			no-map;
> +		};
> +
> +		vdev0vring1: vdev0vring1@55008000 {
> +			reg = <0 0x55008000 0 0x8000>;
> +			no-map;
> +		};
> +
> +		rsc_table: rsc-table@550ff000 {
> +			reg = <0 0x550ff000 0 0x1000>;
> +			no-map;
> +		};
> +
> +		vdev0buffer: vdev0buffer@55400000 {
> +			compatible = "shared-dma-pool";
> +			reg = <0 0x55400000 0 0x100000>;
> +			no-map;
> +		};

[Severity: Medium]
Are we sure we want to unconditionally reserve these memory regions for
all boards using this base SoM?

Because of the no-map properties, this removes roughly 1.1MB of RAM
from the kernel's linear mapping permanently across all Verdin i.MX8MM
boards, even when the M4 coprocessor is not used.

>  	};
>  };

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