On Tue, Jun 10, 2025 at 03:42:57AM +0000, Junhui Liu wrote: >Hi Peng, >Thanks for your review. > >On 09/06/2025 16:43, Peng Fan wrote: >> 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-fd4f1c676...@pigmoral.tech/ >>> [1] >>>Signed-off-by: Junhui Liu <junhui....@pigmoral.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....@pigmoral.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 >> > >Will do in v2. > >>>+ >>>+#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)"? > >Will do in v2. > >> >>>+ 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? > >I'll exclude it. > >> >>>+ >>>+ 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? > >I checked the remoteproc framework code and found that the attach >function will only be called when 'rproc->state == RPROC_DETACHED', and >it seems that rproc->state will not be set to RPROC_DETACHED unless I do >so explicitly in the driver or an implemented detach function is called, >neither of which happens in this driver. > >Given this, do we still need to add an attach hook even though it will >not be called in practice?
There is no need, I overlooked RPROC_DETACHED Regards, Peng
