[PATCH 00/16] PM / OPP: multiple regulators & opp-transition support

[PATCH 00/16] PM / OPP: multiple regulators & opp-transition support

[Viresh Kumar]

Hi Guys,

This is rebased over following series that adds debugfs support to OPP
core: http://marc.info/?i=cover.1441354424.git.viresh.kumar%40linaro.org

This series extends V2 bindings support further to make it usable to
most of the platforms.

[1-2] update the bindings a bit to get them working for multiple
      regulators case.

[3-4] cleanups.
[5-7] Multiple regulator support
[8-16] OPP transition support, so that the user drivers can directly ask
       to switch device to a particular OPP, instead of them dealing
       with the complexity of handling clocks and voltages.

I have also got cpufreq-dt driver updated to work with the new bindings,
but holded-off those changes to keep this series smaller. Those were
another Nine patches.

For curious developers/reviewers, all required code (debugfs, this and
cpufreq-dt) is pushed to:

https://git.linaro.org/people/viresh.kumar/linux.git opp/multi-regulator-v1

Please help in getting this reviewed :)

Viresh Kumar (16):
  PM / OPP: Add 'supply-names' binding
  PM / OPP: Add 'opp-microvolt-triplets' binding
  PM / OPP: Improve debug print messages with pr_fmt
  PM / OPP: Rename routines specific to old bindings with _v1
  PM / OPP: Parse all power-supply related bindings together
  PM / OPP: Create separate structure for regulator/supplies
  PM / OPP: Add multiple regulators support
  PM / OPP: get/put regulators from OPP core
  PM / OPP: Disable OPPs that aren't supported by the regulators
  PM / OPP: Introduce dev_pm_opp_get_max_volt_latency()
  PM / OPP: Introduce dev_pm_opp_get_max_transition_latency()
  PM / OPP: Parse clock and voltage tolerance for v1 bindings
  PM / OPP: Manage device clk as well
  PM / OPP: Add dev_pm_opp_set_regulator() to specify regulator
  PM / OPP: Add dev_pm_opp_set_rate()
  PM / OPP: don't print error message for deferred probing

 Documentation/devicetree/bindings/opp/opp.txt |  40 +-
 drivers/base/power/opp/core.c                 | 637 +++++++++++++++++++++++---
 drivers/base/power/opp/cpu.c                  |   8 +-
 drivers/base/power/opp/debugfs.c              |  52 ++-
 drivers/base/power/opp/opp.h                  |  44 +-
 include/linux/pm_opp.h                        |  25 +
 6 files changed, 722 insertions(+), 84 deletions(-)

-- 
2.4.0

[PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

Regulators already have stable DT bindings, wherein the consumer (of
supplies) will have following for each regulator/supply.

<name>-supply: <phandle to the regulator node>;

Current OPP bindings extend above, by transforming it into a list of
phandles. But we missed the <name> string, which is used to identify the
regulator.

And looking from regulators perspective, having two different ways of
specifying regulators doesn't seem like a step forward, it also means we
have to update every single device binding. And things will become
complex.

Another way to support multiple regulators per device (in OPP V2
bindings) is to leave regulator consumer bindings as is, and create a
'supply-names' property in the opp-table node, which will contain a list
of strings. The names in this list shall match 'name' from the
'<name>-supply' strings present in the device node.

The strings in this list also specify the order in which values must be
present in 'opp-microvolt' and 'opp-microamp' properties.

Cc: Mark Brown <broonie@kernel.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 0cb44dc21f97..8759bc4783ed 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following
 - compatible: Allow OPPs to express their compatibility. It should be:
   "operating-points-v2".
 
+- supply-names: This is a required property, only if multiple supplies are
+  available for the device. Otherwise it is optional.
+
+  This list is used to pass names of all the device supplies. The order of names
+  present here is important, as that should match the order in which values are
+  present in 'opp-microvolt' and 'opp-microamp' properties.
+
 - OPP nodes: One or more OPP nodes describing voltage-current-frequency
   combinations. Their name isn't significant but their phandle can be used to
   reference an OPP.
@@ -97,8 +104,8 @@ properties.
   Single entry is for target voltage and three entries are for <target min max>
   voltages.
 
-  Entries for multiple regulators must be present in the same order as
-  regulators are specified in device's DT node.
+  Entries for multiple regulators must be present in the same order as their
+  names are present in 'supply-names' property of the opp-table.
 
 - opp-microamp: The maximum current drawn by the device in microamperes
   considering system specific parameters (such as transients, process, aging,
@@ -107,10 +114,12 @@ properties.
 
   Should only be set if opp-microvolt is set for the OPP.
 
-  Entries for multiple regulators must be present in the same order as
-  regulators are specified in device's DT node. If this property isn't required
-  for few regulators, then this should be marked as zero for them. If it isn't
-  required for any regulator, then this property need not be present.
+  Entries for multiple regulators must be present in the same order as their
+  names are present in 'supply-names' property of the opp-table.
+
+  If this property isn't required for few regulators, then this should be marked
+  as zero for them. If it isn't required for any regulator, then this property
+  need not be present.
 
 - clock-latency-ns: Specifies the maximum possible transition latency (in
   nanoseconds) for switching to this OPP from any other OPP.
@@ -369,13 +378,16 @@ Example 4: Handling multiple regulators
 			compatible = "arm,cortex-a7";
 			...
 
-			cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
+			vcc0-supply = <&cpu_supply0>;
+			vcc1-supply = <&cpu_supply1>;
+			vcc2-supply = <&cpu_supply2>;
 			operating-points-v2 = <&cpu0_opp_table>;
 		};
 	};
 
 	cpu0_opp_table: opp_table0 {
 		compatible = "operating-points-v2";
+		supply-names = "vcc0", "vcc1", "vcc2";
 		opp-shared;
 
 		opp00 {
-- 
2.4.0

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Rob Herring]

On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> Regulators already have stable DT bindings, wherein the consumer (of
> supplies) will have following for each regulator/supply.
> 
> <name>-supply: <phandle to the regulator node>;
> 
> Current OPP bindings extend above, by transforming it into a list of
> phandles. But we missed the <name> string, which is used to identify the
> regulator.
> 
> And looking from regulators perspective, having two different ways of
> specifying regulators doesn't seem like a step forward, it also means we
> have to update every single device binding. And things will become
> complex.
> 
> Another way to support multiple regulators per device (in OPP V2
> bindings) is to leave regulator consumer bindings as is, and create a
> 'supply-names' property in the opp-table node, which will contain a list
> of strings. The names in this list shall match 'name' from the
> '<name>-supply' strings present in the device node.
> 
> The strings in this list also specify the order in which values must be
> present in 'opp-microvolt' and 'opp-microamp' properties.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 0cb44dc21f97..8759bc4783ed 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following
>  - compatible: Allow OPPs to express their compatibility. It should be:
>    "operating-points-v2".
>  
> +- supply-names: This is a required property, only if multiple supplies are
> +  available for the device. Otherwise it is optional.
> +
> +  This list is used to pass names of all the device supplies. The order of names
> +  present here is important, as that should match the order in which values are
> +  present in 'opp-microvolt' and 'opp-microamp' properties.
> +

What if we have a 2nd device and supply rail? For example, what if the
L2$ has a separate rail from the cores but is linked to the OPPs.

>  - OPP nodes: One or more OPP nodes describing voltage-current-frequency
>    combinations. Their name isn't significant but their phandle can be used to
>    reference an OPP.
> @@ -97,8 +104,8 @@ properties.
>    Single entry is for target voltage and three entries are for <target min max>
>    voltages.
>  
> -  Entries for multiple regulators must be present in the same order as
> -  regulators are specified in device's DT node.
> +  Entries for multiple regulators must be present in the same order as their
> +  names are present in 'supply-names' property of the opp-table.
>  
>  - opp-microamp: The maximum current drawn by the device in microamperes
>    considering system specific parameters (such as transients, process, aging,
> @@ -107,10 +114,12 @@ properties.
>  
>    Should only be set if opp-microvolt is set for the OPP.
>  
> -  Entries for multiple regulators must be present in the same order as
> -  regulators are specified in device's DT node. If this property isn't required
> -  for few regulators, then this should be marked as zero for them. If it isn't
> -  required for any regulator, then this property need not be present.
> +  Entries for multiple regulators must be present in the same order as their
> +  names are present in 'supply-names' property of the opp-table.
> +
> +  If this property isn't required for few regulators, then this should be marked
> +  as zero for them. If it isn't required for any regulator, then this property
> +  need not be present.

Remind me of when do we have multiple regulators for a cpu? The number
of supplies should be defined by the cpu binding as function of how many
supply rails a cpu has.

>  
>  - clock-latency-ns: Specifies the maximum possible transition latency (in
>    nanoseconds) for switching to this OPP from any other OPP.
> @@ -369,13 +378,16 @@ Example 4: Handling multiple regulators
>  			compatible = "arm,cortex-a7";
>  			...
>  
> -			cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
> +			vcc0-supply = <&cpu_supply0>;
> +			vcc1-supply = <&cpu_supply1>;
> +			vcc2-supply = <&cpu_supply2>;

This may just be an example, but a CA7 doesn't have 3 supply rails.

Rob

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 14-09-15, 15:22, Rob Herring wrote:
> What if we have a 2nd device and supply rail? For example, what if the
> L2$ has a separate rail from the cores but is linked to the OPPs.

Right, so that is the case with the Mediatek SoC as well, AFAIR. How
do we plan to treat L2 devices? For example, in the mediatek cpufreq
driver, they change L2's supplies together with CPUs.

One way to get that done, with such very closely bound devices is to
add two regulators:

cpu-supply = ...;
l2-supply = ...;

And then a property in OPP table:

supply-name = "cpu", "l2";

And maybe fix the order in which the supplies which be updated, based
on the order in which entries are present in the above property.

Any other way you suggest for doing that ?

> Remind me of when do we have multiple regulators for a cpu?

I haven't seen that yet, but its more like what I explained above.
i.e. one for the CPU and other one for the L2 cache.

But, these bindings do apply for other devices as well, where it
should be very much possible.

-- 
viresh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

Rob,

On 15-09-15, 08:17, Viresh Kumar wrote:
> On 14-09-15, 15:22, Rob Herring wrote:
> > What if we have a 2nd device and supply rail? For example, what if the
> > L2$ has a separate rail from the cores but is linked to the OPPs.
> 
> Right, so that is the case with the Mediatek SoC as well, AFAIR. How
> do we plan to treat L2 devices? For example, in the mediatek cpufreq
> driver, they change L2's supplies together with CPUs.
> 
> One way to get that done, with such very closely bound devices is to
> add two regulators:
> 
> cpu-supply = ...;
> l2-supply = ...;
> 
> And then a property in OPP table:
> 
> supply-name = "cpu", "l2";
> 
> And maybe fix the order in which the supplies which be updated, based
> on the order in which entries are present in the above property.
> 
> Any other way you suggest for doing that ?
> 
> > Remind me of when do we have multiple regulators for a cpu?
> 
> I haven't seen that yet, but its more like what I explained above.
> i.e. one for the CPU and other one for the L2 cache.
> 
> But, these bindings do apply for other devices as well, where it
> should be very much possible.

Not sure if you still have any objections to this patch?

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Stephen Boyd]

On 09/14, Rob Herring wrote:
> On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> > Regulators already have stable DT bindings, wherein the consumer (of
> > supplies) will have following for each regulator/supply.
> > 
> > <name>-supply: <phandle to the regulator node>;
> > 
> > Current OPP bindings extend above, by transforming it into a list of
> > phandles. But we missed the <name> string, which is used to identify the
> > regulator.
> > 
> > And looking from regulators perspective, having two different ways of
> > specifying regulators doesn't seem like a step forward, it also means we
> > have to update every single device binding. And things will become
> > complex.
> > 
> > Another way to support multiple regulators per device (in OPP V2
> > bindings) is to leave regulator consumer bindings as is, and create a
> > 'supply-names' property in the opp-table node, which will contain a list
> > of strings. The names in this list shall match 'name' from the
> > '<name>-supply' strings present in the device node.
> > 
> > The strings in this list also specify the order in which values must be
> > present in 'opp-microvolt' and 'opp-microamp' properties.
> > 
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 0cb44dc21f97..8759bc4783ed 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following
> >  - compatible: Allow OPPs to express their compatibility. It should be:
> >    "operating-points-v2".
> >  
> > +- supply-names: This is a required property, only if multiple supplies are
> > +  available for the device. Otherwise it is optional.
> > +
> > +  This list is used to pass names of all the device supplies. The order of names
> > +  present here is important, as that should match the order in which values are
> > +  present in 'opp-microvolt' and 'opp-microamp' properties.
> > +
> 
> What if we have a 2nd device and supply rail? For example, what if the
> L2$ has a separate rail from the cores but is linked to the OPPs.

I'm lost why we need this property at all. What happened to using

 opp-microvolt-0 = <1 2 3>;
 opp-microvolt-1 = <1>;
 opp-microvolt-2 = <3 4 5>;
 etc.

That seems to avoid any problem with 3 vs. 1 element properties
combined into one large array. Having supply-names seems too
brittle and would tie us to a particular OPP user's decision to
call supplies by some name.

Also, I've seen devices that are split across two power domains.
These devices aren't CPUs, but they are other devices including
L2 caches. So we're going to need either multiple regulator
support or multiple "power domain at a particular performance
levels" support somehow.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 15-10-15, 17:22, Stephen Boyd wrote:
> I'm lost why we need this property at all. What happened to using
> 
>  opp-microvolt-0 = <1 2 3>;
>  opp-microvolt-1 = <1>;
>  opp-microvolt-2 = <3 4 5>;
>  etc.

Perhaps you are confusing this with the bindings we came up for
picking right voltage levels based on the cuts/version of the hardware
we are running on. The problem that Lee Jones mentioned and that can
be used in your case as well.

> That seems to avoid any problem with 3 vs. 1 element properties
> combined into one large array.

That's not the problem I was trying to solve here.

> Having supply-names seems too
> brittle and would tie us to a particular OPP user's decision to
> call supplies by some name.

No. The name has to match the <name>-supply property present in the
device's node, that's why we need this property :)

