[RFC] OPP: Redefine bindings to overcome shortcomings

[RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

Hi Rob, et al..

Current OPP (Operating performance point) DT bindings are proven to be
insufficient at multiple instances.

There had been multiple band-aid approaches to get them fixed (The latest one
being: http://www.mail-archive.com/devicetree@vger.kernel.org/msg53398.html).
For obvious reasons Rob rejected them and shown the right path forward. And this
is the first try to get those with a pen and paper.

The shortcomings we are trying to solve here:

- Some kind of compatibility string to probe the right cpufreq driver for
  platforms, when multiple drivers are available. For example: how to choose
  between cpufreq-dt and arm_big_little drivers.

- Getting clock sharing information between CPUs. Single shared clock vs.
  independent clock per core vs. shared clock per cluster.

- Support for turbo modes

- Other per OPP settings: transition latencies, disabled status, etc.?

The below document should be enough to describe how I am trying to fix these.
Please let me know what all I need to fix, surely there would be lots of
obstacles. I am prepared to get beaten up :)

I accept in advance that naming is extremely bad here, I need some suggestions
for sure.

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

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5..5efd8d4 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -4,6 +4,153 @@ SoCs have a standard set of tuples consisting of frequency and
 voltage pairs that the device will support per voltage domain. These
 are called Operating Performance Points or OPPs.
 
+This documents defines OPP bindings with its required/optional properties.
+OPPs can be defined for any device, this file uses CPU device as an example to
+illustrate how to define OPPs.
+
+linux,operating-points, opp-lists and opps:
+
+- linux,operating-points:
+  Container of all OPP nodes.
+
+  Required properties:
+  - opp nodes (explained below)
+
+  Optional properties:
+  - compatible: allow OPPs to express their compatibility with devices
+
+
+- opp-list@*:
+  List of nodes defining performance points. Following belong to the nodes
+  within the opp-lists.
+
+  Required properties:
+  - frequency-kHz: Frequency in kHz
+  - voltage-uV: voltage in micro Volts
+
+  Optional properties:
+  - turbo-mode: Marks the volt-freq pair as turbo pair.
+  - status: Marks the node enabled/disabled.
+
+
+- opp@*:
+  Operating performance point node per device. Multiple devices sharing it can
+  use its phandle in their 'opp' property.
+
+  Required properties:
+  - opp-list: phandle to opp-list defined above.
+
+  Optional properties:
+  - clocks: Tuple of clock providers
+  - clock-names: Clock names
+  - opp-supply: phandle to the parent supply/regulator node
+  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+  - clock-latency: Specify the possible maximum transition latency for clock,
+    in unit of nanoseconds.
+
+Example: Multi-cluster system with separate clock lines for clusters. All CPUs
+         in the clusters share same clock lines.
+
+/ {
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		linux,operating-points {
+			compatible = "linux,cpufreq-dt";
+
+			opp-list0: opp-list@0 {
+				{
+					frequency-kHz = <1000000>;
+					voltage-uV = <975000>;
+					status = "okay";
+				};
+				{
+					frequency-kHz = <1100000>;
+					voltage-uV = <1000000>;
+					status = "okay";
+				};
+				{
+					frequency-kHz = <1200000>;
+					voltage-uV = <1025000>;
+					status = "okay";
+					turbo-mode;
+				};
+			};
+
+			opp-list1: opp-list@1 {
+				{
+					frequency-kHz = <1300000>;
+					voltage-uV = <1050000>;
+					status = "okay";
+				};
+				{
+					frequency-kHz = <1400000>;
+					voltage-uV = <1075000>;
+					status = "disabled";
+				};
+				{
+					frequency-kHz = <1500000>;
+					voltage-uV = <1100000>;
+					status = "okay";
+					turbo-mode;
+				};
+			};
+
+			opp0: opp@0 {
+				clocks = <&clk-controller 0>;
+				clock-names = "cpu";
+				opp-supply = <&cpu-supply0>;
+				voltage-tolerance = <2>; /* percentage */
+				clock-latency = <300000>;
+				opp-list = <&opp-list0>;
+			};
+
+			opp1: opp@1 {
+				clocks = <&clk-controller 1>;
+				clock-names = "cpu";
+				opp-supply = <&cpu-supply1>;
+				voltage-tolerance = <2>; /* percentage */
+				clock-latency = <400000>;
+				opp-list = <&opp-list1>;
+			};
+		};
+
+		cpu@0 {
+			compatible = "arm,cortex-a7";
+			reg = <0>;
+			next-level-cache = <&L2>;
+			opps = <opp0>;
+		};
+
+		cpu@1 {
+			compatible = "arm,cortex-a7";
+			reg = <1>;
+			next-level-cache = <&L2>;
+			opps = <opp0>;
+		};
+
+		cpu@100 {
+			compatible = "arm,cortex-a15";
+			reg = <100>;
+			next-level-cache = <&L2>;
+			opps = <opp1>;
+		};
+
+		cpu@101 {
+			compatible = "arm,cortex-a15";
+			reg = <101>;
+			next-level-cache = <&L2>;
+			opps = <opp1>;
+		};
+	};
+};
+
+
+
+Deprecated Bindings
+-------------------
+
 Properties:
 - operating-points: An array of 2-tuples items, and each item consists
   of frequency and voltage like <freq-kHz vol-uV>.
