linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver
@ 2025-09-26 12:33 Peng Fan
  2025-09-26 12:33 ` [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path Peng Fan
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Tested on
i.MX8MP-EVK, i.MX8MM-EVK, i.MX93-11x11-EVK, i.MX8QXP-MEK, and i.MX8ULP-EVK.

Retested all the patches for V3 on above platforms. And pass build
with patch incremental applied with ARM64 defconfig. pass build for
imx_v6_v7_defconfig with all patches applied.

This is the 2nd series to cleanup the driver.

Patch 1:
Fix the runtime usage. This is not critical bug fix, so it could be
defered to 6.18.

Patch 2-6:
Use devres managed API to cleanup the error handling path and remove path.

Thanks to Ulf for the suggestion on the runtime PM fix in patch 1.
Thanks to Daniel and Frank for the internal reviewing.

Signed-off-by: Peng Fan <peng.fan@nxp.com>

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
Changes in v3:
- Follow Ulf's suggestion to address the runtime PM in patch 1, and add
  Ulf's suggested-by tag. I dropped Frank and Daniel's tag in patch 1.
- With the changes in patch 1, the remove() is kept, then there are very
  minor conflicts when picking remaining patches in V2, so I still keep
  R-b tag from Frank and Daniel for patch 2-6.
- Link to v2: https://lore.kernel.org/r/20250923-imx_rproc_c2-v2-0-d31c437507e5@nxp.com

Changes in v2:
- Address a build warning in patch 4/6
- Add R-b from Frank and Daniel
- Link to v1: https://lore.kernel.org/r/20250917-imx_rproc_c2-v1-0-00ce23dc9c6e@nxp.com

---
Peng Fan (6):
      remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path
      remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup
      remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup
      remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup
      remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
      remoteproc: imx_rproc: Use devm_rproc_add() helper

 drivers/remoteproc/imx_rproc.c | 100 +++++++++++++++++++----------------------
 1 file changed, 47 insertions(+), 53 deletions(-)
---
base-commit: 8e2755d7779a95dd61d8997ebce33ff8b1efd3fb
change-id: 20250926-imx_rproc_v3-a50abed3288a

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-09-29 13:09   ` Ulf Hansson
  2025-09-26 12:33 ` [PATCH v3 2/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup Peng Fan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Proper cleanup should be done when rproc_add() fails by invoking both
pm_runtime_disable() and pm_runtime_put_noidle() to avoid leaving the
device in an inconsistent power state.

Fix it by adding pm_runtime_put_noidle() and pm_runtime_disable()
in the error path.

Also Update the remove() callback to use pm_runtime_put_noidle() instead of
pm_runtime_put(), to clearly indicate that only need to restore the usage
count.

Fixes: a876a3aacc43 ("remoteproc: imx_rproc: detect and attach to pre-booted remote cores")
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Hiago De Franco <hiago.franco@toradex.com>
Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index bb25221a4a8987ff427d68e2a5535f0e156b0097..8424e6ea5569b9ba6b07525643ce795faaeb2898 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1136,11 +1136,16 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	ret = rproc_add(rproc);
 	if (ret) {
 		dev_err(dev, "rproc_add failed\n");
-		goto err_put_clk;
+		goto err_put_pm;
 	}
 
 	return 0;
 
+err_put_pm:
+	if (dcfg->method == IMX_RPROC_SCU_API) {
+		pm_runtime_disable(dev);
+		pm_runtime_put_noidle(dev);
+	}
 err_put_clk:
 	clk_disable_unprepare(priv->clk);
 err_put_scu:
@@ -1160,7 +1165,7 @@ static void imx_rproc_remove(struct platform_device *pdev)
 
 	if (priv->dcfg->method == IMX_RPROC_SCU_API) {
 		pm_runtime_disable(priv->dev);
-		pm_runtime_put(priv->dev);
+		pm_runtime_put_noidle(priv->dev);
 	}
 	clk_disable_unprepare(priv->clk);
 	rproc_del(rproc);

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
  2025-09-26 12:33 ` [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-09-26 12:33 ` [PATCH v3 3/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup Peng Fan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Replace manual destroy_workqueue() calls in error and remove paths with a
devm_add_action_or_reset() helper. Ensure the workqueue is properly
cleaned up with the device lifecycle, and simplify error handling in probe
by removing now-unnecessary labels and cleanup steps.

No functional changes.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 8424e6ea5569b9ba6b07525643ce795faaeb2898..9c44ce56f1ab044ca5dccfeb0aaa0f7cd810aab1 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1046,6 +1046,13 @@ static int imx_rproc_sys_off_handler(struct sys_off_data *data)
 	return NOTIFY_DONE;
 }
 
+static void imx_rproc_destroy_workqueue(void *data)
+{
+	struct workqueue_struct *workqueue = data;
+
+	destroy_workqueue(workqueue);
+}
+
 static int imx_rproc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1077,11 +1084,15 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	ret = devm_add_action_or_reset(dev, imx_rproc_destroy_workqueue, priv->workqueue);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add devm destroy workqueue action\n");
+
 	INIT_WORK(&priv->rproc_work, imx_rproc_vq_work);
 
 	ret = imx_rproc_xtr_mbox_init(rproc, true);
 	if (ret)
-		goto err_put_wkq;
+		return ret;
 
 	ret = imx_rproc_addr_init(priv, pdev);
 	if (ret) {
@@ -1152,8 +1163,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	imx_rproc_put_scu(rproc);
 err_put_mbox:
 	imx_rproc_free_mbox(rproc);
-err_put_wkq:
-	destroy_workqueue(priv->workqueue);
 
 	return ret;
 }
@@ -1171,7 +1180,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
 	rproc_del(rproc);
 	imx_rproc_put_scu(rproc);
 	imx_rproc_free_mbox(rproc);
-	destroy_workqueue(priv->workqueue);
 }
 
 static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
  2025-09-26 12:33 ` [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path Peng Fan
  2025-09-26 12:33 ` [PATCH v3 2/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-09-26 12:33 ` [PATCH v3 4/6] remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup Peng Fan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Convert imx_rproc_free_mbox() to a devm-managed cleanup action using
devm_add_action_or_reset(). Ensure the mailbox resources are freed
automatically with the device lifecycle, simplify error handling and
removing the need for manual cleanup in probe and remove paths.

Also improve error reporting by using dev_err_probe() for consistency and
clarity.

No functional changes.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 9c44ce56f1ab044ca5dccfeb0aaa0f7cd810aab1..3260fd55a713994e1d39cdf677265edd4192ae45 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -93,7 +93,7 @@ struct imx_rproc_mem {
 #define ATT_CORE(I)     BIT((I))
 
 static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block);
-static void imx_rproc_free_mbox(struct rproc *rproc);
+static void imx_rproc_free_mbox(void *data);
 
 struct imx_rproc {
 	struct device			*dev;
@@ -780,8 +780,9 @@ static int imx_rproc_xtr_mbox_init(struct rproc *rproc, bool tx_block)
 	return 0;
 }
 
-static void imx_rproc_free_mbox(struct rproc *rproc)
+static void imx_rproc_free_mbox(void *data)
 {
+	struct rproc *rproc = data;
 	struct imx_rproc *priv = rproc->priv;
 
 	if (priv->tx_ch) {
@@ -1094,15 +1095,18 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = devm_add_action_or_reset(dev, imx_rproc_free_mbox, rproc);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to add devm free mbox action: %d\n", ret);
+
 	ret = imx_rproc_addr_init(priv, pdev);
-	if (ret) {
-		dev_err(dev, "failed on imx_rproc_addr_init\n");
-		goto err_put_mbox;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed on imx_rproc_addr_init\n");
 
 	ret = imx_rproc_detect_mode(priv);
 	if (ret)
-		goto err_put_mbox;
+		return dev_err_probe(dev, ret, "failed on detect mode\n");
 
 	ret = imx_rproc_clk_enable(priv);
 	if (ret)
@@ -1161,8 +1165,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 	clk_disable_unprepare(priv->clk);
 err_put_scu:
 	imx_rproc_put_scu(rproc);
-err_put_mbox:
-	imx_rproc_free_mbox(rproc);
 
 	return ret;
 }
@@ -1179,7 +1181,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
 	clk_disable_unprepare(priv->clk);
 	rproc_del(rproc);
 	imx_rproc_put_scu(rproc);
-	imx_rproc_free_mbox(rproc);
 }
 
 static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/6] remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
                   ` (2 preceding siblings ...)
  2025-09-26 12:33 ` [PATCH v3 3/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-09-26 12:33 ` [PATCH v3 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup Peng Fan
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Replace separate calls to devm_clk_get() and clk_prepare_enable() with
devm_clk_get_enabled(), which combines clock acquisition and enabling
into a single managed step. Simplify the probe logic and remove the need
for manual clock disable in error and remove paths.

Also, update error handling to eliminate redundant cleanup steps and use
return-based error propagation where appropriate. Improve code clarity and
reduce the chance of resource leaks or incorrect ordering in cleanup paths.

No functional changes.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 3260fd55a713994e1d39cdf677265edd4192ae45..f353a680ba993dbe3dd7866ca725d18aa58303a1 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1006,26 +1006,19 @@ static int imx_rproc_clk_enable(struct imx_rproc *priv)
 {
 	const struct imx_rproc_dcfg *dcfg = priv->dcfg;
 	struct device *dev = priv->dev;
-	int ret;
 
 	/* Remote core is not under control of Linux or it is managed by SCU API */
 	if (dcfg->method == IMX_RPROC_NONE || dcfg->method == IMX_RPROC_SCU_API)
 		return 0;
 
-	priv->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(priv->clk)) {
-		dev_err(dev, "Failed to get clock\n");
-		return PTR_ERR(priv->clk);
-	}
-
 	/*
 	 * clk for M4 block including memory. Should be
 	 * enabled before .start for FW transfer.
 	 */
-	ret = clk_prepare_enable(priv->clk);
-	if (ret) {
+	priv->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(priv->clk)) {
 		dev_err(dev, "Failed to enable clock\n");
-		return ret;
+		return PTR_ERR(priv->clk);
 	}
 
 	return 0;
@@ -1127,7 +1120,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 						    imx_rproc_sys_off_handler, rproc);
 		if (ret) {
 			dev_err(dev, "register power off handler failure\n");
-			goto err_put_clk;
+			goto err_put_scu;
 		}
 
 		ret = devm_register_sys_off_handler(dev, SYS_OFF_MODE_RESTART_PREPARE,
@@ -1135,7 +1128,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 						    imx_rproc_sys_off_handler, rproc);
 		if (ret) {
 			dev_err(dev, "register restart handler failure\n");
-			goto err_put_clk;
+			goto err_put_scu;
 		}
 	}
 
@@ -1144,7 +1137,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		ret = pm_runtime_resume_and_get(dev);
 		if (ret) {
 			dev_err(dev, "pm_runtime get failed: %d\n", ret);
-			goto err_put_clk;
+			goto err_put_scu;
 		}
 	}
 
@@ -1161,8 +1154,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		pm_runtime_disable(dev);
 		pm_runtime_put_noidle(dev);
 	}
-err_put_clk:
-	clk_disable_unprepare(priv->clk);
 err_put_scu:
 	imx_rproc_put_scu(rproc);
 
@@ -1178,7 +1169,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
 		pm_runtime_disable(priv->dev);
 		pm_runtime_put_noidle(priv->dev);
 	}
-	clk_disable_unprepare(priv->clk);
 	rproc_del(rproc);
 	imx_rproc_put_scu(rproc);
 }

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
                   ` (3 preceding siblings ...)
  2025-09-26 12:33 ` [PATCH v3 4/6] remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-09-26 12:33 ` [PATCH v3 6/6] remoteproc: imx_rproc: Use devm_rproc_add() helper Peng Fan
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

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.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index f353a680ba993dbe3dd7866ca725d18aa58303a1..76feda868cb619b79922bcee4c6fdb3e16fc81e3 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.
@@ -1103,7 +1103,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");
@@ -1118,27 +1118,21 @@ 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) {
 		pm_runtime_enable(dev);
 		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 = rproc_add(rproc);
@@ -1154,8 +1148,6 @@ static int imx_rproc_probe(struct platform_device *pdev)
 		pm_runtime_disable(dev);
 		pm_runtime_put_noidle(dev);
 	}
-err_put_scu:
-	imx_rproc_put_scu(rproc);
 
 	return ret;
 }
@@ -1170,7 +1162,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
 		pm_runtime_put_noidle(priv->dev);
 	}
 	rproc_del(rproc);
-	imx_rproc_put_scu(rproc);
 }
 
 static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 6/6] remoteproc: imx_rproc: Use devm_rproc_add() helper
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
                   ` (4 preceding siblings ...)
  2025-09-26 12:33 ` [PATCH v3 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup Peng Fan
@ 2025-09-26 12:33 ` Peng Fan
  2025-10-01 14:22 ` [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Mathieu Poirier
  2025-10-14 15:19 ` Mathieu Poirier
  7 siblings, 0 replies; 10+ messages in thread
From: Peng Fan @ 2025-09-26 12:33 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Ulf Hansson,
	Hiago De Franco, Frank Li, Daniel Baluta
  Cc: linux-remoteproc, imx, linux-arm-kernel, linux-kernel, Peng Fan

Replace manual rproc_add() and cleanup logic with devm_rproc_add(), which
ties the remoteproc lifecycle to the device's lifecycle. This simplifies
error handling and ensures proper cleanup.

With no need to invoke rproc_del(), the remove() ops could be removed.

No functional changes.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/remoteproc/imx_rproc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index 76feda868cb619b79922bcee4c6fdb3e16fc81e3..68e01b647b66910627fb2256c96c152f3c22c83b 100644
--- a/drivers/remoteproc/imx_rproc.c
+++ b/drivers/remoteproc/imx_rproc.c
@@ -1135,7 +1135,7 @@ static int imx_rproc_probe(struct platform_device *pdev)
 			return dev_err_probe(dev, ret, "pm_runtime get failed\n");
 	}
 
-	ret = rproc_add(rproc);
+	ret = devm_rproc_add(dev, rproc);
 	if (ret) {
 		dev_err(dev, "rproc_add failed\n");
 		goto err_put_pm;
@@ -1161,7 +1161,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
 		pm_runtime_disable(priv->dev);
 		pm_runtime_put_noidle(priv->dev);
 	}
-	rproc_del(rproc);
 }
 
 static const struct imx_rproc_plat_ops imx_rproc_ops_arm_smc = {

-- 
2.37.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path
  2025-09-26 12:33 ` [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path Peng Fan
@ 2025-09-29 13:09   ` Ulf Hansson
  0 siblings, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2025-09-29 13:09 UTC (permalink / raw)
  To: Peng Fan
  Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Hiago De Franco, Frank Li,
	Daniel Baluta, linux-remoteproc, imx, linux-arm-kernel,
	linux-kernel

On Fri, 26 Sept 2025 at 14:33, Peng Fan <peng.fan@nxp.com> wrote:
>
> Proper cleanup should be done when rproc_add() fails by invoking both
> pm_runtime_disable() and pm_runtime_put_noidle() to avoid leaving the
> device in an inconsistent power state.
>
> Fix it by adding pm_runtime_put_noidle() and pm_runtime_disable()
> in the error path.
>
> Also Update the remove() callback to use pm_runtime_put_noidle() instead of
> pm_runtime_put(), to clearly indicate that only need to restore the usage
> count.
>
> Fixes: a876a3aacc43 ("remoteproc: imx_rproc: detect and attach to pre-booted remote cores")
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Hiago De Franco <hiago.franco@toradex.com>
> Suggested-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe


> ---
>  drivers/remoteproc/imx_rproc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index bb25221a4a8987ff427d68e2a5535f0e156b0097..8424e6ea5569b9ba6b07525643ce795faaeb2898 100644
> --- a/drivers/remoteproc/imx_rproc.c
> +++ b/drivers/remoteproc/imx_rproc.c
> @@ -1136,11 +1136,16 @@ static int imx_rproc_probe(struct platform_device *pdev)
>         ret = rproc_add(rproc);
>         if (ret) {
>                 dev_err(dev, "rproc_add failed\n");
> -               goto err_put_clk;
> +               goto err_put_pm;
>         }
>
>         return 0;
>
> +err_put_pm:
> +       if (dcfg->method == IMX_RPROC_SCU_API) {
> +               pm_runtime_disable(dev);
> +               pm_runtime_put_noidle(dev);
> +       }
>  err_put_clk:
>         clk_disable_unprepare(priv->clk);
>  err_put_scu:
> @@ -1160,7 +1165,7 @@ static void imx_rproc_remove(struct platform_device *pdev)
>
>         if (priv->dcfg->method == IMX_RPROC_SCU_API) {
>                 pm_runtime_disable(priv->dev);
> -               pm_runtime_put(priv->dev);
> +               pm_runtime_put_noidle(priv->dev);
>         }
>         clk_disable_unprepare(priv->clk);
>         rproc_del(rproc);
>
> --
> 2.37.1
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
                   ` (5 preceding siblings ...)
  2025-09-26 12:33 ` [PATCH v3 6/6] remoteproc: imx_rproc: Use devm_rproc_add() helper Peng Fan
@ 2025-10-01 14:22 ` Mathieu Poirier
  2025-10-14 15:19 ` Mathieu Poirier
  7 siblings, 0 replies; 10+ messages in thread
From: Mathieu Poirier @ 2025-10-01 14:22 UTC (permalink / raw)
  To: Peng Fan
  Cc: Bjorn Andersson, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Ulf Hansson, Hiago De Franco, Frank Li,
	Daniel Baluta, linux-remoteproc, imx, linux-arm-kernel,
	linux-kernel

On Fri, Sep 26, 2025 at 08:33:14PM +0800, Peng Fan wrote:
> Tested on
> i.MX8MP-EVK, i.MX8MM-EVK, i.MX93-11x11-EVK, i.MX8QXP-MEK, and i.MX8ULP-EVK.
> 
> Retested all the patches for V3 on above platforms. And pass build
> with patch incremental applied with ARM64 defconfig. pass build for
> imx_v6_v7_defconfig with all patches applied.
> 
> This is the 2nd series to cleanup the driver.
> 
> Patch 1:
> Fix the runtime usage. This is not critical bug fix, so it could be
> defered to 6.18.
> 
> Patch 2-6:
> Use devres managed API to cleanup the error handling path and remove path.
> 
> Thanks to Ulf for the suggestion on the runtime PM fix in patch 1.
> Thanks to Daniel and Frank for the internal reviewing.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Changes in v3:
> - Follow Ulf's suggestion to address the runtime PM in patch 1, and add
>   Ulf's suggested-by tag. I dropped Frank and Daniel's tag in patch 1.
> - With the changes in patch 1, the remove() is kept, then there are very
>   minor conflicts when picking remaining patches in V2, so I still keep
>   R-b tag from Frank and Daniel for patch 2-6.
> - Link to v2: https://lore.kernel.org/r/20250923-imx_rproc_c2-v2-0-d31c437507e5@nxp.com
> 
> Changes in v2:
> - Address a build warning in patch 4/6
> - Add R-b from Frank and Daniel
> - Link to v1: https://lore.kernel.org/r/20250917-imx_rproc_c2-v1-0-00ce23dc9c6e@nxp.com
> 
> ---
> Peng Fan (6):
>       remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup
>       remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
>       remoteproc: imx_rproc: Use devm_rproc_add() helper
> 
>  drivers/remoteproc/imx_rproc.c | 100 +++++++++++++++++++----------------------
>  1 file changed, 47 insertions(+), 53 deletions(-)

I will apply this set when 6.18-rc1 comes out.

Mathieu


> ---
> base-commit: 8e2755d7779a95dd61d8997ebce33ff8b1efd3fb
> change-id: 20250926-imx_rproc_v3-a50abed3288a
> 
> Best regards,
> -- 
> Peng Fan <peng.fan@nxp.com>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver
  2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
                   ` (6 preceding siblings ...)
  2025-10-01 14:22 ` [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Mathieu Poirier
@ 2025-10-14 15:19 ` Mathieu Poirier
  7 siblings, 0 replies; 10+ messages in thread
From: Mathieu Poirier @ 2025-10-14 15:19 UTC (permalink / raw)
  To: Peng Fan
  Cc: Bjorn Andersson, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, Ulf Hansson, Hiago De Franco, Frank Li,
	Daniel Baluta, linux-remoteproc, imx, linux-arm-kernel,
	linux-kernel

On Fri, Sep 26, 2025 at 08:33:14PM +0800, Peng Fan wrote:
> Tested on
> i.MX8MP-EVK, i.MX8MM-EVK, i.MX93-11x11-EVK, i.MX8QXP-MEK, and i.MX8ULP-EVK.
> 
> Retested all the patches for V3 on above platforms. And pass build
> with patch incremental applied with ARM64 defconfig. pass build for
> imx_v6_v7_defconfig with all patches applied.
> 
> This is the 2nd series to cleanup the driver.
> 
> Patch 1:
> Fix the runtime usage. This is not critical bug fix, so it could be
> defered to 6.18.
> 
> Patch 2-6:
> Use devres managed API to cleanup the error handling path and remove path.
> 
> Thanks to Ulf for the suggestion on the runtime PM fix in patch 1.
> Thanks to Daniel and Frank for the internal reviewing.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> Changes in v3:
> - Follow Ulf's suggestion to address the runtime PM in patch 1, and add
>   Ulf's suggested-by tag. I dropped Frank and Daniel's tag in patch 1.
> - With the changes in patch 1, the remove() is kept, then there are very
>   minor conflicts when picking remaining patches in V2, so I still keep
>   R-b tag from Frank and Daniel for patch 2-6.
> - Link to v2: https://lore.kernel.org/r/20250923-imx_rproc_c2-v2-0-d31c437507e5@nxp.com
> 
> Changes in v2:
> - Address a build warning in patch 4/6
> - Add R-b from Frank and Daniel
> - Link to v1: https://lore.kernel.org/r/20250917-imx_rproc_c2-v1-0-00ce23dc9c6e@nxp.com
> 
> ---
> Peng Fan (6):
>       remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup
>       remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup
>       remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup
>       remoteproc: imx_rproc: Use devm_rproc_add() helper
> 
>  drivers/remoteproc/imx_rproc.c | 100 +++++++++++++++++++----------------------
>  1 file changed, 47 insertions(+), 53 deletions(-)

I have applied this set.

Mathieu

> ---
> base-commit: 8e2755d7779a95dd61d8997ebce33ff8b1efd3fb
> change-id: 20250926-imx_rproc_v3-a50abed3288a
> 
> Best regards,
> -- 
> Peng Fan <peng.fan@nxp.com>
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-10-14 15:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26 12:33 [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Peng Fan
2025-09-26 12:33 ` [PATCH v3 1/6] remoteproc: imx_rproc: Fix runtime PM cleanup and improve remove path Peng Fan
2025-09-29 13:09   ` Ulf Hansson
2025-09-26 12:33 ` [PATCH v3 2/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for workqueue cleanup Peng Fan
2025-09-26 12:33 ` [PATCH v3 3/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for mailbox cleanup Peng Fan
2025-09-26 12:33 ` [PATCH v3 4/6] remoteproc: imx_rproc: Use devm_clk_get_enabled() and simplify cleanup Peng Fan
2025-09-26 12:33 ` [PATCH v3 5/6] remoteproc: imx_rproc: Use devm_add_action_or_reset() for scu cleanup Peng Fan
2025-09-26 12:33 ` [PATCH v3 6/6] remoteproc: imx_rproc: Use devm_rproc_add() helper Peng Fan
2025-10-01 14:22 ` [PATCH v3 0/6] remoteproc: imx_rproc: Use device managed API to clean up the driver Mathieu Poirier
2025-10-14 15:19 ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).