> Also, I've seen devices that are split across two power domains.
> These devices aren't CPUs, but they are other devices including
> L2 caches. So we're going to need either multiple regulator
> support or multiple "power domain at a particular performance
> levels" support somehow.

Right, that's a good example of why we need multi-regulator support :)

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Stephen Boyd]

On 10/16, Viresh Kumar wrote:
> On 15-10-15, 17:22, Stephen Boyd wrote:
> > I'm lost why we need this property at all. What happened to using
> > 
> >  opp-microvolt-0 = <1 2 3>;
> >  opp-microvolt-1 = <1>;
> >  opp-microvolt-2 = <3 4 5>;
> >  etc.
> 
> Perhaps you are confusing this with the bindings we came up for
> picking right voltage levels based on the cuts/version of the hardware
> we are running on. The problem that Lee Jones mentioned and that can
> be used in your case as well.

Isn't that what this patch series is for?

> 
> > That seems to avoid any problem with 3 vs. 1 element properties
> > combined into one large array.
> 
> That's not the problem I was trying to solve here.

What problem are you trying to solve then?

> 
> > Having supply-names seems too
> > brittle and would tie us to a particular OPP user's decision to
> > call supplies by some name.
> 
> No. The name has to match the <name>-supply property present in the
> device's node, that's why we need this property :)

Why does it need to match? Sorry I'm totally lost now.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 16-10-15, 12:16, Stephen Boyd wrote:
> On 10/16, Viresh Kumar wrote:
> > On 15-10-15, 17:22, Stephen Boyd wrote:
> > > I'm lost why we need this property at all. What happened to using
> > > 
> > >  opp-microvolt-0 = <1 2 3>;
> > >  opp-microvolt-1 = <1>;
> > >  opp-microvolt-2 = <3 4 5>;
> > >  etc.
> > 
> > Perhaps you are confusing this with the bindings we came up for
> > picking right voltage levels based on the cuts/version of the hardware
> > we are running on. The problem that Lee Jones mentioned and that can
> > be used in your case as well.
> 
> Isn't that what this patch series is for?

Hehe, no.

Okay here is the problem statement:

We have two supplies for a device and the device node will have
something like:

name1-supply = <&supply1>;
name2-supply = <&supply2>;

And the OPP node needs to have voltages for both of them:

opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;

Where XYZ(1) are for supply1 and XYZ(2) are for supply2.

Now we need to identify the supplies for which the values are present
here and their order as well. How do we do that?

The way I am suggesting is to add a property in opp node which will
keep "name1" and "name2" in it.

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 17-10-15, 09:40, Viresh Kumar wrote:
> Hehe, no.
> 
> Okay here is the problem statement:
> 
> We have two supplies for a device and the device node will have
> something like:
> 
> name1-supply = <&supply1>;
> name2-supply = <&supply2>;
> 
> And the OPP node needs to have voltages for both of them:
> 
> opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;
> 
> Where XYZ(1) are for supply1 and XYZ(2) are for supply2.
> 
> Now we need to identify the supplies for which the values are present
> here and their order as well. How do we do that?
> 
> The way I am suggesting is to add a property in opp node which will
> keep "name1" and "name2" in it.

Any more comments? Acks ? I want to close this series :)

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

On Sat, Oct 17, 2015 at 09:40:55AM +0530, Viresh Kumar wrote:

> Okay here is the problem statement:

> We have two supplies for a device and the device node will have
> something like:

> name1-supply = <&supply1>;
> name2-supply = <&supply2>;

> And the OPP node needs to have voltages for both of them:

> opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;

> Where XYZ(1) are for supply1 and XYZ(2) are for supply2.

> Now we need to identify the supplies for which the values are present
> here and their order as well. How do we do that?

> The way I am suggesting is to add a property in opp node which will
> keep "name1" and "name2" in it.

When we start doing this we also start having to worry about things like
the sequencing of the updates between the various supplies and end up in
full on power sequencing (or at least baking some sequencing into the DT
which will doubtless need extending at some point).  I'm not sure that's
a place we want to end up just yet, I think it's safer to just have a
little bit of code in the kernel that glues things together in the cases
where this is needed.  

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 23-10-15, 01:39, Mark Brown wrote:
> When we start doing this we also start having to worry about things like
> the sequencing of the updates between the various supplies and end up in
> full on power sequencing (or at least baking some sequencing into the DT
> which will doubtless need extending at some point).

Absolutely.

> I'm not sure that's
> a place we want to end up just yet, I think it's safer to just have a
> little bit of code in the kernel that glues things together in the cases
> where this is needed.  

So you are effectively saying that we shouldn't go ahead with multi
regulator support in OPP library, right?

I went ahead with it as it came as a requirement (specially from
Qcom).

To the problem of sequencing, maybe we can just support that for the
simple case, where supplies will be programmed in the order in which
they are present in the property I added in this patch. And not try to
solve problem for the complex cases, if we feel it is getting ugly.

@Stephen ?

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Tue, Oct 27, 2015 at 01:49:17PM +0530, Viresh Kumar wrote:
> On 23-10-15, 01:39, Mark Brown wrote:

> > I'm not sure that's
> > a place we want to end up just yet, I think it's safer to just have a
> > little bit of code in the kernel that glues things together in the cases
> > where this is needed.  

> So you are effectively saying that we shouldn't go ahead with multi
> regulator support in OPP library, right?

Well, I think things like libraries for getting the data tables out of
DT are fine but I'm not convinced that trying to avoid having any device
specific code at all is sufficiently clear yet - as far as I know we're
mostly looking at a fairly small subset of devices still and with things
like sequencing in the mix it's a bit worrying to me to be putting it
all into an ABI intended to be used with no knowledge of the platform.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Stephen Boyd]

On 10/28, Mark Brown wrote:
> On Tue, Oct 27, 2015 at 01:49:17PM +0530, Viresh Kumar wrote:
> > On 23-10-15, 01:39, Mark Brown wrote:
> 
> > > I'm not sure that's
> > > a place we want to end up just yet, I think it's safer to just have a
> > > little bit of code in the kernel that glues things together in the cases
> > > where this is needed.  
> 
> > So you are effectively saying that we shouldn't go ahead with multi
> > regulator support in OPP library, right?
> 
> Well, I think things like libraries for getting the data tables out of
> DT are fine but I'm not convinced that trying to avoid having any device
> specific code at all is sufficiently clear yet - as far as I know we're
> mostly looking at a fairly small subset of devices still and with things
> like sequencing in the mix it's a bit worrying to me to be putting it
> all into an ABI intended to be used with no knowledge of the platform.

Agreed. Looking at the current oppv2 binding I'm confused how it
even works. It shows cpu-supply = <&phandle1>, <&phandle2>, etc.
which doesn't work with the regulator bindings design. It seems
that Viresh figured that out when implementing support for
multiple regulators, so the binding was changed to put names in
the opp tables and then use that to get regulators in the opp
core. Urgh.

I think I understand the goal here though. The goal is to move
all the clk_get()/regulator_get() and clock and regulator API
interactions into the OPP framework so that we can say "go to OPP
at index 4" from the cpufreq core and the OPP framework takes
care of actually doing the transition. Given that doing such a
transition could be very machine specific, we probably ought to
make the OPP framework flexible enough to let us decide how to do
that.

Perhaps we need to expand on the compatible string in the opp
node to have compatible = "vendor,cool-transition",
"operating-points-v2"? Then we can plug in vendor specific
drivers that handle the frequency/voltage transition and they'll
know that the fourth column in the voltage entry corresponds to
this particular device's foo-supply and in what order to change
voltages. A generic driver could exist for the simple case of one
regulator and one clock that matches on "operating-points-v2".

Or we can avoid doing any clk_get()/regulator_get() stuff inside
the OPP framework, and let OPP consumers tell the OPP framework
about each clock and regulator it wants the framework to manage
on the consumer's behalf. It could even tell the framework which
regulator corresponds to which voltage column and in what order
to change the voltages so that the OPP framework doesn't need to
know these device specific details. Then the consumer can still
say go to OPP level 4 and that all works without introducing some
new DT ABI.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

[Removed few people and LKML from CC as its not really about DT
change anymore but OPP]

Hi Guys,

On 15 September 2015 at 01:52, Rob Herring <robh@kernel.org> wrote:

> Remind me of when do we have multiple regulators for a cpu? The number
> of supplies should be defined by the cpu binding as function of how many
> supply rails a cpu has.

This thread died long back and we never worked out support for multiple
regulators into the OPP core.

But, Dave and Nishanth (from TI) pinged me last week about it. According
to them, their platform has got two regulators for the CPU device and they
want OPP layer to have this support.

I was expecting them to reignite this thread, but don't know what happened.

And so, I am starting it here.

@Dave/Nishanth: Can you please explain the use-case here please? So
that we can get convinced that you really need OPP-core to support multiple
regulators..

--
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Nishanth Menon]

On 03/01/2016 12:45 AM, Viresh Kumar wrote:
> [Removed few people and LKML from CC as its not really about DT
> change anymore but OPP]
> 
> Hi Guys,
> 
> On 15 September 2015 at 01:52, Rob Herring <robh@kernel.org> wrote:
> 
>> Remind me of when do we have multiple regulators for a cpu? The number
>> of supplies should be defined by the cpu binding as function of how many
>> supply rails a cpu has.
> 
> This thread died long back and we never worked out support for multiple
> regulators into the OPP core.
> 
> But, Dave and Nishanth (from TI) pinged me last week about it. According
> to them, their platform has got two regulators for the CPU device and they
> want OPP layer to have this support.
> 
> I was expecting them to reignite this thread, but don't know what happened.

