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

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Fri, Jun 19, 2026 at 09:38:54AM -0700, 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 ++----------------

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

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