Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Tue, 28 Nov 2023 11:29:11 +0100
From:   Stephan Gerhold <stephan.gerhold@kernkonzept.com>
To:     Viresh Kumar <viresh.kumar@linaro.org>
Cc:     Ulf Hansson <ulf.hansson@linaro.org>, Nishanth Menon <nm@ti.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Viresh Kumar <vireshk@kernel.org>, linux-pm@vger.kernel.org,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3 0/3]  OPP: Simplify required-opp handling
Message-ID: <ZWXA9_VDRKzMA9Nj@kernkonzept.com>
References: <cover.1700131353.git.viresh.kumar@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cover.1700131353.git.viresh.kumar@linaro.org>
Precedence: bulk

On Thu, Nov 16, 2023 at 04:13:04PM +0530, Viresh Kumar wrote:
> Configuring the required OPP was never properly implemented, we just
> took an exception for genpds and configured them directly, while leaving
> out all other required OPP types.
> 
> Now that a standard call to dev_pm_opp_set_opp() takes care of
> configuring the opp->level too, the special handling for genpds can be
> avoided by simply calling dev_pm_opp_set_opp() for the required OPPs,
> which shall eventually configure the corresponding level for genpds.
> 
> This also makes it possible for us to configure other type of required
> OPPs (no concrete users yet though), via the same path. This is how
> other frameworks take care of parent nodes, like clock, regulators, etc,
> where we recursively call the same helper.
> 
> Pushed here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/required-opps
> 

Sorry for the delay. I tested the "opp/linux-next" branch (which seems
to contain the changes in this series already now) with the following
configurations:

 - Single genpd used for cpufreq (MSM8909): Works
 - Multiple genpd used for cpufreq (MSM8916): Works
 - Single genpd used for cpufreq + parent genpd (MSM8916): Works, warning gone

Thanks for fixing this! :-)

I guess I'm too late now but FWIW:

Tested-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>

> V2->V3:
> - Dropped patch 1/3, merged.
> - Added a new commit to avoid propagation and a WARN() for parent genpd case.
> 
> V1->V2:
> - Support opp-level 0, drop vote i.e..
> - Fix OPP pointer while calling dev_pm_opp_set_opp() recursively.
> - Minor checks and fixes.
> - Add Reviewed-by from Ulf.
> 
> --
> Viresh
> 
> Viresh Kumar (3):
>   OPP: Use _set_opp_level() for single genpd case
>   OPP: Call dev_pm_opp_set_opp() for required OPPs
>   OPP: Don't set OPP recursively for a parent genpd
> 
>  drivers/opp/core.c     | 180 ++++++++++++++++++++++-------------------
>  drivers/opp/of.c       |  49 ++++++++---
>  drivers/opp/opp.h      |   8 +-
>  include/linux/pm_opp.h |   7 +-
>  4 files changed, 144 insertions(+), 100 deletions(-)
> 
> -- 
> 2.31.1.272.g89b43f80a514
> 

-- 
Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth