Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 25 Jul 2019 08:31:37 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Saravana Kannan <saravanak@google.com>
Cc:     MyungJoo Ham <myungjoo.ham@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
        Stephen Boyd <sboyd@kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Sibi Sankar <sibis@codeaurora.org>,
        Android Kernel Team <kernel-team@android.com>,
        Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 5/5] PM / devfreq: Add required OPPs support to
 passive governor
Message-ID: <20190725030137.uz22iwvdv37fsw56@vireshk-i7>
References: <20190717222340.137578-1-saravanak@google.com>
 <20190717222340.137578-6-saravanak@google.com>
 <20190723100406.7zchvflrmoaipxek@vireshk-i7>
 <CAGETcx89X0Xra+3HK+jbuCHXMgRL7QCwSShyMy7DY2Bg1eVjDQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAGETcx89X0Xra+3HK+jbuCHXMgRL7QCwSShyMy7DY2Bg1eVjDQ@mail.gmail.com>
User-Agent: NeoMutt/20180716-391-311a52
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 23-07-19, 17:26, Saravana Kannan wrote:
> On Tue, Jul 23, 2019 at 3:04 AM Viresh Kumar <viresh.kumar@linaro.org> 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