Re: [PATCH v3] arm64: dts: freescale: imx8mp-verdin: Add Cortex-M7 remoteproc

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Oct 22, 2026 at 02:34:25PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] arm64: dts: imx8mp-verdin: unregistered IMX8MP_CLK_M7_DIV clock in imx8mp-cm7
> --
>
> commit 37e821a019361403bc773ac48a7bf68be6a133ac
> Author: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
>
> arm64: dts: freescale: imx8mp-verdin: Add Cortex-M7 remoteproc
>
> This commit adds the Cortex-M7 remoteproc node and its associated
> reserved-memory regions to the Verdin iMX8M Plus device tree.
>
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> > index 9fee2cf9ef546..45a4713c260f4 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> > @@ -77,6 +77,18 @@ key-wakeup {
> >  		};
> >  	};
> >
> > +	imx8mp-cm7 {
> > +		compatible = "fsl,imx8mp-cm7";
> > +		clocks = <&clk IMX8MP_CLK_M7_DIV>;
>
> [Severity: High]
> Should this clock specifier be IMX8MP_CLK_M7_CORE instead of
> IMX8MP_CLK_M7_DIV?

look like make sense.

Frank

>
> In drivers/clk/imx/clk-imx8mp.c, commit 8c83a8ff4dd9 ("clk: imx8mp: use
> imx8m_clk_hw_composite_core to simplify code") replaced
> hws[IMX8MP_CLK_M7_DIV] in imx8mp_clocks_probe() with
> hws[IMX8MP_CLK_M7_CORE] without keeping an alias at IMX8MP_CLK_M7_DIV:
>
> 	hws[IMX8MP_CLK_A53_DIV] = imx8m_clk_hw_composite_core("arm_a53_div", imx8mp_a53_sels, ccm_base + 0x8000);
> 	hws[IMX8MP_CLK_A53_SRC] = hws[IMX8MP_CLK_A53_DIV];
> 	hws[IMX8MP_CLK_A53_CG] = hws[IMX8MP_CLK_A53_DIV];
> 	hws[IMX8MP_CLK_M7_CORE] = imx8m_clk_hw_composite_core("m7_core", imx8mp_m7_sels, ccm_base + 0x8080);
>
> Because clk_hw_data is zero-allocated, hws[IMX8MP_CLK_M7_DIV] remains
> NULL. When imx_rproc_probe() in drivers/remoteproc/imx_rproc.c requests
> the clock for "fsl,imx8mp-cm7" (which sets IMX_RPROC_NEED_CLKS in
> imx_rproc_cfg_imx8mn):
>
> 	if (dcfg->flags & IMX_RPROC_NEED_CLKS) {
> 		priv->clk = devm_clk_get_enabled(dev, NULL);
> 		if (IS_ERR(priv->clk))
> 			return dev_err_probe(dev, PTR_ERR(priv->clk), "Failed to enable clock\n");
> 	}
>
> of_clk_hw_onecell_get() returns NULL for hw_data->hws[IMX8MP_CLK_M7_DIV],
> causing devm_clk_get_enabled() to return NULL without an error. As a
> result, m7_core is never prepared or enabled by imx_rproc_probe(), and
> clk_disable_unused() can gate it off at late boot when the Cortex-M7 was
> not pre-booted by the bootloader, leaving the core unclocked when
> imx_rproc accesses TCM or starts the remote processor.
>
> Other i.MX8MP Cortex-M7 device tree descriptions such as
> arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc-overlay-cm7.dtso
> use IMX8MP_CLK_M7_CORE:
>
> 	imx8mp-cm7 {
> 		compatible = "fsl,imx8mp-cm7-mmio";
> 		clocks = <&clk IMX8MP_CLK_M7_CORE>;
>
> > +		mbox-names = "tx", "rx", "rxdb";
> > +		mboxes = <&mu 0 1
> > +			  &mu 1 1
> > +			  &mu 3 1>;
> > +		memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>,
> > +				<&rsc_table>;
> > +		syscon = <&src>;
> > +	};
> > +
> >  	sound_hdmi: sound-hdmi {
> >  		compatible = "fsl,imx-audio-hdmi";
> >  		model = "audio-hdmi";
>
> [ ... ]
>
> >
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261022-verdin-imx8mp-m7-v3-1-af284e28531d@toradex.com?part=1