Thanks a ton viresh bringing this back up again.

> 
> And so, I am starting it here.
> 
> @Dave/Nishanth: Can you please explain the use-case here please? So
> that we can get convinced that you really need OPP-core to support multiple
> regulators..
> 

For controlling leakage and switching frequency of the transistors,
traditional controls are usually around the voltage rail alone. This is
not entirely accurate. many SoC vendors including TI [1] and others[2]
use Body Biasing to control leakage. in fact this is already modeled as
a regulator for controlling the SoC internal LDO for TI devices in Linux
kernel (drivers/regulator/ti-abb-regulator.c).


What this means is something as simple as follows:

Typical usage:
    VDD
    ---
     |
     |
     /
   |/
 --|
   |\
     V
     |
     |
    GND


Here we provide voltage optimized for the switching frequency required
for operation.

With Body Biasing: (simplified diagram)
    VDD   VBB (Body Bias)
    ---   ---
     |     |
     |     |
     /     |
   |/      |
 --|   ----/
   |\
     V
     |
     |
    GND


This implies that for an OPP, we have the triplet {frequency, VDD(SMPS
voltage), VBB(Bias voltage) } - Multi regulator is key for all OMAP
generation SoCs since OMAP3 to function for frequencies -> DRA7 for
example mandates ABB for all OPPs.

For some reason, this entire thread slipped out attention :( - Thanks
viresh for bringing it back to focus.


[1] http://www.ti.com/product/DM3730/technicaldocuments including
http://www.ti.com/product/am5728?keyMatch=am5728  (TI) OMAP3, OMAP4,
OMAP5, DRA7, AM57xx SoCs all have this

[2] http://www.techdesignforums.com/practice/guides/dvfs-body-back-bias/
(ST Micro for example)


-- 
Regards,
Nishanth Menon

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

On Tue, Mar 01, 2016 at 12:15:48PM +0530, Viresh Kumar wrote:

> [Removed few people and LKML from CC as its not really about DT
> change anymore but OPP]

Please don't take things off-list unless there is a really strong reason
to do so.  Sending things to the list ensures that everyone gets a
chance to read and comment on things.

I'm really not sure what I can say here that I've not already said
repeatedly, what's new here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Viresh Kumar]

On 02-03-16, 11:50, Mark Brown wrote:
> Please don't take things off-list unless there is a really strong reason
> to do so.  Sending things to the list ensures that everyone gets a
> chance to read and comment on things.

Okay. I thought keeping that on linux-pm would be sufficient for
interested folks to follow.

> I'm really not sure what I can say here that I've not already said
> repeatedly, what's new here?

Yeah, you have raised some genuine concerns earlier, like the sequence
in which the voltages of multiple supplies should be changed, and if
we should rely on some DT sequence for that.

I started it again, because we have a real case now (not like the
virtual case earlier) where a platform has multiple supplies for a
device (CPU). I just wanted to hear from all of you on how should we
go ahead and solve it.

TI guys had some crazy ideas, like using OPP-notifiers to update the
second regulator, etc and I really hated it :)

-- 
viresh

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On Wed, Mar 02, 2016 at 03:58:31PM +0530, Viresh Kumar wrote:
> On 02-03-16, 11:50, Mark Brown wrote:

> > I'm really not sure what I can say here that I've not already said
> > repeatedly, what's new here?

> Yeah, you have raised some genuine concerns earlier, like the sequence
> in which the voltages of multiple supplies should be changed, and if
> we should rely on some DT sequence for that.

> I started it again, because we have a real case now (not like the
> virtual case earlier) where a platform has multiple supplies for a
> device (CPU). I just wanted to hear from all of you on how should we
> go ahead and solve it.

> TI guys had some crazy ideas, like using OPP-notifiers to update the
> second regulator, etc and I really hated it :)

My main thought with this stuff is that if it's really complicated to
write generic code that might be a sign that it's sensible to write a
custom driver.  We can always writee something later if we figure out
some general patterns.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Nishanth Menon]

On Wed, Mar 2, 2016 at 5:24 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Mar 02, 2016 at 03:58:31PM +0530, Viresh Kumar wrote:
>> On 02-03-16, 11:50, Mark Brown wrote:
>
>> > I'm really not sure what I can say here that I've not already said
>> > repeatedly, what's new here?
>
>> Yeah, you have raised some genuine concerns earlier, like the sequence
>> in which the voltages of multiple supplies should be changed, and if
>> we should rely on some DT sequence for that.
>
>> I started it again, because we have a real case now (not like the
>> virtual case earlier) where a platform has multiple supplies for a
>> device (CPU). I just wanted to hear from all of you on how should we
>> go ahead and solve it.
>
>> TI guys had some crazy ideas, like using OPP-notifiers to update the
>> second regulator, etc and I really hated it :)

Ummm... We prefer not to do this via notifier solution.. yeah doing
that around notifiers is indeed a crazier alternative

>
> My main thought with this stuff is that if it's really complicated to
> write generic code that might be a sign that it's sensible to write a
> custom driver.  We can always writee something later if we figure out
> some general patterns.

We did propose to go via voltage domain[1] to actually do an OPP
transition and provide hooks that can implement required regulator set
for the SoC. After seeing dev_pm_opp_set_rate is available, it does
not make sense to go down the voltagedomain path anymore.

We do have a real need to control multiple regulators around an OPP
change. this logic is valid for more than CPU device -> it is valid
for devices like DSP, GPU etc in the SoC as well across multiple
families in the OMAP generation SoCs. The logic by itself it not too
complicated. a voltage plane for a device has two regulators that need
to be controlled in tandem for a given target frequency. by allowing
dev_pm_opp_set_rate to control "n" regulators it actually does
simplify the problem down for a generic solution -> DVFS for devices
triggered from cpufreq or via devfreq.


[1] http://marc.info/?l=linux-omap&m=139275579731801&w=2


---
Regards,
Nishanth Menon

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On Wed, Mar 02, 2016 at 09:26:03AM -0600, Nishanth Menon wrote:

> We do have a real need to control multiple regulators around an OPP
> change. this logic is valid for more than CPU device -> it is valid
> for devices like DSP, GPU etc in the SoC as well across multiple
> families in the OMAP generation SoCs. The logic by itself it not too
> complicated. a voltage plane for a device has two regulators that need
> to be controlled in tandem for a given target frequency. by allowing
> dev_pm_opp_set_rate to control "n" regulators it actually does
> simplify the problem down for a generic solution -> DVFS for devices
> triggered from cpufreq or via devfreq.

That's n regulators that need to be changed in arbitrary orders
(together with some other number of clocks).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Nishanth Menon]

On Wed, Mar 2, 2016 at 9:30 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Mar 02, 2016 at 09:26:03AM -0600, Nishanth Menon wrote:
>
>> We do have a real need to control multiple regulators around an OPP
>> change. this logic is valid for more than CPU device -> it is valid
>> for devices like DSP, GPU etc in the SoC as well across multiple
>> families in the OMAP generation SoCs. The logic by itself it not too
>> complicated. a voltage plane for a device has two regulators that need
>> to be controlled in tandem for a given target frequency. by allowing
>> dev_pm_opp_set_rate to control "n" regulators it actually does
>> simplify the problem down for a generic solution -> DVFS for devices
>> triggered from cpufreq or via devfreq.
>
> That's n regulators that need to be changed in arbitrary orders
> (together with some other number of clocks).

I agree, the "n" regulators that control supplies to a device cannot
obviously be transitioned in random order and ofcourse we want to
describe what supplies are present in the hardware in dts, not how the
supplies need to be transitioned. There will be a sequencing required
based on platfiorm for these n regulators (and or optimization of
voltages) - similar to requirement for voltage Vs frequency scale
needs.

One alternative I can think of is to reintroduce voltage domain driver
back in (we will get rid of the clock notifier tieups etc).. this
voltage domain mechanism will handle all the required sequencing based
on platform and any optimizations or quirks needed, then,
_set_opp_voltage would see if a voltagedomain is present, if yes, it
will invoke that instead of a single regulator. obviously two things
take place with this approach:
a) dev_pm_opp_set_rate still remain the single entry point for all OPP
transition (I think this is a good idea)
b) a level of abstraction introduced by complex drivers via the
voltage domain framework ( the original proposed framework had ability
already to handle 1-n regulators based on platform voltage domain
drivers).

What other alternatives can folks suggest we go towards?

---
Regards,
Nishanth Menon

Re: [PATCH 01/16] PM / OPP: Add 'supply-names' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

On Fri, Sep 11, 2015 at 05:31:57PM +0530, Viresh Kumar wrote:

> +- supply-names: This is a required property, only if multiple supplies are
> +  available for the device. Otherwise it is optional.
> +
> +  This list is used to pass names of all the device supplies. The order of names
> +  present here is important, as that should match the order in which values are
> +  present in 'opp-microvolt' and 'opp-microamp' properties.

If we were going to add something like this (which I'm really not that
thrilled about due to their encouragement of bad practice as previously
discussed) it seems wrong to add it at the level of a specific binding
rather than as a general facility.  I think I'm not entirely sold on
this use case though, I'll follow up to a later message in this
thread...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 02/16] PM / OPP: Add 'opp-microvolt-triplets' binding

[Viresh Kumar]

If 'opp-microvolt' is used to specify values for multiple regulators,
then we need this additional information to know if the values passed
should be treated as <target> or <target min max>.

This is because, DT doesn't differentiate between these two styles:

prop = <x>, <y>, <z>;
prop = <x y z>;

Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 8759bc4783ed..719603b87353 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -76,6 +76,16 @@ This describes the OPPs belonging to a device. This node can have following
   present here is important, as that should match the order in which values are
   present in 'opp-microvolt' and 'opp-microamp' properties.
 
+- opp-microvolt-triplets: This is a required property, only if multiple supplies
+  are available for the device. Otherwise it is ignored.
+
+  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
+  power-supply. 'opp-microvolt-triplets' property is used to find the form in
+  which 'opp-microvolt' is present.
+
+  If present, then 'opp-microvolt' must be present in <target min max> form,
+  else in <target> form.
+
 - OPP nodes: One or more OPP nodes describing voltage-current-frequency
   combinations. Their name isn't significant but their phandle can be used to
   reference an OPP.
@@ -403,6 +413,8 @@ Example 4: Handling multiple regulators
 
 		/* OR */
 
