lkml.org 
[lkml]   [2020]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] OPP: Check for bandwidth values before creating icc paths
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@codeaurora.org>
>> ---
>> 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@codeaurora.org>
> 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@codeaurora.org>
> [ Viresh: Support the case of !opp_table and massaged changelog ]
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> 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@codeaurora.org>
Reviewed-by: Sibi Sankar <sibis@codeaurora.org>

> + }
> +
> + /* 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.

\
 
 \ /
  Last update: 2020-05-29 16:19    [W:0.086 / U:0.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site