Re: [PATCH v3 5/5] PM / devfreq: Add required OPPs support to passive governor

[Viresh Kumar]

On 23-07-19, 17:26, Saravana Kannan wrote:
> On Tue, Jul 23, 2019 at 3:04 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> > On 17-07-19, 15:23, Saravana Kannan wrote:
> > >       /*
> > > @@ -56,13 +56,20 @@ static int devfreq_passive_get_target_freq(struct devfreq *devfreq,
> > >        * list of parent device. Because in this case, *freq is temporary
> > >        * value which is decided by ondemand governor.
> > >        */
> > > -     opp = devfreq_recommended_opp(parent_devfreq->dev.parent, freq, 0);
> > > -     if (IS_ERR(opp)) {
> > > -             ret = PTR_ERR(opp);
> > > +     p_opp = devfreq_recommended_opp(parent_devfreq->dev.parent, freq, 0);
> > > +     if (IS_ERR(p_opp)) {
> > > +             ret = PTR_ERR(p_opp);
> > >               goto out;
> > >       }
> > >
> > > -     dev_pm_opp_put(opp);
> > > +     if (devfreq->opp_table && parent_devfreq->opp_table)
> > > +             opp = dev_pm_opp_xlate_opp(parent_devfreq->opp_table,
> > > +                                        devfreq->opp_table, p_opp);
> >
> > you put p_opp right here.

What about this comment ?

> >
> > Also shouldn't you try to get p_opp under the above if block only? As
> > that is the only user of it ?
> 
> No, p_opp (used to be called opp) was used even before my changes. If
> there's no required-opps mapping this falls back to assuming the slave
> device OPP to pick should be the same index as the master device's
> opp.
> 
> So I believe this patch is correct as-is.

Right.

-- 
viresh