linux-kernel - Re: [v5, PATCH 4/5] cpufreq: mediatek: add opp notification for SVS support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20191210064319.f4ksrxozp3gv4xry@vireshk-i7>
Date:   Tue, 10 Dec 2019 12:13:19 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     "andrew-sh.cheng" <andrew-sh.cheng@...iatek.com>
Cc:     Mark Rutland <mark.rutland@....com>, Nishanth Menon <nm@...com>,
        srv_heupstream@...iatek.com, linux-pm@...r.kernel.org,
        Stephen Boyd <sboyd@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        linux-kernel@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        fan.chen@...iatek.com, devicetree@...r.kernel.org
Subject: Re: [v5, PATCH 4/5] cpufreq: mediatek: add opp notification for SVS
 support

On 09-12-19, 14:56, andrew-sh.cheng wrote:
> On Wed, 2019-11-27 at 14:06 +0530, Viresh Kumar wrote:
> > On 26-11-19, 19:50, Andrew-sh.Cheng wrote:
> > > +		if (!IS_ERR(opp_item))
> > > +			dev_pm_opp_put(opp_item);
> > > +		else
> > > +			freq = 0;
> > > +
> > 
> > What is the purpose of the above code ?
> When dev_pm_opp_find_freq_ceil() doesn't find matching opp item, freq
> value won't be set.
> Set it as 0 for below checking
> > 
> > > +		/* case of current opp is disabled */
> > > +		if (freq == 0 || freq != info->opp_freq) {
> > > +			// find an enable opp item
> > > +			freq = 1;
> > > +			opp_item = dev_pm_opp_find_freq_ceil(info->cpu_dev,
> > > +							     &freq);
> > > +			if (!IS_ERR(opp_item)) {
> > > +				dev_pm_opp_put(opp_item);
> > > +				policy = cpufreq_cpu_get(info->opp_cpu);
> > > +				if (policy) {
> > > +					cpufreq_driver_target(policy,
> > > +						freq / 1000,
> > > +						CPUFREQ_RELATION_L);
> > 
> > Why don't you simply call this instead of all the code in the else
> > block ?
> These else code is used to check "current opp item is disabled or not".
> If not, do nothing.
> If current opp item is disabled, need to find an not-disabled opp item,
> and set frequency to it.

Right. So this notifier helper of yours receive the opp which is getting
disabled, why don't you compare its frequency directly to see if the current OPP
is getting disabled ?

-- 
viresh