linux-kernel - Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 28 Apr 2022 19:16:32 +0800
From:   Rex-BC Chen <rex-bc.chen@...iatek.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
CC:     <rafael@...nel.org>, <robh+dt@...nel.org>, <krzk+dt@...nel.org>,
        <matthias.bgg@...il.com>, <jia-wei.chang@...iatek.com>,
        <roger.lu@...iatek.com>, <hsinyi@...gle.com>,
        <khilman@...libre.com>, <angelogioacchino.delregno@...labora.com>,
        <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>,
        <Project_Global_Chrome_Upstream_Group@...iatek.com>
Subject: Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function

On Wed, 2022-04-27 at 08:41 +0530, Viresh Kumar wrote:
> On 26-04-22, 19:13, Rex-BC Chen wrote:
> > We have a non-upstream driver which tries to get frequency by
> > 'cpufreq_get'.
> 
> This is the right thing to do there.
> 
> > When we use that non-upstream driver, 'cpufreq_verify_current_freq'
> > will be further invoked by 'cpufreq_get' and it would cause voltage
> > pulse issue as I described previously.
> 
> I see this will eventually resolve to __cpufreq_driver_target(),
> which
> should return without any frequency updates.
> 

Hello Viresh,

Yes, the call stack will eventually go to __cpufreq_driver_target.
However, we can observe the mismatch between target_freq and policy-cur 
with a tiny difference.
e.g.
[ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
requested 500000 kHz
[ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

We check the assignment of policy->cur could be either from
cpufreq_driver->get_intermediate or from cpufreq_driver->get.
But it is strange to have the frequency value like 499999 kHz.
Is the result of tiny frequency difference expected from your point of
view?

> What do you mean by "voltage pulse" here? What actually happens which
> you want to avoid.
> 

When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
rising and falling phenomenon which can be observed if 'cpufreq_get' is
invoked.
>From top of view, if 'cpufreq_get' is NOT invoked in that condition,
the voltage pulse will no longer occur.
That's why we add this patch for this series.

> > Therefore, we apply the solution in this series.
> 
> I won't call it a solution but a Bug as .get() is supposed to read
> real freq of the hardware.
> 
> > Recently, we found that using 'cpufreq_generic_get' directly in our
> > non-upstream driver can do the same thing without pulse issue.
> 
> That would be an abuse of the cpufreq_generic_get() API. It is ONLY
> allowed to be used while setting .get callback in the driver.
> 

Thank you for sharing the correct information.
Is it possible to get frequency (API) a simple way, like get current
opp frequency?

BRs,
Rex