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

On Fri, Jul 24, 2026 at 04:41:32PM +0000, [email protected] wrote:
> 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.
> 

This is true and intentional from our side. The SoM DTSI now describes
the hardware including the M4 for the common use cases, instead of
requiring users to enable it specifically in an overlay. The drawback
mentioned here is a small tradeoff we considered.

> > +		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.
> 

Yes. We lose only a small amount of RAM, in a region that is fine for
all our Verdin iMX8MM SoMs. The DTSI is now set up to use the M4
copressor by default, in line with our other SoMs.

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