[RFC V2] OPP: Redefine bindings to overcome shortcomings

[RFC V2] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

Rob et al,

This is another attempt to redefine OPP bindings which we concluded to after
first round of reviews.

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.

The shortcomings we are trying to solve here:

- How to select which driver to probe for a platform, 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

- Support for intermediate frequencies

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

Please see the below bindings for further details.

I haven't incorporated the comments given by Mark Brown as I had some doubts.
@broonie: Is this what you wanted to mention earlier ? : http://pastebin.com/1RZTccmm

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---

 Documentation/devicetree/bindings/power/opp.txt | 309 +++++++++++++++++++++++-
 1 file changed, 308 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
index 74499e5033fc..9cdc0c9b09af 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -1,9 +1,316 @@
-* 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 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:
+  - opp-khz: Frequency in kHz
+  - opp-microvolt: voltage in micro Volts
+
+  Optional properties:
+  - turbo-mode: Marks the volt-freq pair as turbo pair.
+  - status: Marks the node enabled/disabled.
+  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
+  - clock-latency-ns: Specify the possible maximum transition latency (in
+    nanoseconds) for switching to this opp from any other opp.
+
+- 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:
+  - opp-intermediate: Stable opp we *must* switch to, before switching to the
+    target opp. Contains phandle of one of the opp-node present in opp-list.
+
+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>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply0>;
+			operating-points-v2 = <&cpu0_opp>;
+		};
+
+		cpu@1 {
+			compatible = "arm,cortex-a9";
+			reg = <1>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply0>;
+			operating-points-v2 = <&cpu0_opp>;
+		};
+	};
+
+	cpu0_opp: opp0 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+		opp-intermediate = <&cpu0_intermediate>;
+
+		cpu0_opplist: opp-list0 {
+			entry00 {
+				opp-khz = <1000000>;
+				opp-microvolt = <975000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+			};
+			cpu0_intermediate: entry01 {
+				opp-khz = <1100000>;
+				opp-microvolt = <1000000>;
+				voltage-tolerance = <3>;
+				clock-latency-ns = <310000>;
+				status = "okay";
+			};
+			entry02 {
+				opp-khz = <1200000>;
+				opp-microvolt = <1025000>;
+				voltage-tolerance = <1>;
+				clock-latency-ns = <290000>;
+				status = "okay";
+				turbo-mode;
+			};
+		};
+	};
+};
+
+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>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply0>;
+			operating-points-v2 = <&cpu0_opp>;
+		};
+
+		cpu@1 {
+			compatible = "qcom,krait";
+			reg = <1>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 1>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply1>;
+			operating-points-v2 = <&cpu1_opp>;
+		};
+
+		cpu@2 {
+			compatible = "qcom,krait";
+			reg = <2>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 2>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply2>;
+			operating-points-v2 = <&cpu2_opp>;
+		};
+
+		cpu@3 {
+			compatible = "qcom,krait";
+			reg = <3>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 3>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply3>;
+			operating-points-v2 = <&cpu3_opp>;
+		};
+	};
+
+	cpu0_opp: opp0 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+		opp-intermediate = <&cpu0_intermediate>;
+
+		cpu0_opplist: opp-list0 {
+			entry00 {
+				opp-khz = <1000000>;
+				opp-microvolt = <975000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+			};
+			cpu0_intermediate: entry01 {
+				opp-khz = <1100000>;
+				opp-microvolt = <1000000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+			};
+			entry02 {
+				opp-khz = <1200000>;
+				opp-microvolt = <1025000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+				turbo-mode;
+			};
+		};
+	};
+
+	cpu1_opp: opp1 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+		opp-intermediate = <&cpu0_intermediate>;
+	};
+
+	cpu2_opp: opp2 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+	};
+
+	cpu3_opp: opp3 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+		opp-intermediate = <&cpu0_intermediate>;
+	};
+};
+
+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>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply0>;
+			operating-points-v2 = <&cpu0_opp>;
+		};
+
+		cpu@1 {
+			compatible = "arm,cortex-a7";
+			reg = <1>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply0>;
+			operating-points-v2 = <&cpu0_opp>;
+		};
+
+		cpu@100 {
+			compatible = "arm,cortex-a15";
+			reg = <100>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 1>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply1>;
+			operating-points-v2 = <&cpu100_opp>;
+		};
+
+		cpu@101 {
+			compatible = "arm,cortex-a15";
+			reg = <101>;
+			next-level-cache = <&L2>;
+			clocks = <&clk_controller 1>;
+			clock-names = "cpu";
+			opp-supply = <&cpu_supply1>;
+			operating-points-v2 = <&cpu100_opp>;
+		};
+	};
+
+	cpu0_opp: opp0 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu0_opplist>;
+		opp-intermediate = <&cpu0_intermediate>;
+
+		cpu0_opplist: opp-list0 {
+			entry00 {
+				opp-khz = <1000000>;
+				opp-microvolt = <975000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+			};
+			cpu0_intermediate: entry01 {
+				opp-khz = <1100000>;
+				opp-microvolt = <1000000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+			};
+			entry02 {
+				opp-khz = <1200000>;
+				opp-microvolt = <1025000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <300000>;
+				status = "okay";
+				turbo-mode;
+			};
+		};
+	};
+
+	cpu100_opp: opp1 {
+		compatible = "linux,cpu-dvfs";
+		opp-list = <&cpu100_opplist>;
+		opp-intermediate = <&cpu100_intermediate>;
+
+		cpu100_opplist: opp-list1 {
+			entry10 {
+				opp-khz = <1300000>;
+				opp-microvolt = <1050000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <400000>;
+				status = "okay";
+			};
+			cpu100_intermediate: entry11 {
+				opp-khz = <1400000>;
+				opp-microvolt = <1075000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <400000>;
+				status = "okay";
+			};
+			entry12 {
+				opp-khz = <1500000>;
+				opp-microvolt = <1100000>;
+				voltage-tolerance = <2>;
+				clock-latency-ns = <400000>;
+				status = "okay";
+				turbo-mode;
+			};
+		};
+	};
+};
+
+
+
+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.3.0.rc0.44.ga94655d

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

