lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250609083803.GA13113@nxa18884-linux>
Date: Mon, 9 Jun 2025 16:43:30 +0800
From: Peng Fan <peng.fan@....nxp.com>
To: Junhui Liu <junhui.liu@...moral.tech>
Cc: Bjorn Andersson <andersson@...nel.org>,
	Mathieu Poirier <mathieu.poirier@...aro.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Chen Wang <unicorn_wang@...look.com>,
	Inochi Amaoto <inochiama@...il.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>,
	linux-remoteproc@...r.kernel.org, devicetree@...r.kernel.org,
	sophgo@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-riscv@...ts.infradead.org
Subject: Re: [PATCH 2/2] drivers: remoteproc: Add C906L controller for Sophgo
 CV1800B SoC

On Sun, Jun 08, 2025 at 10:37:40AM +0800, Junhui Liu wrote:
>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 [1]. Support for mailbox-based functionality will be added in
>a separate patch.
>
>Link: https://lore.kernel.org/linux-riscv/20250520-cv18xx-mbox-v4-0-fd4f1c676d6e@pigmoral.tech/ [1]
>Signed-off-by: Junhui Liu <junhui.liu@...moral.tech>
>---
> drivers/remoteproc/Kconfig                |   9 ++
> drivers/remoteproc/Makefile               |   1 +
> drivers/remoteproc/sophgo_cv1800b_c906l.c | 233 ++++++++++++++++++++++++++++++
> 3 files changed, 243 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..f3c8d8fd4f796d0cf64f8ab0dd797e017b8e8be7
>--- /dev/null
>+++ b/drivers/remoteproc/sophgo_cv1800b_c906l.c
>@@ -0,0 +1,233 @@
>+// SPDX-License-Identifier: GPL-2.0-or-later
>+/*
>+ * Copyright (C) 2025 Junhui Liu <junhui.liu@...moral.tech>
>+ */
>+
>+#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)

Align the format.

'#include <linux/bits.h>' should be added for BIT

>+
>+#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 *va;
>+
>+	va = ioremap_wc(mem->dma, mem->len);
>+	if (IS_ERR_OR_NULL(va))

Use "if (!va)"?

>+		return -ENOMEM;
>+
>+	/* Update memory entry va */
>+	mem->va = va;
>+
>+	return 0;
>+}
>+
>+static int cv1800b_c906l_mem_release(struct rproc *rproc,
>+				     struct rproc_mem_entry *mem)
>+{
>+	iounmap(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;
>+
>+	/* 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;
>+		}

Is there a need to handle vdev0buffer?

>+
>+		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);
>+	}
>+
>+	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,

Seems your setup does not support attach mode, so better add
attach hook and return -ENOTSUPP?

>+};
>+
>+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);
>+
>+	if (atomic_read(&rproc->power) > 0)
>+		rproc_shutdown(rproc);

I think the remoteproc framework should block remove to be executed
if 'power > 0'.  If not, the framework should be enhanced.

Regards,
Peng

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