+		opp-microvolt-triplets;
+
 		opp00 {
 			opp-hz = /bits/ 64 <1000000000>;
 			opp-microvolt = <970000 975000 985000>, /* Supply 0 */
@@ -416,6 +428,8 @@ Example 4: Handling multiple regulators
 
 		/* OR */
 
+		opp-microvolt-triplets;
+
 		opp00 {
 			opp-hz = /bits/ 64 <1000000000>;
 			opp-microvolt = <970000 975000 985000>, /* Supply 0 */
-- 
2.4.0

Re: [PATCH 02/16] PM / OPP: Add 'opp-microvolt-triplets' binding

[Rob Herring]

On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> If 'opp-microvolt' is used to specify values for multiple regulators,
> then we need this additional information to know if the values passed
> should be treated as <target> or <target min max>.

Can't you determine this implicitly from # of cells / # of regulators
being either 1 or 3?

> 
> This is because, DT doesn't differentiate between these two styles:
> 
> prop = <x>, <y>, <z>;
> prop = <x y z>;
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 8759bc4783ed..719603b87353 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -76,6 +76,16 @@ This describes the OPPs belonging to a device. This node can have following
>    present here is important, as that should match the order in which values are
>    present in 'opp-microvolt' and 'opp-microamp' properties.
>  
> +- opp-microvolt-triplets: This is a required property, only if multiple supplies
> +  are available for the device. Otherwise it is ignored.
> +
> +  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
> +  power-supply. 'opp-microvolt-triplets' property is used to find the form in
> +  which 'opp-microvolt' is present.
> +
> +  If present, then 'opp-microvolt' must be present in <target min max> form,
> +  else in <target> form.

This implies that bindings currently with triplets are wrong since they
will be missing opp-microvolt-triplets.

Rob

Re: [PATCH 02/16] PM / OPP: Add 'opp-microvolt-triplets' binding

[Viresh Kumar]

[+Cc Mark, I thought I cc'd him earlier, but no, I cc'd him only for
the first patch]

On 14-09-15, 15:30, Rob Herring wrote:
> On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> > If 'opp-microvolt' is used to specify values for multiple regulators,
> > then we need this additional information to know if the values passed
> > should be treated as <target> or <target min max>.
> 
> Can't you determine this implicitly from # of cells / # of regulators
> being either 1 or 3?

I thought the #<name>-cells property is used to pass arguments along with the
phandle, so something like this:

supply0: regulator@f8000000 {
        regulator-cells = 1;
        ...
}

cpu@1 {
        cpu-supply = <&supply0 XYZ>
}

But if we can define something like:

supply0: regulator@f8000000 {
        regulator-cells or microvolt-cells = 1 or 3;
        ...
}

And then do:

cpu@1 {
        cpu-supply = <&supply0>
        operating-points-v2 = "&opp-table";
}

opp-table: table {

        ...

        opp0 {
                opp-hz = ...;
                ...
                opp-microvolt = <one or three values here>
        }
}

then it will be very simple. Also, this would mean that with multiple
regulators, we can have one regulator supporting single microvolts
value and other supporting tar/min/max values..

> > 
> > This is because, DT doesn't differentiate between these two styles:
> > 
> > prop = <x>, <y>, <z>;
> > prop = <x y z>;
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 8759bc4783ed..719603b87353 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -76,6 +76,16 @@ This describes the OPPs belonging to a device. This node can have following
> >    present here is important, as that should match the order in which values are
> >    present in 'opp-microvolt' and 'opp-microamp' properties.
> >  
> > +- opp-microvolt-triplets: This is a required property, only if multiple supplies
> > +  are available for the device. Otherwise it is ignored.
> > +
> > +  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
> > +  power-supply. 'opp-microvolt-triplets' property is used to find the form in
> > +  which 'opp-microvolt' is present.
> > +
> > +  If present, then 'opp-microvolt' must be present in <target min max> form,
> > +  else in <target> form.
> 
> This implies that bindings currently with triplets are wrong since they
> will be missing opp-microvolt-triplets.

Yeah, the bindings are incomplete, but there are no users yet, which
need the triplet thing. So, its all working just fine.

-- 
viresh

Re: [PATCH 02/16] PM / OPP: Add 'opp-microvolt-triplets' binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

On Tue, Sep 15, 2015 at 09:00:27AM +0530, Viresh Kumar wrote:

> [+Cc Mark, I thought I cc'd him earlier, but no, I cc'd him only for
> the first patch]

I'm reading this on a plane so have no other context and to be honest
I'm struggling to understand what is being discussed here.  It would be
really helpful if you were to describe in words what proposed bindings
are intended to do as well as presenting examples, the examples by
themselves require the reader to reverse engineer what the semantics are
intended to be.

> But if we can define something like:

> supply0: regulator@f8000000 {
>         regulator-cells or microvolt-cells = 1 or 3;
>         ...
> }

As far as I can tell this is proposing adding something to the regulator
binding specifying if users must present either a single value or a
min/target/max triplet.  This is obviously problematic since regulators
can be shared - the needs of one user may not match the needs of another
user, and of course most users should not be specifying voltages at all
in the device tree in the first place.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH 03/16] PM / OPP: Improve debug print messages with pr_fmt

[Viresh Kumar]

With debug options on, it is difficult to locate OPP core's debug
prints. Fix this by prefixing OPP debug prints with KBUILD_MODNAME.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 2 ++
 drivers/base/power/opp/cpu.c  | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 8a72f0e586e8..113fdd0ce6d9 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -11,6 +11,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/slab.h>
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 3d946b508a13..9ac691341fb0 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -10,6 +10,9 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
 #include <linux/err.h>
-- 
2.4.0

[PATCH 04/16] PM / OPP: Rename routines specific to old bindings with _v1

[Viresh Kumar]

Clearly distinguish routines based on what version of bindings they
parse. We have already postfixed routines properly with _v2 for new
bindings. Postfix the older ones now with _v1.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 113fdd0ce6d9..6bebc503d727 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -718,7 +718,7 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 }
 
 /**
- * _opp_add_dynamic() - Allocate a dynamic OPP.
+ * _opp_add_v1() - Allocate a OPP based on v1 bindings.
  * @dev:	device for which we do this operation
  * @freq:	Frequency in Hz for this OPP
  * @u_volt:	Voltage in uVolts for this OPP
@@ -744,8 +744,8 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
  *		Duplicate OPPs (both freq and volt are same) and !opp->available
  * -ENOMEM	Memory allocation failure
  */
-static int _opp_add_dynamic(struct device *dev, unsigned long freq,
-			    long u_volt, bool dynamic)
+static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
+		       bool dynamic)
 {
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
@@ -949,7 +949,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
  */
 int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 {
-	return _opp_add_dynamic(dev, freq, u_volt, true);
+	return _opp_add_v1(dev, freq, u_volt, true);
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_add);
 
@@ -1251,7 +1251,7 @@ static int _of_add_opp_table_v1(struct device *dev)
 		unsigned long freq = be32_to_cpup(val++) * 1000;
 		unsigned long volt = be32_to_cpup(val++);
 
-		if (_opp_add_dynamic(dev, freq, volt, false))
+		if (_opp_add_v1(dev, freq, volt, false))
 			dev_warn(dev, "%s: Failed to add OPP %ld\n",
 				 __func__, freq);
 		nr -= 2;
-- 
2.4.0

[PATCH 05/16] PM / OPP: Parse all power-supply related bindings together

[Viresh Kumar]

Move all DT parsing for the power supplies to a single function, rather
than keeping them at separate places. This will help manage things
properly.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 6bebc503d727..463417b6838c 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -787,9 +787,10 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 }
 
 /* TODO: Support multiple regulators */
-static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev)
+static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
 {
 	u32 microvolt[3] = {0};
+	u32 val;
 	int count, ret;
 
 	count = of_property_count_u32_elems(opp->np, "opp-microvolt");
@@ -815,6 +816,9 @@ static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev)
 	opp->u_volt_min = microvolt[1];
 	opp->u_volt_max = microvolt[2];
 
+	if (!of_property_read_u32(opp->np, "opp-microamp", &val))
+		opp->u_amp = val;
+
 	return 0;
 }
 
@@ -879,13 +883,10 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	if (!of_property_read_u32(np, "clock-latency-ns", &val))
 		new_opp->clock_latency_ns = val;
 
-	ret = opp_get_microvolt(new_opp, dev);
+	ret = opp_parse_supplies(new_opp, dev);
 	if (ret)
 		goto free_opp;
 
-	if (!of_property_read_u32(new_opp->np, "opp-microamp", &val))
-		new_opp->u_amp = val;
-
 	ret = _opp_add(dev, new_opp, dev_opp);
 	if (ret)
 		goto free_opp;
-- 
2.4.0

[PATCH 06/16] PM / OPP: Create separate structure for regulator/supplies

[Viresh Kumar]

Support for multiple regulators will be added later, until then move
all power-supply related information in a separate structure.

To make allocating/freeing memory for these supply structures easy,
allocate memory for them along with opp.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c    | 57 ++++++++++++++++++++++++++--------------
 drivers/base/power/opp/debugfs.c |  9 ++++---
 drivers/base/power/opp/opp.h     | 29 ++++++++++++++------
 3 files changed, 63 insertions(+), 32 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 463417b6838c..14e5fa10be2d 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -127,7 +127,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 	if (IS_ERR_OR_NULL(tmp_opp) || !tmp_opp->available)
 		pr_err("%s: Invalid parameters\n", __func__);
 	else
-		v = tmp_opp->u_volt;
+		v = tmp_opp->supplies[0].u_volt;
 
 	return v;
 }
@@ -491,13 +491,14 @@ struct device_list_opp *_add_list_dev(const struct device *dev,
 /**
  * _add_device_opp() - Find device OPP table or allocate a new one
  * @dev:	device for which we do this operation
+ * @supply_count: Number of supplies available for each OPP.
  *
  * It tries to find an existing table first, if it couldn't find one, it
  * allocates a new OPP table and returns that.
  *
  * Return: valid device_opp pointer if success, else NULL.
  */
-static struct device_opp *_add_device_opp(struct device *dev)
+static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 {
 	struct device_opp *dev_opp;
 	struct device_list_opp *list_dev;
@@ -515,6 +516,7 @@ static struct device_opp *_add_device_opp(struct device *dev)
 	if (!dev_opp)
 		return NULL;
 
+	dev_opp->supply_count = supply_count;
 	INIT_LIST_HEAD(&dev_opp->dev_list);
 
 	list_dev = _add_list_dev(dev, dev_opp);
@@ -652,24 +654,31 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_remove);
 
-static struct dev_pm_opp *_allocate_opp(struct device *dev,
-					struct device_opp **dev_opp)
+static struct dev_pm_opp *
+_allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count)
 {
 	struct dev_pm_opp *opp;
+	size_t size = sizeof(*opp);
+
+	/* Memory for the OPP and its supplies is allocated together */
+	size += supply_count * sizeof(*opp->supplies);
 
 	/* allocate new OPP node */
-	opp = kzalloc(sizeof(*opp), GFP_KERNEL);
+	opp = kzalloc(size, GFP_KERNEL);
 	if (!opp)
 		return NULL;
 
 	INIT_LIST_HEAD(&opp->node);
 
-	*dev_opp = _add_device_opp(dev);
+	*dev_opp = _add_device_opp(dev, supply_count);
 	if (!*dev_opp) {
 		kfree(opp);
 		return NULL;
 	}
 
+	/* Supplies points to the memory right after the opp */
+	opp->supplies = (struct opp_supply *)(opp + 1);
+
 	return opp;
 }
 
@@ -699,10 +708,12 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 
 		/* Duplicate OPPs */
 		dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n",
-			 __func__, opp->rate, opp->u_volt, opp->available,
-			 new_opp->rate, new_opp->u_volt, new_opp->available);
+			 __func__, opp->rate, opp->supplies[0].u_volt,
+			 opp->available, new_opp->rate,
+			 new_opp->supplies[0].u_volt, new_opp->available);
 
-		return opp->available && new_opp->u_volt == opp->u_volt ?
+		return opp->available &&
+			opp->supplies[0].u_volt == new_opp->supplies[0].u_volt ?
 			0 : -EEXIST;
 	}
 
@@ -754,7 +765,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 	/* Hold our list modification lock here */
 	mutex_lock(&dev_opp_list_lock);
 
-	new_opp = _allocate_opp(dev, &dev_opp);
+	new_opp = _allocate_opp(dev, &dev_opp, 1);
 	if (!new_opp) {
 		ret = -ENOMEM;
 		goto unlock;
@@ -762,7 +773,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 
 	/* populate the opp table */
 	new_opp->rate = freq;
-	new_opp->u_volt = u_volt;
+	new_opp->supplies[0].u_volt = u_volt;
 	new_opp->available = true;
 	new_opp->dynamic = dynamic;
 
@@ -789,6 +800,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 /* TODO: Support multiple regulators */
 static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
 {
+	struct opp_supply *supply = &opp->supplies[0];
 	u32 microvolt[3] = {0};
 	u32 val;
 	int count, ret;
@@ -812,12 +824,12 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
 		return -EINVAL;
 	}
 
-	opp->u_volt = microvolt[0];
-	opp->u_volt_min = microvolt[1];
-	opp->u_volt_max = microvolt[2];
+	supply->u_volt = microvolt[0];
+	supply->u_volt_min = microvolt[1];
+	supply->u_volt_max = microvolt[2];
 
 	if (!of_property_read_u32(opp->np, "opp-microamp", &val))
-		opp->u_amp = val;
+		supply->u_amp = val;
 
 	return 0;
 }