-- 
2.0.3.693.g996b0fd

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Lucas Stach]

Hi Viresh,

not commenting on the overall structure as I have to think a bit more
about this. But small comments below.

Am Donnerstag, den 04.12.2014, 16:44 +0530 schrieb Viresh Kumar:
> Hi Rob, et al..
> 
> Current OPP (Operating performance point) DT bindings are proven to be
> insufficient at multiple instances.
> 
> There had been multiple band-aid approaches to get them fixed (The latest one
> being: http://www.mail-archive.com/devicetree@vger.kernel.org/msg53398.html).
> For obvious reasons Rob rejected them and shown the right path forward. And this
> is the first try to get those with a pen and paper.
> 
> The shortcomings we are trying to solve here:
> 
> - Some kind of compatibility string to probe the right cpufreq driver for
>   platforms, when multiple drivers are available. For example: how to choose
>   between cpufreq-dt and arm_big_little drivers.
> 
> - Getting clock sharing information between CPUs. Single shared clock vs.
>   independent clock per core vs. shared clock per cluster.
> 
> - Support for turbo modes
> 
> - Other per OPP settings: transition latencies, disabled status, etc.?
> 
> The below document should be enough to describe how I am trying to fix these.
> Please let me know what all I need to fix, surely there would be lots of
> obstacles. I am prepared to get beaten up :)
> 
> I accept in advance that naming is extremely bad here, I need some suggestions
> for sure.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/power/opp.txt | 147 ++++++++++++++++++++++++
>  1 file changed, 147 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5..5efd8d4 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -4,6 +4,153 @@ SoCs have a standard set of tuples consisting of frequency and
>  voltage pairs that the device will support per voltage domain. These
>  are called Operating Performance Points or OPPs.
>  
> +This documents defines OPP bindings with its required/optional properties.
> +OPPs can be defined for any device, this file uses CPU device as an example to
> +illustrate how to define OPPs.
> +
> +linux,operating-points, opp-lists and opps:
> +
> +- linux,operating-points:
> +  Container of all OPP nodes.
> +
> +  Required properties:
> +  - opp nodes (explained below)
> +
> +  Optional properties:
> +  - compatible: allow OPPs to express their compatibility with devices
> +
> +
> +- opp-list@*:
> +  List of nodes defining performance points. Following belong to the nodes
> +  within the opp-lists.
> +
> +  Required properties:
> +  - frequency-kHz: Frequency in kHz
> +  - voltage-uV: voltage in micro Volts
> +
> +  Optional properties:
> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> +  - status: Marks the node enabled/disabled.

What about devices with multiple different turbo states? We have seen
CPUs that boost to different states in the x86 world, surely we will
encounter something like this in the ARM world too. Do we just mark them
all as turbo OPPs and let the driver decide what to do? If we want to
keep using cpufreq-dt for as much devices as possible is it really
sufficient to know that this is a turbo state, without knowing the
conditions required for activating the state?

> +
> +
> +- opp@*:
> +  Operating performance point node per device. Multiple devices sharing it can
> +  use its phandle in their 'opp' property.
> +
> +  Required properties:
> +  - opp-list: phandle to opp-list defined above.
> +
> +  Optional properties:
> +  - clocks: Tuple of clock providers
> +  - clock-names: Clock names
> +  - opp-supply: phandle to the parent supply/regulator node
> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.

