linux-kernel - Re: [PATCH 2/2] OPP: Call dev_pm_opp_set

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231025152000.vt42gvti2e4rewwc@vireshk-i7>
Date:   Wed, 25 Oct 2023 20:50:00 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Stephan Gerhold <stephan@...hold.net>
Cc:     Stephan Gerhold <stephan.gerhold@...nkonzept.com>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] OPP: Call dev_pm_opp_set_opp() for required OPPs

On 25-10-23, 14:17, Stephan Gerhold wrote:
> Thanks, this seems to work fine.

Thanks a lot.

> I found another small problem: In my OPP setup for MSM8916, some of the
> required-opps point to an OPP with "opp-level = <0>" (see [1], the
> <&rpmpd_opp_none> in the cpu-opp-table). This is because the vote for
> the "CX" domain is for the CPU PLL clock source, which is only used for
> the higher CPU frequencies (>= 998.4 MHz). With the previous code you
> made it possible for me to vote for opp-level = <0> in commit
> a5663c9b1e31 ("opp: Allow opp-level to be set to 0", discussion in [2]).
> I think now it's broken because the _set_opp_level() added by Uffe
> checks for if (!opp->level) as a sign that there is no opp-level defined
> at all.

Yes, we broke that. I think a simple fix is to initialize the level
with an impossible value, like -1 and then 0 becomes valid.

> Based on my longer discussion with Uffe recently [3] it's not entirely
> clear yet if I will still have the reference to &rpmpd_opp_none in the
> future. Alternatively, we discussed describing this differently, e.g. as
> a parent power domain (which would bypass most of the OPP code), or
> moving it directly to an OPP table of CPU PLL device node (which would
> only describe the actual "active" required-opps).
> 
> I'm not sure if anyone else has a reasonable use case for pointing to a
> required-opp with opp-level = <0>, so we could potentially also postpone
> solving this to later.

I would like to fix this respectively. Thanks for bringing this up.

-- 
viresh