Skip to contentSkip to conversation

remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI.

PATCH Applied [email protected] 2 participants 5 messages Oct 23 – Nov 2

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v9 cover remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI.

The driver currently derives the memory region type from the referenced memory node name, creating a implicit DT ABI.

rsc_table: rsc-table@90000000

           ^^^^^^^^
{	ret  = <0x90000000>;
	no-map;
}
m4 {
	...
	memory-region = <&rsc_table>;
}

Current require node name have to be rsc-table, but no any binding doc to restrict it.

Since memory node names are not constrained by the binding, incorrect names can silently pass DT validation, as seen in examples using "vdevbuffer" instead of "vdev0buffer" ([1] and [2]).

Add optional memory-region-names property to define such ABI. To keep back compactiblity, use "optional".

Prevous version
https://lore.kernel.org/imx/ail0sH1UhnQBPRkr@SMW015318/

---
Changes in v9:
- s/hidden/implicit/g, sorry, missed at v8
- Link to v8: https://patch.msgid.link/[email protected]

Changes in v8:
- remove dev_warning
- remove dts patches
- Link to v7: https://patch.msgid.link/[email protected]

Changes in v7:
- move imx_rproc_rmem_to_resource() close to caller
- collect Francesco and peng fan's review tag
- Link to v6: https://patch.msgid.link/[email protected]

Changes in v6:
- collect rob's review-by tags for binding
- move imx_rproc_rmem_to_resource to c file
- dts move related memory region node into overlay file
- Link to v5: https://patch.msgid.link/[email protected]

Changes in v5:
- fix binding region-memory-names's pattern
- add vdev1vring0 and vdev1vring1 at dts
- collect pan peng's review by tags
- Link to v4: https://patch.msgid.link/[email protected]

Change in v3
- rephrase commit message and cover letter to correct descritpion and reasons.

To: Bjorn Andersson <[email protected]>
To: Mathieu Poirier <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Sascha Hauer <[email protected]>
To: Pengutronix Kernel Team <[email protected]>
To: Fabio Estevam <[email protected]>
To: Peng Fan <[email protected]>

---
Laurentiu Mihalcea (2):
dt-bindings: remoteproc: imx_rproc: document memory-region-names to avoid implicit ABI
remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI

 .../bindings/remoteproc/fsl,imx-rproc.yaml         | 11 ++++++++
 drivers/remoteproc/imx_rproc.c                     | 29 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

---
base-commit: d419d682257d813f6cb7a28e42b35d0edf4ad038
change-id: 20260915-memory-region-name-f950eea2b78d

Best regards,

Signature · 1 line
Frank Li <[email protected]>
#1 lore raw

PATCH v9 1/2 dt-bindings: remoteproc: imx_rproc: document memory-region-names to avoid implicit ABI

From: Laurentiu Mihalcea <[email protected]>

Document the optional memory-region-names property.

The driver currently derives the memory region type from the referenced memory node name, creating a implicit DT ABI. Since memory node names are not constrained by the binding, incorrect names can silently pass DT validation, as seen in examples using "vdevbuffer" instead of "vdev0buffer" ([1] and [2]).

Add memory-region-names to explicitly identify the expected memory regions and avoid relying on memory node names as ABI.

---
Use optional property to keep back compatible with old platform. New platform should use memory-region-names.

---
 .../devicetree/bindings/remoteproc/fsl,imx-rproc.yaml         | 11 +++++++++++
 1 file changed, 11 insertions(+)
Documentation/devicetree/bindings/remoteproc/fsl,imx-rproc.yaml +11 -0
index c18f71b648890..f87036299a7b6 100644
@@ -62,6 +62,15 @@ properties:
     minItems: 1
     maxItems: 32
 
+  memory-region-names:
+    minItems: 1
+    maxItems: 32
+    items:
+      anyOf:
+        - const: rsc-table
+        - pattern: "^vdev[0-9]+(buffer|vring[0-9]+)$"
+        - pattern: "^m(4|7|33)-reserved(-[a-z0-9-]+)+$"
+
   power-domains:
     minItems: 2
     maxItems: 8
@@ -134,6 +143,7 @@ examples:
     imx7d-cm4 {
       compatible = "fsl,imx7d-cm4";
       memory-region = <&m4_reserved_sysmem1>, <&m4_reserved_sysmem2>;
+      memory-region-names = "m4-reserved-sysmem1", "m4-reserved-sysmem2";
       syscon = <&src>;
       clocks = <&clks IMX7D_ARM_M4_ROOT_CLK>;
     };