This is extremely ill defined. It doesn't say in which direction the
tolerance is to be applied. Can you go below or above the OPP specified
voltage? For now everyone just assumes that it has to work both ways.
Also with this binding the tolerance is applied for all OPPs, where is
very much depends on the individual OPP.

If you are going to redefine OPPs anyway I would really like to see this
property die and rather have a min/max voltage per OPP. That way you can
properly express the OPP constraints. Most OPPs will likely allow a much
higher voltage than their minimal specified one, except when you go over
thermal limits with a high clock/voltage combination.

> +  - clock-latency: Specify the possible maximum transition latency for clock,
> +    in unit of nanoseconds.

Why do we need this? This is property of the clock. We should be able to
handle this completely internally in the kernel. I don't know if the
clock API has something like this right now, but it should be a trivial
addition.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

Hi Lucas,

On 4 December 2014 at 17:04, Lucas Stach <l.stach@pengutronix.de> wrote:

>> +- opp-list@*:
>> +  List of nodes defining performance points. Following belong to the nodes
>> +  within the opp-lists.
>> +
>> +  Required properties:
>> +  - frequency-kHz: Frequency in kHz
>> +  - voltage-uV: voltage in micro Volts
>> +
>> +  Optional properties:
>> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
>> +  - status: Marks the node enabled/disabled.
>
> What about devices with multiple different turbo states? We have seen

You mean that a state may or maynot be turbo at some point of time ?

> CPUs that boost to different states in the x86 world, surely we will
> encounter something like this in the ARM world too. Do we just mark them
> all as turbo OPPs and let the driver decide what to do? If we want to

Maybe yes. But the good thing about binding this time is, it is expandable.
So, if there is a future need that we can't think of today, then we can surely
do incremental changes here.

> keep using cpufreq-dt for as much devices as possible is it really

Its not about cpufreq-dt alone. We maybe using other drivers as well..

> sufficient to know that this is a turbo state, without knowing the
> conditions required for activating the state?

Can you elaborate more on this? If something is required and we
know what exactly it is, then we can put up the right binding right
now as well..

>> +- opp@*:
>> +  Operating performance point node per device. Multiple devices sharing it can
>> +  use its phandle in their 'opp' property.
>> +
>> +  Required properties:
>> +  - opp-list: phandle to opp-list defined above.
>> +
>> +  Optional properties:
>> +  - clocks: Tuple of clock providers
>> +  - clock-names: Clock names
>> +  - opp-supply: phandle to the parent supply/regulator node
>> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
>
> This is extremely ill defined. It doesn't say in which direction the
> tolerance is to be applied. Can you go below or above the OPP specified
> voltage? For now everyone just assumes that it has to work both ways.

Yes, the binding is as per today's requirements (or rather implementations).
So it is both ways. But if everybody agrees on it, we can improve it..

> Also with this binding the tolerance is applied for all OPPs, where is
> very much depends on the individual OPP.

Hmm, Not only this but the same is true for clock latency as well. We
*may* need that per opp node sometime..

> If you are going to redefine OPPs anyway I would really like to see this
> property die and rather have a min/max voltage per OPP. That way you can

Maybe yes.

> properly express the OPP constraints. Most OPPs will likely allow a much
> higher voltage than their minimal specified one, except when you go over
> thermal limits with a high clock/voltage combination.

Yes.

>> +  - clock-latency: Specify the possible maximum transition latency for clock,
>> +    in unit of nanoseconds.
>
> Why do we need this? This is property of the clock. We should be able to
> handle this completely internally in the kernel. I don't know if the
> clock API has something like this right now, but it should be a trivial
> addition.

This is not only clock's latency, but is somehow named this way. This should
give the time it takes to change from frequency A to frequency B, which include
change in supplies as well.. So, this probably is dvfs-latency ..

This is required by cpufreq right now, but would be useful for the energy aware
scheduler as well. So, yes this is important. Also, it might also be required to
be per OPP...

Probably we can use voltage-tolerance and clock-latency at both
levels. list-level
and OPP level. list level being at higher priority ?

