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

From: Sibi Sankar
Date: Mon Jun 01 2020 - 06:01:08 EST


On 2020-06-01 12:43, Viresh Kumar wrote:
On 01-06-20, 12:09, Sibi Sankar wrote:
On 2020-06-01 09:37, Viresh Kumar wrote:
> On 29-05-20, 19:47, Sibi Sankar wrote:
> > opp_np needs to be subjected
> > to NULL check as well.
>
> No, it isn't. It should already be valid and is set by the OPP core.
> Actually we don't need to do of_node_get(opp_table->np) and just use
> np, I did that to not have a special case while putting the resource.
>

I should have phrased it differently.
opp_np needs to be checked to deal
with cases where devices don't have
"operating-points-v2" associated with
it.

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index a5d87ca0ab571..06976d14e6ccb 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -344,14 +344,14 @@ static int _bandwidth_supported(struct device *dev,
struct opp_table *opp_table)

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);
}

+ /* Lets not fail in case we are parsing opp-v1 bindings */
+ if (!opp_np)
+ return 0;
+

sdhci_msm 7c4000.sdhci: OPP table empty
sdhci_msm 7c4000.sdhci: _allocate_opp_table: Error finding interconnect
paths: -22

I see the following errors without
the check.

My reply unfortunately only considered the case where this routine was
called from within the opp table. Are you testing it for the case
where you are adding OPPs dynamically from the code ?

Yeah dev_pm_opp_add/dev_pm_opp_set_clkname
or pretty much any api doing a
dev_pm_opp_get_opp_table without
a opp_table node associated with
it will run into this issue.

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