Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Date:   Wed, 25 Oct 2023 20:50:00 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     Stephan Gerhold <stephan@gerhold.net>
Cc:     Stephan Gerhold <stephan.gerhold@kernkonzept.com>,
        Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
        Stephen Boyd <sboyd@kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>, linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] OPP: Call dev_pm_opp_set_opp() for required OPPs
Message-ID: <20231025152000.vt42gvti2e4rewwc@vireshk-i7>
References: <cover.1697710527.git.viresh.kumar@linaro.org>
 <6de4fcb5bb943a131d0cdf0a858bd35af02a2f88.1697710527.git.viresh.kumar@linaro.org>
 <ZTeoGiMQZ-OoYJBG@kernkonzept.com>
 <20231025073634.4et5epfog25o2pxf@vireshk-i7>
 <ZTkHVVq61lrpRUOv@gerhold.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZTkHVVq61lrpRUOv@gerhold.net>
Precedence: bulk

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