[PATCH v2 0/2] remoteproc: cv1800b: Add initial support for C906L processor

[PATCH v2 0/2] remoteproc: cv1800b: Add initial support for C906L processor

[Junhui Liu]

This patch series introduces initial support for the C906L remote
processor in the Sophgo CV1800B SoC. The CV1800B SoC integrates multiple
cores, including a main application core running Linux, a C906L core
typically running RTOS, and an 8051 MCU.

The C906L is an asymmetric processor designed to typically run RTOS.
This patch adds the basic infrastructure to support remoteproc
management of the C906L processor, including firmware loading and basic
control (start/stop) from the main Linux core. Mailbox-related
functionality will be added in a separate patch, after Zephyr mainline
support for the C906L is ready [1].

The C906L remoteproc relies on the reset controller [1] to function
correctly [2].

A branch for testing is available at [3].

Link: https://github.com/zephyrproject-rtos/zephyr/pull/69594 [1]
Link: https://lore.kernel.org/linux-riscv/20250617070144.1149926-1-inochiama@gmail.com [2]
Link: https://github.com/pigmoral/linux/tree/cv1800b/rproc-test [3]

---
Changes in v2:
- Add mbox-related properties in dt-bindings
- Remove reserved-memory node in dt examples
- Fix warnings in driver by handling conversions of `void *` and `void
  __iomem *` correctly
- Add missing <linux/bits.h> header
- Use `rproc_of_resm_mem_entry_init()` for vdev0buffer region
- Remove redundant rproc_shutdown in remove() function
- Link to v1: https://lore.kernel.org/r/20250608-cv1800-rproc-v1-0-57cf66cdf6a3@pigmoral.tech

---
Junhui Liu (2):
      dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC
      drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

 .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  |  79 +++++++
 drivers/remoteproc/Kconfig                         |   9 +
 drivers/remoteproc/Makefile                        |   1 +
 drivers/remoteproc/sophgo_cv1800b_c906l.c          | 239 +++++++++++++++++++++
 4 files changed, 328 insertions(+)
---
base-commit: 038d61fd642278bab63ee8ef722c50d10ab01e8f
change-id: 20250527-cv1800-rproc-08b3a9d2e461

Best regards,
-- 
Junhui Liu <junhui.liu@pigmoral.tech>

[PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

Add C906L remote processor for CV1800B SoC, which is an asymmetric
processor typically running RTOS.

Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
---
 .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo C906L remote processor controller for CV1800B SoC
+
+maintainers:
+  - Junhui Liu <junhui.liu@pigmoral.tech>
+
+description:
+  Document the bindings for the C906L remoteproc component that loads and boots
+  firmwares on the CV1800B SoC.
+
+properties:
+  compatible:
+    const: sophgo,cv1800b-c906l
+
+  firmware-name:
+    maxItems: 1
+
+  mbox-names:
+    items:
+      - const: tx
+      - const: rx
+
+  mboxes:
+    description:
+      This property is required only if the rpmsg/virtio functionality is used.
+      (see mailbox/sophgo,cv1800b-mailbox.yaml)
+    items:
+      - description: mailbox channel to send data to C906L
+      - description: mailbox channel to receive data from C906L
+
+  memory-region:
+    description:
+      List of phandles to reserved memory regions used by the remote processor.
+      The first region is required and provides the firmware region for the
+      remote processor. The following regions (vdev buffer, vrings) are optional
+      and are only required if rpmsg/virtio functionality is used.
+    minItems: 1
+    items:
+      - description: firmware region
+      - description: vdev buffer
+      - description: vring0
+      - description: vring1
+    additionalItems: true
+
+  resets:
+    maxItems: 1
+
+  sophgo,syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A phandle to the SEC_SYS region, used for configuration of the remote
+      processor.
+
+required:
+  - compatible
+  - firmware-name
+  - memory-region
+  - resets
+  - sophgo,syscon
+
+additionalProperties: false
+
+examples:
+  - |
+    c906l-rproc {
+        compatible = "sophgo,cv1800b-c906l";
+        firmware-name = "c906l-firmware.elf";
+        mbox-names = "tx", "rx";
+        mboxes = <&mbox 0 2>, <&mbox 1 1>;
+        memory-region = <&c906l_mem>, <&vdev0buffer>,
+                        <&vdev0vring0>, <&vdev0vring1>;
+        resets = <&rst 294>;
+        sophgo,syscon = <&sec_sys>;
+    };

-- 
2.50.1

[PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[Junhui Liu]

Add initial support for the C906L remote processor found in the Sophgo
CV1800B SoC. The C906L is an asymmetric core typically used to run an
RTOS. This driver enables firmware loading and start/stop control of the
C906L processor via the remoteproc framework.

The C906L and the main application processor can communicate through
mailboxes. Support for mailbox-based functionality will be added in
a separate patch.

Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
---
 drivers/remoteproc/Kconfig                |   9 ++
 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/sophgo_cv1800b_c906l.c | 239 ++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 83962a114dc9fdb3260e6e922602f2da53106265..7b09a8f00332605ee528ff7c21c31091c10c2bf5 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -299,6 +299,15 @@ config RCAR_REMOTEPROC
 	  This can be either built-in or a loadable module.
 	  If compiled as module (M), the module name is rcar_rproc.
 
+config SOPHGO_CV1800B_C906L
+	tristate "Sophgo CV1800B C906L remoteproc support"
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	help
+	  Say y here to support CV1800B C906L remote processor via the remote
+	  processor framework.
+
+	  It's safe to say N here.
+
 config ST_REMOTEPROC
 	tristate "ST remoteproc support"
 	depends on ARCH_STI
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 1c7598b8475d6057a3e044b41e3515103b7aa9f1..3c1e9387491cedc9dda8219f1e9130a84538156f 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_QCOM_WCNSS_PIL)		+= qcom_wcnss_pil.o
 qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_RCAR_REMOTEPROC)		+= rcar_rproc.o
+obj-$(CONFIG_SOPHGO_CV1800B_C906L)	+= sophgo_cv1800b_c906l.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.o
 obj-$(CONFIG_STM32_RPROC)		+= stm32_rproc.o
