[<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