Thanks for your quick comments :)

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Mark Brown]

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

On Thu, Dec 04, 2014 at 12:34:28PM +0100, Lucas Stach wrote:
> Am Donnerstag, den 04.12.2014, 16:44 +0530 schrieb Viresh Kumar:

> > +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.

> This is extremely ill defined. It doesn't say in which direction the
> tolerance is to be applied. Can you go below or above the OPP specified
> voltage? For now everyone just assumes that it has to work both ways.
> Also with this binding the tolerance is applied for all OPPs, where is
> very much depends on the individual OPP.

Almost all specifications for voltages are done as either min/typ/max or
+/- a target voltage.

> If you are going to redefine OPPs anyway I would really like to see this
> property die and rather have a min/max voltage per OPP. That way you can
> properly express the OPP constraints. Most OPPs will likely allow a much
> higher voltage than their minimal specified one, except when you go over
> thermal limits with a high clock/voltage combination.

If you've got a minimum and maximum you also need to specify a target,
generally it's going to be better to go for the target voltage which may
not be the midpoint and is unlikely to be one of the bounds.  I do think
it's sensible to have the option of doing both to more closely match
datasheets.

> > +  - clock-latency: Specify the possible maximum transition latency for clock,
> > +    in unit of nanoseconds.

> Why do we need this? This is property of the clock. We should be able to
> handle this completely internally in the kernel. I don't know if the
> clock API has something like this right now, but it should be a trivial
> addition.

Or have it be part of the clock binding at any rate.

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

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

On 4 December 2014 at 19:37, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> This is not only clock's latency, but is somehow named this way. This should
> give the time it takes to change from frequency A to frequency B, which include
> change in supplies as well.. So, this probably is dvfs-latency ..

Oops. No this is just clock-latency. We are calculating voltage-latency
separately.

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

On 4 December 2014 at 16:44, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> The shortcomings we are trying to solve here:
>
> - Some kind of compatibility string to probe the right cpufreq driver for
>   platforms, when multiple drivers are available. For example: how to choose
>   between cpufreq-dt and arm_big_little drivers.
>
> - Getting clock sharing information between CPUs. Single shared clock vs.
>   independent clock per core vs. shared clock per cluster.
>
> - Support for turbo modes
>
> - Other per OPP settings: transition latencies, disabled status, etc.?

Some updates on the structure of bindings which I got up to with help of Arnd
and Rob over IRC, have got better examples to show how things would look
like:

diff --git a/Documentation/devicetree/bindings/power/opp.txt
b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5033fc..8ae574b84650 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -1,9 +1,292 @@
-* Generic OPP Interface
+Generic OPP (Operating Performance Points) Interface
+----------------------------------------------------

 SoCs have a standard set of tuples consisting of frequency and
 voltage pairs that the device will support per voltage domain. These
 are called Operating Performance Points or OPPs.