diff --git a/drivers/remoteproc/sophgo_cv1800b_c906l.c b/drivers/remoteproc/sophgo_cv1800b_c906l.c
new file mode 100644
index 0000000000000000000000000000000000000000..42258f072619bed25d307300135d04cf26d54e44
--- /dev/null
+++ b/drivers/remoteproc/sophgo_cv1800b_c906l.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2025 Junhui Liu <junhui.liu@pigmoral.tech>
+ */
+
+#include <linux/bits.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+
+#include "remoteproc_internal.h"
+
+#define CV1800B_SYS_C906L_CTRL_REG	0x04
+#define CV1800B_SYS_C906L_CTRL_EN	BIT(13)
+
+#define CV1800B_SYS_C906L_BOOTADDR_REG	0x20
+
+/**
+ * struct cv1800b_c906l - C906L remoteproc structure
+ * @dev: private pointer to the device
+ * @reset: reset control handle
+ * @rproc: the remote processor handle
+ * @syscon: regmap for accessing security system registers
+ */
+struct cv1800b_c906l {
+	struct device *dev;
+	struct reset_control *reset;
+	struct rproc *rproc;
+	struct regmap *syscon;
+};
+
+static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
+				   struct rproc_mem_entry *mem)
+{
+	void __iomem *va;
+
+	va = ioremap_wc(mem->dma, mem->len);
+	if (!va)
+		return -ENOMEM;
+
+	/* Update memory entry va */
+	mem->va = (void *)va;
+
+	return 0;
+}
+
+static int cv1800b_c906l_mem_release(struct rproc *rproc,
+				     struct rproc_mem_entry *mem)
+{
+	iounmap((void __iomem *)mem->va);
+	return 0;
+}
+
+static int cv1800b_c906l_add_carveout(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct device_node *np = dev->of_node;
+	struct of_phandle_iterator it;
+	struct rproc_mem_entry *mem;
+	struct reserved_mem *rmem;
+	int i = 0;
+
+	/* Register associated reserved memory regions */
+	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
+	while (of_phandle_iterator_next(&it) == 0) {
+		rmem = of_reserved_mem_lookup(it.node);
+		if (!rmem) {
+			of_node_put(it.node);
+			return -EINVAL;
+		}
+
+		if (!strcmp(it.node->name, "vdev0buffer")) {
+			mem = rproc_of_resm_mem_entry_init(&rproc->dev, i,
+							   rmem->size,
+							   rmem->base,
+							   it.node->name);
+		} else {
+			mem = rproc_mem_entry_init(dev, NULL, (dma_addr_t)rmem->base,
+						   rmem->size, rmem->base,
+						   cv1800b_c906l_mem_alloc,
+						   cv1800b_c906l_mem_release,
+						   it.node->name);
+		}
+
+		if (!mem) {
+			of_node_put(it.node);
+			return -ENOMEM;
+		}
+
+		rproc_add_carveout(rproc, mem);
+		i++;
+	}
+
+	return 0;
+}
+
+static int cv1800b_c906l_prepare(struct rproc *rproc)
+{
+	struct cv1800b_c906l *priv = rproc->priv;
+	int ret;
+
+	ret = cv1800b_c906l_add_carveout(rproc);
+	if (ret)
+		return ret;
+
+	/*
+	 * This control bit must be set to enable the C906L remote processor.
+	 * Note that once the remote processor is running, merely clearing
+	 * this bit will not stop its execution.
+	 */
+	return regmap_update_bits(priv->syscon, CV1800B_SYS_C906L_CTRL_REG,
+				  CV1800B_SYS_C906L_CTRL_EN,
+				  CV1800B_SYS_C906L_CTRL_EN);
+}
+
+static int cv1800b_c906l_start(struct rproc *rproc)
+{
+	struct cv1800b_c906l *priv = rproc->priv;
+	u32 bootaddr[2];
+	int ret;
+
+	bootaddr[0] = lower_32_bits(rproc->bootaddr);
+	bootaddr[1] = upper_32_bits(rproc->bootaddr);
+
+	ret = regmap_bulk_write(priv->syscon, CV1800B_SYS_C906L_BOOTADDR_REG,
+				bootaddr, ARRAY_SIZE(bootaddr));
+	if (ret)
+		return ret;
+
+	return reset_control_deassert(priv->reset);
+}
+
+static int cv1800b_c906l_stop(struct rproc *rproc)
+{
+	struct cv1800b_c906l *priv = rproc->priv;
+
+	return reset_control_assert(priv->reset);
+}
+
+static int cv1800b_c906l_parse_fw(struct rproc *rproc,
+				  const struct firmware *fw)
+{
+	int ret;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret == -EINVAL) {
+		dev_info(&rproc->dev, "No resource table in elf\n");
+		ret = 0;
+	}
+
+	return ret;
+}
+
+static const struct rproc_ops cv1800b_c906l_ops = {
+	.prepare = cv1800b_c906l_prepare,
+	.start = cv1800b_c906l_start,
+	.stop = cv1800b_c906l_stop,
+	.load = rproc_elf_load_segments,
+	.parse_fw = cv1800b_c906l_parse_fw,
+	.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+	.sanity_check = rproc_elf_sanity_check,
+	.get_boot_addr = rproc_elf_get_boot_addr,
+};
+
+static int cv1800b_c906l_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct cv1800b_c906l *priv;
+	struct rproc *rproc;
+	const char *fw_name;
+	int ret;
+
+	ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+	if (ret)
+		return dev_err_probe(dev, ret, "No firmware filename given\n");
+
+	rproc = devm_rproc_alloc(dev, dev_name(dev), &cv1800b_c906l_ops,
+				 fw_name, sizeof(*priv));
+	if (!rproc)
+		return dev_err_probe(dev, -ENOMEM,
+				     "unable to allocate remoteproc\n");
+
+	rproc->has_iommu = false;
+
+	priv = rproc->priv;
+	priv->dev = dev;
+	priv->rproc = rproc;
+
+	priv->syscon = syscon_regmap_lookup_by_phandle(np, "sophgo,syscon");
+	if (IS_ERR(priv->syscon))
+		return PTR_ERR(priv->syscon);
+
+	priv->reset = devm_reset_control_get_exclusive(dev, NULL);
+	if (IS_ERR(priv->reset))
+		return dev_err_probe(dev, PTR_ERR(priv->reset),
+				     "failed to get reset control handle\n");
+
+	platform_set_drvdata(pdev, rproc);
+
+	ret = devm_rproc_add(dev, rproc);
+	if (ret)
+		return dev_err_probe(dev, ret, "rproc_add failed\n");
+
+	return 0;
+}
+
+static void cv1800b_c906l_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+
+	rproc_del(rproc);
+}
+
+static const struct of_device_id cv1800b_c906l_of_match[] = {
+	{ .compatible = "sophgo,cv1800b-c906l" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cv1800b_c906l_of_match);
+
+static struct platform_driver cv1800b_c906l_driver = {
+	.probe = cv1800b_c906l_probe,
+	.remove = cv1800b_c906l_remove,
+	.driver = {
+		.name = "cv1800b-c906l",
+		.of_match_table = cv1800b_c906l_of_match,
+	},
+};
+
+module_platform_driver(cv1800b_c906l_driver);
+
+MODULE_AUTHOR("Junhui Liu <junhui.liu@pigmoral.tech>");
+MODULE_DESCRIPTION("Sophgo CV1800B C906L remote processor control driver");
+MODULE_LICENSE("GPL");

-- 
2.50.1

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Krzysztof Kozlowski]

On 28/07/2025 13:03, Junhui Liu wrote:
> +
> +  firmware-name:
> +    maxItems: 1
> +
> +  mbox-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  mboxes:

First go mboxes, then mboxes-names. ALWAYS.

> +    description:
> +      This property is required only if the rpmsg/virtio functionality is used.
> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
> +    items:
> +      - description: mailbox channel to send data to C906L
> +      - description: mailbox channel to receive data from C906L
> +
> +  memory-region:
> +    description:
> +      List of phandles to reserved memory regions used by the remote processor.
> +      The first region is required and provides the firmware region for the
> +      remote processor. The following regions (vdev buffer, vrings) are optional
> +      and are only required if rpmsg/virtio functionality is used.
> +    minItems: 1

Why isn't this constrained?

> +    items:
> +      - description: firmware region
> +      - description: vdev buffer
> +      - description: vring0
> +      - description: vring1
> +    additionalItems: true

No, drop. This needs to be constrained.


> +
> +  resets:
> +    maxItems: 1
> +
> +  sophgo,syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle to the SEC_SYS region, used for configuration of the remote
> +      processor.
> +
> +required:
> +  - compatible
> +  - firmware-name
> +  - memory-region
> +  - resets
> +  - sophgo,syscon

Why mboxes are not required?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    c906l-rproc {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

On 28/07/2025 15:11, Krzysztof Kozlowski wrote:
> On 28/07/2025 13:03, Junhui Liu wrote:
>> +
>> +  firmware-name:
>> +    maxItems: 1
>> +
>> +  mbox-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +  mboxes:
> 
> First go mboxes, then mboxes-names. ALWAYS.

Thanks, I will fix the order in the next version.

> 
>> +    description:
>> +      This property is required only if the rpmsg/virtio functionality is used.
>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>> +    items:
>> +      - description: mailbox channel to send data to C906L
>> +      - description: mailbox channel to receive data from C906L
>> +
>> +  memory-region:
>> +    description:
>> +      List of phandles to reserved memory regions used by the remote processor.
>> +      The first region is required and provides the firmware region for the
>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>> +      and are only required if rpmsg/virtio functionality is used.
>> +    minItems: 1
> 
> Why isn't this constrained?

Do you mean a maxItems should be added here?

> 
>> +    items:
>> +      - description: firmware region
>> +      - description: vdev buffer
>> +      - description: vring0
>> +      - description: vring1
>> +    additionalItems: true
> 
> No, drop. This needs to be constrained.

My intention is that RPMsg/OpenAMP is not the only use case for
remoteproc. There are scenarios where the remoteproc is just used for
booting the remote processor without any communication with Linux. In
such case, only the firmware region is needed, and the other regions may
not be necessary.

Additionally, the remote processor might reserve extra memory for custom
buffers or other firmware-specific purposes beyond virtio/rpmsg.
Therefore, I think only the firmware region should be required and
constrained, while allowing flexibility for additional/custom memory
regions as needed.

> 
> 
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  sophgo,syscon:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      A phandle to the SEC_SYS region, used for configuration of the remote
>> +      processor.
>> +
>> +required:
>> +  - compatible
>> +  - firmware-name
>> +  - memory-region
>> +  - resets
>> +  - sophgo,syscon
> 
> Why mboxes are not required?

The reason is similar to "memory-region" above. The mboxes property is
only needed when RPMsg/virtio communication is used. For some use cases,
the remote processor does not need to communicate with Linux at all, so
the mailbox is not required. Would it be necessary to require the mboxes
property even in scenarios where mailbox communication is not involved?

> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    c906l-rproc {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Thanks for the information, I will change the node name to a generic
"remoteproc" in the example. (Although it's not in the list, I think
it's generic enough)

> 
> 
> Best regards,
> Krzysztof

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Krzysztof Kozlowski]

On 28/07/2025 19:13, Junhui Liu wrote:
>>
>>> +    description:
>>> +      This property is required only if the rpmsg/virtio functionality is used.
>>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>>> +    items:
>>> +      - description: mailbox channel to send data to C906L
>>> +      - description: mailbox channel to receive data from C906L
>>> +
>>> +  memory-region:
>>> +    description:
>>> +      List of phandles to reserved memory regions used by the remote processor.
>>> +      The first region is required and provides the firmware region for the
>>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>>> +      and are only required if rpmsg/virtio functionality is used.
>>> +    minItems: 1
>>
>> Why isn't this constrained?
> 
> Do you mean a maxItems should be added here?
> >>
>>> +    items:
>>> +      - description: firmware region
>>> +      - description: vdev buffer
>>> +      - description: vring0
>>> +      - description: vring1
>>> +    additionalItems: true
>>
>> No, drop. This needs to be constrained.
> 
> My intention is that RPMsg/OpenAMP is not the only use case for

We don't allow such syntax, that's not negotiable. Why 322 redundant
memory regions are fine now?

> remoteproc. There are scenarios where the remoteproc is just used for
> booting the remote processor without any communication with Linux. In
> such case, only the firmware region is needed, and the other regions may
> not be necessary.
> 
> Additionally, the remote processor might reserve extra memory for custom
> buffers or other firmware-specific purposes beyond virtio/rpmsg.
> Therefore, I think only the firmware region should be required and
> constrained, while allowing flexibility for additional/custom memory
> regions as needed.

So how does this work exactly without the rest? Remote processor boots
and works fine? How do you communicate with it?

Please describe exactly the usecase.


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Inochi Amaoto]

On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
> Add C906L remote processor for CV1800B SoC, which is an asymmetric
> processor typically running RTOS.
> 
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo C906L remote processor controller for CV1800B SoC
> +
> +maintainers:
> +  - Junhui Liu <junhui.liu@pigmoral.tech>
> +
> +description:
> +  Document the bindings for the C906L remoteproc component that loads and boots
> +  firmwares on the CV1800B SoC.
> +
> +properties:
> +  compatible:
> +    const: sophgo,cv1800b-c906l
> +
> +  firmware-name:
> +    maxItems: 1
> +
> +  mbox-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  mboxes:
> +    description:
> +      This property is required only if the rpmsg/virtio functionality is used.
> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
> +    items:
> +      - description: mailbox channel to send data to C906L
> +      - description: mailbox channel to receive data from C906L
> +
> +  memory-region:
> +    description:
> +      List of phandles to reserved memory regions used by the remote processor.
> +      The first region is required and provides the firmware region for the
> +      remote processor. The following regions (vdev buffer, vrings) are optional
> +      and are only required if rpmsg/virtio functionality is used.
> +    minItems: 1
> +    items:
> +      - description: firmware region
> +      - description: vdev buffer
> +      - description: vring0
> +      - description: vring1
> +    additionalItems: true
> +

Why not allocating these region dynamicly? I do not think firware is
always avaible before staring. Allowing dynamic firmware give us max
flexiblity.

Regards,
Inochi

Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[kernel test robot]

Hi Junhui,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 038d61fd642278bab63ee8ef722c50d10ab01e8f]

url:    https://github.com/intel-lab-lkp/linux/commits/Junhui-Liu/dt-bindings-remoteproc-Add-C906L-rproc-for-Sophgo-CV1800B-SoC/20250728-190847
base:   038d61fd642278bab63ee8ef722c50d10ab01e8f
patch link:    https://lore.kernel.org/r/20250728-cv1800-rproc-v2-2-5bbee4abe9dc%40pigmoral.tech
patch subject: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC
config: parisc-randconfig-r123-20250729 (https://download.01.org/0day-ci/archive/20250729/202507291829.aB1UgzrA-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 8.5.0
reproduce: (https://download.01.org/0day-ci/archive/20250729/202507291829.aB1UgzrA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507291829.aB1UgzrA-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/remoteproc/sophgo_cv1800b_c906l.c:47:20: sparse: sparse: cast removes address space '__iomem' of expression

vim +/__iomem +47 drivers/remoteproc/sophgo_cv1800b_c906l.c

    36	
    37	static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
    38					   struct rproc_mem_entry *mem)
    39	{
    40		void __iomem *va;
    41	
    42		va = ioremap_wc(mem->dma, mem->len);
    43		if (!va)
    44			return -ENOMEM;
    45	
    46		/* Update memory entry va */
  > 47		mem->va = (void *)va;
    48	
    49		return 0;
    50	}
    51	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

On 29/07/2025 08:27, Krzysztof Kozlowski wrote:
> On 28/07/2025 19:13, Junhui Liu wrote:
>>>
>>>> +    description:
>>>> +      This property is required only if the rpmsg/virtio functionality is used.
>>>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>>>> +    items:
>>>> +      - description: mailbox channel to send data to C906L
>>>> +      - description: mailbox channel to receive data from C906L
>>>> +
>>>> +  memory-region:
>>>> +    description:
>>>> +      List of phandles to reserved memory regions used by the remote processor.
>>>> +      The first region is required and provides the firmware region for the
>>>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>>>> +      and are only required if rpmsg/virtio functionality is used.
>>>> +    minItems: 1
>>>
>>> Why isn't this constrained?
>> 
>> Do you mean a maxItems should be added here?
>> >>
>>>> +    items:
>>>> +      - description: firmware region
>>>> +      - description: vdev buffer
>>>> +      - description: vring0
>>>> +      - description: vring1
>>>> +    additionalItems: true
>>>
>>> No, drop. This needs to be constrained.
>> 
>> My intention is that RPMsg/OpenAMP is not the only use case for
> 
> We don't allow such syntax, that's not negotiable. Why 322 redundant
> memory regions are fine now?
> 
>> remoteproc. There are scenarios where the remoteproc is just used for
>> booting the remote processor without any communication with Linux. In
>> such case, only the firmware region is needed, and the other regions may
>> not be necessary.
>> 
>> Additionally, the remote processor might reserve extra memory for custom
>> buffers or other firmware-specific purposes beyond virtio/rpmsg.
>> Therefore, I think only the firmware region should be required and
>> constrained, while allowing flexibility for additional/custom memory
>> regions as needed.
> 
> So how does this work exactly without the rest? Remote processor boots
> and works fine? How do you communicate with it?
> 
> Please describe exactly the usecase.

Thank you for your clarification.

The C906L remoteproc can run at two use cases:
1. Standalone mode: Only the firmware region is used. In this case, the
   remoteproc driver loads the firmware into the firmware region and
   boots the C906L. The C906L runs independently, without communication
   with Linux, and the mailbox is not used.
2. OpenAMP/RPMsg mode: The firmware region, vdev buffer, and vrings are
   used. In this scenario, the C906L runs firmware with OpenAMP support
   and communicates with Linux via the virtio memory regions and mailbox.

To summarize:
- Required: firmware region
- Optional: vdev buffer, vrings, mailbox

Then I can add a "maxItems: 4" to the memory-region property and remove
the "additionalItems: true" line.

Is this approach acceptable for you?

> 
> 
> Best regards,
> Krzysztof

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

On 29/07/2025 16:31, Inochi Amaoto wrote:
> On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
>> Add C906L remote processor for CV1800B SoC, which is an asymmetric
>> processor typically running RTOS.
>> 
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---
>>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
>>  1 file changed, 79 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> @@ -0,0 +1,79 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Sophgo C906L remote processor controller for CV1800B SoC
>> +
>> +maintainers:
>> +  - Junhui Liu <junhui.liu@pigmoral.tech>
>> +
>> +description:
>> +  Document the bindings for the C906L remoteproc component that loads and boots
>> +  firmwares on the CV1800B SoC.
>> +
>> +properties:
>> +  compatible:
>> +    const: sophgo,cv1800b-c906l
>> +
>> +  firmware-name:
>> +    maxItems: 1
>> +
>> +  mbox-names:
>> +    items:
>> +      - const: tx
>> +      - const: rx
>> +
>> +  mboxes:
>> +    description:
>> +      This property is required only if the rpmsg/virtio functionality is used.
>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>> +    items:
>> +      - description: mailbox channel to send data to C906L
>> +      - description: mailbox channel to receive data from C906L
>> +
>> +  memory-region:
>> +    description:
>> +      List of phandles to reserved memory regions used by the remote processor.
>> +      The first region is required and provides the firmware region for the
>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>> +      and are only required if rpmsg/virtio functionality is used.
>> +    minItems: 1
>> +    items:
>> +      - description: firmware region
>> +      - description: vdev buffer
>> +      - description: vring0
>> +      - description: vring1
>> +    additionalItems: true
>> +
> 
> Why not allocating these region dynamicly? I do not think firware is
> always avaible before staring. Allowing dynamic firmware give us max
> flexiblity.

I'm afraid it's not easy to do this.

For firmware region, the RTOS firmware usually needs a physical address
to link to, and I have researched and tested two RTOS (RT-Thread and
Zephyr) on the C906L, both of them do not support position-independent
execution or runtime relocation. Therefore, a reserved memory region is
needed to provide a fixed physical address for the RTOS firmware.
(In fact, there is already a reserved memory region for the C906L in
cv1800b-milkv-duo.dts)

For virtio-related regions, the RTOS firmware also needs to know the
shared memory regions for communications at compile time.

So, reserving memory through DT is necessary for now.

> 
> Regards,
> Inochi

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Inochi Amaoto]

On Wed, Jul 30, 2025 at 03:57:09AM +0000, Junhui Liu wrote:
> On 29/07/2025 16:31, Inochi Amaoto wrote:
> > On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
> >> Add C906L remote processor for CV1800B SoC, which is an asymmetric
> >> processor typically running RTOS.
> >> 
> >> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> >> ---
> >>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
> >>  1 file changed, 79 insertions(+)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> >> new file mode 100644
> >> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> >> @@ -0,0 +1,79 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Sophgo C906L remote processor controller for CV1800B SoC
> >> +
> >> +maintainers:
> >> +  - Junhui Liu <junhui.liu@pigmoral.tech>
> >> +
> >> +description:
> >> +  Document the bindings for the C906L remoteproc component that loads and boots
> >> +  firmwares on the CV1800B SoC.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: sophgo,cv1800b-c906l
> >> +
> >> +  firmware-name:
> >> +    maxItems: 1
> >> +
> >> +  mbox-names:
> >> +    items:
> >> +      - const: tx
> >> +      - const: rx
> >> +
> >> +  mboxes:
> >> +    description:
> >> +      This property is required only if the rpmsg/virtio functionality is used.
> >> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
> >> +    items:
> >> +      - description: mailbox channel to send data to C906L
> >> +      - description: mailbox channel to receive data from C906L
> >> +
> >> +  memory-region:
> >> +    description:
> >> +      List of phandles to reserved memory regions used by the remote processor.
> >> +      The first region is required and provides the firmware region for the
> >> +      remote processor. The following regions (vdev buffer, vrings) are optional
> >> +      and are only required if rpmsg/virtio functionality is used.
> >> +    minItems: 1
> >> +    items:
> >> +      - description: firmware region
> >> +      - description: vdev buffer
> >> +      - description: vring0
> >> +      - description: vring1
> >> +    additionalItems: true
> >> +
> > 
> > Why not allocating these region dynamicly? I do not think firware is
> > always avaible before staring. Allowing dynamic firmware give us max
> > flexiblity.
> 
> I'm afraid it's not easy to do this.
> 
> For firmware region, the RTOS firmware usually needs a physical address
> to link to, and I have researched and tested two RTOS (RT-Thread and
> Zephyr) on the C906L, both of them do not support position-independent
> execution or runtime relocation. Therefore, a reserved memory region is
> needed to provide a fixed physical address for the RTOS firmware.

I think it is simple and possible to add PIE support for these RTOS. As
the memory of CV18XX is limited, I do not want to see some reserved
regions. This may hurt users who do not need this.

> (In fact, there is already a reserved memory region for the C906L in
> cv1800b-milkv-duo.dts)

This is just preserved for vendor zsbl and I have a plan to remove it.
Always let linux take care of all memory. It is good to support all
firmware implementation for CV18XX.

I think it is always good to use remoteproc like this:
https://www.kernel.org/doc/html/latest/staging/remoteproc.html

> 
> For virtio-related regions, the RTOS firmware also needs to know the
> shared memory regions for communications at compile time.
> 

I think you should investigate this and check if there is something you
missed. I haven't see any reserved region in remoteproc binding mentions
virtio.

Regards,
Inochi

Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[Krzysztof Kozlowski]

On 28/07/2025 13:03, Junhui Liu wrote:
> +
> +static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
> +				   struct rproc_mem_entry *mem)
> +{
> +	void __iomem *va;
> +
> +	va = ioremap_wc(mem->dma, mem->len);
> +	if (!va)
> +		return -ENOMEM;
> +
> +	/* Update memory entry va */
> +	mem->va = (void *)va;
> +
> +	return 0;
> +}
> +
> +static int cv1800b_c906l_mem_release(struct rproc *rproc,
> +				     struct rproc_mem_entry *mem)
> +{
> +	iounmap((void __iomem *)mem->va);
> +	return 0;
> +}
> +
> +static int cv1800b_c906l_add_carveout(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct device_node *np = dev->of_node;
> +	struct of_phandle_iterator it;
> +	struct rproc_mem_entry *mem;
> +	struct reserved_mem *rmem;
> +	int i = 0;
> +
> +	/* Register associated reserved memory regions */
> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
> +	while (of_phandle_iterator_next(&it) == 0) {
> +		rmem = of_reserved_mem_lookup(it.node);
> +		if (!rmem) {
> +			of_node_put(it.node);
> +			return -EINVAL;
> +		}
> +
> +		if (!strcmp(it.node->name, "vdev0buffer")) {

Why are you adding undocumented ABI? And so hidden, not even using
standard OF API!

How does this behaves when I change your DTS to call it
"whateverbuffer"? Does it work? Obviously not.

No, stop doing that.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Krzysztof Kozlowski]

On 30/07/2025 05:31, Junhui Liu wrote:
> On 29/07/2025 08:27, Krzysztof Kozlowski wrote:
>> On 28/07/2025 19:13, Junhui Liu wrote:
>>>>
>>>>> +    description:
>>>>> +      This property is required only if the rpmsg/virtio functionality is used.
>>>>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>>>>> +    items:
>>>>> +      - description: mailbox channel to send data to C906L
>>>>> +      - description: mailbox channel to receive data from C906L
>>>>> +
>>>>> +  memory-region:
>>>>> +    description:
>>>>> +      List of phandles to reserved memory regions used by the remote processor.
>>>>> +      The first region is required and provides the firmware region for the
>>>>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>>>>> +      and are only required if rpmsg/virtio functionality is used.
>>>>> +    minItems: 1
>>>>
>>>> Why isn't this constrained?
>>>
>>> Do you mean a maxItems should be added here?
>>>>>
>>>>> +    items:
>>>>> +      - description: firmware region
>>>>> +      - description: vdev buffer
>>>>> +      - description: vring0
>>>>> +      - description: vring1
>>>>> +    additionalItems: true
>>>>
>>>> No, drop. This needs to be constrained.
>>>
>>> My intention is that RPMsg/OpenAMP is not the only use case for
>>
>> We don't allow such syntax, that's not negotiable. Why 322 redundant
>> memory regions are fine now?
>>
>>> remoteproc. There are scenarios where the remoteproc is just used for
>>> booting the remote processor without any communication with Linux. In
>>> such case, only the firmware region is needed, and the other regions may
>>> not be necessary.
>>>
>>> Additionally, the remote processor might reserve extra memory for custom
>>> buffers or other firmware-specific purposes beyond virtio/rpmsg.
>>> Therefore, I think only the firmware region should be required and
>>> constrained, while allowing flexibility for additional/custom memory
>>> regions as needed.
>>
>> So how does this work exactly without the rest? Remote processor boots
>> and works fine? How do you communicate with it?
>>
>> Please describe exactly the usecase.
> 
> Thank you for your clarification.
> 
> The C906L remoteproc can run at two use cases:
> 1. Standalone mode: Only the firmware region is used. In this case, the
>    remoteproc driver loads the firmware into the firmware region and
>    boots the C906L. The C906L runs independently, without communication
>    with Linux, and the mailbox is not used.
> 2. OpenAMP/RPMsg mode: The firmware region, vdev buffer, and vrings are
>    used. In this scenario, the C906L runs firmware with OpenAMP support
>    and communicates with Linux via the virtio memory regions and mailbox.
> 
> To summarize:
> - Required: firmware region
> - Optional: vdev buffer, vrings, mailbox

How does your driver behave in (1)? Does it work?

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

On 30/07/2025 14:05, Inochi Amaoto wrote:
> On Wed, Jul 30, 2025 at 03:57:09AM +0000, Junhui Liu wrote:
>> On 29/07/2025 16:31, Inochi Amaoto wrote:
>> > On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
>> >> Add C906L remote processor for CV1800B SoC, which is an asymmetric
>> >> processor typically running RTOS.
>> >> 
>> >> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> >> ---
>> >>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
>> >>  1 file changed, 79 insertions(+)
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> >> new file mode 100644
>> >> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
>> >> @@ -0,0 +1,79 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Sophgo C906L remote processor controller for CV1800B SoC
>> >> +
>> >> +maintainers:
>> >> +  - Junhui Liu <junhui.liu@pigmoral.tech>
>> >> +
>> >> +description:
>> >> +  Document the bindings for the C906L remoteproc component that loads and boots
>> >> +  firmwares on the CV1800B SoC.
>> >> +
>> >> +properties:
>> >> +  compatible:
>> >> +    const: sophgo,cv1800b-c906l
>> >> +
>> >> +  firmware-name:
>> >> +    maxItems: 1
>> >> +
>> >> +  mbox-names:
>> >> +    items:
>> >> +      - const: tx
>> >> +      - const: rx
>> >> +
>> >> +  mboxes:
>> >> +    description:
>> >> +      This property is required only if the rpmsg/virtio functionality is used.
>> >> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>> >> +    items:
>> >> +      - description: mailbox channel to send data to C906L
>> >> +      - description: mailbox channel to receive data from C906L
>> >> +
>> >> +  memory-region:
>> >> +    description:
>> >> +      List of phandles to reserved memory regions used by the remote processor.
>> >> +      The first region is required and provides the firmware region for the
>> >> +      remote processor. The following regions (vdev buffer, vrings) are optional
>> >> +      and are only required if rpmsg/virtio functionality is used.
>> >> +    minItems: 1
>> >> +    items:
>> >> +      - description: firmware region
>> >> +      - description: vdev buffer
>> >> +      - description: vring0
>> >> +      - description: vring1
>> >> +    additionalItems: true
>> >> +
>> > 
>> > Why not allocating these region dynamicly? I do not think firware is
>> > always avaible before staring. Allowing dynamic firmware give us max
>> > flexiblity.
>> 
>> I'm afraid it's not easy to do this.
>> 
>> For firmware region, the RTOS firmware usually needs a physical address
>> to link to, and I have researched and tested two RTOS (RT-Thread and
>> Zephyr) on the C906L, both of them do not support position-independent
>> execution or runtime relocation. Therefore, a reserved memory region is
>> needed to provide a fixed physical address for the RTOS firmware.
> 
> I think it is simple and possible to add PIE support for these RTOS. As
> the memory of CV18XX is limited, I do not want to see some reserved
> regions. This may hurt users who do not need this.

Thank you for sharing your concern about the limited memory.

However, I think I have to wait until some RTOS supports PIE before I
can continue to advance this patch series. At least I haven't found any
guide on compiling RTOS firmware with PIE support for the two RTOSs
(RT-Thread and Zephyr) I'm currently testing on the C906L.

Besides, I have searched the existing remoteproc drivers in the kernel,
and haven't found any driver using dynamic memory allocation for the
firmware region. It may take some time to implement this feature if we
really need it on CV18XX SoCs.

> 
>> (In fact, there is already a reserved memory region for the C906L in
>> cv1800b-milkv-duo.dts)
> 
> This is just preserved for vendor zsbl and I have a plan to remove it.
> Always let linux take care of all memory. It is good to support all
> firmware implementation for CV18XX.

Got it.

> 
> I think it is always good to use remoteproc like this:
> https://www.kernel.org/doc/html/latest/staging/remoteproc.html
> 
>> 
>> For virtio-related regions, the RTOS firmware also needs to know the
>> shared memory regions for communications at compile time.
>> 
> 
> I think you should investigate this and check if there is something you
> missed. I haven't see any reserved region in remoteproc binding mentions
> virtio.

Currently, in Zephyr, the only boards with OpenAMP sample support are
the i.MX and STM32MP series [1]. Both of them define reserved memory
regions for virtio and vrings in their respective Linux kernel device
trees [2][3]. These are the only available reference targets I have at
the moment.

Furthermore, searching for the keyword "vring" in the remoteproc
bindings yields many results, which I believe mostly pertain to reserved
memory regions for rpmsg/virtio.

$grep "vring" -r Documentation/devicetree/bindings/remoteproc | wc -l
24

So, at present, all my references use reserved memory for firmware
regions (unless there is specific memory for the processor in hardware)
and for virtio-related regions. Do you think there is anything I might
have missed, or should some new feature be implemented?

[1] https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/subsys/ipc/openamp_rsc_table/boards
[2] https://github.com/torvalds/linux/blob/v6.16/arch/arm/boot/dts/st/stm32mp15xx-dkx.dtsi#L33-L49
[3] https://github.com/torvalds/linux/blob/v6.16/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts#L68-L97

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[Junhui Liu]

On 30/07/2025 08:46, Krzysztof Kozlowski wrote:
> On 28/07/2025 13:03, Junhui Liu wrote:
>> +
>> +static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
>> +				   struct rproc_mem_entry *mem)
>> +{
>> +	void __iomem *va;
>> +
>> +	va = ioremap_wc(mem->dma, mem->len);
>> +	if (!va)
>> +		return -ENOMEM;
>> +
>> +	/* Update memory entry va */
>> +	mem->va = (void *)va;
>> +
>> +	return 0;
>> +}
>> +
>> +static int cv1800b_c906l_mem_release(struct rproc *rproc,
>> +				     struct rproc_mem_entry *mem)
>> +{
>> +	iounmap((void __iomem *)mem->va);
>> +	return 0;
>> +}
>> +
>> +static int cv1800b_c906l_add_carveout(struct rproc *rproc)
>> +{
>> +	struct device *dev = rproc->dev.parent;
>> +	struct device_node *np = dev->of_node;
>> +	struct of_phandle_iterator it;
>> +	struct rproc_mem_entry *mem;
>> +	struct reserved_mem *rmem;
>> +	int i = 0;
>> +
>> +	/* Register associated reserved memory regions */
>> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
>> +	while (of_phandle_iterator_next(&it) == 0) {
>> +		rmem = of_reserved_mem_lookup(it.node);
>> +		if (!rmem) {
>> +			of_node_put(it.node);
>> +			return -EINVAL;
>> +		}
>> +
>> +		if (!strcmp(it.node->name, "vdev0buffer")) {
> 
> Why are you adding undocumented ABI? And so hidden, not even using
> standard OF API!
> 
> How does this behaves when I change your DTS to call it
> "whateverbuffer"? Does it work? Obviously not.
> 
> No, stop doing that.

Yes, you're right. I will consider introducing a "memory-region-names"
property in the bindings, instead of relying on the node labels directly.

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[Krzysztof Kozlowski]

On 30/07/2025 11:27, Junhui Liu wrote:
> On 30/07/2025 08:46, Krzysztof Kozlowski wrote:
>> On 28/07/2025 13:03, Junhui Liu wrote:
>>> +
>>> +static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
>>> +				   struct rproc_mem_entry *mem)
>>> +{
>>> +	void __iomem *va;
>>> +
>>> +	va = ioremap_wc(mem->dma, mem->len);
>>> +	if (!va)
>>> +		return -ENOMEM;
>>> +
>>> +	/* Update memory entry va */
>>> +	mem->va = (void *)va;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int cv1800b_c906l_mem_release(struct rproc *rproc,
>>> +				     struct rproc_mem_entry *mem)
>>> +{
>>> +	iounmap((void __iomem *)mem->va);
>>> +	return 0;
>>> +}
>>> +
>>> +static int cv1800b_c906l_add_carveout(struct rproc *rproc)
>>> +{
>>> +	struct device *dev = rproc->dev.parent;
>>> +	struct device_node *np = dev->of_node;
>>> +	struct of_phandle_iterator it;
>>> +	struct rproc_mem_entry *mem;
>>> +	struct reserved_mem *rmem;
>>> +	int i = 0;
>>> +
>>> +	/* Register associated reserved memory regions */
>>> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
>>> +	while (of_phandle_iterator_next(&it) == 0) {
>>> +		rmem = of_reserved_mem_lookup(it.node);
>>> +		if (!rmem) {
>>> +			of_node_put(it.node);
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		if (!strcmp(it.node->name, "vdev0buffer")) {
>>
>> Why are you adding undocumented ABI? And so hidden, not even using
>> standard OF API!
>>
>> How does this behaves when I change your DTS to call it
>> "whateverbuffer"? Does it work? Obviously not.
>>
>> No, stop doing that.
> 
> Yes, you're right. I will consider introducing a "memory-region-names"
> property in the bindings, instead of relying on the node labels directly.


You don't need it. First, you use some old code as template, but you
should look how or re-use Rob's code rewriting this completely.

Second, list has strict order, so you know exactly where the vdev0
buffer is. It cannot be on any other position of the list.

This is why you define the ABI. Use then the ABI.


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Junhui Liu]

On 30/07/2025 08:47, Krzysztof Kozlowski wrote:
> On 30/07/2025 05:31, Junhui Liu wrote:
>> On 29/07/2025 08:27, Krzysztof Kozlowski wrote:
>>> On 28/07/2025 19:13, Junhui Liu wrote:
>>>>>
>>>>>> +    description:
>>>>>> +      This property is required only if the rpmsg/virtio functionality is used.
>>>>>> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
>>>>>> +    items:
>>>>>> +      - description: mailbox channel to send data to C906L
>>>>>> +      - description: mailbox channel to receive data from C906L
>>>>>> +
>>>>>> +  memory-region:
>>>>>> +    description:
>>>>>> +      List of phandles to reserved memory regions used by the remote processor.
>>>>>> +      The first region is required and provides the firmware region for the
>>>>>> +      remote processor. The following regions (vdev buffer, vrings) are optional
>>>>>> +      and are only required if rpmsg/virtio functionality is used.
>>>>>> +    minItems: 1
>>>>>
>>>>> Why isn't this constrained?
>>>>
>>>> Do you mean a maxItems should be added here?
>>>>>>
>>>>>> +    items:
>>>>>> +      - description: firmware region
>>>>>> +      - description: vdev buffer
>>>>>> +      - description: vring0
>>>>>> +      - description: vring1
>>>>>> +    additionalItems: true
>>>>>
>>>>> No, drop. This needs to be constrained.
>>>>
>>>> My intention is that RPMsg/OpenAMP is not the only use case for
>>>
>>> We don't allow such syntax, that's not negotiable. Why 322 redundant
>>> memory regions are fine now?
>>>
>>>> remoteproc. There are scenarios where the remoteproc is just used for
>>>> booting the remote processor without any communication with Linux. In
>>>> such case, only the firmware region is needed, and the other regions may
>>>> not be necessary.
>>>>
>>>> Additionally, the remote processor might reserve extra memory for custom
>>>> buffers or other firmware-specific purposes beyond virtio/rpmsg.
>>>> Therefore, I think only the firmware region should be required and
>>>> constrained, while allowing flexibility for additional/custom memory
>>>> regions as needed.
>>>
>>> So how does this work exactly without the rest? Remote processor boots
>>> and works fine? How do you communicate with it?
>>>
>>> Please describe exactly the usecase.
>> 
>> Thank you for your clarification.
>> 
>> The C906L remoteproc can run at two use cases:
>> 1. Standalone mode: Only the firmware region is used. In this case, the
>>    remoteproc driver loads the firmware into the firmware region and
>>    boots the C906L. The C906L runs independently, without communication
>>    with Linux, and the mailbox is not used.
>> 2. OpenAMP/RPMsg mode: The firmware region, vdev buffer, and vrings are
>>    used. In this scenario, the C906L runs firmware with OpenAMP support
>>    and communicates with Linux via the virtio memory regions and mailbox.
>> 
>> To summarize:
>> - Required: firmware region
>> - Optional: vdev buffer, vrings, mailbox
> 
> How does your driver behave in (1)? Does it work?

The driver inits the firmware region and loads the firmware into it.
Then it sets the enable bit in the C906L's control register, set the
reset address for the C906L and deassert the reset bit for the C906L to boot it.

Actually, the current driver only supports the first case, and it works.
The second case is not implemented yet, but I believe it can be
supported based on the current code structure. I originally intended to
submit the OpenAMP/RPMsg-related bindings together with the driver
implementation; however, I was advised to finalize the bindings in v1:

https://lore.kernel.org/linux-riscv/PN0P287MB22589781F2D49353E7C66C46FE75A@PN0P287MB2258.INDP287.PROD.OUTLOOK.COM/

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 2/2] drivers: remoteproc: Add C906L controller for Sophgo CV1800B SoC

[Junhui Liu]

On 30/07/2025 11:35, Krzysztof Kozlowski wrote:
> On 30/07/2025 11:27, Junhui Liu wrote:
>> On 30/07/2025 08:46, Krzysztof Kozlowski wrote:
>>> On 28/07/2025 13:03, Junhui Liu wrote:
>>>> +
>>>> +static int cv1800b_c906l_mem_alloc(struct rproc *rproc,
>>>> +				   struct rproc_mem_entry *mem)
>>>> +{
>>>> +	void __iomem *va;
>>>> +
>>>> +	va = ioremap_wc(mem->dma, mem->len);
>>>> +	if (!va)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	/* Update memory entry va */
>>>> +	mem->va = (void *)va;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int cv1800b_c906l_mem_release(struct rproc *rproc,
>>>> +				     struct rproc_mem_entry *mem)
>>>> +{
>>>> +	iounmap((void __iomem *)mem->va);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int cv1800b_c906l_add_carveout(struct rproc *rproc)
>>>> +{
>>>> +	struct device *dev = rproc->dev.parent;
>>>> +	struct device_node *np = dev->of_node;
>>>> +	struct of_phandle_iterator it;
>>>> +	struct rproc_mem_entry *mem;
>>>> +	struct reserved_mem *rmem;
>>>> +	int i = 0;
>>>> +
>>>> +	/* Register associated reserved memory regions */
>>>> +	of_phandle_iterator_init(&it, np, "memory-region", NULL, 0);
>>>> +	while (of_phandle_iterator_next(&it) == 0) {
>>>> +		rmem = of_reserved_mem_lookup(it.node);
>>>> +		if (!rmem) {
>>>> +			of_node_put(it.node);
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		if (!strcmp(it.node->name, "vdev0buffer")) {
>>>
>>> Why are you adding undocumented ABI? And so hidden, not even using
>>> standard OF API!
>>>
>>> How does this behaves when I change your DTS to call it
>>> "whateverbuffer"? Does it work? Obviously not.
>>>
>>> No, stop doing that.
>> 
>> Yes, you're right. I will consider introducing a "memory-region-names"
>> property in the bindings, instead of relying on the node labels directly.
> 
> 
> You don't need it. First, you use some old code as template, but you
> should look how or re-use Rob's code rewriting this completely.

Sorry, I didn't catch up with that patch. I will look into it and update
my implementation accordingly.

> 
> Second, list has strict order, so you know exactly where the vdev0
> buffer is. It cannot be on any other position of the list.

Thanks for the advice, I will use the order in the list to identify the
memory region.

> 
> This is why you define the ABI. Use then the ABI.
> 

Understood, I will reconsider this.

-- 
Best regards,
Junhui Liu

Re: [PATCH v2 1/2] dt-bindings: remoteproc: Add C906L rproc for Sophgo CV1800B SoC

[Inochi Amaoto]

On Wed, Jul 30, 2025 at 08:59:15AM +0000, Junhui Liu wrote:
> On 30/07/2025 14:05, Inochi Amaoto wrote:
> > On Wed, Jul 30, 2025 at 03:57:09AM +0000, Junhui Liu wrote:
> >> On 29/07/2025 16:31, Inochi Amaoto wrote:
> >> > On Mon, Jul 28, 2025 at 07:03:23PM +0800, Junhui Liu wrote:
> >> >> Add C906L remote processor for CV1800B SoC, which is an asymmetric
> >> >> processor typically running RTOS.
> >> >> 
> >> >> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> >> >> ---
> >> >>  .../bindings/remoteproc/sophgo,cv1800b-c906l.yaml  | 79 ++++++++++++++++++++++
> >> >>  1 file changed, 79 insertions(+)
> >> >> 
> >> >> diff --git a/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> >> >> new file mode 100644
> >> >> index 0000000000000000000000000000000000000000..2061c2fd6ba343c09b1a91700ea4a695d2b57f81
> >> >> --- /dev/null
> >> >> +++ b/Documentation/devicetree/bindings/remoteproc/sophgo,cv1800b-c906l.yaml
> >> >> @@ -0,0 +1,79 @@
> >> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> >> +%YAML 1.2
> >> >> +---
> >> >> +$id: http://devicetree.org/schemas/remoteproc/sophgo,cv1800b-c906l.yaml#
> >> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> >> +
> >> >> +title: Sophgo C906L remote processor controller for CV1800B SoC
> >> >> +
> >> >> +maintainers:
> >> >> +  - Junhui Liu <junhui.liu@pigmoral.tech>
> >> >> +
> >> >> +description:
> >> >> +  Document the bindings for the C906L remoteproc component that loads and boots
> >> >> +  firmwares on the CV1800B SoC.
> >> >> +
> >> >> +properties:
> >> >> +  compatible:
> >> >> +    const: sophgo,cv1800b-c906l
> >> >> +
> >> >> +  firmware-name:
> >> >> +    maxItems: 1
> >> >> +
> >> >> +  mbox-names:
> >> >> +    items:
> >> >> +      - const: tx
> >> >> +      - const: rx
> >> >> +
> >> >> +  mboxes:
> >> >> +    description:
> >> >> +      This property is required only if the rpmsg/virtio functionality is used.
> >> >> +      (see mailbox/sophgo,cv1800b-mailbox.yaml)
> >> >> +    items:
> >> >> +      - description: mailbox channel to send data to C906L
> >> >> +      - description: mailbox channel to receive data from C906L
> >> >> +
> >> >> +  memory-region:
> >> >> +    description:
> >> >> +      List of phandles to reserved memory regions used by the remote processor.
> >> >> +      The first region is required and provides the firmware region for the
> >> >> +      remote processor. The following regions (vdev buffer, vrings) are optional
> >> >> +      and are only required if rpmsg/virtio functionality is used.
> >> >> +    minItems: 1
> >> >> +    items:
> >> >> +      - description: firmware region
> >> >> +      - description: vdev buffer
> >> >> +      - description: vring0
> >> >> +      - description: vring1
> >> >> +    additionalItems: true
> >> >> +
> >> > 
> >> > Why not allocating these region dynamicly? I do not think firware is
> >> > always avaible before staring. Allowing dynamic firmware give us max
> >> > flexiblity.
> >> 
> >> I'm afraid it's not easy to do this.
> >> 
> >> For firmware region, the RTOS firmware usually needs a physical address
> >> to link to, and I have researched and tested two RTOS (RT-Thread and
> >> Zephyr) on the C906L, both of them do not support position-independent
> >> execution or runtime relocation. Therefore, a reserved memory region is
> >> needed to provide a fixed physical address for the RTOS firmware.
> > 
> > I think it is simple and possible to add PIE support for these RTOS. As
> > the memory of CV18XX is limited, I do not want to see some reserved
> > regions. This may hurt users who do not need this.
> 
> Thank you for sharing your concern about the limited memory.
> 
> However, I think I have to wait until some RTOS supports PIE before I
> can continue to advance this patch series. At least I haven't found any
> guide on compiling RTOS firmware with PIE support for the two RTOSs
> (RT-Thread and Zephyr) I'm currently testing on the C906L.
> 
> Besides, I have searched the existing remoteproc drivers in the kernel,
> and haven't found any driver using dynamic memory allocation for the
> firmware region. It may take some time to implement this feature if we
> really need it on CV18XX SoCs.
> 
> > 
> >> (In fact, there is already a reserved memory region for the C906L in
> >> cv1800b-milkv-duo.dts)
> > 
> > This is just preserved for vendor zsbl and I have a plan to remove it.
> > Always let linux take care of all memory. It is good to support all
> > firmware implementation for CV18XX.
> 
> Got it.
> 
> > 
> > I think it is always good to use remoteproc like this:
> > https://www.kernel.org/doc/html/latest/staging/remoteproc.html
> > 
> >> 
> >> For virtio-related regions, the RTOS firmware also needs to know the
> >> shared memory regions for communications at compile time.
> >> 
> > 
> > I think you should investigate this and check if there is something you
> > missed. I haven't see any reserved region in remoteproc binding mentions
> > virtio.
> 

> Currently, in Zephyr, the only boards with OpenAMP sample support are
> the i.MX and STM32MP series [1]. Both of them define reserved memory
> regions for virtio and vrings in their respective Linux kernel device
> trees [2][3]. These are the only available reference targets I have at
> the moment.
> 
> Furthermore, searching for the keyword "vring" in the remoteproc
> bindings yields many results, which I believe mostly pertain to reserved
> memory regions for rpmsg/virtio.
> 
> $grep "vring" -r Documentation/devicetree/bindings/remoteproc | wc -l
> 24
> 

#grep -rl vring | wc -l
9

In fact, it seems not many boards add vring as memory region,
But for simplifity, it is OK for me to leave the vring as an
fixed region. Or just not support virtio at all... I see the
document of OpenAMP says it support no-virtio mode.

> So, at present, all my references use reserved memory for firmware
> regions (unless there is specific memory for the processor in hardware)
> and for virtio-related regions. Do you think there is anything I might
> have missed, or should some new feature be implemented?
> 
> [1] https://github.com/zephyrproject-rtos/zephyr/tree/main/samples/subsys/ipc/openamp_rsc_table/boards
> [2] https://github.com/torvalds/linux/blob/v6.16/arch/arm/boot/dts/st/stm32mp15xx-dkx.dtsi#L33-L49
> [3] https://github.com/torvalds/linux/blob/v6.16/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts#L68-L97
> 

Great, I think it is convincing. Thanks.

Regards,
Inochi