lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 20 Feb 2017 16:02:35 +0200
From:   Leonard Crestez <leonard.crestez@....com>
To:     <linux-pm@...r.kernel.org>, Viresh Kumar <viresh.kumar@...aro.org>
CC:     Leonard Crestez <leonard.crestez@....com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Jacky Bai <ping.bai@....com>, Shawn Guo <shawnguo@...nel.org>,
        Lucas Stach <l.stach@...gutronix.de>,
        <linux-kernel@...r.kernel.org>
Subject: [PATCH] cpufreq: imx6q: use devm to fetch resources

Replace the clk_get and regulator_get will devm variants to free the
resources automatically when probe failed or driver is removed.

The device we are probing is not cpu_dev but the cpufreq platform_device
(pdev->dev). In order for this to actually work correctly we assign to
the cpufreq device the of_node from cpu_dev.

Signed-off-by: Leonard Crestez <leonard.crestez@....com>
---
 drivers/cpufreq/imx6q-cpufreq.c | 75 +++++++++++++----------------------------
 1 file changed, 24 insertions(+), 51 deletions(-)

Something similar was rejected in the past: https://patchwork.kernel.org/patch/7099051/

New version assigns the of_node from the cpu device to the cpufreq device so
that devm_clk_get(pdev->dev, ...) works as expected. It is not clear if this is
allowed but I don't see why it wouldn't.

diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c
index ef1fa81..374bc72 100644
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ * Copyright (C) 2017 NXP
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -198,35 +199,37 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		return -ENOENT;
 	}
 
-	arm_clk = clk_get(cpu_dev, "arm");
-	pll1_sys_clk = clk_get(cpu_dev, "pll1_sys");
-	pll1_sw_clk = clk_get(cpu_dev, "pll1_sw");
-	step_clk = clk_get(cpu_dev, "step");
-	pll2_pfd2_396m_clk = clk_get(cpu_dev, "pll2_pfd2_396m");
+	pdev->dev.of_node = cpu_dev->of_node;
+
+	arm_clk = devm_clk_get(&pdev->dev, "arm");
+	pll1_sys_clk = devm_clk_get(&pdev->dev, "pll1_sys");
+	pll1_sw_clk = devm_clk_get(&pdev->dev, "pll1_sw");
+	step_clk = devm_clk_get(&pdev->dev, "step");
+	pll2_pfd2_396m_clk = devm_clk_get(&pdev->dev, "pll2_pfd2_396m");
 	if (IS_ERR(arm_clk) || IS_ERR(pll1_sys_clk) || IS_ERR(pll1_sw_clk) ||
 	    IS_ERR(step_clk) || IS_ERR(pll2_pfd2_396m_clk)) {
 		dev_err(cpu_dev, "failed to get clocks\n");
 		ret = -ENOENT;
-		goto put_clk;
+		goto put_node;
 	}
 
 	if (of_machine_is_compatible("fsl,imx6ul")) {
-		pll2_bus_clk = clk_get(cpu_dev, "pll2_bus");
-		secondary_sel_clk = clk_get(cpu_dev, "secondary_sel");
+		pll2_bus_clk = devm_clk_get(&pdev->dev, "pll2_bus");
+		secondary_sel_clk = devm_clk_get(&pdev->dev, "secondary_sel");
 		if (IS_ERR(pll2_bus_clk) || IS_ERR(secondary_sel_clk)) {
 			dev_err(cpu_dev, "failed to get clocks specific to imx6ul\n");
 			ret = -ENOENT;
-			goto put_clk;
+			goto put_node;
 		}
 	}
 