[Viresh Kumar]

Fixing Olof's email address ..

On 23 January 2015 at 16:14, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Rob et al,
>
> This is another attempt to redefine OPP bindings which we concluded to after
> first round of reviews.
>
> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg53398.html).
> For obvious reasons Rob rejected them and shown the right path forward.
>
> The shortcomings we are trying to solve here:
>
> - How to select which driver to probe for a platform, 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
>
> - Support for intermediate frequencies
>
> - Other per OPP settings: transition latencies, disabled status, etc.?
>
> Please see the below bindings for further details.
>
> I haven't incorporated the comments given by Mark Brown as I had some doubts.
> @broonie: Is this what you wanted to mention earlier ? : http://pastebin.com/1RZTccmm
>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
>  Documentation/devicetree/bindings/power/opp.txt | 309 +++++++++++++++++++++++-
>  1 file changed, 308 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5033fc..9cdc0c9b09af 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -1,9 +1,316 @@
> -* 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 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:
> +  - opp-khz: Frequency in kHz
> +  - opp-microvolt: voltage in micro Volts
> +
> +  Optional properties:
> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> +  - status: Marks the node enabled/disabled.
> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +  - clock-latency-ns: Specify the possible maximum transition latency (in
> +    nanoseconds) for switching to this opp from any other opp.
> +
> +- 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:
> +  - opp-intermediate: Stable opp we *must* switch to, before switching to the
> +    target opp. Contains phandle of one of the opp-node present in opp-list.
> +
> +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>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +
> +               cpu@1 {
> +                       compatible = "arm,cortex-a9";
> +                       reg = <1>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +       };
> +
> +       cpu0_opp: opp0 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +
> +               cpu0_opplist: opp-list0 {
> +                       entry00 {
> +                               opp-khz = <1000000>;
> +                               opp-microvolt = <975000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       cpu0_intermediate: entry01 {
> +                               opp-khz = <1100000>;
> +                               opp-microvolt = <1000000>;
> +                               voltage-tolerance = <3>;
> +                               clock-latency-ns = <310000>;
> +                               status = "okay";
> +                       };
> +                       entry02 {
> +                               opp-khz = <1200000>;
> +                               opp-microvolt = <1025000>;
> +                               voltage-tolerance = <1>;
> +                               clock-latency-ns = <290000>;
> +                               status = "okay";
> +                               turbo-mode;
> +                       };
> +               };
> +       };
> +};
> +
> +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>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +
> +               cpu@1 {
> +                       compatible = "qcom,krait";
> +                       reg = <1>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 1>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply1>;
> +                       operating-points-v2 = <&cpu1_opp>;
> +               };
> +
> +               cpu@2 {
> +                       compatible = "qcom,krait";
> +                       reg = <2>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 2>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply2>;
> +                       operating-points-v2 = <&cpu2_opp>;
> +               };
> +
> +               cpu@3 {
> +                       compatible = "qcom,krait";
> +                       reg = <3>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 3>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply3>;
> +                       operating-points-v2 = <&cpu3_opp>;
> +               };
> +       };
> +
> +       cpu0_opp: opp0 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +
> +               cpu0_opplist: opp-list0 {
> +                       entry00 {
> +                               opp-khz = <1000000>;
> +                               opp-microvolt = <975000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       cpu0_intermediate: entry01 {
> +                               opp-khz = <1100000>;
> +                               opp-microvolt = <1000000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       entry02 {
> +                               opp-khz = <1200000>;
> +                               opp-microvolt = <1025000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                               turbo-mode;
> +                       };
> +               };
> +       };
> +
> +       cpu1_opp: opp1 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +       };
> +
> +       cpu2_opp: opp2 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +       };
> +
> +       cpu3_opp: opp3 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +       };
> +};
> +
> +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>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +
> +               cpu@1 {
> +                       compatible = "arm,cortex-a7";
> +                       reg = <1>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +
> +               cpu@100 {
> +                       compatible = "arm,cortex-a15";
> +                       reg = <100>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 1>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply1>;
> +                       operating-points-v2 = <&cpu100_opp>;
> +               };
> +
> +               cpu@101 {
> +                       compatible = "arm,cortex-a15";
> +                       reg = <101>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 1>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply1>;
> +                       operating-points-v2 = <&cpu100_opp>;
> +               };
> +       };
> +
> +       cpu0_opp: opp0 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +
> +               cpu0_opplist: opp-list0 {
> +                       entry00 {
> +                               opp-khz = <1000000>;
> +                               opp-microvolt = <975000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       cpu0_intermediate: entry01 {
> +                               opp-khz = <1100000>;
> +                               opp-microvolt = <1000000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       entry02 {
> +                               opp-khz = <1200000>;
> +                               opp-microvolt = <1025000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                               turbo-mode;
> +                       };
> +               };
> +       };
> +
> +       cpu100_opp: opp1 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu100_opplist>;
> +               opp-intermediate = <&cpu100_intermediate>;
> +
> +               cpu100_opplist: opp-list1 {
> +                       entry10 {
> +                               opp-khz = <1300000>;
> +                               opp-microvolt = <1050000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <400000>;
> +                               status = "okay";
> +                       };
> +                       cpu100_intermediate: entry11 {
> +                               opp-khz = <1400000>;
> +                               opp-microvolt = <1075000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <400000>;
> +                               status = "okay";
> +                       };
> +                       entry12 {
> +                               opp-khz = <1500000>;
> +                               opp-microvolt = <1100000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <400000>;
> +                               status = "okay";
> +                               turbo-mode;
> +                       };
> +               };
> +       };
> +};
> +
> +
> +
> +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.3.0.rc0.44.ga94655d
>
--
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: [RFC V2] OPP: Redefine bindings to overcome shortcomings