+This documents defines OPP bindings with its required/optional properties.
+OPPs can be defined for any device, this file uses CPU device as an example to
+illustrate how to define OPPs.
+
+opp nodes and opp-lists
+
+- opp-listN:
+  List of nodes defining performance points. Following belong to the nodes
+  within the opp-lists.
+
+  Required properties:
+  - frequency-kHz: Frequency in kHz
+  - voltage-uV: voltage in micro Volts
+
+  Optional properties:
+  - turbo-mode: Marks the volt-freq pair as turbo pair.
+  - status: Marks the node enabled/disabled.
+
+- oppN:
+  Operating performance point node per device. Devices using it should have its
+  phandle in their "operating-points-v2" property.
+
+  Required properties:
+  - compatible: allow OPPs to express their compatibility
+  - opp-list: phandle to opp-list defined above.
+
+  Optional properties:
+  - clocks: Tuple of clock providers
+  - clock-names: Clock names
+  - opp-supply: phandle to the parent supply/regulator node
+  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+  - clock-latency: Specify the possible maximum transition latency for clock,
+    in unit of nanoseconds.
+
+Example 1: Simple case of dual-core cortex A9-single cluster, sharing
clock line.
+
+/ {
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               cpu@0 {
+                       compatible = "arm,cortex-a9";
+                       reg = <0>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp0>;
+
+                       opp0: opp0 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 0>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply0>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+
+                               opplist0: opp-list0 {
+                                       entry00 {
+                                               frequency-kHz = <1000000>;
+                                               voltage-uV = <975000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1100000>;
+                                               voltage-uV = <1000000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1200000>;
+                                               voltage-uV = <1025000>;
+                                               status = "okay";
+                                               turbo-mode;
+                                       };
+                               };
+                       };
+               };
+
+               cpu@1 {
+                       compatible = "arm,cortex-a9";
+                       reg = <1>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp0>;
+               };
+       };
+};
+
+Example 2: Quad-core krait (All CPUs have independent clock lines but
have same set of OPPs)
+
+/ {
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               cpu@0 {
+                       compatible = "qcom,krait";
+                       reg = <0>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp0>;
+
+                       opp0: opp0 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 0>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply0>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+
+                               opplist0: opp-list0 {
+                                       entry00 {
+                                               frequency-kHz = <1000000>;
+                                               voltage-uV = <975000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1100000>;
+                                               voltage-uV = <1000000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1200000>;
+                                               voltage-uV = <1025000>;
+                                               status = "okay";
+                                               turbo-mode;
+                                       };
+                               };
+                       };
+               };
+
+               cpu@1 {
+                       compatible = "qcom,krait";
+                       reg = <1>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp1>;
+
+                       opp1: opp1 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 1>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply1>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+                       };
+               };
+
+               cpu@2 {
+                       compatible = "qcom,krait";
+                       reg = <2>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp2>;
+
+                       opp2: opp2 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 2>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply2>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+                       };
+               };
+
+               cpu@3 {
+                       compatible = "qcom,krait";
+                       reg = <3>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp3>;
+
+                       opp3: opp3 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 3>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply3>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+                       };
+               };
+       };
+};
+
+Example 3: Multi-cluster system with separate clock line per cluster.
+
+/ {
+       cpus {
+               #address-cells = <1>;
+               #size-cells = <0>;
+
+               cpu@0 {
+                       compatible = "arm,cortex-a7";
+                       reg = <0>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp0>;
+
+                       opp0: opp0 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 0>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply0>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <300000>;
+                               opp-list = <&opplist0>;
+
+                               opplist0: opp-list0 {
+                                       entry00 {
+                                               frequency-kHz = <1000000>;
+                                               voltage-uV = <975000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1100000>;
+                                               voltage-uV = <1000000>;
+                                               status = "okay";
+                                       };
+                                       entry01 {
+                                               frequency-kHz = <1200000>;
+                                               voltage-uV = <1025000>;
+                                               status = "okay";
+                                               turbo-mode;
+                                       };
+                               };
+                       };
+               };
+
+               cpu@1 {
+                       compatible = "arm,cortex-a7";
+                       reg = <1>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp0>;
+               };
+
+               cpu@100 {
+                       compatible = "arm,cortex-a15";
+                       reg = <100>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp1>;
+
+                       opp1: opp1 {
+                               compatible = "linux,cpu-dvfs";
+                               clocks = <&clk-controller 1>;
+                               clock-names = "cpu";
+                               opp-supply = <&cpu-supply1>;
+                               voltage-tolerance = <2>; /* percentage */
+                               clock-latency = <400000>;
+                               opp-list = <&opplist1>;
+
+                               opplist1: opp-list1 {
+                                       entry10 {
+                                               frequency-kHz = <1300000>;
+                                               voltage-uV = <1050000>;
+                                               status = "okay";
+                                       };
+                                       entry11 {
+                                               frequency-kHz = <1400000>;
+                                               voltage-uV = <1075000>;
+                                               status = "disabled";
+                                       };
+                                       entry12 {
+                                               frequency-kHz = <1500000>;
+                                               voltage-uV = <1100000>;
+                                               status = "okay";
+                                               turbo-mode;
+                                       };
+                               };
+                       };
+               };
+
+               cpu@101 {
+                       compatible = "arm,cortex-a15";
+                       reg = <101>;
+                       next-level-cache = <&L2>;
+                       operating-points-v2 = <&opp1>;
+               };
+       };
+};
+
+
+
+Deprecated Bindings
+-------------------
+

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Rob Herring]

On Tue, Dec 9, 2014 at 9:51 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 4 December 2014 at 16:44, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> The shortcomings we are trying to solve here:
>>
>> - Some kind of compatibility string to probe the right cpufreq driver for
>>   platforms, when multiple drivers are available. For example: how to choose
>>   between cpufreq-dt and arm_big_little drivers.
>>
>> - Getting clock sharing information between CPUs. Single shared clock vs.
>>   independent clock per core vs. shared clock per cluster.
>>
>> - Support for turbo modes
>>
>> - Other per OPP settings: transition latencies, disabled status, etc.?
>
> Some updates on the structure of bindings which I got up to with help of Arnd
> and Rob over IRC, have got better examples to show how things would look
> like:
>
> diff --git a/Documentation/devicetree/bindings/power/opp.txt
> b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5033fc..8ae574b84650 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -1,9 +1,292 @@
> -* Generic OPP Interface
> +Generic OPP (Operating Performance Points) Interface
> +----------------------------------------------------
>
>  SoCs have a standard set of tuples consisting of frequency and
>  voltage pairs that the device will support per voltage domain. These
>  are called Operating Performance Points or OPPs.
>
> +This documents defines OPP bindings with its required/optional properties.
> +OPPs can be defined for any device, this file uses CPU device as an example to
> +illustrate how to define OPPs.
> +
> +opp nodes and opp-lists
> +
> +- opp-listN:
> +  List of nodes defining performance points. Following belong to the nodes
> +  within the opp-lists.
> +
> +  Required properties:
> +  - frequency-kHz: Frequency in kHz

s/kHz/khz/

> +  - voltage-uV: voltage in micro Volts

-microvolt is more consistent with regulator binding.

The names are a bit redundant. perhaps opp-khz and opp-microvolt instead.

> +
> +  Optional properties:
> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> +  - status: Marks the node enabled/disabled.
> +
> +- oppN:
> +  Operating performance point node per device. Devices using it should have its
> +  phandle in their "operating-points-v2" property.
> +
> +  Required properties:
> +  - compatible: allow OPPs to express their compatibility
> +  - opp-list: phandle to opp-list defined above.
> +
> +  Optional properties:
> +  - clocks: Tuple of clock providers
> +  - clock-names: Clock names
> +  - opp-supply: phandle to the parent supply/regulator node
> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +  - clock-latency: Specify the possible maximum transition latency for clock,
> +    in unit of nanoseconds.
> +
> +Example 1: Simple case of dual-core cortex A9-single cluster, sharing
> clock line.
> +
> +/ {
> +       cpus {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               cpu@0 {
> +                       compatible = "arm,cortex-a9";
> +                       reg = <0>;
> +                       next-level-cache = <&L2>;
> +                       operating-points-v2 = <&opp0>;
> +
> +                       opp0: opp0 {

I don't really like having this under cpu0 when it applies to all
cpus. I would move it out of /cpus.

> +                               compatible = "linux,cpu-dvfs";

This should not be linux specific. Probably not cpu specific either.

> +                               clocks = <&clk-controller 0>;
> +                               clock-names = "cpu";

clocks are an input to the cpu, not really an opp. You could have a an
OPP which uses a different parent clock, but that is most likely a
switch within the clock controller rather than 2 clock inputs to the
cpu.

I think the clock binding for cpus should stand on its own independent of OPPs.

> +                               opp-supply = <&cpu-supply0>;

Same comment as clocks applies here.

> +                               voltage-tolerance = <2>; /* percentage */
> +                               clock-latency = <300000>;

These could be per entry. I'm not sure it is worth the savings to not
just always specify them per entry.

We should append units (-us) to clock-latency unless there is a good
reason to maintain compatibility.

> +                               opp-list = <&opplist0>;

With the above changes, having this list is unnecessary.

So, what I envision is like this:

/cpus {
  cpu@0 {
    clocks = <...>;
    cpu-supply = <...>;
    operating-point-v2 = <&cpu0-opp>;
   };
  cpu@1 {
    clocks = <...>;
    cpu-supply = <...>;
    operating-point-v2 = <&cpu1-opp>;
   };
};

cpu0-opp : opp0 {
  compatible = "operating-point-table";
  entry0 {
    opp-khz = <500000>;
    opp-microvolt = <900000>;
  };
  entry1 {
    opp-khz = <1000000>;
    opp-microvolt = <1000000>;
    turbo-mode;
  };
};

cpu1-opp : opp1 {
  compatible = "operating-point-table";
...
};

We need to also consider if this all works for other non-cpu OPPs like
GPUs or DRAM/bus.

Rob

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

On 29 December 2014 at 22:35, Rob Herring <robherring2@gmail.com> wrote:
>> +  - frequency-kHz: Frequency in kHz
>
> s/kHz/khz/
>
>> +  - voltage-uV: voltage in micro Volts
>
> -microvolt is more consistent with regulator binding.
>
> The names are a bit redundant. perhaps opp-khz and opp-microvolt instead.

All accepted.

>> +Example 1: Simple case of dual-core cortex A9-single cluster, sharing
>> clock line.
>> +
>> +/ {
>> +       cpus {
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               cpu@0 {
>> +                       compatible = "arm,cortex-a9";
>> +                       reg = <0>;
>> +                       next-level-cache = <&L2>;
>> +                       operating-points-v2 = <&opp0>;
>> +
>> +                       opp0: opp0 {
>
> I don't really like having this under cpu0 when it applies to all
> cpus. I would move it out of /cpus.

That's how I had it initially, but then Arnd didn't like it much.

>> +                               compatible = "linux,cpu-dvfs";
>
> This should not be linux specific. Probably not cpu specific either.

This was just an example of one of the bindings and there will be others
as well.

'linux' here doesn't mean linux specific, but just that its first used by
Linux. That's what my understanding is atleast.

>> +                               clocks = <&clk-controller 0>;
>> +                               clock-names = "cpu";
>
> clocks are an input to the cpu, not really an opp. You could have a an
> OPP which uses a different parent clock, but that is most likely a
> switch within the clock controller rather than 2 clock inputs to the
> cpu.
>
> I think the clock binding for cpus should stand on its own independent of OPPs.
>
>> +                               opp-supply = <&cpu-supply0>;
>
> Same comment as clocks applies here.

Both will be kept in the cpu node where they were initially.

>> +                               voltage-tolerance = <2>; /* percentage */
>> +                               clock-latency = <300000>;
>
> These could be per entry. I'm not sure it is worth the savings to not
> just always specify them per entry.

Done.

> We should append units (-us) to clock-latency unless there is a good
> reason to maintain compatibility.

So you meant something like this:

clock-latency-us = <300000>;

right?

>> +                               opp-list = <&opplist0>;
>
> With the above changes, having this list is unnecessary.

It might be for the use case I mentioned earlier about something
like Krait.

> So, what I envision is like this:
>
> /cpus {
>   cpu@0 {
>     clocks = <...>;
>     cpu-supply = <...>;
>     operating-point-v2 = <&cpu0-opp>;
>    };
>   cpu@1 {
>     clocks = <...>;
>     cpu-supply = <...>;
>     operating-point-v2 = <&cpu1-opp>;
>    };
> };

Looks fine..

> cpu0-opp : opp0 {
>   compatible = "operating-point-table";
>   entry0 {
>     opp-khz = <500000>;
>     opp-microvolt = <900000>;
>   };
>   entry1 {
>     opp-khz = <1000000>;
>     opp-microvolt = <1000000>;
>     turbo-mode;
>   };
> };
> cpu1-opp : opp1 {
>   compatible = "operating-point-table";
> ...
> };

What about something like Krait which wants to use exactly same
bindings for all CPUs but want to specify they are controlled separately.

So I had it like:

cpu0-opp : opp0 {
  compatible = "operating-point-table";
  opp-list = <&opplist0>;

  opplist0: opp-list0 {
          entry0 {
            opp-khz = <500000>;
            opp-microvolt = <900000>;
          };
          entry1 {
            opp-khz = <1000000>;
            opp-microvolt = <1000000>;
            turbo-mode;
          };
  };
};

cpu1-opp : opp1 {
  compatible = "operating-point-table";
  opp-list = <&opplist0>;
};

> We need to also consider if this all works for other non-cpu OPPs like
> GPUs or DRAM/bus.

Do you any input here? Or if you know somebody who can give inputs
about them?

Re: [RFC] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

On 31 December 2014 at 10:17, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29 December 2014 at 22:35, Rob Herring <robherring2@gmail.com> wrote:
>>> +  - frequency-kHz: Frequency in kHz
>>
>> s/kHz/khz/
>>
>>> +  - voltage-uV: voltage in micro Volts
>>
>> -microvolt is more consistent with regulator binding.
>>
>> The names are a bit redundant. perhaps opp-khz and opp-microvolt instead.
>
> All accepted.
>
>>> +Example 1: Simple case of dual-core cortex A9-single cluster, sharing
>>> clock line.
>>> +
>>> +/ {
>>> +       cpus {
>>> +               #address-cells = <1>;
>>> +               #size-cells = <0>;
>>> +
>>> +               cpu@0 {
>>> +                       compatible = "arm,cortex-a9";
>>> +                       reg = <0>;
>>> +                       next-level-cache = <&L2>;
>>> +                       operating-points-v2 = <&opp0>;
>>> +
>>> +                       opp0: opp0 {
>>
>> I don't really like having this under cpu0 when it applies to all
>> cpus. I would move it out of /cpus.
>
> That's how I had it initially, but then Arnd didn't like it much.
>
>>> +                               compatible = "linux,cpu-dvfs";
>>
>> This should not be linux specific. Probably not cpu specific either.
>
> This was just an example of one of the bindings and there will be others
> as well.
>
> 'linux' here doesn't mean linux specific, but just that its first used by
> Linux. That's what my understanding is atleast.
>
>>> +                               clocks = <&clk-controller 0>;
>>> +                               clock-names = "cpu";
>>
>> clocks are an input to the cpu, not really an opp. You could have a an
>> OPP which uses a different parent clock, but that is most likely a
>> switch within the clock controller rather than 2 clock inputs to the
>> cpu.
>>
>> I think the clock binding for cpus should stand on its own independent of OPPs.
>>
>>> +                               opp-supply = <&cpu-supply0>;
>>
>> Same comment as clocks applies here.
>
> Both will be kept in the cpu node where they were initially.
>
>>> +                               voltage-tolerance = <2>; /* percentage */
>>> +                               clock-latency = <300000>;
>>
>> These could be per entry. I'm not sure it is worth the savings to not
>> just always specify them per entry.
>
> Done.
>
>> We should append units (-us) to clock-latency unless there is a good
>> reason to maintain compatibility.
>
> So you meant something like this:
>
> clock-latency-us = <300000>;
>
> right?
>
>>> +                               opp-list = <&opplist0>;
>>
>> With the above changes, having this list is unnecessary.
>
> It might be for the use case I mentioned earlier about something
> like Krait.
>
>> So, what I envision is like this:
>>
>> /cpus {
>>   cpu@0 {
>>     clocks = <...>;
>>     cpu-supply = <...>;
>>     operating-point-v2 = <&cpu0-opp>;
>>    };
>>   cpu@1 {
>>     clocks = <...>;
>>     cpu-supply = <...>;
>>     operating-point-v2 = <&cpu1-opp>;
>>    };
>> };
>
> Looks fine..
>
>> cpu0-opp : opp0 {
>>   compatible = "operating-point-table";
>>   entry0 {
>>     opp-khz = <500000>;
>>     opp-microvolt = <900000>;
>>   };
>>   entry1 {
>>     opp-khz = <1000000>;
>>     opp-microvolt = <1000000>;
>>     turbo-mode;
>>   };
>> };
>> cpu1-opp : opp1 {
>>   compatible = "operating-point-table";
>> ...
>> };
>
> What about something like Krait which wants to use exactly same
> bindings for all CPUs but want to specify they are controlled separately.
>
> So I had it like:
>
> cpu0-opp : opp0 {
>   compatible = "operating-point-table";
>   opp-list = <&opplist0>;
>
>   opplist0: opp-list0 {
>           entry0 {
>             opp-khz = <500000>;
>             opp-microvolt = <900000>;
>           };
>           entry1 {
>             opp-khz = <1000000>;
>             opp-microvolt = <1000000>;
>             turbo-mode;
>           };
>   };
> };
>
> cpu1-opp : opp1 {
>   compatible = "operating-point-table";
>   opp-list = <&opplist0>;
> };

Gentle reminder, so that we can close this long standing issue soon..