-	arm_reg = regulator_get(cpu_dev, "arm");
-	pu_reg = regulator_get_optional(cpu_dev, "pu");
-	soc_reg = regulator_get(cpu_dev, "soc");
+	arm_reg = devm_regulator_get(&pdev->dev, "arm");
+	pu_reg = devm_regulator_get_optional(&pdev->dev, "pu");
+	soc_reg = devm_regulator_get(&pdev->dev, "soc");
 	if (IS_ERR(arm_reg) || IS_ERR(soc_reg)) {
 		dev_err(cpu_dev, "failed to get regulators\n");
 		ret = -ENOENT;
-		goto put_reg;
+		goto put_node;
 	}
 
 	/*
@@ -239,7 +242,7 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		ret = dev_pm_opp_of_add_table(cpu_dev);
 		if (ret < 0) {
 			dev_err(cpu_dev, "failed to init OPP table: %d\n", ret);
-			goto put_reg;
+			goto put_node;
 		}
 
 		/* Because we have added the OPPs here, we must free them */
@@ -256,11 +259,11 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 	ret = dev_pm_opp_init_cpufreq_table(cpu_dev, &freq_table);
 	if (ret) {
 		dev_err(cpu_dev, "failed to init cpufreq table: %d\n", ret);
-		goto put_reg;
+		goto out_free_opp;
 	}
 
 	/* Make imx6_soc_volt array's size same as arm opp number */
-	imx6_soc_volt = devm_kzalloc(cpu_dev, sizeof(*imx6_soc_volt) * num, GFP_KERNEL);
+	imx6_soc_volt = devm_kzalloc(&pdev->dev, sizeof(*imx6_soc_volt) * num, GFP_KERNEL);
 	if (imx6_soc_volt == NULL) {
 		ret = -ENOMEM;
 		goto free_freq_table;
@@ -339,7 +342,6 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 		goto free_freq_table;
 	}
 
-	of_node_put(np);
 	return 0;
 
 free_freq_table:
@@ -347,29 +349,9 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev)
 out_free_opp:
 	if (free_opp)
 		dev_pm_opp_of_remove_table(cpu_dev);
-put_reg:
-	if (!IS_ERR(arm_reg))
-		regulator_put(arm_reg);
-	if (!IS_ERR(pu_reg))
-		regulator_put(pu_reg);
-	if (!IS_ERR(soc_reg))
-		regulator_put(soc_reg);
-put_clk:
-	if (!IS_ERR(arm_clk))
-		clk_put(arm_clk);
-	if (!IS_ERR(pll1_sys_clk))
-		clk_put(pll1_sys_clk);
-	if (!IS_ERR(pll1_sw_clk))
-		clk_put(pll1_sw_clk);
-	if (!IS_ERR(step_clk))
-		clk_put(step_clk);
-	if (!IS_ERR(pll2_pfd2_396m_clk))
-		clk_put(pll2_pfd2_396m_clk);
-	if (!IS_ERR(pll2_bus_clk))
-		clk_put(pll2_bus_clk);
-	if (!IS_ERR(secondary_sel_clk))
-		clk_put(secondary_sel_clk);
-	of_node_put(np);
+put_node:
+	of_node_put(pdev->dev.of_node);
+	pdev->dev.of_node = NULL;
 	return ret;
 }
 
@@ -379,17 +361,8 @@ static int imx6q_cpufreq_remove(struct platform_device *pdev)
 	dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table);
 	if (free_opp)
 		dev_pm_opp_of_remove_table(cpu_dev);
-	regulator_put(arm_reg);
-	if (!IS_ERR(pu_reg))
-		regulator_put(pu_reg);
-	regulator_put(soc_reg);
-	clk_put(arm_clk);
-	clk_put(pll1_sys_clk);
-	clk_put(pll1_sw_clk);
-	clk_put(step_clk);
-	clk_put(pll2_pfd2_396m_clk);
-	clk_put(pll2_bus_clk);
-	clk_put(secondary_sel_clk);
+	of_node_put(pdev->dev.of_node);
+	pdev->dev.of_node = NULL;
 
 	return 0;
 }
-- 
2.7.4

Powered by blists - more mailing lists