mbox series

[0/5] OPP: Allow power/current values without voltage

Message ID cover.1667473008.git.viresh.kumar@linaro.org
Headers show
Series OPP: Allow power/current values without voltage | expand

Message

Viresh Kumar Nov. 3, 2022, 11:01 a.m. UTC
Hello,

Some platforms, such as Apple Silicon, do not describe their device's
voltage regulators in the DT as they cannot be controlled by the kernel
and/or rely on opaque firmware algorithms to control their voltage and
current characteristics at runtime. They can, however, experimentally
determine the power consumption of a given device at a given OPP, taking
advantage of opp-microwatt to provide EAS on such devices as was initially
intended.

But the OPP core currently doesn't parse the opp-microwatt property if
opp-microvolt isn't present. This patch series targets to change this approach.

This first fixes few mistakes in the DT bindings, followed by code
reorganization. And the last commit, from James, fixes the problem at hand.

I have tested all combinations on my Hikey board, hope it doesn't break
anything.

James Calligeros (1):
  OPP: decouple dt properties in opp_parse_supplies()

Viresh Kumar (4):
  dt-bindings: opp: Fix usage of current in microwatt property
  dt-bindings: opp: Fix named microwatt property
  OPP: Parse named opp-microwatt property too
  OPP: Simplify opp_parse_supplies() by restructuring it

 .../devicetree/bindings/opp/opp-v2-base.yaml  |   6 +-
 drivers/opp/of.c                              | 228 ++++++++----------
 2 files changed, 102 insertions(+), 132 deletions(-)

Comments

Viresh Kumar Nov. 4, 2022, 5:08 a.m. UTC | #1
On 03-11-22, 16:31, Viresh Kumar wrote:
> From: James Calligeros <jcalligeros99@gmail.com>
> 
> The opp-microwatt property was added with the intention of providing
> platforms a way to specify a precise value for the power consumption
> of a device at a given OPP to enable better energy-aware scheduling
> decisions by informing the kernel of the total static and dynamic
> power of a device at a given OPP, removing the reliance on the EM
> subsystem's often flawed estimations. This property is parsed by
> opp_parse_supplies(), which creates a hard dependency on the
> opp-microvolt property.
> 
> Some platforms, such as Apple Silicon, do not describe their device's
> voltage regulators in the DT as they cannot be controlled by the kernel
> and/or rely on opaque firmware algorithms to control their voltage and
> current characteristics at runtime. We can, however, experimentally
> determine the power consumption of a given device at a given OPP, taking
> advantage of opp-microwatt to provide EAS on such devices as was
> initially intended.
> 
> Allow platforms to specify and consume any subset of opp-microvolt,
> opp-microamp, or opp-microwatt without a hard dependency on
> opp-microvolt to enable this functionality on such platforms.
> 
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> Co-developed-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V2: Rewritten by Viresh on top of his changes.
> 
>  drivers/opp/of.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)

Plus this fix:

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 273fa9c0e1c0..e55c6095adf0 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -673,7 +673,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
                              struct opp_table *opp_table)
 {
        u32 *microvolt, *microamp, *microwatt;
-       int ret, i, j;
+       int ret = 0, i, j;
        bool triplet;

        microvolt = opp_parse_microvolt(opp, dev, opp_table, &triplet);
Viresh Kumar Nov. 4, 2022, 5:28 a.m. UTC | #2
On 04-11-22, 15:25, James Calligeros wrote:
> On Friday, 4 November 2022 3:07:52 PM AEST Viresh Kumar wrote:
> > It should happen sequentially.
> 
>  I noticed the mutexes after I got some sleep :)
> 
> > Initialize "ret = 0" in opp_parse_supplies().
> > 
> > Or pick patches from:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
> > 
> 
> It didn't even cross my mind that opp_parse_supplies() returning
> ret uninitialised was causing the failure further up the chain. Applied
> and tested, everything's working as expected now. 
> 
> > Sorry for the trouble.
> 
> This is on me, I should have heeded the compiler warning.

Thanks James for giving this a try. Really appreciate it. The changes are
applied and pushed for linux-next along with your Tested-by.