@@ -826,6 +838,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
  * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings)
  * @dev:	device for which we do this operation
  * @np:		device node
+ * @supply_count: Number of supplies available for each OPP
  *
  * This function adds an opp definition to the opp list and returns status. The
  * opp can be controlled using dev_pm_opp_enable/disable functions and may be
@@ -845,10 +858,12 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
  * -ENOMEM	Memory allocation failure
  * -EINVAL	Failed parsing the OPP node
  */
-static int _opp_add_static_v2(struct device *dev, struct device_node *np)
+static int _opp_add_static_v2(struct device *dev, struct device_node *np,
+			      int supply_count)
 {
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
+	struct opp_supply *supply;
 	u64 rate;
 	u32 val;
 	int ret;
@@ -856,7 +871,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 	/* Hold our list modification lock here */
 	mutex_lock(&dev_opp_list_lock);
 
-	new_opp = _allocate_opp(dev, &dev_opp);
+	new_opp = _allocate_opp(dev, &dev_opp, supply_count);
 	if (!new_opp) {
 		ret = -ENOMEM;
 		goto unlock;
@@ -906,9 +921,10 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
 
 	mutex_unlock(&dev_opp_list_lock);
 
+	supply = &new_opp->supplies[0];
 	pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n",
-		 __func__, new_opp->turbo, new_opp->rate, new_opp->u_volt,
-		 new_opp->u_volt_min, new_opp->u_volt_max,
+		 __func__, new_opp->turbo, new_opp->rate, supply->u_volt,
+		 supply->u_volt_min, supply->u_volt_max,
 		 new_opp->clock_latency_ns);
 
 	/*
@@ -1195,7 +1211,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	for_each_available_child_of_node(opp_np, np) {
 		count++;
 
-		ret = _opp_add_static_v2(dev, np);
+		/* Todo: Add support for multiple supplies */
+		ret = _opp_add_static_v2(dev, np, 1);
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index 865cbfa24640..e6ba29c04513 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -34,6 +34,7 @@ void opp_debug_remove_one(struct dev_pm_opp *opp)
 int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp)
 {
 	struct dentry *pdentry = dev_opp->dentry;
+	struct opp_supply *supply = &opp->supplies[0];
 	struct dentry *d;
 	char name[15];
 
@@ -59,18 +60,18 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp)
 		return -ENOMEM;
 
 	if (!debugfs_create_u32("u_volt_target", S_IRUGO, d,
-				(u32 *)&opp->u_volt))
+				(u32 *)&supply->u_volt))
 		return -ENOMEM;
 
 	if (!debugfs_create_u32("u_volt_min", S_IRUGO, d,
-				(u32 *)&opp->u_volt_min))
+				(u32 *)&supply->u_volt_min))
 		return -ENOMEM;
 
 	if (!debugfs_create_u32("u_volt_max", S_IRUGO, d,
-				(u32 *)&opp->u_volt_max))
+				(u32 *)&supply->u_volt_max))
 		return -ENOMEM;
 
-	if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&opp->u_amp))
+	if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&supply->u_amp))
 		return -ENOMEM;
 
 	if (!debugfs_create_u32("clock_latency_ns", S_IRUGO, d,
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 0e4c27fb0c4a..a7a6917d6fbd 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -39,6 +39,22 @@
  */
 
 /**
+ * struct opp_supply - Per power-supply structure
+ * @u_volt:	Target voltage in microvolts corresponding to this OPP
+ * @u_volt_min:	Minimum voltage in microvolts corresponding to this OPP
+ * @u_volt_max:	Maximum voltage in microvolts corresponding to this OPP
+ * @u_amp:	Maximum current drawn by the device in microamperes
+ *
+ * This structure stores the OPP power-supply information for a given device.
+ */
+struct opp_supply {
+	unsigned long u_volt;
+	unsigned long u_volt_min;
+	unsigned long u_volt_max;
+	unsigned long u_amp;
+};
+
+/**
  * struct dev_pm_opp - Generic OPP description structure
  * @node:	opp list node. The nodes are maintained throughout the lifetime
  *		of boot. It is expected only an optimal set of OPPs are
@@ -52,10 +68,7 @@
  * @available:	true/false - marks if this OPP as available or not
  * @turbo:	true if turbo (boost) OPP
  * @rate:	Frequency in hertz
- * @u_volt:	Target voltage in microvolts corresponding to this OPP
- * @u_volt_min:	Minimum voltage in microvolts corresponding to this OPP
- * @u_volt_max:	Maximum voltage in microvolts corresponding to this OPP
- * @u_amp:	Maximum current drawn by the device in microamperes
+ * @supplies:	Array of power-supplies for the device.
  * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
  *		frequency from any other OPP's frequency.
  * @dev_opp:	points back to the device_opp struct this opp belongs to
@@ -73,10 +86,8 @@ struct dev_pm_opp {
 	bool turbo;
 	unsigned long rate;
 
-	unsigned long u_volt;
-	unsigned long u_volt_min;
-	unsigned long u_volt_max;
-	unsigned long u_amp;
+	struct opp_supply *supplies;
+
 	unsigned long clock_latency_ns;
 
 	struct device_opp *dev_opp;
@@ -123,6 +134,7 @@ struct device_list_opp {
  * @dev_list:	list of devices that share these OPPs
  * @opp_list:	list of opps
  * @np:		struct device_node pointer for opp's DT node.
+ * @supply_count: Number of power-supplies
  * @shared_opp: OPP is shared between multiple devices.
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
@@ -145,6 +157,7 @@ struct device_opp {
 
 	struct device_node *np;
 	unsigned long clock_latency_ns_max;
+	unsigned int supply_count;
 	bool shared_opp;
 	struct dev_pm_opp *suspend_opp;
 
-- 
2.4.0

[PATCH 07/16] PM / OPP: Add multiple regulators support

[Viresh Kumar]

This adds support to parse multiple regulators or power-supplies in OPP
core. This doesn't use those values yet.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c    | 197 +++++++++++++++++++++++++++++++--------
 drivers/base/power/opp/debugfs.c |  53 +++++++----
 drivers/base/power/opp/opp.h     |   4 +
 3 files changed, 201 insertions(+), 53 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 14e5fa10be2d..d6e945ec6467 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -502,21 +502,27 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 {
 	struct device_opp *dev_opp;
 	struct device_list_opp *list_dev;
+	size_t size;
 
 	/* Check for existing list for 'dev' first */
 	dev_opp = _find_device_opp(dev);
 	if (!IS_ERR(dev_opp))
 		return dev_opp;
 
+	/* Allocate size for supply-names with dev_opp */
+	size = sizeof(*dev_opp) + supply_count * sizeof(*dev_opp->supply_names);
+
 	/*
 	 * Allocate a new device OPP table. In the infrequent case where a new
 	 * device is needed to be added, we pay this penalty.
 	 */
-	dev_opp = kzalloc(sizeof(*dev_opp), GFP_KERNEL);
+	dev_opp = kzalloc(size, GFP_KERNEL);
 	if (!dev_opp)
 		return NULL;
 
 	dev_opp->supply_count = supply_count;
+	dev_opp->supply_names = (const char **)(dev_opp + 1);
+
 	INIT_LIST_HEAD(&dev_opp->dev_list);
 
 	list_dev = _add_list_dev(dev, dev_opp);
@@ -525,6 +531,13 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 		return NULL;
 	}
 
+	/*
+	 * Initialize supply-name as dev-name for single supplies. This is
+	 * required for the debugfs code.
+	 */
+	if (supply_count == 1)
+		*dev_opp->supply_names = dev_name(dev);
+
 	srcu_init_notifier_head(&dev_opp->srcu_head);
 	INIT_LIST_HEAD(&dev_opp->opp_list);
 
@@ -682,6 +695,23 @@ _allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count)
 	return opp;
 }
 
+static bool _supplies_match(struct device_opp *dev_opp,
+			    struct dev_pm_opp *old_opp,
+			    struct dev_pm_opp *new_opp)
+{
+	struct opp_supply *old = old_opp->supplies;
+	struct opp_supply *new = new_opp->supplies;
+	int i;
+
+	for (i = 0; i < dev_opp->supply_count; i++)
+		if (old[i].u_volt != new[i].u_volt ||
+		    old[i].u_volt_min != new[i].u_volt_min ||
+		    old[i].u_volt_max != new[i].u_volt_max)
+			return false;
+
+	return true;
+}
+
 static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 		    struct device_opp *dev_opp)
 {
@@ -712,9 +742,8 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 			 opp->available, new_opp->rate,
 			 new_opp->supplies[0].u_volt, new_opp->available);
 
-		return opp->available &&
-			opp->supplies[0].u_volt == new_opp->supplies[0].u_volt ?
-			0 : -EEXIST;
+		return opp->available && _supplies_match(dev_opp, opp, new_opp)
+			? 0 : -EEXIST;
 	}
 
 	new_opp->dev_opp = dev_opp;
@@ -797,41 +826,99 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 	return ret;
 }
 
-/* TODO: Support multiple regulators */
-static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
+/* returns the number of entries used from microvolt */
+static void opp_parse_single_supply(struct opp_supply *supply, bool triplet,
+				    u32 *microvolt, u32 *microamp)
 {
-	struct opp_supply *supply = &opp->supplies[0];
-	u32 microvolt[3] = {0};
-	u32 val;
-	int count, ret;
+	if (triplet) {
+		supply->u_volt = microvolt[0];
+		supply->u_volt_min = microvolt[1];
+		supply->u_volt_max = microvolt[2];
+	} else {
+		supply->u_volt = microvolt[0];
+		supply->u_volt_min = microvolt[0];
+		supply->u_volt_max = microvolt[0];
+	}
+
+	supply->u_amp = *microamp;
+}
+
+static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
+			      int supply_count, bool triplets)
+{
+	struct opp_supply *supply = opp->supplies;
+	int i, vcount, icount, ret, step;
+	u32 *microvolt, *microamp;
 
-	count = of_property_count_u32_elems(opp->np, "opp-microvolt");
-	if (!count)
+	vcount = of_property_count_u32_elems(opp->np, "opp-microvolt");
+	if (!vcount)
 		return 0;
 
-	/* There can be one or three elements here */
-	if (count != 1 && count != 3) {
-		dev_err(dev, "%s: Invalid number of elements in opp-microvolt property (%d)\n",
-			__func__, count);
-		return -EINVAL;
-	}
+	icount = of_property_count_u32_elems(opp->np, "opp-microamp");
+	if (!icount)
+		return 0;
+
+	/* Allocate memory for volt/amp */
+	microvolt = kcalloc(vcount, sizeof(*microvolt), GFP_KERNEL);
+	microamp = kcalloc(icount, sizeof(*microamp), GFP_KERNEL);
+	if (!microvolt || !microamp)
+		return -ENOMEM;
 
 	ret = of_property_read_u32_array(opp->np, "opp-microvolt", microvolt,
-					 count);
+					 vcount);
 	if (ret) {
 		dev_err(dev, "%s: error parsing opp-microvolt: %d\n", __func__,
 			ret);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto free_microvolt;
 	}
 
-	supply->u_volt = microvolt[0];
-	supply->u_volt_min = microvolt[1];
-	supply->u_volt_max = microvolt[2];
+	ret = of_property_read_u32_array(opp->np, "opp-microamp", microamp,
+					 icount);
+	if (ret) {
+		dev_err(dev, "%s: error parsing opp-microamp: %d\n", __func__,
+			ret);
+		ret = -EINVAL;
+		goto free_microvolt;
+	}
 