@@ -149,6 +159,7 @@ examples:
                 &mu 1 1
                 &mu 3 1>;
       memory-region = <&vdev0buffer>, <&vdev0vring0>, <&vdev0vring1>, <&rsc_table>;
+      memory-region-names = "vdev0buffer", "vdev0vring0", "vdev0vring1", "rsc-table";
       syscon = <&src>;
     };
 ...
Signature · 1 line
2.43.0
#2 lore raw

PATCH v9 2/2 remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI

In reply to [email protected]

From: Laurentiu Mihalcea <[email protected]>

The driver currently derives the memory region type from the referenced memory node name, creating a implicit DT ABI. Since memory node names are not constrained by the binding, incorrect names can silently pass DT validation, as seen in examples using "vdevbuffer" instead of "vdev0buffer" ([1] and [2]).

Parse memory-region-names to identify the expected memory regions and avoid relying on memory node names as ABI.

---
 drivers/remoteproc/imx_rproc.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/remoteproc/imx_rproc.c +27 -2
index 581bb07f5893f..2707040f16d06 100644
@@ -635,6 +635,25 @@ static int imx_rproc_sm_lmm_prepare(struct rproc *rproc)
 	return 0;
 }
 
+static int imx_rproc_rmem_to_resource(struct device_node *np,
+				      int index,
+				      struct resource *res)
+{
+	int ret;
+
+	ret = of_reserved_mem_region_to_resource(np, index, res);
+	if (ret)
+		return ret;
+
+	/* "memory-region-names" is optional */
+	ret = of_property_read_string_index(np, "memory-region-names",
+					    index, &res->name);
+	if (ret == -EINVAL)
+		return 0;
+
+	return ret;
+}
+
 static int imx_rproc_prepare(struct rproc *rproc)
 {
 	struct imx_rproc *priv = rproc->priv;
@@ -648,7 +667,7 @@ static int imx_rproc_prepare(struct rproc *rproc)
 		int err;
 		struct resource res;
 
-		err = of_reserved_mem_region_to_resource(np, i++, &res);
+		err = imx_rproc_rmem_to_resource(np, i++, &res);
 		if (err)
 			break;
 
@@ -823,11 +842,17 @@ static int imx_rproc_addr_init(struct imx_rproc *priv,
 	if (nph <= 0)
 		return 0;
 
+	if (of_property_present(np, "memory-region-names") &&
+	    nph != of_property_count_strings(np, "memory-region-names")) {
+		dev_err(dev, "invalid reserved memory name count\n");
+		return -EINVAL;
+	}
+
 	/* remap optional addresses */
 	for (a = 0; a < nph; a++) {
 		struct resource res;
 
-		err = of_reserved_mem_region_to_resource(np, a, &res);
+		err = imx_rproc_rmem_to_resource(np, a, &res);
 		if (err) {
 			dev_err(dev, "unable to resolve memory region\n");
 			return err;
Signature · 1 line
2.43.0
maintainer
#3 lore raw
In reply to [email protected]

I will apply this patchset when 7.4-rc1 comes out.

Thanks,
Mathieu

Quoted 94 lines · On Fri, Oct 23, 2026 at 03:52:48PM -0400, [email protected] wrote:

The driver currently derives the memory region type from the referenced memory node name, creating a implicit DT ABI.

rsc_table: rsc-table@90000000

           ^^^^^^^^
{	ret  = <0x90000000>;
	no-map;
}
m4 {
	...
	memory-region = <&rsc_table>;
}

Current require node name have to be rsc-table, but no any binding doc to restrict it.

Since memory node names are not constrained by the binding, incorrect names can silently pass DT validation, as seen in examples using "vdevbuffer" instead of "vdev0buffer" ([1] and [2]).

Add optional memory-region-names property to define such ABI. To keep back compactiblity, use "optional".

Prevous version
https://lore.kernel.org/imx/ail0sH1UhnQBPRkr@SMW015318/

---
Changes in v9:
- s/hidden/implicit/g, sorry, missed at v8
- Link to v8: https://patch.msgid.link/[email protected]

Changes in v8:
- remove dev_warning
- remove dts patches
- Link to v7: https://patch.msgid.link/[email protected]

Changes in v7:
- move imx_rproc_rmem_to_resource() close to caller
- collect Francesco and peng fan's review tag
- Link to v6: https://patch.msgid.link/[email protected]

Changes in v6:
- collect rob's review-by tags for binding
- move imx_rproc_rmem_to_resource to c file
- dts move related memory region node into overlay file
- Link to v5: https://patch.msgid.link/[email protected]

Changes in v5:
- fix binding region-memory-names's pattern
- add vdev1vring0 and vdev1vring1 at dts
- collect pan peng's review by tags
- Link to v4: https://patch.msgid.link/[email protected]

Change in v3
- rephrase commit message and cover letter to correct descritpion and reasons.

To: Bjorn Andersson <[email protected]>
To: Mathieu Poirier <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Sascha Hauer <[email protected]>
To: Pengutronix Kernel Team <[email protected]>
To: Fabio Estevam <[email protected]>
To: Peng Fan <[email protected]>

---
Laurentiu Mihalcea (2):
dt-bindings: remoteproc: imx_rproc: document memory-region-names to avoid implicit ABI
remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI

 .../bindings/remoteproc/fsl,imx-rproc.yaml         | 11 ++++++++
 drivers/remoteproc/imx_rproc.c                     | 29 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

---
base-commit: d419d682257d813f6cb7a28e42b35d0edf4ad038
change-id: 20260915-memory-region-name-f950eea2b78d

Best regards,

Signature · 1 line
Frank Li <[email protected]>
maintainer Applied
#4 lore raw
In reply to [email protected]
Quoted 17 lines · On Fri, Oct 23, 2026 at 03:52:48PM -0400, [email protected] wrote:

The driver currently derives the memory region type from the referenced memory node name, creating a implicit DT ABI.

rsc_table: rsc-table@90000000

           ^^^^^^^^
{	ret  = <0x90000000>;
	no-map;
}
m4 {
	...
	memory-region = <&rsc_table>;
}

Current require node name have to be rsc-table, but no any binding doc to restrict it.

Quoted 7 lines

Since memory node names are not constrained by the binding, incorrect names can silently pass DT validation, as seen in examples using "vdevbuffer" instead of "vdev0buffer" ([1] and [2]).

Add optional memory-region-names property to define such ABI. To keep back compactiblity, use "optional".

Applied.

Thanks,
Mathieu

Quoted 70 lines

Prevous version
https://lore.kernel.org/imx/ail0sH1UhnQBPRkr@SMW015318/

---
Changes in v9:
- s/hidden/implicit/g, sorry, missed at v8
- Link to v8: https://patch.msgid.link/[email protected]

Changes in v8:
- remove dev_warning
- remove dts patches
- Link to v7: https://patch.msgid.link/[email protected]

Changes in v7:
- move imx_rproc_rmem_to_resource() close to caller
- collect Francesco and peng fan's review tag
- Link to v6: https://patch.msgid.link/[email protected]

Changes in v6:
- collect rob's review-by tags for binding
- move imx_rproc_rmem_to_resource to c file
- dts move related memory region node into overlay file
- Link to v5: https://patch.msgid.link/[email protected]

Changes in v5:
- fix binding region-memory-names's pattern
- add vdev1vring0 and vdev1vring1 at dts
- collect pan peng's review by tags
- Link to v4: https://patch.msgid.link/[email protected]

Change in v3
- rephrase commit message and cover letter to correct descritpion and reasons.

To: Bjorn Andersson <[email protected]>
To: Mathieu Poirier <[email protected]>
To: Rob Herring <[email protected]>
To: Krzysztof Kozlowski <[email protected]>
To: Conor Dooley <[email protected]>
To: Sascha Hauer <[email protected]>
To: Pengutronix Kernel Team <[email protected]>
To: Fabio Estevam <[email protected]>
To: Peng Fan <[email protected]>

---
Laurentiu Mihalcea (2):
dt-bindings: remoteproc: imx_rproc: document memory-region-names to avoid implicit ABI
remoteproc: imx_rpoc: use memory-region-names to avoid implicit ABI

 .../bindings/remoteproc/fsl,imx-rproc.yaml         | 11 ++++++++
 drivers/remoteproc/imx_rproc.c                     | 29 ++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

---
base-commit: d419d682257d813f6cb7a28e42b35d0edf4ad038
change-id: 20260915-memory-region-name-f950eea2b78d

Best regards,

Signature · 1 line
Frank Li <[email protected]>