[Rob Herring]

On Fri, Jan 23, 2015 at 4:44 AM, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Rob et al,
>
> This is another attempt to redefine OPP bindings which we concluded to after
> first round of reviews.
>
> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg53398.html).
> For obvious reasons Rob rejected them and shown the right path forward.
>
> The shortcomings we are trying to solve here:
>
> - How to select which driver to probe for a platform, 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.

I'd like to see acks from Qualcomm folks to make sure this works for them.

> - Support for turbo modes
>
> - Support for intermediate frequencies
>
> - Other per OPP settings: transition latencies, disabled status, etc.?
>
> Please see the below bindings for further details.
>
> I haven't incorporated the comments given by Mark Brown as I had some doubts.
> @broonie: Is this what you wanted to mention earlier ? : http://pastebin.com/1RZTccmm
>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>
>  Documentation/devicetree/bindings/power/opp.txt | 309 +++++++++++++++++++++++-
>  1 file changed, 308 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5033fc..9cdc0c9b09af 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -1,9 +1,316 @@
> -* 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 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.

I still don't think we need this extra level. More below on this.

> +
> +  Required properties:
> +  - opp-khz: Frequency in kHz
> +  - opp-microvolt: voltage in micro Volts

This can be optional as it is valid to have frequency scaling without
voltage scaling. More so for bus scaling than cpu scaling.

