Re: [PATCH v2 02/12] arm64: dts: imx8qm-mek: fix reserved memory node names for remoteproc

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Renaming the vring memory nodes for cm4-1 to `vdev2vring0` and `vdev3vring0` breaks static vring matching in the remoteproc core, causing a fallback to dynamic allocation and breaking IPC.
- [High] The DSP's vdev buffer and vring nodes in `imx8qm-mek.dts` were missed during the rename, leaving them with the generic `memory` node name, which breaks the driver's memory region parsing and IPC.
--

--- Patch [2]: [PATCH v2 02/12] arm64: dts: imx8qm-mek: fix reserved memory node names for remoteproc ---
commit b33c57fd7f12629e7e4a66a20b899d545ad3c226
Author: Peng Fan <[email protected]>

arm64: dts: imx8qm-mek: fix reserved memory node names for remoteproc
    
The imx_rproc driver uses node names to identify reserved memory regions.
It checks for "vdev0buffer" to skip mapping, "rsc-table" to locate the
resource table, and "vdev" prefix to filter vdev regions.

> diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
> index 9e49ac377162b..8b67fb5d670da 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
> @@ -44,57 +44,57 @@ reserved-memory {
>  		#size-cells = <2>;
>  		ranges;

[ ... ]

> -		vdev2vring0: memory@90100000 {
> +		vdev2vring0: vdev2vring0@90100000 {

[Severity: High]
This isn't a bug introduced by this patch, but will this renaming actually
work for the cm4-1 remoteproc instance?

Since imx8qm-cm4-1 is an independent rproc instance, its first virtio device
index is 0. Looking at the remoteproc core, rproc_alloc_vring() searches for
vring carveouts using the format string "vdev%dvring%d".

Because of this, won't the core search for "vdev0vring0" and "vdev0vring1"
instead of "vdev2vring0"? If so, the mismatch would cause rproc_alloc_vring()
to fail finding the carveout and fall back to dynamic allocation, potentially
breaking IPC.

>  			reg = <0 0x90100000 0 0x8000>;
>  			no-map;
>  		};

[ ... ]

[Severity: High]
This is a pre-existing issue, but were the DSP vdev buffer and vring nodes
missed during this rename?

Looking at dsp_vdev0buffer in the same file (which currently remains as
memory@94300000), the imx_dsp_rproc driver will fail to skip the node
because it evaluates strstarts(res.name, "vdev0buffer").

Since the node name remains "memory", won't remoteproc_virtio fall back to
dynamic allocation from system memory and break IPC for the DSP?

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