-	if (!of_property_read_u32(opp->np, "opp-microamp", &val))
-		supply->u_amp = val;
+	/*
+	 * For single supply, "opp-microvolt-triplets" is not mandatory and we
+	 * need to find it ourselves.
+	 */
+	if (supply_count == 1) {
+		if (vcount == 1) {
+			triplets = false;
+		} else if (vcount == 3) {
+			triplets = true;
+		} else {
+			dev_err(dev, "%s: opp-microvolt property should have 1 or 3 elements (%d)\n",
+				__func__, vcount);
+			ret = -EINVAL;
+			goto free_microvolt;
+		}
+	}
 
-	return 0;
+	step = triplets ? 3 : 1;
+
+	/* microvolt sanity check */
+	if ((vcount != supply_count * step) || (icount != supply_count)) {
+		dev_err(dev, "%s: Invalid number of elements in opp-microvolt/amp property (v=%d i=%d c=%d t=%d)\n",
+			__func__, vcount, icount, supply_count * step,
+			triplets);
+		ret = -EINVAL;
+		goto free_microvolt;
+	}
+
+	for (i = 0; i < supply_count; i++)
+		opp_parse_single_supply(supply + i, triplets,
+					microvolt + step * i, microamp + i);
+
+free_microvolt:
+	kfree(microamp);
+	kfree(microvolt);
+
+	return ret;
 }
 
 /**
@@ -839,6 +926,8 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
  * @dev:	device for which we do this operation
  * @np:		device node
  * @supply_count: Number of supplies available for each OPP
+ * @triplets:	If true, microvolt property should be in form <target min max>,
+ *		else <target>.
  *
  * This function adds an opp definition to the opp list and returns status. The
  * opp can be controlled using dev_pm_opp_enable/disable functions and may be
@@ -859,7 +948,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev)
  * -EINVAL	Failed parsing the OPP node
  */
 static int _opp_add_static_v2(struct device *dev, struct device_node *np,
-			      int supply_count)
+			      int supply_count, bool triplets)
 {
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
@@ -898,7 +987,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np,
 	if (!of_property_read_u32(np, "clock-latency-ns", &val))
 		new_opp->clock_latency_ns = val;
 
-	ret = opp_parse_supplies(new_opp, dev);
+	ret = opp_parse_supplies(new_opp, dev, supply_count, triplets);
 	if (ret)
 		goto free_opp;
 
@@ -1165,6 +1254,10 @@ void dev_pm_opp_of_remove_table(struct device *dev)
 
 	/* Find if dev_opp manages a single device */
 	if (list_is_singular(&dev_opp->dev_list)) {
+		/* Free dev_opp if no OPPs are added yet */
+		if (list_empty(&dev_opp->opp_list))
+			_remove_device_opp(dev_opp);
+
 		/* Free static OPPs */
 		list_for_each_entry_safe(opp, tmp, &dev_opp->opp_list, node) {
 			if (!opp->dynamic)
@@ -1197,7 +1290,9 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 {
 	struct device_node *np;
 	struct device_opp *dev_opp;
-	int ret = 0, count = 0;
+	const char **name;
+	int ret = 0, count, supply_count, string_count;
+	bool triplets;
 
 	dev_opp = _managed_opp(opp_np);
 	if (dev_opp) {
@@ -1207,12 +1302,44 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 		return ret;
 	}
 
+	triplets = of_property_read_bool(opp_np, "opp-microvolt-triplets");
+	string_count = of_property_count_strings(opp_np, "supply-names");
+
+	/* Fallback to single power-supply if multiple aren't present */
+	if (string_count <= 0) {
+		supply_count = 1;
+		string_count = 0;
+	} else {
+		supply_count = string_count;
+	}
+
+	/*
+	 * We need to add dev_opp before adding any OPPs, so that supply_names
+	 * are valid while the OPPs are getting added.
+	 */
+	dev_opp = _add_device_opp(dev, supply_count);
+	if (!dev_opp)
+		return -ENOMEM;
+
+	/* Parse supply names */
+	name = dev_opp->supply_names;
+	for (count = 0; count < string_count; count++, name++) {
+		/* Parse supply names */
+		ret = of_property_read_string_index(opp_np, "supply-names",
+						    count, name);
+		if (ret) {
+			dev_err(dev, "%s: read supply names (%s) error (%d)\n",
+				__func__, opp_np->name, ret);
+			goto free_table;
+		}
+	}
+
 	/* We have opp-list node now, iterate over it and add OPPs */
+	count = 0;
 	for_each_available_child_of_node(opp_np, np) {
 		count++;
 
-		/* Todo: Add support for multiple supplies */
-		ret = _opp_add_static_v2(dev, np, 1);
+		ret = _opp_add_static_v2(dev, np, supply_count, triplets);
 		if (ret) {
 			dev_err(dev, "%s: Failed to add OPP, %d\n", __func__,
 				ret);
@@ -1221,12 +1348,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	}
 
 	/* There should be one of more OPP defined */
-	if (WARN_ON(!count))
-		return -ENOENT;
-
-	dev_opp = _find_device_opp(dev);
-	if (WARN_ON(IS_ERR(dev_opp))) {
-		ret = PTR_ERR(dev_opp);
+	if (WARN_ON(!count)) {
+		ret = -ENOENT;
 		goto free_table;
 	}
 
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c
index e6ba29c04513..de2083d69297 100644
--- a/drivers/base/power/opp/debugfs.c
+++ b/drivers/base/power/opp/debugfs.c
@@ -31,12 +31,44 @@ void opp_debug_remove_one(struct dev_pm_opp *opp)
 	debugfs_remove_recursive(opp->dentry);
 }
 
+int opp_debug_create_supplies(struct dev_pm_opp *opp,
+			      struct device_opp *dev_opp, struct dentry *dentry)
+{
+	struct opp_supply *supply = opp->supplies;
+	char name[NAME_MAX];
+	const char **supply_name = dev_opp->supply_names;
+	int i;
+
+	for (i = 0; i < dev_opp->supply_count; i++, supply_name++) {
+		snprintf(name, sizeof(name), "%s_u_volt_target", *supply_name);
+		if (!debugfs_create_u32(name, S_IRUGO, dentry,
+					(u32 *)&supply->u_volt))
+			return -ENOMEM;
+
+		snprintf(name, sizeof(name), "%s_u_volt_min", *supply_name);
+		if (!debugfs_create_u32(name, S_IRUGO, dentry,
+					(u32 *)&supply->u_volt_min))
+			return -ENOMEM;
+
+		snprintf(name, sizeof(name), "%s_u_volt_max", *supply_name);
+		if (!debugfs_create_u32(name, S_IRUGO, dentry,
+					(u32 *)&supply->u_volt_max))
+			return -ENOMEM;
+	}
+
+	if (!debugfs_create_u32("u_amp", S_IRUGO, dentry,
+				(u32 *)&supply->u_amp))
+		return -ENOMEM;
+
+	return 0;
+}
+
 int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp)
 {
 	struct dentry *pdentry = dev_opp->dentry;
-	struct opp_supply *supply = &opp->supplies[0];
 	struct dentry *d;
 	char name[15];
+	int ret;
 
 	/* Rate is unique to each OPP, use it to give opp-name */
 	sprintf(name, "opp:%lu", opp->rate);
@@ -59,25 +91,14 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp)
 	if (!debugfs_create_u32("rate_hz", S_IRUGO, d, (u32 *)&opp->rate))
 		return -ENOMEM;
 
-	if (!debugfs_create_u32("u_volt_target", S_IRUGO, d,
-				(u32 *)&supply->u_volt))
-		return -ENOMEM;
-
-	if (!debugfs_create_u32("u_volt_min", S_IRUGO, d,
-				(u32 *)&supply->u_volt_min))
-		return -ENOMEM;
-
-	if (!debugfs_create_u32("u_volt_max", S_IRUGO, d,
-				(u32 *)&supply->u_volt_max))
-		return -ENOMEM;
-
-	if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&supply->u_amp))
-		return -ENOMEM;
-
 	if (!debugfs_create_u32("clock_latency_ns", S_IRUGO, d,
 				(u32 *)&opp->clock_latency_ns))
 		return -ENOMEM;
 
+	ret = opp_debug_create_supplies(opp, dev_opp, d);
+	if (ret)
+		return ret;
+
 	opp->dentry = d;
 	return 0;
 }
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index a7a6917d6fbd..16575268f6ce 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -135,6 +135,7 @@ struct device_list_opp {
  * @opp_list:	list of opps
  * @np:		struct device_node pointer for opp's DT node.
  * @supply_count: Number of power-supplies
+ * @supply_names: Array of strings containing names of the power-supplies
  * @shared_opp: OPP is shared between multiple devices.
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
@@ -157,7 +158,10 @@ struct device_opp {
 
 	struct device_node *np;
 	unsigned long clock_latency_ns_max;
+
 	unsigned int supply_count;
+	const char **supply_names;
+
 	bool shared_opp;
 	struct dev_pm_opp *suspend_opp;
 
-- 
2.4.0

[PATCH 08/16] PM / OPP: get/put regulators from OPP core

[Viresh Kumar]

When the 'supply_names' property is present, the OPP core can request
regulators for them. All the regulators from the 'supply_names' property
must be present, in order to move forward.

If an error occurs, undo all allocations and return the appropriate
error. This also helps in returning error on -EPROBE_DEFER case, where
the driver must try again at a later point of time.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 30 +++++++++++++++++++++++++++---
 drivers/base/power/opp/opp.h  |  3 +++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index d6e945ec6467..42ea5a332ddc 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -509,8 +509,10 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 	if (!IS_ERR(dev_opp))
 		return dev_opp;
 
-	/* Allocate size for supply-names with dev_opp */
-	size = sizeof(*dev_opp) + supply_count * sizeof(*dev_opp->supply_names);
+	/* Allocate memory for supply-names & regulators with dev_opp */
+	size = sizeof(*dev_opp);
+	size += supply_count * sizeof(*dev_opp->supply_names);
+	size += supply_count * sizeof(*dev_opp->regulators);
 
 	/*
 	 * Allocate a new device OPP table. In the infrequent case where a new
@@ -521,7 +523,8 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 		return NULL;
 
 	dev_opp->supply_count = supply_count;
-	dev_opp->supply_names = (const char **)(dev_opp + 1);
+	dev_opp->regulators = (struct regulator **)(dev_opp + 1);
+	dev_opp->supply_names = (const char **)(dev_opp->regulators + supply_count);
 
 	INIT_LIST_HEAD(&dev_opp->dev_list);
 
@@ -566,10 +569,15 @@ static void _kfree_device_rcu(struct rcu_head *head)
 static void _remove_device_opp(struct device_opp *dev_opp)
 {
 	struct device_list_opp *list_dev;
+	int count;
 
 	if (!list_empty(&dev_opp->opp_list))
 		return;
 
+	/* Release regulators */
+	for (count = dev_opp->supply_count - 1; count >= 0; count--)
+		regulator_put(dev_opp->regulators[count]);
+
 	list_dev = list_first_entry(&dev_opp->dev_list, struct device_list_opp,
 				    node);
 
@@ -1293,6 +1301,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	const char **name;
 	int ret = 0, count, supply_count, string_count;
 	bool triplets;
+	struct regulator *reg;
 
 	dev_opp = _managed_opp(opp_np);
 	if (dev_opp) {
@@ -1334,6 +1343,21 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 		}
 	}
 
+	/* Get supply regulators */
+	name = dev_opp->supply_names;
+	for (count = 0; count < supply_count; count++, name++) {
+		/* Parse regulators */
+		reg = regulator_get_optional(dev, *name);
+		if (IS_ERR(reg)) {
+			ret = PTR_ERR(reg);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "%s: no regulator (%s) found: %d\n",
+					__func__, *name, ret);
+			goto free_table;
+		}
+		dev_opp->regulators[count] = reg;
+	}
+
 	/* We have opp-list node now, iterate over it and add OPPs */
 	count = 0;
 	for_each_available_child_of_node(opp_np, np) {
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 16575268f6ce..830c4b654a51 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/limits.h>
 #include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
 #include <linux/rculist.h>
 #include <linux/rcupdate.h>
 
@@ -135,6 +136,7 @@ struct device_list_opp {
  * @opp_list:	list of opps
  * @np:		struct device_node pointer for opp's DT node.
  * @supply_count: Number of power-supplies
+ * @regulators:	Array of regulators
  * @supply_names: Array of strings containing names of the power-supplies
  * @shared_opp: OPP is shared between multiple devices.
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
@@ -160,6 +162,7 @@ struct device_opp {
 	unsigned long clock_latency_ns_max;
 
 	unsigned int supply_count;
+	struct regulator **regulators;
 	const char **supply_names;
 
 	bool shared_opp;
-- 
2.4.0

[PATCH 09/16] PM / OPP: Disable OPPs that aren't supported by the regulators

[Viresh Kumar]

Disable any OPPs where the connected regulators aren't able to provide
the specified voltage.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 42ea5a332ddc..04fe181b8132 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -834,6 +834,34 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 	return ret;
 }
 
+static bool opp_supported_by_regulators(struct dev_pm_opp *opp,
+					struct device_opp *dev_opp)
+{
+	struct opp_supply *supply;
+	struct regulator *reg;
+	int count;
+
+	for (count = 0; count < dev_opp->supply_count; count++) {
+		supply = opp->supplies + count;
+		reg = dev_opp->regulators[count];
+
+		/* Regulator may not be available for device */
+		if (IS_ERR(reg))
+			continue;
+
+		if (!regulator_is_supported_voltage(reg, supply->u_volt_min,
+						    supply->u_volt_max)) {
+			pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator: %s\n",
+				__func__, supply->u_volt_min,
+				supply->u_volt_max,
+				dev_opp->supply_names[count]);
+			return false;
+		}
+	}
+
+	return true;
+}
+
 /* returns the number of entries used from microvolt */
 static void opp_parse_single_supply(struct opp_supply *supply, bool triplet,
 				    u32 *microvolt, u32 *microamp)
@@ -1016,6 +1044,12 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np,
 	if (new_opp->clock_latency_ns > dev_opp->clock_latency_ns_max)
 		dev_opp->clock_latency_ns_max = new_opp->clock_latency_ns;
 
+	if (!opp_supported_by_regulators(new_opp, dev_opp)) {
+		new_opp->available = false;
+		dev_warn(dev, "%s: OPP not supported by regulators (%lu)\n",
+			 __func__, new_opp->rate);
+	}
+
 	mutex_unlock(&dev_opp_list_lock);
 
 	supply = &new_opp->supplies[0];
-- 
2.4.0

[PATCH 10/16] PM / OPP: Introduce dev_pm_opp_get_max_volt_latency()

[Viresh Kumar]

In few use cases (like: cpufreq), it is desired to get the maximum
voltage latency for changing OPPs. Add support for that.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 59 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h        |  6 +++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 04fe181b8132..e272e738a8d0 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -226,6 +226,65 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
 
 /**
+ * dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds
+ * @dev:	device for which we do this operation
+ *
+ * Return: This function returns the max voltage latency in nanoseconds.
+ *
+ * Locking: This function takes rcu_read_lock().
+ */
+unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
+{
+	struct device_opp *dev_opp;
+	struct dev_pm_opp *opp;
+	struct opp_supply *supply;
+	struct regulator *reg;
+	unsigned long latency_ns = 0;
+	unsigned long min_uV, max_uV;
+	int count, ret;
+
+	rcu_read_lock();
+
+	dev_opp = _find_device_opp(dev);
+	if (IS_ERR(dev_opp))
+		goto unlock;
+
+	/* Sum max latencies for all supplies */
+	for (count = 0; count < dev_opp->supply_count; count++) {
+		min_uV = ~0;
+		max_uV = 0;
+
+		reg = dev_opp->regulators[count];
+
+		/* Regulator may not be available for device */
+		if (IS_ERR(reg))
+			continue;
+
+		list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) {
+			if (!opp->available)
+				continue;
+
+			supply = opp->supplies + count;
+
+			if (supply->u_volt_min < min_uV)
+				min_uV = supply->u_volt_min;
+			if (supply->u_volt_max > max_uV)
+				max_uV = supply->u_volt_max;
+		}
+
+		ret = regulator_set_voltage_time(reg, min_uV, max_uV);
+		if (ret > 0)
+			latency_ns += ret * 1000;
+	}
+
+unlock:
+	rcu_read_unlock();
+
+	return latency_ns;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_volt_latency);
+
+/**
  * dev_pm_opp_get_suspend_opp() - Get suspend opp
  * @dev:	device for which we do this operation
  *
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 9a2e50337af9..fd58e2074721 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -34,6 +34,7 @@ bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp);
 
 int dev_pm_opp_get_opp_count(struct device *dev);
 unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev);
+unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev);
 struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
 
 struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
@@ -81,6 +82,11 @@ static inline unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
 	return 0;
 }
 
+static inline unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
+{
+	return 0;
+}
+
 static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
 {
 	return NULL;
-- 
2.4.0

[PATCH 11/16] PM / OPP: Introduce dev_pm_opp_get_max_transition_latency()

[Viresh Kumar]

In few use cases (like: cpufreq), it is desired to get the maximum
latency for changing OPPs. Add support for that.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 17 +++++++++++++++++
 include/linux/pm_opp.h        |  6 ++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index e272e738a8d0..d0678e804a75 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -285,6 +285,23 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_volt_latency);
 
 /**
+ * dev_pm_opp_get_max_transition_latency() - Get max transition latency in
+ *					     nanoseconds
+ * @dev:	device for which we do this operation
+ *
+ * Return: This function returns the max transition latency, in nanoseconds, to
+ * switch from one OPP to other.
+ *
+ * Locking: This function takes rcu_read_lock().
+ */
+unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev)
+{
+	return dev_pm_opp_get_max_volt_latency(dev) +
+		dev_pm_opp_get_max_clock_latency(dev);
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_transition_latency);
+
+/**
  * dev_pm_opp_get_suspend_opp() - Get suspend opp
  * @dev:	device for which we do this operation
  *
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index fd58e2074721..e36f347ff32b 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -35,6 +35,7 @@ bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp);
 int dev_pm_opp_get_opp_count(struct device *dev);
 unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev);
 unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev);
+unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev);
 struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
 
 struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
@@ -87,6 +88,11 @@ static inline unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
 	return 0;
 }
 
+static inline unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev)
+{
+	return 0;
+}
+
 static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev)
 {
 	return NULL;
-- 
2.4.0

[PATCH 12/16] PM / OPP: Parse clock and voltage tolerance for v1 bindings

[Viresh Kumar]

V2 bindings have better support for that and doesn't need special care.
To use callbacks, like dev_pm_opp_get_max_{transition|volt}_latency(),
irrespective of the bindings, the core needs to know clock/voltage
tolerance for the earlier bindings.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 29 ++++++++++++++++++++++++++---
 drivers/base/power/opp/opp.h  |  5 +++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index d0678e804a75..9633f2fc3481 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -568,16 +568,19 @@ struct device_list_opp *_add_list_dev(const struct device *dev,
  * _add_device_opp() - Find device OPP table or allocate a new one
  * @dev:	device for which we do this operation
  * @supply_count: Number of supplies available for each OPP.
+ * @opp_v2:	true if parsing v2 bindings, else false.
  *
  * It tries to find an existing table first, if it couldn't find one, it
  * allocates a new OPP table and returns that.
  *
  * Return: valid device_opp pointer if success, else NULL.
  */