> +  Optional properties:
> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> +  - status: Marks the node enabled/disabled.
> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
> +  - clock-latency-ns: Specify the possible maximum transition latency (in
> +    nanoseconds) for switching to this opp from any other opp.
> +
> +- 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:
> +  - opp-intermediate: Stable opp we *must* switch to, before switching to the
> +    target opp. Contains phandle of one of the opp-node present in opp-list.

What about cases where perhaps you have a more complex sequencing
arrangement? For example, what if you have to hit each step (to go
from A -> D you have to go A -> B -> C -> D). Perhaps each OPP could
have an opp-next property which lists other OPPs you can transition to
(and no property means no restriction).

Do you have a specific user in mind? If not, I think we can defer this
issue. I think the binding is flexible enough to accommodate this in
the future.

[...]

> +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>;
> +                       clocks = <&clk_controller 0>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply0>;
> +                       operating-points-v2 = <&cpu0_opp>;
> +               };
> +
> +               cpu@1 {
> +                       compatible = "qcom,krait";
> +                       reg = <1>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 1>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply1>;
> +                       operating-points-v2 = <&cpu1_opp>;
> +               };
> +
> +               cpu@2 {
> +                       compatible = "qcom,krait";
> +                       reg = <2>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 2>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply2>;
> +                       operating-points-v2 = <&cpu2_opp>;
> +               };
> +
> +               cpu@3 {
> +                       compatible = "qcom,krait";
> +                       reg = <3>;
> +                       next-level-cache = <&L2>;
> +                       clocks = <&clk_controller 3>;
> +                       clock-names = "cpu";
> +                       opp-supply = <&cpu_supply3>;
> +                       operating-points-v2 = <&cpu3_opp>;
> +               };
> +       };
> +
> +       cpu0_opp: opp0 {
> +               compatible = "linux,cpu-dvfs";

As I said before, drop the linux part. I'm not sure about cpu-dvfs
either. Nothing is specific to cpus and you are necessarily doing
voltage scaling. You could want to find all cpu OPPs though. Perhaps:
"cpu-operating-point", "operating-point";

It is not documented either.

> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +
> +               cpu0_opplist: opp-list0 {
> +                       entry00 {
> +                               opp-khz = <1000000>;
> +                               opp-microvolt = <975000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       cpu0_intermediate: entry01 {
> +                               opp-khz = <1100000>;
> +                               opp-microvolt = <1000000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                       };
> +                       entry02 {
> +                               opp-khz = <1200000>;
> +                               opp-microvolt = <1025000>;
> +                               voltage-tolerance = <2>;
> +                               clock-latency-ns = <300000>;
> +                               status = "okay";
> +                               turbo-mode;
> +                       };
> +               };
> +       };
> +
> +       cpu1_opp: opp1 {
> +               compatible = "linux,cpu-dvfs";
> +               opp-list = <&cpu0_opplist>;
> +               opp-intermediate = <&cpu0_intermediate>;
> +       };

This doesn't add anything other than some indirection to imply
independent OPPs. The only way I know the clocks are not shared is you
told me so in the example. I'd still prefer to determine from the
clock api whether 2 clocks can be changed independently. That didn't
seem to be agreed on, so you could simply add a "shared-opp" property
to opp0 to mark an OPP as shared or not. Then you can remove the
opp-list and these other cpuX_opp nodes.

Rob
--
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: [RFC V2] OPP: Redefine bindings to overcome shortcomings

[Viresh Kumar]

On 29 January 2015 at 21:52, Rob Herring <robherring2@gmail.com> wrote:
> On Fri, Jan 23, 2015 at 4:44 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

>> - Getting clock sharing information between CPUs. Single shared clock vs
>>   independent clock per core vs shared clock per cluster.
>
> I'd like to see acks from Qualcomm folks to make sure this works for them.

@Stephen: Can I have your inputs as well here?

>> +  Required properties:
>> +  - opp-khz: Frequency in kHz
>> +  - opp-microvolt: voltage in micro Volts
>
> This can be optional as it is valid to have frequency scaling without
> voltage scaling. More so for bus scaling than cpu scaling.

Right.

>> +  Optional properties:
>> +  - opp-intermediate: Stable opp we *must* switch to, before switching to the
>> +    target opp. Contains phandle of one of the opp-node present in opp-list.
>
> What about cases where perhaps you have a more complex sequencing
> arrangement? For example, what if you have to hit each step (to go
> from A -> D you have to go A -> B -> C -> D). Perhaps each OPP could
> have an opp-next property which lists other OPPs you can transition to
> (and no property means no restriction).

I haven't seen such examples yet but yeah that might be required
at some point of time.

> Do you have a specific user in mind? If not, I think we can defer this
> issue. I think the binding is flexible enough to accommodate this in
> the future.

Yeah, Tegra and Mediatek already need it. Even cpufreq core does
support it currently.

What about keeping it as is for now and extending to opp-next in case
it is required. Or maybe add opp-next right now. So, for the case of
single intermediate-freq, all OPPs except the intermediate one will have
opp-next set to intermediate opp. And intermediate opp doesn't fill this.

I will think about it a bit more though.

>> +       cpu0_opp: opp0 {
>> +               compatible = "linux,cpu-dvfs";
>
> As I said before, drop the linux part. I'm not sure about cpu-dvfs
> either. Nothing is specific to cpus and you are necessarily doing
> voltage scaling. You could want to find all cpu OPPs though. Perhaps:
> "cpu-operating-point", "operating-point";

What about "operating-point-v2", as that's the real version.

> It is not documented either.

Will do.

>> +       cpu1_opp: opp1 {
>> +               compatible = "linux,cpu-dvfs";
>> +               opp-list = <&cpu0_opplist>;
>> +               opp-intermediate = <&cpu0_intermediate>;
>> +       };
>
> This doesn't add anything other than some indirection to imply
> independent OPPs. The only way I know the clocks are not shared is you
> told me so in the example. I'd still prefer to determine from the

Will document that clearly.

> clock api whether 2 clocks can be changed independently. That didn't

Mike Turquette has objected to that earlier and so we moved
to DT to find this out..

> seem to be agreed on, so you could simply add a "shared-opp" property
> to opp0 to mark an OPP as shared or not. Then you can remove the
> opp-list and these other cpuX_opp nodes.

Okay.

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

[Lucas Stach]

Am Freitag, den 23.01.2015, 16:14 +0530 schrieb Viresh Kumar:
> Rob et al,
> 
> This is another attempt to redefine OPP bindings which we concluded to after
> first round of reviews.
> 
> 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.
> 
> The shortcomings we are trying to solve here:
> 
> - How to select which driver to probe for a platform, 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
> 
> - Support for intermediate frequencies
> 
> - Other per OPP settings: transition latencies, disabled status, etc.?
> 
> Please see the below bindings for further details.
> 
> I haven't incorporated the comments given by Mark Brown as I had some doubts.
> @broonie: Is this what you wanted to mention earlier ? : http://pastebin.com/1RZTccmm
> 

I think we should work this out for the new bindings, as
voltage-tolerance is a completely bogus value.

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> 
>  Documentation/devicetree/bindings/power/opp.txt | 309 +++++++++++++++++++++++-
>  1 file changed, 308 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt
> index 74499e5033fc..9cdc0c9b09af 100644
> --- a/Documentation/devicetree/bindings/power/opp.txt
> +++ b/Documentation/devicetree/bindings/power/opp.txt
> @@ -1,9 +1,316 @@
> -* 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 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:
> +  - opp-khz: Frequency in kHz
> +  - opp-microvolt: voltage in micro Volts

Each OPP voltage should be defined by the triplet of minimum,
nominal/typical, maximum. This lets you specify exact tolerances in each
direction and should cover most use-cases.

IMHO it would make sense to just define opp-microvolt as an array of
those 3 values, so the DT doesn't get bloated with a lot more
properties.

A typical value for a CPU could then look like this:
opp-microvolt = <800000 850000 1100000>

For devices without any tolerance you can just specify the same value
three times and be done with it:
opp-microvolt = <900000 900000 900000>

> +
> +  Optional properties:
> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> +  - status: Marks the node enabled/disabled.
> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.

Please let's drop this.

> +  - clock-latency-ns: Specify the possible maximum transition latency (in
> +    nanoseconds) for switching to this opp from any other opp.
> +
> +- 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:
> +  - opp-intermediate: Stable opp we *must* switch to, before switching to the
> +    target opp. Contains phandle of one of the opp-node present in opp-list.
> +
[...]

Regards,
Lucas

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

[Mark Brown]

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

On Fri, Jan 23, 2015 at 12:39:14PM +0100, Lucas Stach wrote:

> > +  Required properties:
> > +  - opp-khz: Frequency in kHz
> > +  - opp-microvolt: voltage in micro Volts

> Each OPP voltage should be defined by the triplet of minimum,
> nominal/typical, maximum. This lets you specify exact tolerances in each
> direction and should cover most use-cases.

> IMHO it would make sense to just define opp-microvolt as an array of
> those 3 values, so the DT doesn't get bloated with a lot more
> properties.

> A typical value for a CPU could then look like this:
> opp-microvolt = <800000 850000 1100000>

I tend to agree that this is clearer.  It might be nice to have variants
for specifying directly as a percentage but I don't think it's really
worth the complexity.

> For devices without any tolerance you can just specify the same value
> three times and be done with it:
> opp-microvolt = <900000 900000 900000>

If we change the binding to be typ/min/max rather than min/typ/max then
we could also do this by allowing either one or three values to be
specified.  That might be more worth the complexity especially given...

> > +  Optional properties:
> > +  - turbo-mode: Marks the volt-freq pair as turbo pair.
> > +  - status: Marks the node enabled/disabled.
> > +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.

> Please let's drop this.

DT bindings are supposed to be stable, this means the code should accept
old bindings and they should be documented as deprecated.

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

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

[Viresh Kumar]

On 23 January 2015 at 17:22, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jan 23, 2015 at 12:39:14PM +0100, Lucas Stach wrote:

> I tend to agree that this is clearer.  It might be nice to have variants
> for specifying directly as a percentage but I don't think it's really
> worth the complexity.

> If we change the binding to be typ/min/max rather than min/typ/max then
> we could also do this by allowing either one or three values to be
> specified.  That might be more worth the complexity especially given...

+1

>> > +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
>
>> Please let's drop this.
>
> DT bindings are supposed to be stable, this means the code should accept
> old bindings and they should be documented as deprecated.

Yes, I have marked the earlier OPP bindings as deprecated. Will mark
this one too separately.

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

[Viresh Kumar]

On 23 January 2015 at 17:09, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Freitag, den 23.01.2015, 16:14 +0530 schrieb Viresh Kumar:
>> I haven't incorporated the comments given by Mark Brown as I had some doubts.
>> @broonie: Is this what you wanted to mention earlier ? : http://pastebin.com/1RZTccmm
>>
>
> I think we should work this out for the new bindings, as
> voltage-tolerance is a completely bogus value.

Yeah, that's what I was saying. I just wanted to confirm that I understood
it clearly. And so didn't made the changes here.

>> +  - opp-microvolt: voltage in micro Volts
>
> Each OPP voltage should be defined by the triplet of minimum,
> nominal/typical, maximum. This lets you specify exact tolerances in each
> direction and should cover most use-cases.
>
> IMHO it would make sense to just define opp-microvolt as an array of
> those 3 values, so the DT doesn't get bloated with a lot more
> properties.
>
> A typical value for a CPU could then look like this:
> opp-microvolt = <800000 850000 1100000>
>
> For devices without any tolerance you can just specify the same value
> three times and be done with it:
> opp-microvolt = <900000 900000 900000>

Ok.

>> +  Optional properties:
>> +  - turbo-mode: Marks the volt-freq pair as turbo pair.
>> +  - status: Marks the node enabled/disabled.
>> +  - voltage-tolerance: Specify the CPU voltage tolerance in percentage.
>
> Please let's drop this.

Yes, I did dropped it in the pastebin link I gave above.

Thanks again for your feedback.

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

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 976 bytes --]

On Fri, Jan 23, 2015 at 04:14:50PM +0530, Viresh Kumar wrote:

> +- opp-listN:
> +  List of nodes defining performance points. Following belong to the nodes
> +  within the opp-lists.

Why is there the N here?  It doesn't correspond to the examples...

> +  Required properties:
> +  - opp-khz: Frequency in kHz
> +  - opp-microvolt: voltage in micro Volts

I thought the goal here was to specify ranges?

> +- 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.

I don't understand what that compatible property is intended to mean and
I expect other readers might be similarly confused - is it a standard
compatbile property meaning this noe corresponds to some sort of device?

There also appears to be no code matching these bindings...

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Viresh Kumar]

