Re: [PATCH v3 4/4] arm64: dts: imx93: Cortex-A Core remoteproc device node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Use of undocumented and unsupported property `init-on-array` in the clock controller node.
- [Medium] Encoding software-specific use-cases into top-level static .dts files.
- [Medium] The `m33_reserved` memory is declared but not linked to any remoteproc node.
- [Low] Grammatical errors in the inline device tree comments.
--

commit 63c01069d7a6eb875616e06076ef323b131b3915
Author: Jiafei Pan <Jiafei.Pan@nxp.com>

arm64: dts: imx93: Cortex-A Core remoteproc device node

This commit creates new top-level device tree files for the i.MX93
14x14 and 11x11 EVKs to support a multicore RTOS configuration, adding
Cortex-A Core remoteproc device nodes.

> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 711e36cc2c990..f067e5c44daef 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
[ ... ]
>  dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx93-11x11-evk-multicore-rtos.dtb

Device trees are generally intended to describe the physical hardware
of a board rather than specific software deployment policies. Does
adding dedicated top-level device tree files for an RTOS use-case
risk a combinatoric explosion of files whenever the base hardware
changes?

Could these software-specific configurations be implemented as Device
Tree Overlays (.dtbo) or applied dynamically via bootloader fixups
instead?

[ ... ]
> diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk-multicore-rtos.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk-multicore-rtos.dts
> new file mode 100644
> index 0000000000000..9fb2b94b83b4f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk-multicore-rtos.dts
[ ... ]
> +		/*
> +		 * Reserve up to 16MB for one possible RTOS instances running on
> +		 * one Cortex-A Cores when booting Linux on at least one Cortex-A Core.
> +		 */

This isn't a bug, but "instances" and "Cores" should probably be
singular ("RTOS instance" and "Cortex-A Core").

> +		rtos_ca55_reserved: rtos-ca55@d0000000 {
> +			no-map;
> +			reg = <0 0xd0000000 0x0 0x1000000>;
> +		};
> +
> +		/* Reserve 16MB for FreeRTOS on M33 */
> +		m33_reserved: m33@a5000000 {
> +			no-map;
> +			reg = <0 0xa5000000 0 0x1000000>;
> +		};

Is this memory-region intentionally left unlinked to any remoteproc
node?

Since it is not assigned to the cm33 node and is marked with no-map,
it will not be mapped by Linux. If the imx_rproc driver attempts to
parse an M33 firmware ELF and load segments into this memory, won't
it fail to validate or translate the physical address?

> +	};
> +};
> +
> +&lpuart2 {
> +	status = "disabled";
> +};
> +
> +&clk {
> +	init-on-array = <IMX93_CLK_LPUART2_GATE>;
> +};

Is init-on-array supported by the upstream Common Clock Framework?

This property appears to be absent from upstream dt-bindings and is
not parsed by mainline clock drivers. Because the framework will
silently ignore it, won't the late clk_disable_unused initcall
disable the LPUART2 clock, given the lpuart2 device is marked
disabled?

If the RTOS running on the remoteproc attempts to access the gated
UART, will it cause a bus fault and crash the remote processor?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511023928.39640-1-Jiafei.Pan@nxp.com?part=4