-static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
+static struct device_opp *_add_device_opp(struct device *dev, int supply_count,
+					  bool opp_v2)
 {
 	struct device_opp *dev_opp;
 	struct device_list_opp *list_dev;
+	struct device_node *np;
 	size_t size;
 
 	/* Check for existing list for 'dev' first */
@@ -598,6 +601,22 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count)
 	if (!dev_opp)
 		return NULL;
 
+	if (!opp_v2) {
+		/*
+		 * Only required for backward compatibility with v1 bindings.
+		 */
+		np = of_node_get(dev->of_node);
+		if (np) {
+			u32 val;
+
+			if (!of_property_read_u32(np, "clock-latency", &val))
+				dev_opp->clock_latency_ns_max = val;
+			of_property_read_u32(np, "voltage-tolerance",
+					     &dev_opp->voltage_tolerance_v1);
+			of_node_put(np);
+		}
+	}
+
 	dev_opp->supply_count = supply_count;
 	dev_opp->regulators = (struct regulator **)(dev_opp + 1);
 	dev_opp->supply_names = (const char **)(dev_opp->regulators + supply_count);
@@ -767,7 +786,7 @@ _allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count)
 
 	INIT_LIST_HEAD(&opp->node);
 
-	*dev_opp = _add_device_opp(dev, supply_count);
+	*dev_opp = _add_device_opp(dev, supply_count, false);
 	if (!*dev_opp) {
 		kfree(opp);
 		return NULL;
@@ -873,6 +892,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 {
 	struct device_opp *dev_opp;
 	struct dev_pm_opp *new_opp;
+	unsigned long tol;
 	int ret;
 
 	/* Hold our list modification lock here */
@@ -886,7 +906,10 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
 
 	/* populate the opp table */
 	new_opp->rate = freq;
+	tol = u_volt * dev_opp->voltage_tolerance_v1 / 100;
 	new_opp->supplies[0].u_volt = u_volt;
+	new_opp->supplies[0].u_volt_min = u_volt - tol;
+	new_opp->supplies[0].u_volt_max = u_volt + tol;
 	new_opp->available = true;
 	new_opp->dynamic = dynamic;
 
@@ -1436,7 +1459,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 	 * We need to add dev_opp before adding any OPPs, so that supply_names
 	 * are valid while the OPPs are getting added.
 	 */
-	dev_opp = _add_device_opp(dev, supply_count);
+	dev_opp = _add_device_opp(dev, supply_count, true);
 	if (!dev_opp)
 		return -ENOMEM;
 
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 830c4b654a51..55205cbe59dc 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -142,6 +142,8 @@ struct device_list_opp {
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
  * @dentry_name: Name of the real dentry.
  *
+ * @voltage_tolerance_v1: In percentage, for v1 bindings only.
+ *
  * This is an internal data structure maintaining the link to opps attached to
  * a device. This structure is not meant to be shared to users as it is
  * meant for book keeping and private to OPP library.
@@ -173,6 +175,9 @@ struct device_opp {
 	struct dentry *dentry;
 	char dentry_name[NAME_MAX];
 #endif
+
+	/* For backward compatibility with v1 bindings */
+	unsigned int voltage_tolerance_v1;
 };
 
 /* Routines internal to opp core */
-- 
2.4.0

[PATCH 13/16] PM / OPP: Manage device clk as well

[Viresh Kumar]

OPP core has got almost everything now to manage device's OPP
transitions, the only thing left is device's clk. Get that as well.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 9 +++++++++
 drivers/base/power/opp/opp.h  | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 9633f2fc3481..a04dcacb8a07 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -636,6 +636,11 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count,
 	if (supply_count == 1)
 		*dev_opp->supply_names = dev_name(dev);
 
+	/* Find clk for the device */
+	dev_opp->clk = clk_get(dev, NULL);
+	if (IS_ERR(dev_opp->clk))
+		dev_dbg(dev, "%s: Couldn't find clock\n", __func__);
+
 	srcu_init_notifier_head(&dev_opp->srcu_head);
 	INIT_LIST_HEAD(&dev_opp->opp_list);
 
@@ -669,6 +674,10 @@ static void _remove_device_opp(struct device_opp *dev_opp)
 	if (!list_empty(&dev_opp->opp_list))
 		return;
 
+	/* Release clk */
+	if (!IS_ERR(dev_opp->clk))
+		clk_put(dev_opp->clk);
+
 	/* Release regulators */
 	for (count = dev_opp->supply_count - 1; count >= 0; count--)
 		regulator_put(dev_opp->regulators[count]);
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 55205cbe59dc..e36ee6296f1a 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -14,6 +14,7 @@
 #ifndef __DRIVER_OPP_H__
 #define __DRIVER_OPP_H__
 
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -135,6 +136,7 @@ struct device_list_opp {
  * @dev_list:	list of devices that share these OPPs
  * @opp_list:	list of opps
  * @np:		struct device_node pointer for opp's DT node.
+ * @clk:	struct clk pointer
  * @supply_count: Number of power-supplies
  * @regulators:	Array of regulators
  * @supply_names: Array of strings containing names of the power-supplies
@@ -163,6 +165,7 @@ struct device_opp {
 	struct device_node *np;
 	unsigned long clock_latency_ns_max;
 
+	struct clk *clk;
 	unsigned int supply_count;
 	struct regulator **regulators;
 	const char **supply_names;
-- 
2.4.0

[PATCH 14/16] PM / OPP: Add dev_pm_opp_set_regulator() to specify regulator

[Viresh Kumar]

The new OPP V2 bindings have a way to find supplies for a particular
device. But the old V1 bindings doesn't have any API to do it today.

This is required in order to move the complexity of switching OPPs (i.e.
changing clock and voltages), into the OPP core, rather then keeping
that in individual drivers.

This patch adds another API, to be used only for V1 bindings, which
first finds the regulator for the dev_opp and then disables all OPPs
that aren't supported by the regulator.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 74 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_opp.h        |  6 ++++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index a04dcacb8a07..4ee0911b97ea 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -970,6 +970,73 @@ static bool opp_supported_by_regulators(struct dev_pm_opp *opp,
 	return true;
 }
 
+/**
+ * dev_pm_opp_set_regulator() - Set regulator for the device OPP
+ * @dev: Device for which the regulator has to be set.
+ * @id: String used to find regulator.
+ *
+ * This is only required for the V1 bindings, as regulator is automatically
+ * found for the V2 bindings. OPP core finds a regulator with name <reg>-supply,
+ * and then get/put it automatically. The name of the debugfs files isn't
+ * changed however, to keep it simple. They are named as <dev_name>_*.
+ */
+int dev_pm_opp_set_regulator(struct device *dev, const char *id)
+{
+	struct device_opp *dev_opp;
+	struct dev_pm_opp *opp;
+	struct regulator *reg;
+	int ret = 0;
+
+	rcu_read_lock();
+
+	dev_opp = _find_device_opp(dev);
+	if (IS_ERR(dev_opp)) {
+		ret = PTR_ERR(dev_opp);
+		dev_err(dev, "%s: no device opp found: %d\n", __func__, ret);
+		goto unlock;
+	}
+
+	/* Do we already have a regulator attached to this dev_opp? */
+	if (!IS_ERR_OR_NULL(*dev_opp->regulators)) {
+		dev_err(dev, "%s: can't add (%s), regulator already present\n",
+			__func__, id);
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	reg = regulator_get_optional(dev, id);
+
+	/*
+	 * Few platforms may not have regulators for the device, and we need to
+	 * save error number in that case as well.
+	 */
+	*dev_opp->regulators = reg;
+
+	if (IS_ERR(reg)) {
+		ret = PTR_ERR(reg);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "%s: no regulator (%s) found: %d\n",
+				__func__, id, ret);
+		goto unlock;
+	}
+
+	list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) {
+		/* Disable OPPs, that aren't supported by the regulator */
+		if (opp_supported_by_regulators(opp, dev_opp))
+			continue;
+
+		opp->available = false;
+		dev_warn(dev, "%s: disabled OPP (%lu), not supported by regulator (%s)\n",
+			 __func__, opp->rate, id);
+	}
+
+unlock:
+	rcu_read_unlock();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator);
+
 /* returns the number of entries used from microvolt */
 static void opp_parse_single_supply(struct opp_supply *supply, bool triplet,
 				    u32 *microvolt, u32 *microamp)
@@ -1493,9 +1560,14 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np)
 		if (IS_ERR(reg)) {
 			ret = PTR_ERR(reg);
 			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "%s: deferring, no regulator (%s) found: %d\n",
+					__func__, *name, ret);
+			/* Regulator may not be compulsory for the device */
+			if (!string_count)
 				dev_err(dev, "%s: no regulator (%s) found: %d\n",
 					__func__, *name, ret);
-			goto free_table;
+			else
+				goto free_table;
 		}
 		dev_opp->regulators[count] = reg;
 	}
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index e36f347ff32b..e8aee03b974a 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -57,6 +57,7 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq);
 int dev_pm_opp_disable(struct device *dev, unsigned long freq);
 
 struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
