diff mbox series

[v8,08/10] cpufreq: dt: Validate all interconnect paths

Message ID 20200512125327.1868-9-georgi.djakov@linaro.org (mailing list archive)
State New, archived
Delegated to: viresh kumar
Headers show
Series Introduce OPP bandwidth bindings | expand

Commit Message

Georgi Djakov May 12, 2020, 12:53 p.m. UTC
Currently when we check for the available resources, we assume that there
is only one interconnect path, but in fact it could be more than one. Do
some validation to determine the number of paths and verify if each one
of them is available.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
v8:
* New patch.

 drivers/cpufreq/cpufreq-dt.c | 49 ++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 5 deletions(-)

Comments

Matthias Kaehlcke May 12, 2020, 9:47 p.m. UTC | #1
Hi Georgi,

On Tue, May 12, 2020 at 03:53:25PM +0300, Georgi Djakov wrote:
> Currently when we check for the available resources, we assume that there
> is only one interconnect path, but in fact it could be more than one. Do
> some validation to determine the number of paths and verify if each one
> of them is available.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> v8:
> * New patch.
> 
>  drivers/cpufreq/cpufreq-dt.c | 49 ++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
> index 4ecef3257532..3dd28c2c1633 100644
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -91,12 +91,54 @@ static const char *find_supply_name(struct device *dev)
>  	return name;
>  }
>  
> +static int find_icc_paths(struct device *dev)
> +{
> +	struct device_node *np;
> +	struct icc_path **paths;
> +	int i, count, num_paths;
> +	int ret = 0;
> +
> +	np = of_node_get(dev->of_node);
> +	if (!np)
> +		return 0;
> +
> +	count = of_count_phandle_with_args(np, "interconnects",
> +					   "#interconnect-cells");
> +	of_node_put(np);
> +	if (count < 0)
> +		return 0;
> +
> +	/* two phandles when #interconnect-cells = <1> */
> +	if (count % 2) {
> +		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	num_paths = count / 2;
> +	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
> +	if (!paths)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < num_paths; i++) {
> +		paths[i] = of_icc_get_by_index(dev, i);
> +		ret = PTR_ERR_OR_ZERO(paths[i]);
> +		if (ret)
> +			break;
> +	}
> +
> +	while (i--)
> +		icc_put(paths[i]);

Since the function only does a validation and throws the paths away
afterwards you don't really need the dynamic allocation and 'icc_put'
loop. Just have a single 'struct icc_path' pointer and call icc_put()
inside the for loop.
Viresh Kumar May 13, 2020, 6:42 a.m. UTC | #2
On 12-05-20, 15:53, Georgi Djakov wrote:
> Currently when we check for the available resources, we assume that there
> is only one interconnect path, but in fact it could be more than one. Do
> some validation to determine the number of paths and verify if each one
> of them is available.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> v8:
> * New patch.

Merged this with the 7th patch and applied this delta:

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 15d70112454c..79742bbd221f 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -13,7 +13,6 @@
 #include <linux/cpufreq.h>
 #include <linux/cpumask.h>
 #include <linux/err.h>
-#include <linux/interconnect.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pm_opp.h>
@@ -91,49 +90,6 @@ static const char *find_supply_name(struct device *dev)
 	return name;
 }
 
-static int find_icc_paths(struct device *dev)
-{
-	struct device_node *np;
-	struct icc_path **paths;
-	int i, count, num_paths;
-	int ret = 0;
-
-	np = of_node_get(dev->of_node);
-	if (!np)
-		return 0;
-
-	count = of_count_phandle_with_args(np, "interconnects",
-					   "#interconnect-cells");
-	of_node_put(np);
-	if (count < 0)
-		return 0;
-
-	/* two phandles when #interconnect-cells = <1> */
-	if (count % 2) {
-		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
-		return -EINVAL;
-	}
-
-	num_paths = count / 2;
-	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
-	if (!paths)
-		return -ENOMEM;
-
-	for (i = 0; i < num_paths; i++) {
-		paths[i] = of_icc_get_by_index(dev, i);
-		ret = PTR_ERR_OR_ZERO(paths[i]);
-		if (ret)
-			break;
-	}
-
-	while (i--)
-		icc_put(paths[i]);
-
-	kfree(paths);
-
-	return ret;
-}
-
 static int resources_available(void)
 {
 	struct device *cpu_dev;
@@ -165,15 +121,9 @@ static int resources_available(void)
 
 	clk_put(cpu_clk);
 
-	ret = find_icc_paths(cpu_dev);
-	if (ret) {
-		if (ret == -EPROBE_DEFER)
-			dev_dbg(cpu_dev, "defer icc path: %d\n", ret);
-		else
-			dev_err(cpu_dev, "failed to get icc path: %d\n", ret);
-
+	ret = dev_pm_opp_of_find_icc_paths(cpu_dev, NULL);
+	if (ret)
 		return ret;
-	}
 
 	name = find_supply_name(cpu_dev);
 	/* Platform doesn't require regulator */
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c
index 4ecef3257532..3dd28c2c1633 100644
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -91,12 +91,54 @@  static const char *find_supply_name(struct device *dev)
 	return name;
 }
 
+static int find_icc_paths(struct device *dev)
+{
+	struct device_node *np;
+	struct icc_path **paths;
+	int i, count, num_paths;
+	int ret = 0;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return 0;
+
+	count = of_count_phandle_with_args(np, "interconnects",
+					   "#interconnect-cells");
+	of_node_put(np);
+	if (count < 0)
+		return 0;
+
+	/* two phandles when #interconnect-cells = <1> */
+	if (count % 2) {
+		dev_err(dev, "%s: Invalid interconnects values\n", __func__);
+		return -EINVAL;
+	}
+
+	num_paths = count / 2;
+	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
+	if (!paths)
+		return -ENOMEM;
+
+	for (i = 0; i < num_paths; i++) {
+		paths[i] = of_icc_get_by_index(dev, i);
+		ret = PTR_ERR_OR_ZERO(paths[i]);
+		if (ret)
+			break;
+	}
+
+	while (i--)
+		icc_put(paths[i]);
+
+	kfree(paths);
+
+	return ret;
+}
+
 static int resources_available(void)
 {
 	struct device *cpu_dev;
 	struct regulator *cpu_reg;
 	struct clk *cpu_clk;
-	struct icc_path *cpu_path;
 	int ret = 0;
 	const char *name;
 
@@ -123,8 +165,7 @@  static int resources_available(void)
 
 	clk_put(cpu_clk);
 
-	cpu_path = of_icc_get(cpu_dev, NULL);
-	ret = PTR_ERR_OR_ZERO(cpu_path);
+	ret = find_icc_paths(cpu_dev);
 	if (ret) {
 		if (ret == -EPROBE_DEFER)
 			dev_dbg(cpu_dev, "defer icc path: %d\n", ret);
@@ -134,8 +175,6 @@  static int resources_available(void)
 		return ret;
 	}
 
-	icc_put(cpu_path);
-
 	name = find_supply_name(cpu_dev);
 	/* Platform doesn't require regulator */
 	if (!name)