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

[Viresh Kumar]

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 ?

-- 
viresh