Re: [PATCH v2 01/12] arm64: dts: imx8dxl-evk: fix reserved memory node names for remoteproc

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

New issues:
- [Medium] The newly introduced reserved-memory node names violate Devicetree naming conventions and schema.

Pre-existing issues:
- [High] Incomplete fix leaving remoteproc broken on sibling i.MX device trees.
--

--- Patch [1]: [PATCH v2 01/12] arm64: dts: imx8dxl-evk: fix reserved memory node names for remoteproc ---
commit e2a2dbe393a11711fef30985552292a1360dcdae
Author: Peng Fan <[email protected]>

arm64: dts: imx8dxl-evk: 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.

Rename the reserved memory nodes to use the expected names so the driver
can correctly identify and handle each region.

Fixes: ee39dbd9a6cb ("arm64: dts: imx8dxl-evk: add imx8dxl_cm4, lsio mu5, related memory region")
Signed-off-by: Peng Fan <[email protected]>

> diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> index 0ede0c7ed422c..71eff3efa6d32 100644
> --- a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts

[Severity: High]
This isn't a bug introduced by this patch, but does this leave remoteproc
broken on sibling i.MX device trees?

When the imx_rproc driver calls of_reserved_mem_region_to_resource(), it
expects res.name to start with vdev or rsc-table. Unconditionally on boot,
other i.MX boards still use generic memory@... node names. For example, in
arch/arm64/boot/dts/freescale/imx8dxl-sr-som.dtsi under reserved-memory:

vdev0vring0: memory0@90000000 {

And similarly in arch/arm64/boot/dts/freescale/imx8qm-mek.dts:

vdev0vring0: memory@90000000 {

Could these sibling device trees be updated as well so virtio rings and
buffers are mapped correctly instead of as regular memory carveouts?

[ ... ]

> @@ -65,32 +65,32 @@ linux,cma {
>  			linux,cma-default;
>  		};
>  
> -		vdev0vring0: memory0@90000000 {
> +		vdev0vring0: vdev0vring0@90000000 {

[Severity: Medium]
Do these new reserved-memory node names violate Devicetree naming conventions
and schema validation?

Building the device tree with schema validation enabled triggers dtbs_check
warnings because nodes like vdev0vring0@90000000 violate the strict
reserved-memory regex: ^([a-z0-9-]+-)?(memory|sram|ram)(@[0-9a-f]+)?$

The underlying driver code in drivers/remoteproc/imx_rproc.c enforces this
via hardcoded string matching:

imx_rproc_addr_init() {
    ...
    if (strstarts(res.name, "vdev"))
        continue;
    ...
}

Could the node names append -memory (e.g., vdev0vring0-memory@90000000) to
satisfy both the schema's optional prefix and the driver's strstarts()
checks? 

Alternatively, could the driver be updated to use memory-region-names via
of_reserved_mem_region_to_resource_byname() while keeping the generic
memory node names?

[ ... ]

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