Re: [PATCH] OPP: Check for bandwidth values before creating icc paths

From: Sibi Sankar
Date: Fri May 29 2020 - 10:17:27 EST


On 2020-05-29 10:50, Viresh Kumar wrote:
On 28-05-20, 00:54, Sibi Sankar wrote:
Prevent the core from creating and voting on icc paths when the
opp-table does not have the bandwidth values populated. Currently
this check is performed on the first OPP node.

Signed-off-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
---
drivers/opp/of.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 61fce1284f012..95cf6f1312765 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -338,6 +338,21 @@ int dev_pm_opp_of_find_icc_paths(struct device *dev,
struct device_node *np;
int ret = 0, i, count, num_paths;
struct icc_path **paths;
+ struct property *prop;
+
+ /* Check for bandwidth values on the first OPP node */
+ if (opp_table && opp_table->np) {
+ np = of_get_next_available_child(opp_table->np, NULL);
+ if (!np) {
+ dev_err(dev, "Empty OPP table\n");
+ return 0;
+ }
+
+ prop = of_find_property(np, "opp-peak-kBps", NULL);
+ of_node_put(np);
+ if (!prop || !prop->length)
+ return 0;
+ }

This doesn't support the call made from cpufreq-dt driver. Pushed
this, please give this a try:

Viresh,
Thanks for the patch!


From: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
Date: Thu, 28 May 2020 00:54:18 +0530
Subject: [PATCH] opp: Don't parse icc paths unnecessarily

The DT node of the device may contain interconnect paths while the OPP
table doesn't have the bandwidth values. There is no need to parse the
paths in such cases.

Signed-off-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
[ Viresh: Support the case of !opp_table and massaged changelog ]
Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
---
drivers/opp/of.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 61fce1284f01..8c1bf01f0e50 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -332,13 +332,56 @@ static int _of_opp_alloc_required_opps(struct
opp_table *opp_table,
return ret;
}

+static int _bandwidth_supported(struct device *dev, struct opp_table
*opp_table)
+{
+ struct device_node *np, *opp_np;
+ struct property *prop;
+
+ if (!opp_table) {
+ np = of_node_get(dev->of_node);
+ if (!np)
+ return -ENODEV;
+
+ opp_np = _opp_of_get_opp_desc_node(np, 0);
+ of_node_put(np);
+
+ /* Lets not fail in case we are parsing opp-v1 bindings */
+ if (!opp_np)
+ return 0;
+ } else {
+ opp_np = of_node_get(opp_table->np);

opp_np needs to be subjected
to NULL check as well. Lets
move "if (!opp_np)" to outside
the if/else. With the above
change in place:

Tested-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>
Reviewed-by: Sibi Sankar <sibis@xxxxxxxxxxxxxx>

+ }
+
+ /* Checking only first OPP is sufficient */
+ np = of_get_next_available_child(opp_np, NULL);
+ if (!np) {
+ dev_err(dev, "OPP table empty\n");
+ return -EINVAL;
+ }
+ of_node_put(opp_np);
+
+ prop = of_find_property(np, "opp-peak-kBps", NULL);
+ of_node_put(np);
+
+ if (!prop || !prop->length)
+ return 0;
+
+ return 1;
+}
+
int dev_pm_opp_of_find_icc_paths(struct device *dev,
struct opp_table *opp_table)
{
struct device_node *np;
- int ret = 0, i, count, num_paths;
+ int ret, i, count, num_paths;
struct icc_path **paths;

+ ret = _bandwidth_supported(dev, opp_table);
+ if (ret <= 0)
+ return ret;
+
+ ret = 0;
+
np = of_node_get(dev->of_node);
if (!np)
return 0;

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.