1. Patchew
  2. linux
  3. [PATCH 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver
  4. View patch

[PATCH 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup

Peng Fan posted 6 patches 2 weeks ago
There is a newer version of this series
[PATCH 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
Posted by Peng Fan 2 weeks ago 
Replace the explicit call to imx_rproc_put_scu() in the remove path with
devm_add_action_or_reset(). Ensure proper cleanup of scu resources and
simplify the code by leveraging the device-managed resource framework.

Additionally:
 - Remove the IMX_RPROC_SCU_API check from imx_rproc_put_scu(), as
   devm_add_action_or_reset() now exclusively handles SCU cleanup.
 - Improve error reporting by using dev_err_probe() for consistency and
   clarity.
 - Drop the err_put_scu label, as it is now redundant due to the updated
   error handling approach.

No functional changes.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 57 ++++++++++++++----------------------------
 1 file changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index c6cfb308ddb376f370fd4492f8a84f734602bac8..a53ff186d218f54123e1ce740b0277a6fe95a902 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -796,13 +796,9 @@ static void imx_rproc_free_mbox(void *data)
 	}
 }
 
-static void imx_rproc_put_scu(struct rproc *rproc)
+static void imx_rproc_put_scu(void *data)
 {
-	struct imx_rproc *priv = rproc->priv;
-	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
-
-	if (dcfg->method != IMX_RPROC_SCU_API)
-		return;
+	struct imx_rproc *priv = data;
 
 	if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
 		dev_pm_domain_detach_list(priv->pd_list);
@@ -944,6 +940,10 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
 	else
 		priv->core_index = 0;
 
+	ret = devm_add_action_or_reset(dev, imx_rproc_put_scu, priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add action for put scu\n");
+
 	/*
 	 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
 	 * and Linux could only do IPC with Mcore and nothing else.
@@ -1110,7 +1110,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 
 	ret = imx_rproc_clk_enable(priv);
 	if (ret)
-		goto err_put_scu;
+		return dev_err_probe(dev, ret, "failed to enable clks\n");
 
 	if (rproc->state != RPROC_DETACHED)
 		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
@@ -1125,61 +1125,42 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
 						    SYS_OFF_PRIO_DEFAULT,
 						    imx_rproc_sys_off_handler, rproc);
-		if (ret) {
-			dev_err(dev, "register power off handler failure\n");
-			goto err_put_scu;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "register power off handler failure\n");
 
 		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
 						    SYS_OFF_PRIO_DEFAULT,
 						    imx_rproc_sys_off_handler, rproc);
-		if (ret) {
-			dev_err(dev, "register restart handler failure\n");
-			goto err_put_scu;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "register restart handler failure\n");
 	}
 
 	if (dcfg->method == IMX_RPROC_SCU_API) {
 		ret = devm_pm_runtime_enable(dev);
-		if (ret) {
-			dev_err(dev, "Failed to enable runtime PM, %d\n", ret);
-			goto err_put_scu;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
 
 		ret = pm_runtime_resume_and_get(dev);
-		if (ret) {
-			dev_err(dev, "pm_runtime get failed: %d\n", ret);
-			goto err_put_scu;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "pm_runtime get failed\n");
 
 		ret = devm_add_action_or_reset(dev, imx_rproc_pm_runtime_put, dev);
-		if (ret) {
-			dev_err(dev, "Failed to add devm disable pm action: %d\n", ret);
-			goto err_put_scu;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret, "Failed to add devm disable pm action\n");
 	}
 
 	ret = rproc_add(rproc);
-	if (ret) {
-		dev_err(dev, "rproc_add failed\n");
-		goto err_put_scu;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "rproc_add failed\n");
 
 	return 0;
-
-err_put_scu:
-	imx_rproc_put_scu(rproc);
-
-	return ret;
 }
 
 static void imx_rproc_remove(struct platform_device *pdev)
 {
 	struct rproc *rproc = platform_get_drvdata(pdev);
-	struct imx_rproc *priv = rproc->priv;
 
 	rproc_del(rproc);
-	imx_rproc_put_scu(rproc);
 }
 
 static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {

-- 
2.37.1
Re: [PATCH 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
Posted by Frank Li 2 weeks ago 
On Wed, Sep 17, 2025 at 09:19:17PM +0800, Peng Fan wrote:
> Replace the explicit call to imx_rproc_put_scu() in the remove path with
> devm_add_action_or_reset(). Ensure proper cleanup of scu resources and
> simplify the code by leveraging the device-managed resource framework.
>
> Additionally:
>  - Remove the IMX_RPROC_SCU_API check from imx_rproc_put_scu(), as
>    devm_add_action_or_reset() now exclusively handles SCU cleanup.
>  - Improve error reporting by using dev_err_probe() for consistency and
>    clarity.
>  - Drop the err_put_scu label, as it is now redundant due to the updated
>    error handling approach.
>
> No functional changes.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/remoteproc/imx_rproc.c | 57 ++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index c6cfb308ddb376f370fd4492f8a84f734602bac8..a53ff186d218f54123e1ce740b0277a6fe95a902 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -796,13 +796,9 @@ static void imx_rproc_free_mbox(void *data)
>  	}
>  }
>
> -static void imx_rproc_put_scu(struct rproc *rproc)
> +static void imx_rproc_put_scu(void *data)
>  {
> -	struct imx_rproc *priv = rproc->priv;
> -	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
> -
> -	if (dcfg->method != IMX_RPROC_SCU_API)
> -		return;
> +	struct imx_rproc *priv = data;
>
>  	if (imx_sc_rm_is_resource_owned(priv->ipc_handle, priv->rsrc_id)) {
>  		dev_pm_domain_detach_list(priv->pd_list);
> @@ -944,6 +940,10 @@ static int imx_rproc_scu_api_detect_mode(struct rproc *rproc)
>  	else
>  		priv->core_index = 0;
>
> +	ret = devm_add_action_or_reset(dev, imx_rproc_put_scu, priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add action for put scu\n");
> +
>  	/*
>  	 * If Mcore resource is not owned by Acore partition, It is kicked by ROM,
>  	 * and Linux could only do IPC with Mcore and nothing else.
> @@ -1110,7 +1110,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
>
>  	ret = imx_rproc_clk_enable(priv);
>  	if (ret)
> -		goto err_put_scu;
> +		return dev_err_probe(dev, ret, "failed to enable clks\n");
>
>  	if (rproc->state != RPROC_DETACHED)
>  		rproc->auto_boot = of_property_read_bool(np, "fsl,auto-boot");
> @@ -1125,61 +1125,42 @@ static int imx_rproc_probe(struct platform_device *pdev)
>  		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_POWER_OFF_PREPARE,
>  						    SYS_OFF_PRIO_DEFAULT,
>  						    imx_rproc_sys_off_handler, rproc);
> -		if (ret) {
> -			dev_err(dev, "register power off handler failure\n");
> -			goto err_put_scu;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "register power off handler failure\n");
>
>  		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
>  						    SYS_OFF_PRIO_DEFAULT,
>  						    imx_rproc_sys_off_handler, rproc);
> -		if (ret) {
> -			dev_err(dev, "register restart handler failure\n");
> -			goto err_put_scu;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "register restart handler failure\n");
>  	}
>
>  	if (dcfg->method == IMX_RPROC_SCU_API) {
>  		ret = devm_pm_runtime_enable(dev);
> -		if (ret) {
> -			dev_err(dev, "Failed to enable runtime PM, %d\n", ret);
> -			goto err_put_scu;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");
>
>  		ret = pm_runtime_resume_and_get(dev);
> -		if (ret) {
> -			dev_err(dev, "pm_runtime get failed: %d\n", ret);
> -			goto err_put_scu;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "pm_runtime get failed\n");
>
>  		ret = devm_add_action_or_reset(dev, imx_rproc_pm_runtime_put, dev);
> -		if (ret) {
> -			dev_err(dev, "Failed to add devm disable pm action: %d\n", ret);
> -			goto err_put_scu;
> -		}
> +		if (ret)
> +			return dev_err_probe(dev, ret, "Failed to add devm disable pm action\n");
>  	}
>
>  	ret = rproc_add(rproc);
> -	if (ret) {
> -		dev_err(dev, "rproc_add failed\n");
> -		goto err_put_scu;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "rproc_add failed\n");
>
>  	return 0;
> -
> -err_put_scu:
> -	imx_rproc_put_scu(rproc);
> -
> -	return ret;
>  }
>
>  static void imx_rproc_remove(struct platform_device *pdev)
>  {
>  	struct rproc *rproc = platform_get_drvdata(pdev);
> -	struct imx_rproc *priv = rproc->priv;
>
>  	rproc_del(rproc);
> -	imx_rproc_put_scu(rproc);
>  }
>
>  static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {
>
> --
> 2.37.1
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.