diff mbox series

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

Message ID 20200512125327.1868-5-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
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.

 drivers/opp/Kconfig    |   1 +
 drivers/opp/core.c     |  27 +++++++--
 drivers/opp/of.c       | 121 ++++++++++++++++++++++++++++++++++++++++-
 drivers/opp/opp.h      |   9 +++
 include/linux/pm_opp.h |  12 ++++
 5 files changed, 164 insertions(+), 6 deletions(-)

Comments

Matthias Kaehlcke May 12, 2020, 9:35 p.m. UTC | #1
On Tue, May 12, 2020 at 03:53:21PM +0300, 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.
> 
>  drivers/opp/Kconfig    |   1 +
>  drivers/opp/core.c     |  27 +++++++--
>  drivers/opp/of.c       | 121 ++++++++++++++++++++++++++++++++++++++++-
>  drivers/opp/opp.h      |   9 +++
>  include/linux/pm_opp.h |  12 ++++
>  5 files changed, 164 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
> index 35dfc7e80f92..230d2b84436c 100644
> --- a/drivers/opp/Kconfig
> +++ b/drivers/opp/Kconfig
> @@ -2,6 +2,7 @@
>  config PM_OPP
>  	bool
>  	select SRCU
> +	depends on INTERCONNECT || !INTERCONNECT
>  	---help---
>  	  SOCs have a standard set of tuples consisting of frequency and
>  	  voltage pairs that the device will support per voltage domain. This
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index ce7e4103ec09..a3dd0bc9b9f6 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -999,6 +999,12 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
>  				ret);
>  	}
>  
> +	/* Find interconnect path(s) for the device */
> +	ret = _of_find_icc_paths(opp_table, dev);
> +	if (ret)
> +		dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
> +			 __func__, ret);
> +
>  	BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
>  	INIT_LIST_HEAD(&opp_table->opp_list);
>  	kref_init(&opp_table->kref);
> @@ -1057,6 +1063,7 @@ static void _opp_table_kref_release(struct kref *kref)
>  {
>  	struct opp_table *opp_table = container_of(kref, struct opp_table, kref);
>  	struct opp_device *opp_dev, *temp;
> +	int i;
>  
>  	_of_clear_opp_table(opp_table);
>  
> @@ -1064,6 +1071,12 @@ static void _opp_table_kref_release(struct kref *kref)
>  	if (!IS_ERR(opp_table->clk))
>  		clk_put(opp_table->clk);
>  
> +	if (opp_table->paths) {
> +		for (i = 0; i < opp_table->path_count; i++)
> +			icc_put(opp_table->paths[i]);
> +		kfree(opp_table->paths);
> +	}
> +
>  	WARN_ON(!list_empty(&opp_table->opp_list));
>  
>  	list_for_each_entry_safe(opp_dev, temp, &opp_table->dev_list, node) {
> @@ -1243,19 +1256,22 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
>  struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>  {
>  	struct dev_pm_opp *opp;
> -	int count, supply_size;
> +	int supply_count, supply_size, icc_size;
>  
>  	/* Allocate space for at least one supply */
> -	count = table->regulator_count > 0 ? table->regulator_count : 1;
> -	supply_size = sizeof(*opp->supplies) * count;
> +	supply_count = table->regulator_count > 0 ? table->regulator_count : 1;
> +	supply_size = sizeof(*opp->supplies) * supply_count;
> +	icc_size = sizeof(*opp->bandwidth) * table->path_count;
>  
>  	/* allocate new OPP node and supplies structures */
> -	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
> +	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
> +
>  	if (!opp)
>  		return NULL;
>  
>  	/* Put the supplies at the end of the OPP structure as an empty array */
>  	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
> +	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
>  	INIT_LIST_HEAD(&opp->node);
>  
>  	return opp;
> @@ -1290,6 +1306,9 @@ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
>  {
>  	if (opp1->rate != opp2->rate)
>  		return opp1->rate < opp2->rate ? -1 : 1;
> +	if (opp1->bandwidth && opp2->bandwidth &&
> +	    opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
> +		return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 : 1;
>  	if (opp1->level != opp2->level)
>  		return opp1->level < opp2->level ? -1 : 1;
>  	return 0;
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index 5179f034751a..d139ad8c8f4f 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -332,6 +332,58 @@ 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)
> +{
> +	struct device_node *np;
> +	int ret, i, count, num_paths;
> +
> +	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;
> +	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
> +				   GFP_KERNEL);
> +	if (!opp_table->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]);
> +			if (ret != -EPROBE_DEFER) {
> +				dev_err(dev, "%s: Unable to get path%d: %d\n",
> +					__func__, i, ret);
> +			}
> +			goto err;
> +		}
> +	}
> +	opp_table->path_count = num_paths;
> +
> +	return 0;
> +
> +err:
> +	while (i--)
> +		icc_put(opp_table->paths[i]);
> +
> +	kfree(opp_table->paths);
> +	opp_table->paths = NULL;
> +
> +	return ret;
> +}
> +
>  static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
>  			      struct device_node *np)
>  {
> @@ -524,8 +576,11 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
>  static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
>  			 bool *rate_not_available)
>  {
> +	struct property *peak, *avg;
> +	u32 *peak_bw, *avg_bw;
>  	u64 rate;
> -	int ret;
> +	int ret, i, count;
> +	bool found = false;
>  
>  	ret = of_property_read_u64(np, "opp-hz", &rate);
>  	if (!ret) {
> @@ -535,10 +590,72 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
>  		 * bit guaranteed in clk API.
>  		 */
>  		new_opp->rate = (unsigned long)rate;
> +		found = true;
>  	}
>  	*rate_not_available = !!ret;
>  
> -	of_property_read_u32(np, "opp-level", &new_opp->level);
> +	peak = of_find_property(np, "opp-peak-kBps", NULL);
> +	if (peak) {
> +		/*
> +		 * Bandwidth consists of peak and average (optional) values:
> +		 * opp-peak-kBps = <path1_value path2_value>;
> +		 * opp-avg-kBps = <path1_value path2_value>;
> +		 */
> +		count = peak->length / sizeof(u32);
> +		peak_bw = kmalloc_array(count, sizeof(*peak_bw), GFP_KERNEL);
> +		if (!peak_bw)
> +			return -ENOMEM;
> +
> +		ret = of_property_read_u32_array(np, "opp-peak-kBps", peak_bw,
> +						 count);
> +		if (ret) {
> +			pr_err("%s: Error parsing opp-peak-kBps: %d\n",
> +			       __func__, ret);
> +			goto free_peak_bw;
> +		}
> +
> +		for (i = 0; i < count; i++)
> +			new_opp->bandwidth[i].peak = kBps_to_icc(peak_bw[i]);
> +
> +		found = true;
> +		kfree(peak_bw);
> +	}
> +
> +	avg = of_find_property(np, "opp-avg-kBps", NULL);
> +	if (peak && avg) {
> +		count = avg->length / sizeof(u32);
> +		avg_bw = kmalloc_array(count, sizeof(*avg_bw), GFP_KERNEL);
> +		if (!avg_bw) {
> +			ret = -ENOMEM;
> +			goto free_peak_bw;
> +		}
> +
> +		ret = of_property_read_u32_array(np, "opp-avg-kBps", avg_bw,
> +						 count);
> +		if (ret) {
> +			pr_err("%s: Error parsing opp-avg-kBps: %d\n",
> +			       __func__, ret);
> +			goto free_avg_bw;
> +		}
> +
> +		for (i = 0; i < count; i++)
> +			new_opp->bandwidth[i].avg = kBps_to_icc(avg_bw[i]);
> +
> +		kfree(avg_bw);
> +	}
> +
> +	if (!of_property_read_u32(np, "opp-level", &new_opp->level))
> +		found = true;
> +
> +	if (found)
> +		return 0;
> +
> +	return ret;
> +
> +free_avg_bw:
> +	kfree(avg_bw);
> +free_peak_bw:
> +	kfree(peak_bw);
>  
>  	return ret;
>  }
> diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
> index bcadb1e328a4..17d45119d9bc 100644
> --- a/drivers/opp/opp.h
> +++ b/drivers/opp/opp.h
> @@ -12,6 +12,7 @@
>  #define __DRIVER_OPP_H__
>  
>  #include <linux/device.h>
> +#include <linux/interconnect.h>
>  #include <linux/kernel.h>
>  #include <linux/kref.h>
>  #include <linux/list.h>
> @@ -59,6 +60,7 @@ extern struct list_head opp_tables;
>   * @rate:	Frequency in hertz
>   * @level:	Performance level
>   * @supplies:	Power supplies voltage/current values
> + * @bandwidth:	Interconnect bandwidth values
>   * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
>   *		frequency from any other OPP's frequency.
>   * @required_opps: List of OPPs that are required by this OPP.
> @@ -81,6 +83,7 @@ struct dev_pm_opp {
>  	unsigned int level;
>  
>  	struct dev_pm_opp_supply *supplies;
> +	struct dev_pm_opp_icc_bw *bandwidth;
>  
>  	unsigned long clock_latency_ns;
>  
> @@ -146,6 +149,8 @@ enum opp_table_access {
>   * @regulator_count: Number of power supply regulators. Its value can be -1
>   * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
>   * property).
> + * @paths: Interconnect path handles
> + * @path_count: Number of interconnect paths
>   * @genpd_performance_state: Device's power domain support performance state.
>   * @is_genpd: Marks if the OPP table belongs to a genpd.
>   * @set_opp: Platform specific set_opp callback
> @@ -189,6 +194,8 @@ struct opp_table {
>  	struct clk *clk;
>  	struct regulator **regulators;
>  	int regulator_count;
> +	struct icc_path **paths;
> +	unsigned int path_count;
>  	bool genpd_performance_state;
>  	bool is_genpd;
>  
> @@ -224,12 +231,14 @@ 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 747861816f4f..cfceb0290401 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -41,6 +41,18 @@ struct dev_pm_opp_supply {
>  	unsigned long u_amp;
>  };
>  
> +/**
> + * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
> + * @avg:	Average bandwidth corresponding to this OPP (in icc units)
> + * @peak:	Peak bandwidth corresponding to this OPP (in icc units)
> + *
> + * This structure stores the bandwidth values for a single interconnect path.
> + */
> +struct dev_pm_opp_icc_bw {
> +	u32 avg;
> +	u32 peak;
> +};
> +
>  /**
>   * struct dev_pm_opp_info - OPP freq/voltage/current values
>   * @rate:	Target clk rate in hz

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Viresh Kumar May 13, 2020, 6:41 a.m. UTC | #2
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 Kumar May 13, 2020, 7:01 a.m. UTC | #3
On 13-05-20, 12:11, Viresh Kumar wrote:
> 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:

+this :)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 15f30ed70bbc..06e38f95116c 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -336,7 +336,7 @@ 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;
+       int ret = 0, i, count, num_paths;
        struct icc_path **paths;
 
        np = of_node_get(dev->of_node);
@@ -375,10 +375,9 @@ int dev_pm_opp_of_find_icc_paths(struct device *dev,
        if (opp_table) {
                opp_table->paths = paths;
                opp_table->path_count = num_paths;
+               return 0;
        }
 
-       return 0;
-
 err:
        while (i--)
                icc_put(paths[i]);
Viresh Kumar May 29, 2020, 4:44 a.m. UTC | #4
On 12-05-20, 15:53, Georgi Djakov wrote:
>  struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>  {
>  	struct dev_pm_opp *opp;
> -	int count, supply_size;
> +	int supply_count, supply_size, icc_size;
>  
>  	/* Allocate space for at least one supply */
> -	count = table->regulator_count > 0 ? table->regulator_count : 1;
> -	supply_size = sizeof(*opp->supplies) * count;
> +	supply_count = table->regulator_count > 0 ? table->regulator_count : 1;
> +	supply_size = sizeof(*opp->supplies) * supply_count;
> +	icc_size = sizeof(*opp->bandwidth) * table->path_count;
>  
>  	/* allocate new OPP node and supplies structures */
> -	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
> +	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
> +
>  	if (!opp)
>  		return NULL;
>  
>  	/* Put the supplies at the end of the OPP structure as an empty array */
>  	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
> +	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
>  	INIT_LIST_HEAD(&opp->node);

Added this delta here.

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 7302f2631f8d..dfbd3d10410c 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1330,7 +1330,8 @@ struct dev_pm_opp *_opp_allocate(struct opp_table *table)
 
        /* Put the supplies at the end of the OPP structure as an empty array */
        opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
-       opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
+       if (icc_size)
+               opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
        INIT_LIST_HEAD(&opp->node);
 
        return opp;
Sibi Sankar May 29, 2020, 11:39 a.m. UTC | #5
On 2020-05-29 10:14, Viresh Kumar wrote:
> On 12-05-20, 15:53, Georgi Djakov wrote:
>>  struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>>  {
>>  	struct dev_pm_opp *opp;
>> -	int count, supply_size;
>> +	int supply_count, supply_size, icc_size;
>> 
>>  	/* Allocate space for at least one supply */
>> -	count = table->regulator_count > 0 ? table->regulator_count : 1;
>> -	supply_size = sizeof(*opp->supplies) * count;
>> +	supply_count = table->regulator_count > 0 ? table->regulator_count : 
>> 1;
>> +	supply_size = sizeof(*opp->supplies) * supply_count;
>> +	icc_size = sizeof(*opp->bandwidth) * table->path_count;
>> 
>>  	/* allocate new OPP node and supplies structures */
>> -	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
>> +	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
>> +
>>  	if (!opp)
>>  		return NULL;
>> 
>>  	/* Put the supplies at the end of the OPP structure as an empty 
>> array */
>>  	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
>> +	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + 
>> supply_count);
>>  	INIT_LIST_HEAD(&opp->node);
> 
> Added this delta here.
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 7302f2631f8d..dfbd3d10410c 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1330,7 +1330,8 @@ struct dev_pm_opp *_opp_allocate(struct opp_table 
> *table)
> 
>         /* Put the supplies at the end of the OPP structure as an empty 
> array */
>         opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
> -       opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies +
> supply_count);
> +       if (icc_size)
> +               opp->bandwidth = (struct dev_pm_opp_icc_bw
> *)(opp->supplies + supply_count);

nice catch!
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>

>         INIT_LIST_HEAD(&opp->node);
> 
>         return opp;
diff mbox series

Patch

diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
index 35dfc7e80f92..230d2b84436c 100644
--- a/drivers/opp/Kconfig
+++ b/drivers/opp/Kconfig
@@ -2,6 +2,7 @@ 
 config PM_OPP
 	bool
 	select SRCU
+	depends on INTERCONNECT || !INTERCONNECT
 	---help---
 	  SOCs have a standard set of tuples consisting of frequency and
 	  voltage pairs that the device will support per voltage domain. This
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index ce7e4103ec09..a3dd0bc9b9f6 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -999,6 +999,12 @@  static struct opp_table *_allocate_opp_table(struct device *dev, int index)
 				ret);
 	}
 
+	/* Find interconnect path(s) for the device */
+	ret = _of_find_icc_paths(opp_table, dev);
+	if (ret)
+		dev_warn(dev, "%s: Error finding interconnect paths: %d\n",
+			 __func__, ret);
+
 	BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
 	INIT_LIST_HEAD(&opp_table->opp_list);
 	kref_init(&opp_table->kref);
@@ -1057,6 +1063,7 @@  static void _opp_table_kref_release(struct kref *kref)
 {
 	struct opp_table *opp_table = container_of(kref, struct opp_table, kref);
 	struct opp_device *opp_dev, *temp;
+	int i;
 
 	_of_clear_opp_table(opp_table);
 
@@ -1064,6 +1071,12 @@  static void _opp_table_kref_release(struct kref *kref)
 	if (!IS_ERR(opp_table->clk))
 		clk_put(opp_table->clk);
 
+	if (opp_table->paths) {
+		for (i = 0; i < opp_table->path_count; i++)
+			icc_put(opp_table->paths[i]);
+		kfree(opp_table->paths);
+	}
+
 	WARN_ON(!list_empty(&opp_table->opp_list));
 
 	list_for_each_entry_safe(opp_dev, temp, &opp_table->dev_list, node) {
@@ -1243,19 +1256,22 @@  EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
 struct dev_pm_opp *_opp_allocate(struct opp_table *table)
 {
 	struct dev_pm_opp *opp;
-	int count, supply_size;
+	int supply_count, supply_size, icc_size;
 
 	/* Allocate space for at least one supply */
-	count = table->regulator_count > 0 ? table->regulator_count : 1;
-	supply_size = sizeof(*opp->supplies) * count;
+	supply_count = table->regulator_count > 0 ? table->regulator_count : 1;
+	supply_size = sizeof(*opp->supplies) * supply_count;
+	icc_size = sizeof(*opp->bandwidth) * table->path_count;
 
 	/* allocate new OPP node and supplies structures */
-	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
+
 	if (!opp)
 		return NULL;
 
 	/* Put the supplies at the end of the OPP structure as an empty array */
 	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count);
 	INIT_LIST_HEAD(&opp->node);
 
 	return opp;
@@ -1290,6 +1306,9 @@  int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2)
 {
 	if (opp1->rate != opp2->rate)
 		return opp1->rate < opp2->rate ? -1 : 1;
+	if (opp1->bandwidth && opp2->bandwidth &&
+	    opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
+		return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 : 1;
 	if (opp1->level != opp2->level)
 		return opp1->level < opp2->level ? -1 : 1;
 	return 0;
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 5179f034751a..d139ad8c8f4f 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -332,6 +332,58 @@  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)
+{
+	struct device_node *np;
+	int ret, i, count, num_paths;
+
+	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;
+	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
+				   GFP_KERNEL);
+	if (!opp_table->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]);
+			if (ret != -EPROBE_DEFER) {
+				dev_err(dev, "%s: Unable to get path%d: %d\n",
+					__func__, i, ret);
+			}
+			goto err;
+		}
+	}
+	opp_table->path_count = num_paths;
+
+	return 0;
+
+err:
+	while (i--)
+		icc_put(opp_table->paths[i]);
+
+	kfree(opp_table->paths);
+	opp_table->paths = NULL;
+
+	return ret;
+}
+
 static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 			      struct device_node *np)
 {
@@ -524,8 +576,11 @@  EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
 static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 			 bool *rate_not_available)
 {
+	struct property *peak, *avg;
+	u32 *peak_bw, *avg_bw;
 	u64 rate;
-	int ret;
+	int ret, i, count;
+	bool found = false;
 
 	ret = of_property_read_u64(np, "opp-hz", &rate);
 	if (!ret) {
@@ -535,10 +590,72 @@  static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 		 * bit guaranteed in clk API.
 		 */
 		new_opp->rate = (unsigned long)rate;
+		found = true;
 	}
 	*rate_not_available = !!ret;
 
-	of_property_read_u32(np, "opp-level", &new_opp->level);
+	peak = of_find_property(np, "opp-peak-kBps", NULL);
+	if (peak) {
+		/*
+		 * Bandwidth consists of peak and average (optional) values:
+		 * opp-peak-kBps = <path1_value path2_value>;
+		 * opp-avg-kBps = <path1_value path2_value>;
+		 */
+		count = peak->length / sizeof(u32);
+		peak_bw = kmalloc_array(count, sizeof(*peak_bw), GFP_KERNEL);
+		if (!peak_bw)
+			return -ENOMEM;
+
+		ret = of_property_read_u32_array(np, "opp-peak-kBps", peak_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-peak-kBps: %d\n",
+			       __func__, ret);
+			goto free_peak_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].peak = kBps_to_icc(peak_bw[i]);
+
+		found = true;
+		kfree(peak_bw);
+	}
+
+	avg = of_find_property(np, "opp-avg-kBps", NULL);
+	if (peak && avg) {
+		count = avg->length / sizeof(u32);
+		avg_bw = kmalloc_array(count, sizeof(*avg_bw), GFP_KERNEL);
+		if (!avg_bw) {
+			ret = -ENOMEM;
+			goto free_peak_bw;
+		}
+
+		ret = of_property_read_u32_array(np, "opp-avg-kBps", avg_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-avg-kBps: %d\n",
+			       __func__, ret);
+			goto free_avg_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].avg = kBps_to_icc(avg_bw[i]);
+
+		kfree(avg_bw);
+	}
+
+	if (!of_property_read_u32(np, "opp-level", &new_opp->level))
+		found = true;
+
+	if (found)
+		return 0;
+
+	return ret;
+
+free_avg_bw:
+	kfree(avg_bw);
+free_peak_bw:
+	kfree(peak_bw);
 
 	return ret;
 }
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index bcadb1e328a4..17d45119d9bc 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -12,6 +12,7 @@ 
 #define __DRIVER_OPP_H__
 
 #include <linux/device.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
@@ -59,6 +60,7 @@  extern struct list_head opp_tables;
  * @rate:	Frequency in hertz
  * @level:	Performance level
  * @supplies:	Power supplies voltage/current values
+ * @bandwidth:	Interconnect bandwidth values
  * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
  *		frequency from any other OPP's frequency.
  * @required_opps: List of OPPs that are required by this OPP.
@@ -81,6 +83,7 @@  struct dev_pm_opp {
 	unsigned int level;
 
 	struct dev_pm_opp_supply *supplies;
+	struct dev_pm_opp_icc_bw *bandwidth;
 
 	unsigned long clock_latency_ns;
 
@@ -146,6 +149,8 @@  enum opp_table_access {
  * @regulator_count: Number of power supply regulators. Its value can be -1
  * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
  * property).
+ * @paths: Interconnect path handles
+ * @path_count: Number of interconnect paths
  * @genpd_performance_state: Device's power domain support performance state.
  * @is_genpd: Marks if the OPP table belongs to a genpd.
  * @set_opp: Platform specific set_opp callback
@@ -189,6 +194,8 @@  struct opp_table {
 	struct clk *clk;
 	struct regulator **regulators;
 	int regulator_count;
+	struct icc_path **paths;
+	unsigned int path_count;
 	bool genpd_performance_state;
 	bool is_genpd;
 
@@ -224,12 +231,14 @@  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 747861816f4f..cfceb0290401 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -41,6 +41,18 @@  struct dev_pm_opp_supply {
 	unsigned long u_amp;
 };
 
+/**
+ * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
+ * @avg:	Average bandwidth corresponding to this OPP (in icc units)
+ * @peak:	Peak bandwidth corresponding to this OPP (in icc units)
+ *
+ * This structure stores the bandwidth values for a single interconnect path.
+ */
+struct dev_pm_opp_icc_bw {
+	u32 avg;
+	u32 peak;
+};
+
 /**
  * struct dev_pm_opp_info - OPP freq/voltage/current values
  * @rate:	Target clk rate in hz