+int dev_pm_opp_set_regulator(struct device *dev, const char *id);
 #else
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
@@ -141,6 +142,11 @@ static inline struct srcu_notifier_head *dev_pm_opp_get_notifier(
 {
 	return ERR_PTR(-EINVAL);
 }
+
+static inline int dev_pm_opp_set_regulator(struct device *dev, const char *id)
+{
+	return -EINVAL;
+}
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
-- 
2.4.0

[PATCH 15/16] PM / OPP: Add dev_pm_opp_set_rate()

[Viresh Kumar]

This adds a routine, dev_pm_opp_set_rate(), responsible for configuring
power-supplies and clock source for on an OPP.

The OPP is found by matching against the target_freq passed to the
routine. This shall replace similar code present in most of the OPP
users and help simplify them a lot.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 152 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h        |   7 ++
 2 files changed, 159 insertions(+)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index 4ee0911b97ea..9fbc38222ca0 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -523,6 +523,158 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
 
+/*
+ * _set_opp_voltages() - Configure device supplies for an OPP
+ * @dev:	device for which we do this operation
+ * @dev_opp:	dev_opp for the device
+ * @opp:	opp for getting supply voltage levels
+ *
+ * This is used to configure all the power supplies, used by the device, to the
+ * voltage levels specified by a particular OPP.
+ */
+static int _set_opp_voltages(struct device *dev, struct device_opp *dev_opp,
+			     struct dev_pm_opp *opp)
+{
+	struct opp_supply *supply;
+	struct regulator *reg;
+	const char *name;
+	int count, ret;
+
+	/* Sum max latencies for all supplies */
+	for (count = 0; count < dev_opp->supply_count; count++) {
+		supply = &opp->supplies[count];
+		reg = dev_opp->regulators[count];
+		name = dev_opp->supply_names[count];
+
+		/* Regulator may not be available for device */
+		if (IS_ERR(reg)) {
+			dev_dbg(dev, "%s: skipping %s regulator: %ld\n",
+				__func__, name, PTR_ERR(reg));
+			continue;
+		}
+
+		dev_dbg(dev, "%s: Supply: %s, voltages (mV): %lu %lu %lu\n",
+			__func__, name, supply->u_volt_min, supply->u_volt,
+			supply->u_volt_max);
+
+		ret = regulator_set_voltage_triplet(reg, supply->u_volt_min,
+						    supply->u_volt,
+						    supply->u_volt_max);
+		if (ret) {
+			dev_err(dev, "%s: failed to set voltage for %s regulator (%lu %lu %lu mV): %d\n",
+				__func__, name, supply->u_volt_min,
+				supply->u_volt, supply->u_volt_max, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * dev_pm_opp_set_rate() - Configure new OPP based on frequency
+ * @dev:	 device for which we do this operation
+ * @target_freq: frequency to achieve
+ *
+ * This configures the power-supplies and clock source to the levels specified
+ * by the OPP corresponding to the target_freq. It takes all necessary locks
+ * required for the operation and the caller doesn't need to care about the rcu
+ * locks.
+ */
+int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
+{
+	struct device_opp *dev_opp;
+	struct dev_pm_opp *old_opp, *opp;
+	struct clk *clk;
+	unsigned long freq, old_freq;
+	int ret;
+
+	if (unlikely(!dev || !target_freq)) {
+		pr_err("%s: Invalid arguments dev=%p, freq=%lu\n", __func__,
+		       dev, target_freq);
+		return -EINVAL;
+	}
+
+	dev_opp = _find_device_opp(dev);
+	if (IS_ERR(dev_opp)) {
+		dev_err(dev, "%s: device opp doesn't exist\n", __func__);
+		return PTR_ERR(dev_opp);
+	}
+
+	clk = dev_opp->clk;
+	if (IS_ERR(clk)) {
+		dev_err(dev, "%s: No clock available for the device\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	freq = clk_round_rate(clk, target_freq);
+	if ((long)freq <= 0)
+		freq = target_freq;
+
+	old_freq = clk_get_rate(clk);
+
+	/* Return early if nothing to do */
+	if (old_freq == freq) {
+		dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n",
+			__func__, freq);
+		return 0;
+	}
+
+	rcu_read_lock();
+
+	old_opp = dev_pm_opp_find_freq_ceil(dev, &old_freq);
+	opp = dev_pm_opp_find_freq_ceil(dev, &freq);
+
+	if (IS_ERR(opp)) {
+		ret = PTR_ERR(opp);
+		dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",
+			__func__, freq, ret);
+		goto unlock;
+	}
+
+	/* Scaling up? Scale voltage before frequency */
+	if (freq > old_freq) {
+		ret = _set_opp_voltages(dev, dev_opp, opp);
+		if (ret)
+			goto restore_voltage;
+	}
+
+	/* Change frequency */
+
+	dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n",
+		__func__, old_freq, freq);
+
+	ret = clk_set_rate(clk, freq);
+	if (ret) {
+		dev_err(dev, "%s: failed to set clock rate: %d\n", __func__,
+			ret);
+		goto restore_voltage;
+	}
+
+	/* Scaling down? Scale voltage after frequency */
+	if (freq < old_freq) {
+		ret = _set_opp_voltages(dev, dev_opp, opp);
+		if (ret)
+			goto restore_freq;
+	}
+
+	goto unlock;
+
+restore_freq:
+	if (clk_set_rate(clk, old_freq))
+		dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+			__func__, old_freq);
+restore_voltage:
+	/* This shouldn't harm if the voltages weren't updated earlier */
+	_set_opp_voltages(dev, dev_opp, old_opp);
+unlock:
+	rcu_read_unlock();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate);
+
 /* List-dev Helpers */
 static void _kfree_list_dev_rcu(struct rcu_head *head)
 {
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index e8aee03b974a..b0f71c0f333a 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -58,6 +58,7 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq);
 
 struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev);
 int dev_pm_opp_set_regulator(struct device *dev, const char *id);
+int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
 #else
 static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp)
 {
@@ -147,6 +148,12 @@ static inline int dev_pm_opp_set_regulator(struct device *dev, const char *id)
 {
 	return -EINVAL;
 }
+
+static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
+{
+	return -EINVAL;
+}
+
 #endif		/* CONFIG_PM_OPP */
 
 #if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
-- 
2.4.0

[PATCH 16/16] PM / OPP: don't print error message for deferred probing

[Viresh Kumar]

dev_pm_opp_of_add_table() can fail with a return value of -EPROBE_DEFER
and in such a case we should skip printing the error message, as its not
really an error.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c
index 9ac691341fb0..f259a994fd16 100644
--- a/drivers/base/power/opp/cpu.c
+++ b/drivers/base/power/opp/cpu.c
@@ -198,8 +198,9 @@ int dev_pm_opp_of_cpumask_add_table(cpumask_var_t cpumask)
 
 		ret = dev_pm_opp_of_add_table(cpu_dev);
 		if (ret) {
-			pr_err("%s: couldn't find opp table for cpu:%d, %d\n",
-			       __func__, cpu, ret);
+			if (ret != -EPROBE_DEFER)
+				pr_err("%s: couldn't find opp table for cpu:%d, %d\n",
+				       __func__, cpu, ret);
 
 			/* Free all other OPPs */
 			dev_pm_opp_of_cpumask_remove_table(cpumask);
-- 
2.4.0