[PATCH] remoteproc: xlnx: refactor start & stop ops

[PATCH] remoteproc: xlnx: refactor start & stop ops

[Tanmay Shah]

Current _start and _stop ops are implemented using various APIs from the
platform management firmware driver. Instead provide respective RPU
start and stop API in the firmware driver and move the logic to interact
with the PM firmware in the firmware driver. The remoteproc driver doesn't
need to know actual logic, but only the final result i.e. RPU start/stop
was success or not. This refactor keeps the remoteproc driver simple and
moves firmware interaction logic to the firmware driver.

Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
---
 drivers/firmware/xilinx/zynqmp.c        | 93 +++++++++++++++++++++++++
 drivers/remoteproc/xlnx_r5_remoteproc.c | 68 ++----------------
 include/linux/firmware/xlnx-zynqmp.h    | 12 ++++
 3 files changed, 110 insertions(+), 63 deletions(-)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index af838b2dc327..f9a3a95b0638 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -1513,6 +1513,99 @@ int zynqmp_pm_request_wake(const u32 node,
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_request_wake);
 
+/**
+ * zynqmp_pm_start_rpu - Boot Real-time Processing Unit (Cortex-R) on SoC
+ *
+ * @node: power-domains id of the core
+ * @bootaddr: Boot address of elf
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_start_rpu(const u32 node, const u64 bootaddr)
+{
+	enum rpu_boot_mem bootmem;
+	int ret;
+
+	/*
+	 * The exception vector pointers (EVP) refer to the base-address of
+	 * exception vectors (for reset, IRQ, FIQ, etc). The reset-vector
+	 * starts at the base-address and subsequent vectors are on 4-byte
+	 * boundaries.
+	 *
+	 * Exception vectors can start either from 0x0000_0000 (LOVEC) or
+	 * from 0xFFFF_0000 (HIVEC) which is mapped in the OCM (On-Chip Memory)
+	 *
+	 * Usually firmware will put Exception vectors at LOVEC.
+	 *
+	 * It is not recommend that you change the exception vector.
+	 * Changing the EVP to HIVEC will result in increased interrupt latency
+	 * and jitter. Also, if the OCM is secured and the Cortex-R5F processor
+	 * is non-secured, then the Cortex-R5F processor cannot access the
+	 * HIVEC exception vectors in the OCM.
+	 */
+	bootmem = (bootaddr >= 0xFFFC0000) ?
+		   PM_RPU_BOOTMEM_HIVEC : PM_RPU_BOOTMEM_LOVEC;
+
+	pr_debug("RPU boot addr 0x%llx from %s.", bootaddr,
+		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
+
+	/* Request node before starting RPU core if new version of API is supported */
+	if (zynqmp_pm_feature(PM_REQUEST_NODE) > PM_API_VERSION_1) {
+		ret = zynqmp_pm_request_node(node,
+					     ZYNQMP_PM_CAPABILITY_ACCESS, 0,
+					     ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+		if (ret < 0) {
+			pr_err("failed to request 0x%x", node);
+			return ret;
+		}
+	}
+
+	ret = zynqmp_pm_request_wake(node, true,
+				     bootmem, ZYNQMP_PM_REQUEST_ACK_NO);
+	if (ret)
+		pr_err("failed to start RPU = 0x%x\n", node);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_start_rpu);
+
+/**
+ * zynqmp_pm_stop_rpu - Stop Real-time Processing Unit (Cortex-R) on SoC
+ *
+ * @node: power-domains id of the core
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_stop_rpu(const u32 node)
+{
+	int ret;
+
+	/* Use release node API to stop core if new version of API is supported */
+	if (zynqmp_pm_feature(PM_RELEASE_NODE) > PM_API_VERSION_1) {
+		ret = zynqmp_pm_release_node(node);
+		if (ret)
+			pr_err("failed to stop remoteproc RPU %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * Check expected version of EEMI call before calling it. This avoids
+	 * any error or warning prints from firmware as it is expected that fw
+	 * doesn't support it.
+	 */
+	if (zynqmp_pm_feature(PM_FORCE_POWERDOWN) != PM_API_VERSION_1) {
+		pr_debug("EEMI interface %d ver 1 not supported\n",
+			 PM_FORCE_POWERDOWN);
+		return -EOPNOTSUPP;
+	}
+
+	/* maintain force pwr down for backward compatibility */
+	ret = zynqmp_pm_force_pwrdwn(node, ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+	if (ret)
+		pr_err("core force power down failed\n");
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_stop_rpu);
+
 /**
  * zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
  * @node:		Node ID of the slave
diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
index 3349d1877751..dcd8a93f031c 100644
--- a/drivers/remoteproc/xlnx_r5_remoteproc.c
+++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
@@ -364,49 +364,12 @@ static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
 static int zynqmp_r5_rproc_start(struct rproc *rproc)
 {
 	struct zynqmp_r5_core *r5_core = rproc->priv;
-	enum rpu_boot_mem bootmem;
 	int ret;
 
-	/*
-	 * The exception vector pointers (EVP) refer to the base-address of
-	 * exception vectors (for reset, IRQ, FIQ, etc). The reset-vector
-	 * starts at the base-address and subsequent vectors are on 4-byte
-	 * boundaries.
-	 *
-	 * Exception vectors can start either from 0x0000_0000 (LOVEC) or
-	 * from 0xFFFF_0000 (HIVEC) which is mapped in the OCM (On-Chip Memory)
-	 *
-	 * Usually firmware will put Exception vectors at LOVEC.
-	 *
-	 * It is not recommend that you change the exception vector.
-	 * Changing the EVP to HIVEC will result in increased interrupt latency
-	 * and jitter. Also, if the OCM is secured and the Cortex-R5F processor
-	 * is non-secured, then the Cortex-R5F processor cannot access the
-	 * HIVEC exception vectors in the OCM.
-	 */
-	bootmem = (rproc->bootaddr >= 0xFFFC0000) ?
-		   PM_RPU_BOOTMEM_HIVEC : PM_RPU_BOOTMEM_LOVEC;
-
-	dev_dbg(r5_core->dev, "RPU boot addr 0x%llx from %s.", rproc->bootaddr,
-		bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
-
-	/* Request node before starting RPU core if new version of API is supported */
-	if (zynqmp_pm_feature(PM_REQUEST_NODE) > 1) {
-		ret = zynqmp_pm_request_node(r5_core->pm_domain_id,
-					     ZYNQMP_PM_CAPABILITY_ACCESS, 0,
-					     ZYNQMP_PM_REQUEST_ACK_BLOCKING);
-		if (ret < 0) {
-			dev_err(r5_core->dev, "failed to request 0x%x",
-				r5_core->pm_domain_id);
-			return ret;
-		}
-	}
-
-	ret = zynqmp_pm_request_wake(r5_core->pm_domain_id, 1,
-				     bootmem, ZYNQMP_PM_REQUEST_ACK_NO);
+	ret = zynqmp_pm_start_rpu(r5_core->pm_domain_id, rproc->bootaddr);
 	if (ret)
-		dev_err(r5_core->dev,
-			"failed to start RPU = 0x%x\n", r5_core->pm_domain_id);
+		dev_err(&rproc->dev, "failed to start RPU\n");
+
 	return ret;
 }
 
@@ -423,30 +386,9 @@ static int zynqmp_r5_rproc_stop(struct rproc *rproc)
 	struct zynqmp_r5_core *r5_core = rproc->priv;
 	int ret;
 
-	/* Use release node API to stop core if new version of API is supported */
-	if (zynqmp_pm_feature(PM_RELEASE_NODE) > 1) {
-		ret = zynqmp_pm_release_node(r5_core->pm_domain_id);
-		if (ret)
-			dev_err(r5_core->dev, "failed to stop remoteproc RPU %d\n", ret);
-		return ret;
-	}
-
-	/*
-	 * Check expected version of EEMI call before calling it. This avoids
-	 * any error or warning prints from firmware as it is expected that fw
-	 * doesn't support it.
-	 */
-	if (zynqmp_pm_feature(PM_FORCE_POWERDOWN) != 1) {
-		dev_dbg(r5_core->dev, "EEMI interface %d ver 1 not supported\n",
-			PM_FORCE_POWERDOWN);
-		return -EOPNOTSUPP;
-	}
-
-	/* maintain force pwr down for backward compatibility */
-	ret = zynqmp_pm_force_pwrdwn(r5_core->pm_domain_id,
-				     ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+	ret = zynqmp_pm_stop_rpu(r5_core->pm_domain_id);
 	if (ret)
-		dev_err(r5_core->dev, "core force power down failed\n");
+		dev_err(&rproc->dev, "failed to stop RPU\n");
 
 	return ret;
 }
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 7e27b0f7bf7e..347df66ee176 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -644,6 +644,8 @@ int zynqmp_pm_get_node_status(const u32 node, u32 *const status,
 			      u32 *const requirements, u32 *const usage);
 int zynqmp_pm_get_rpu_node_status(const u32 node, u32 *const status,
 				  u32 *const requirements, u32 *const usage);
+int zynqmp_pm_start_rpu(const u32 node, const u64 bootaddr);
+int zynqmp_pm_stop_rpu(const u32 node);
 int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
 int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
 			     u32 value);
@@ -960,6 +962,16 @@ static inline int zynqmp_pm_get_rpu_node_status(const u32 node, u32 *const statu
 	return -ENODEV;
 }
 
+static inline int zynqmp_pm_start_rpu(const u32 node, const u64 bootaddr)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_stop_rpu(const u32 node)
+{
+	return -ENODEV;
+}
+
 static inline int zynqmp_pm_set_sd_config(u32 node,
 					  enum pm_sd_config_type config,
 					  u32 value)

base-commit: 721396afea31eac476d88f5db10ba111ba4b8382
-- 
2.34.1

Re: [PATCH] remoteproc: xlnx: refactor start & stop ops

[Michal Simek]

On 6/19/26 18:38, Tanmay Shah wrote:
> Current _start and _stop ops are implemented using various APIs from the
> platform management firmware driver. Instead provide respective RPU
> start and stop API in the firmware driver and move the logic to interact
> with the PM firmware in the firmware driver. The remoteproc driver doesn't
> need to know actual logic, but only the final result i.e. RPU start/stop
> was success or not. This refactor keeps the remoteproc driver simple and
> moves firmware interaction logic to the firmware driver.
> 
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> ---
>   drivers/firmware/xilinx/zynqmp.c        | 93 +++++++++++++++++++++++++
>   drivers/remoteproc/xlnx_r5_remoteproc.c | 68 ++----------------
>   include/linux/firmware/xlnx-zynqmp.h    | 12 ++++
>   3 files changed, 110 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index af838b2dc327..f9a3a95b0638 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -1513,6 +1513,99 @@ int zynqmp_pm_request_wake(const u32 node,
>   }
>   EXPORT_SYMBOL_GPL(zynqmp_pm_request_wake);
>   
> +/**
> + * zynqmp_pm_start_rpu - Boot Real-time Processing Unit (Cortex-R) on SoC
> + *
> + * @node: power-domains id of the core
> + * @bootaddr: Boot address of elf
> + *
> + * Return: status, either success or error+reason
> + */
> +int zynqmp_pm_start_rpu(const u32 node, const u64 bootaddr)
> +{
> +	enum rpu_boot_mem bootmem;
> +	int ret;
> +
> +	/*
> +	 * The exception vector pointers (EVP) refer to the base-address of
> +	 * exception vectors (for reset, IRQ, FIQ, etc). The reset-vector
> +	 * starts at the base-address and subsequent vectors are on 4-byte
> +	 * boundaries.
> +	 *
> +	 * Exception vectors can start either from 0x0000_0000 (LOVEC) or
> +	 * from 0xFFFF_0000 (HIVEC) which is mapped in the OCM (On-Chip Memory)

here

> +	 *
> +	 * Usually firmware will put Exception vectors at LOVEC.
> +	 *
> +	 * It is not recommend that you change the exception vector.
> +	 * Changing the EVP to HIVEC will result in increased interrupt latency
> +	 * and jitter. Also, if the OCM is secured and the Cortex-R5F processor
> +	 * is non-secured, then the Cortex-R5F processor cannot access the
> +	 * HIVEC exception vectors in the OCM.
> +	 */
> +	bootmem = (bootaddr >= 0xFFFC0000) ?

and here you have different values without any explanation why.

The rest looks good to me. It is a step in a right direction.

Thanks,
Michal