On 29 January 2015 at 01:36, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Jan 23, 2015 at 04:14:50PM +0530, Viresh Kumar wrote:
>
>> +- opp-listN:
>> +  List of nodes defining performance points. Following belong to the nodes
>> +  within the opp-lists.
>
> Why is there the N here?  It doesn't correspond to the examples...

I meant [0..N], probably its better to remove it ..

>> +  Required properties:
>> +  - opp-khz: Frequency in kHz
>> +  - opp-microvolt: voltage in micro Volts
>
> I thought the goal here was to specify ranges?

Yeah, I wasn't 100% sure of what you suggested and so asked a question in
the mail earlier. I will change it to what you suggested.

>> +- 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.
>
> I don't understand what that compatible property is intended to mean and
> I expect other readers might be similarly confused - is it a standard
> compatbile property meaning this noe corresponds to some sort of device?

The goal is to choose the driver which we want to probe for a platform. There
can be multiple DT enabled cpufreq drivers present in a build and the platform
needs some way to choose one of them.

For example, if both cpufreq-dt, exynos-cpufreq and arm-big-little drivers are
present in a kernel build, the how do we specify which one a platform wants to
use.

The generic drivers (cpufreq-dt and big LITTLE) should match with some generic
strings, as these aren't specific to any platform. But exynos one can be matched
with the machine or Soc-family's existing compatible names.

> There also appears to be no code matching these bindings...

I still have to post it, was trying to finalize the bindings first.
--
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: [RFC V2] OPP: Redefine bindings to overcome shortcomings

[Mark Brown]

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

On Thu, Jan 29, 2015 at 07:09:23AM +0530, Viresh Kumar wrote:
> On 29 January 2015 at 01:36, Mark Brown <broonie@kernel.org> wrote:

> >> +  Required properties:
> >> +  - compatible: allow OPPs to express their compatibility.
> >> +  - opp-list: phandle to opp-list defined above.

> > I don't understand what that compatible property is intended to mean and
> > I expect other readers might be similarly confused - is it a standard
> > compatbile property meaning this noe corresponds to some sort of device?

> The goal is to choose the driver which we want to probe for a platform. There
> can be multiple DT enabled cpufreq drivers present in a build and the platform
> needs some way to choose one of them.

So you need to document this as a platform device then if that's what
you're doing here; there's more than just specifying a compatible string
involved.

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

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

[Viresh Kumar]

On 29 January 2015 at 16:37, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Jan 29, 2015 at 07:09:23AM +0530, Viresh Kumar wrote:

>> The goal is to choose the driver which we want to probe for a platform. There
>> can be multiple DT enabled cpufreq drivers present in a build and the platform
>> needs some way to choose one of them.
>
> So you need to document this as a platform device then if that's what
> you're doing here; there's more than just specifying a compatible string
> involved.

Will elaborate more on this to make it crystal clear..
--
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: [RFC V2] OPP: Redefine bindings to overcome shortcomings

[Rob Herring]

On Wed, Jan 28, 2015 at 7:39 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 29 January 2015 at 01:36, Mark Brown <broonie@kernel.org> wrote:
>> On Fri, Jan 23, 2015 at 04:14:50PM +0530, Viresh Kumar wrote:

[...]

>>> +  Required properties:
>>> +  - compatible: allow OPPs to express their compatibility.
>>> +  - opp-list: phandle to opp-list defined above.
>>
>> I don't understand what that compatible property is intended to mean and
>> I expect other readers might be similarly confused - is it a standard
>> compatbile property meaning this noe corresponds to some sort of device?
>
> The goal is to choose the driver which we want to probe for a platform. There
> can be multiple DT enabled cpufreq drivers present in a build and the platform
> needs some way to choose one of them.

That was not my original thought, but rather it is versioning the
binding and a way to find OPP nodes. That doesn't mean we can't do
things to make it easier to match to drivers.

> For example, if both cpufreq-dt, exynos-cpufreq and arm-big-little drivers are
> present in a kernel build, the how do we specify which one a platform wants to
> use.

What does the exynos driver do that the other 2 don't?

> The generic drivers (cpufreq-dt and big LITTLE) should match with some generic
> strings, as these aren't specific to any platform. But exynos one can be matched
> with the machine or Soc-family's existing compatible names.

I will likely nak any such strings that are solely there to match to
these 2 drivers. The information is there in DT already. It's just not
a single property to match on. You can tell it is a big.LITTLE system
by the fact your have 2 different cpu types. You can use the top level
compatible strings when you need a machine specific driver. I would
not be trying to get cpufreq platform devices to be created by
of_platform_populate. The other issue I have is you may not always
have 2 drivers for bL and non-bL. Really symmetric multi-cluster or
single cluster should be a subset of functionality of bL.

I think you just need to have an initcall which does this:

if (of_find_matching_node_and_match(machine_specific_driver_match list))
  platform_device_create(match->data);
else if (is_big_little() && has_opp)
  platform_device_create(big_little_device);
else if (has_opp)
  platform_device_create(cpufreq_dt_device);

Or perhaps you handle the generic ones late in boot only if no machine
specific driver has been probed. There is no generic solution ATM for
selecting the best driver when there are multiple matches.

Rob

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

[Viresh Kumar]

On 29 January 2015 at 21:12, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Jan 28, 2015 at 7:39 PM, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

>> The goal is to choose the driver which we want to probe for a platform. There
>> can be multiple DT enabled cpufreq drivers present in a build and the platform
>> needs some way to choose one of them.
>
> That was not my original thought, but rather it is versioning the
> binding and a way to find OPP nodes. That doesn't mean we can't do
> things to make it easier to match to drivers.

Okay.

>> For example, if both cpufreq-dt, exynos-cpufreq and arm-big-little drivers are
>> present in a kernel build, the how do we specify which one a platform wants to
>> use.
>
> What does the exynos driver do that the other 2 don't?

There are efforts going on to start using cpufreq-dt instead
of exynos drivers. So, they might die at some point of time.

Anyway, there can be cpufreq drivers which co-exist
cpufreq-dt, even if they are parsing the same bindings.

> I will likely nak any such strings that are solely there to match to
> these 2 drivers. The information is there in DT already. It's just not
> a single property to match on. You can tell it is a big.LITTLE system
> by the fact your have 2 different cpu types. You can use the top level
> compatible strings when you need a machine specific driver. I would
> not be trying to get cpufreq platform devices to be created by
> of_platform_populate. The other issue I have is you may not always
> have 2 drivers for bL and non-bL. Really symmetric multi-cluster or
> single cluster should be a subset of functionality of bL.
>
> I think you just need to have an initcall which does this:
>
> if (of_find_matching_node_and_match(machine_specific_driver_match list))
>   platform_device_create(match->data);
> else if (is_big_little() && has_opp)
>   platform_device_create(big_little_device);
> else if (has_opp)
>   platform_device_create(cpufreq_dt_device);
>
> Or perhaps you handle the generic ones late in boot only if no machine
> specific driver has been probed. There is no generic solution ATM for
> selecting the best driver when there are multiple matches.

One thing is clear. The compatibility string here isn't going to be
like what I had in mind. So, let it just define the versioning
here and I will see how can I solve the driver-probe problem.
--
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