Re: [PATCH v8 04/10] OPP: Add support for parsing interconnect bandwidth

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Georgi Djakov <georgi.djakov@linaro.org>
Cc: vireshk@kernel.org, nm@ti.com, sboyd@kernel.org,
	rjw@rjwysocki.net, saravanak@google.com, sibis@codeaurora.org,
	mka@chromium.org, robh+dt@kernel.org, rnayak@codeaurora.org,
	bjorn.andersson@linaro.org, vincent.guittot@linaro.org,
	jcrouse@codeaurora.org, evgreen@chromium.org,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 04/10] OPP: Add support for parsing interconnect bandwidth
Date: Wed, 13 May 2020 12:11:39 +0530	[thread overview]
Message-ID: <20200513064139.5kxjjddcooqcvny3@vireshk-i7> (raw)
In-Reply-To: <20200512125327.1868-5-georgi.djakov@linaro.org>

On 12-05-20, 15:53, Georgi Djakov wrote:
> The OPP bindings now support bandwidth values, so add support to parse it
> from device tree and store it into the new dev_pm_opp_icc_bw struct, which
> is part of the dev_pm_opp.
> 
> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> ---
> v8:
> * Drop bandwidth requests and free memory in _opp_table_kref_release.
> * Take into account the supply_count in struct size calculations.
> * Free the temporary arrays for peak and average bandwidth.
> * Fix the check for opp-level.
> * Use dev_warn insted of dev_dbg.
> * Rename _of_find_icc_paths to _of_find_icc_paths.
> * Rename the variable count to supply_count.

Added this delta to this patch:

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index a3dd0bc9b9f6..8b640eac86d5 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1000,7 +1000,7 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 	}
 
 	/* Find interconnect path(s) for the device */
-	ret = _of_find_icc_paths(opp_table, dev);
+	ret = dev_pm_opp_of_find_icc_paths(dev, opp_table);
 	if (ret)
 		dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
 			 __func__, ret);
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index a41c0dc2ac04..15f30ed70bbc 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -332,10 +332,12 @@ static int _of_opp_alloc_required_opps(struct opp_table *opp_table,
 	return ret;
 }
 
-int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
+int dev_pm_opp_of_find_icc_paths(struct device *dev,
+				 struct opp_table *opp_table)
 {
 	struct device_node *np;
 	int ret, i, count, num_paths;
+	struct icc_path **paths;
 
 	np = of_node_get(dev->of_node);
 	if (!np)
@@ -354,15 +356,14 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 	}
 
 	num_paths = count / 2;
-	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
-				   GFP_KERNEL);
-	if (!opp_table->paths)
+	paths = kcalloc(num_paths, sizeof(*paths), GFP_KERNEL);
+	if (!paths)
 		return -ENOMEM;
 
 	for (i = 0; i < num_paths; i++) {
-		opp_table->paths[i] = of_icc_get_by_index(dev, i);
-		if (IS_ERR(opp_table->paths[i])) {
-			ret = PTR_ERR(opp_table->paths[i]);
+		paths[i] = of_icc_get_by_index(dev, i);
+		if (IS_ERR(paths[i])) {
+			ret = PTR_ERR(paths[i]);
 			if (ret != -EPROBE_DEFER) {
 				dev_err(dev, "%s: Unable to get path%d: %d\n",
 					__func__, i, ret);
@@ -370,19 +371,23 @@ int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev)
 			goto err;
 		}
 	}
-	opp_table->path_count = num_paths;
+
+	if (opp_table) {
+		opp_table->paths = paths;
+		opp_table->path_count = num_paths;
+	}
 
 	return 0;
 
 err:
 	while (i--)
-		icc_put(opp_table->paths[i]);
+		icc_put(paths[i]);
 
-	kfree(opp_table->paths);
-	opp_table->paths = NULL;
+	kfree(paths);
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(dev_pm_opp_of_find_icc_paths);
 
 static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 			      struct device_node *np)
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 17d45119d9bc..2b81ffef1ba4 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -231,14 +231,12 @@ void _of_clear_opp_table(struct opp_table *opp_table);
 struct opp_table *_managed_opp(struct device *dev, int index);
 void _of_opp_free_required_opps(struct opp_table *opp_table,
 				struct dev_pm_opp *opp);
-int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev);
 #else
 static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
 static inline void _of_clear_opp_table(struct opp_table *opp_table) {}
 static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
 static inline void _of_opp_free_required_opps(struct opp_table *opp_table,
 					      struct dev_pm_opp *opp) {}
-static inline int _of_find_icc_paths(struct opp_table *opp_table, struct device *dev) { return 0; }
 #endif
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index cfceb0290401..d5c4a329321d 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -372,6 +372,7 @@ int dev_pm_opp_of_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpuma
 struct device_node *dev_pm_opp_of_get_opp_desc_node(struct device *dev);
 struct device_node *dev_pm_opp_get_of_node(struct dev_pm_opp *opp);
 int of_get_required_opp_performance_state(struct device_node *np, int index);
+int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_table *opp_table);
 void dev_pm_opp_of_register_em(struct cpumask *cpus);
 #else
 static inline int dev_pm_opp_of_add_table(struct device *dev)
@@ -420,6 +421,11 @@ static inline int of_get_required_opp_performance_state(struct device_node *np,
 {
 	return -ENOTSUPP;
 }
+
+static inline int dev_pm_opp_of_find_icc_paths(struct device *dev, struct opp_table *opp_table)
+{
+	return -ENOTSUPP;
+}
 #endif
 
 #endif		/* __LINUX_OPP_H__ */

-- 
viresh
