Hello,

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

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

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

On 28-11-23, 11:29, Stephan Gerhold wrote:
> 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! :-)

Thanks a lot.

> I guess I'm too late now but FWIW:
> 
> Tested-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>

Updated the commits with your tag :)

-- 
viresh