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.
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
drivers/remoteproc/imx_rproc.c | 28 +++++++++-------------------
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
index e30b61ee39dacc88f9e938f8c6ffe61fef63dbda..c6cfb308ddb376f370fd4492f8a84f734602bac8 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;
@@ -1134,7 +1127,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,
@@ -1142,7 +1135,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;
}
}
@@ -1150,32 +1143,30 @@ static int imx_rproc_probe(struct platform_device *pdev)
ret = devm_pm_runtime_enable(dev);
if (ret) {
dev_err(dev, "Failed to enable runtime PM, %d\n", ret);
- goto err_put_clk;
+ goto err_put_scu;
}
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;
}
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_clk;
+ goto err_put_scu;
}
}
ret = rproc_add(rproc);
if (ret) {
dev_err(dev, "rproc_add failed\n");
- goto err_put_clk;
+ goto err_put_scu;
}
return 0;
-err_put_clk:
- clk_disable_unprepare(priv->clk);
err_put_scu:
imx_rproc_put_scu(rproc);
@@ -1187,7 +1178,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
struct rproc *rproc = platform_get_drvdata(pdev);
struct imx_rproc *priv = rproc->priv;
- clk_disable_unprepare(priv->clk);
rproc_del(rproc);
imx_rproc_put_scu(rproc);
}
--
2.37.1On Wed, Sep 17, 2025 at 09:19:16PM +0800, Peng Fan wrote:
> 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> drivers/remoteproc/imx_rproc.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index e30b61ee39dacc88f9e938f8c6ffe61fef63dbda..c6cfb308ddb376f370fd4492f8a84f734602bac8 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;
> @@ -1134,7 +1127,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,
> @@ -1142,7 +1135,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;
> }
> }
>
> @@ -1150,32 +1143,30 @@ static int imx_rproc_probe(struct platform_device *pdev)
> ret = devm_pm_runtime_enable(dev);
> if (ret) {
> dev_err(dev, "Failed to enable runtime PM, %d\n", ret);
> - goto err_put_clk;
> + goto err_put_scu;
> }
>
> 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;
> }
>
> 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_clk;
> + goto err_put_scu;
> }
> }
>
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> - goto err_put_clk;
> + goto err_put_scu;
> }
>
> return 0;
>
> -err_put_clk:
> - clk_disable_unprepare(priv->clk);
> err_put_scu:
> imx_rproc_put_scu(rproc);
>
> @@ -1187,7 +1178,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
> struct rproc *rproc = platform_get_drvdata(pdev);
> struct imx_rproc *priv = rproc->priv;
>
> - clk_disable_unprepare(priv->clk);
/home/mpoirier/work/remoteproc/kernel/drivers/remoteproc/imx_rproc.c: In
function ‘imx_rproc_remove’:
/home/mpoirier/work/remoteproc/kernel/drivers/remoteproc/imx_rproc.c:1179:27:
warning: unused variable ‘priv’ [-Wunused-variable]
1179 | struct imx_rproc *priv = rproc->priv;
| ^~~~
AR drivers/remoteproc/built-in.a
AR drivers/built-in.a
AR kernel/module/built-in.a
AR kernel/built-in.a
> rproc_del(rproc);
> imx_rproc_put_scu(rproc);
> }
>
> --
> 2.37.1
>
On Mon, Sep 22, 2025 at 10:40:58AM -0600, Mathieu Poirier wrote: >On Wed, Sep 17, 2025 at 09:19:16PM +0800, Peng Fan wrote: >> Replace separate calls to devm_clk_get() and clk_prepare_enable() with ... >> >> @@ -1187,7 +1178,6 @@ static void imx_rproc_remove(struct platform_device *pdev) >> struct rproc *rproc = platform_get_drvdata(pdev); >> struct imx_rproc *priv = rproc->priv; >> >> - clk_disable_unprepare(priv->clk); > >/home/mpoirier/work/remoteproc/kernel/drivers/remoteproc/imx_rproc.c: In >function ???imx_rproc_remove???: >/home/mpoirier/work/remoteproc/kernel/drivers/remoteproc/imx_rproc.c:1179:27: >warning: unused variable ???priv??? [-Wunused-variable] > 1179 | struct imx_rproc *priv = rproc->priv; > | ^~~~ Oops, this variable is removed in patch 5/6 which should be in this patch. V2 will have this fixed. Thanks, Peng
On Wed, Sep 17, 2025 at 09:19:16PM +0800, Peng Fan wrote:
> 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.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/remoteproc/imx_rproc.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/remoteproc/imx_rproc.c b/drivers/remoteproc/imx_rproc.c
> index e30b61ee39dacc88f9e938f8c6ffe61fef63dbda..c6cfb308ddb376f370fd4492f8a84f734602bac8 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;
> @@ -1134,7 +1127,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,
> @@ -1142,7 +1135,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;
> }
> }
>
> @@ -1150,32 +1143,30 @@ static int imx_rproc_probe(struct platform_device *pdev)
> ret = devm_pm_runtime_enable(dev);
> if (ret) {
> dev_err(dev, "Failed to enable runtime PM, %d\n", ret);
> - goto err_put_clk;
> + goto err_put_scu;
> }
>
> 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;
> }
>
> 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_clk;
> + goto err_put_scu;
> }
> }
>
> ret = rproc_add(rproc);
> if (ret) {
> dev_err(dev, "rproc_add failed\n");
> - goto err_put_clk;
> + goto err_put_scu;
> }
>
> return 0;
>
> -err_put_clk:
> - clk_disable_unprepare(priv->clk);
> err_put_scu:
> imx_rproc_put_scu(rproc);
>
> @@ -1187,7 +1178,6 @@ static void imx_rproc_remove(struct platform_device *pdev)
> struct rproc *rproc = platform_get_drvdata(pdev);
> struct imx_rproc *priv = rproc->priv;
>
> - clk_disable_unprepare(priv->clk);
> rproc_del(rproc);
> imx_rproc_put_scu(rproc);
> }
>
> --
> 2.37.1
>
© 2016 - 2025 Red Hat, Inc.