Re: [PATCH] power: reset: at91-reset: free resources on exit path
From: Nicolas Ferre <nicolas.ferre@microchip.com>
Date: 2021-03-31 08:19:23
Also in:
linux-pm, lkml
On 09/02/2021 at 12:01, Claudiu Beznea wrote:
Free resources on exit path (failure path of probe and remove).
I'm not sure we can use this driver as a module anyway. Otherwise, it looks fine, but isn't it possible to use devm_of_iomap(), even in loop, and avoid having to deal with exit path?
quoted hunk ↗ jump to hunk
Reported-by: kernel test robot <redacted> Reported-by: Dan Carpenter <redacted> Signed-off-by: Claudiu Beznea <redacted> --- drivers/power/reset/at91-reset.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)diff --git a/drivers/power/reset/at91-reset.c b/drivers/power/reset/at91-reset.c index 3ff9d93a5226..2ff7833153b6 100644 --- a/drivers/power/reset/at91-reset.c +++ b/drivers/power/reset/at91-reset.c@@ -206,7 +206,8 @@ static int __init at91_reset_probe(struct platform_device *pdev) if (!reset->ramc_base[idx]) { dev_err(&pdev->dev, "Could not map ram controller address\n"); of_node_put(np); - return -ENODEV; + ret = -ENODEV; + goto unmap; } idx++; }@@ -218,13 +219,15 @@ static int __init at91_reset_probe(struct platform_device *pdev) reset->args = (u32)match->data; reset->sclk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(reset->sclk)) - return PTR_ERR(reset->sclk); + if (IS_ERR(reset->sclk)) { + ret = PTR_ERR(reset->sclk); + goto unmap; + } ret = clk_prepare_enable(reset->sclk); if (ret) { dev_err(&pdev->dev, "Could not enable slow clock\n"); - return ret; + goto unmap; } platform_set_drvdata(pdev, reset);@@ -239,21 +242,33 @@ static int __init at91_reset_probe(struct platform_device *pdev) ret = register_restart_handler(&reset->nb); if (ret) { clk_disable_unprepare(reset->sclk); - return ret; + goto unmap; } at91_reset_status(pdev, reset->rstc_base); return 0; + +unmap: + iounmap(reset->rstc_base); + for (idx = 0; idx < ARRAY_SIZE(reset->ramc_base); idx++) + iounmap(reset->ramc_base[idx]);
But if we keep this loop, I have the feeling that some kind of "of_node_put()" is needed as well.
+
+ return ret;
}
static int __exit at91_reset_remove(struct platform_device *pdev)
{
struct at91_reset *reset = platform_get_drvdata(pdev);
+ int idx;
unregister_restart_handler(&reset->nb);
clk_disable_unprepare(reset->sclk);
+ iounmap(reset->rstc_base);
+ for (idx = 0; idx < ARRAY_SIZE(reset->ramc_base); idx++)
+ iounmap(reset->ramc_base[idx]);Ditto
+ return 0; }
-- Nicolas Ferre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel