Hi Guys,
Finally V4 got some good review comments, Acks, etc.. I have updated the bindings with all review comments and here is V5.
V4->V5: - opp-microamp fixed and rewritten as per Mark's suggestions. - shared-opp renamed as opp-shared, as that's the convention for other properties. - Dropped "[V4 3/3] OPP: Add 'opp-next' in operating-points-v2 bindings" as that was NAK'd by Mike T.. - Added [V5 3/3] based on Nishanth's suggestions. - Added an example for 2/3, multiple OPP nodes. @Stephen/Nishanth: I have kept your tags here as I haven't changed the binding but only an example. Please let me know if you have some comments.
- Other minor formatting.. - Existing binding: "operating-points" isn't deprecated now as platforms looking for simple bindings should be allowed to use them. - opp-khz is changed to opp-hz, examples updated. - turbo-mode explained
V3->V4: - Dropped code changes as we are still concerned about bindings. - separated out into three patches, some of which might be NAK'd. :) - The first patch presents basic OPP stuff that was reviewed earlier. It also has support for multiple regulators, with values for both current and voltage. - Second patch is based on a special concern that Stephen had about multiple OPP tables, one of which the parsing code will select at runtime. - Third one separates out 'opp-next' or Intermediate freq support as Mike T. had few concerns over it. He wanted the clock driver to take care of this and so do not want it to be passed by DT and used by cpufreq. Also, there were concerns like the platform may not want to choose intermediate frequency as a target frequency for longer runs, which wasn't prevented in earlier bindings. And so it is kept separate to be NAK'd quietly, without much disturbances.
---------------x-------------------x------------------------
Current OPP (Operating performance point) DT bindings are proven to be insufficient at multiple instances.
The shortcomings we are trying to solve here:
- Getting clock/voltage/current rails sharing information between CPUs. Shared by all cores vs independent clock per core vs shared clock per cluster.
- Support for specifying current levels along with voltages.
- Support for multiple regulators.
- Support for turbo modes.
- Other per OPP settings: transition latencies, disabled status, etc.?
- Expandability of OPPs in future.
This patchset tries to solve these shortcomings with a new "operating-points-v2" binding, which can be easily extended later if required.
Viresh Kumar (3): OPP: Redefine bindings to overcome shortcomings OPP: Allow multiple OPP tables to be passed via DT OPP: Add binding for 'opp-suspend'
Documentation/devicetree/bindings/power/opp.txt | 437 +++++++++++++++++++++++- 1 file changed, 433 insertions(+), 4 deletions(-)
Current OPP (Operating performance point) DT bindings are proven to be insufficient at multiple instances.
The shortcomings we are trying to solve here:
- Getting clock/voltage/current rails sharing information between CPUs. Shared by all cores vs independent clock per core vs shared clock per cluster.
- Support for specifying current levels along with voltages.
- Support for multiple regulators.
- Support for turbo modes.
- Other per OPP settings: transition latencies, disabled status, etc.?
- Expandability of OPPs in future.
This patch introduces new bindings "operating-points-v2" to get these problems solved. Refer to the bindings for more details.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Documentation/devicetree/bindings/power/opp.txt | 379 +++++++++++++++++++++++- 1 file changed, 375 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 74499e5033fc..d132e2927b21 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -1,8 +1,19 @@ -* Generic OPP Interface +Generic OPP (Operating Performance Points) Bindings +----------------------------------------------------
-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. +Devices work at voltage-current-frequency combinations and some implementations +have the liberty of choosing these. These combinations are called Operating +Performance Points aka OPPs. This document defines bindings for these OPPs +applicable across wide range of devices. For illustration purpose, this document +uses CPU as a device. + +This document contain multiple versions of OPP binding and only one of them +should be used per device. + +Binding 1: operating-points +============================ + +This binding only supports voltage-frequency pairs.
Properties: - operating-points: An array of 2-tuples items, and each item consists @@ -23,3 +34,363 @@ cpu@0 { 198000 850000 >; }; + + + +Binding 2: operating-points-v2 +============================ + +* Property: operating-points-v2 + +Devices supporting OPPs must set their "operating-points-v2" property with +phandle to a OPP descriptor in their DT node. The OPP core will use this phandle +to find the operating points for the device. + + +* OPP Descriptor Node + +This describes the OPPs belonging to a device. This node can have following +properties: + +Required properties: +- compatible: Allow OPPs to express their compatibility. It should be: + "operating-points-v2". + +- OPP nodes: One or more OPP nodes describing voltage-current-frequency + combinations. Their name isn't significant but their phandle can be used to + reference an OPP. + +Optional properties: +- opp-shared: Indicates that device nodes using this OPP descriptor's phandle + switch their DVFS state together, i.e. they share clock/voltage/current lines. + Missing property means devices have independent clock/voltage/current lines, + but they share OPP tables. + + +* OPP Node + +This defines voltage-current-frequency combinations along with other related +properties. + +Required properties: +- opp-hz: Frequency in Hz + +Optional properties: +- opp-microvolt: voltage in micro Volts. + + A single regulator's voltage is specified with an array of size one or three. + Single entry is for target voltage and three entries are for <target min max> + voltages. + + Entries for multiple regulators must be present in the same order as + regulators are specified in device's DT node. + +- opp-microamp: The maximum current drawn by the device in microamperes + considering system specific parameters (such as transients, process, aging, + maximum operating temperature range etc.) as necessary. This may be used to + set the most efficient regulator operating mode. + + Should only be set if opp-microvolt is set for the OPP. + + Entries for multiple regulators must be present in the same order as + regulators are specified in device's DT node. If this property isn't required + for few regulators, then this should be marked as zero for them. If it isn't + required for any regulator, then this property need not be present. + +- clock-latency-ns: Specifies the maximum possible transition latency (in + nanoseconds) for switching to this OPP from any other OPP. + +- turbo-mode: Marks the OPP to be used only for turbo modes. Turbo mode is + available on some platforms, where the device can run over its operating + frequency for a short duration of time limited by the device's power, current + and thermal limits. + +- status: Marks the node enabled/disabled. + +Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. + +/ { + 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 = "operating-points-v2"; + opp-shared; + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000 975000 985000>; + opp-microamp = <70000>; + clock-latency-ns = <300000>; + }; + entry01 { + opp-hz = <1100000000>; + opp-microvolt = <980000 1000000 1010000>; + opp-microamp = <80000>; + clock-latency-ns = <310000>; + }; + entry02 { + opp-hz = <1200000000>; + opp-microvolt = <1025000>; + clock-latency-ns = <290000>; + turbo-mode; + }; + }; +}; + +Example 2: Single cluster, Quad-core Qualcom-krait, switches DVFS states +independently. + +/ { + 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 = <&cpu0_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 = <&cpu0_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 = <&cpu0_opp>; + }; + }; + + cpu0_opp: opp0 { + compatible = "operating-points-v2"; + + /* + * Missing opp-shared property means CPUs switch DVFS states + * independently. + */ + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000 975000 985000>; + opp-microamp = <70000>; + clock-latency-ns = <300000>; + }; + entry01 { + opp-hz = <1100000000>; + opp-microvolt = <980000 1000000 1010000>; + opp-microamp = <80000>; + clock-latency-ns = <310000>; + }; + entry02 { + opp-hz = <1200000000>; + opp-microvolt = <1025000>; + opp-microamp = <90000; + lock-latency-ns = <290000>; + turbo-mode; + }; + }; +}; + +Example 3: Dual-cluster, Dual-core per cluster. CPUs within a cluster switch +DVFS state together. + +/ { + 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 = <&cluster0_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 = <&cluster0_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 = <&cluster1_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 = <&cluster1_opp>; + }; + }; + + cluster0_opp: opp0 { + compatible = "operating-points-v2"; + opp-shared; + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000 975000 985000>; + opp-microamp = <70000>; + clock-latency-ns = <300000>; + }; + entry01 { + opp-hz = <1100000000>; + opp-microvolt = <980000 1000000 1010000>; + opp-microamp = <80000>; + clock-latency-ns = <310000>; + }; + entry02 { + opp-hz = <1200000000>; + opp-microvolt = <1025000>; + opp-microamp = <90000>; + clock-latency-ns = <290000>; + turbo-mode; + }; + }; + + cluster1_opp: opp1 { + compatible = "operating-points-v2"; + opp-shared; + + entry10 { + opp-hz = <1300000000>; + opp-microvolt = <1045000 1050000 1055000>; + opp-microamp = <95000>; + clock-latency-ns = <400000>; + }; + entry11 { + opp-hz = <1400000000>; + opp-microvolt = <1075000>; + opp-microamp = <100000>; + clock-latency-ns = <400000>; + }; + entry12 { + opp-hz = <1500000000>; + opp-microvolt = <1010000 1100000 1110000>; + opp-microamp = <95000>; + clock-latency-ns = <400000>; + turbo-mode; + }; + }; +}; + +Example 4: Handling multiple regulators + +/ { + cpus { + cpu@0 { + compatible = "arm,cortex-a7"; + ... + + opp-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>; + operating-points-v2 = <&cpu0_opp>; + }; + }; + + cpu0_opp: opp0 { + compatible = "operating-points-v2"; + opp-shared; + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000>, /* Supply 0 */ + <960000>, /* Supply 1 */ + <960000>; /* Supply 2 */ + opp-microamp = <70000>, /* Supply 0 */ + <70000>, /* Supply 1 */ + <70000>; /* Supply 2 */ + clock-latency-ns = <300000>; + }; + + /* OR */ + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000 975000 985000>, /* Supply 0 */ + <960000 965000 975000>, /* Supply 1 */ + <960000 965000 975000>; /* Supply 2 */ + opp-microamp = <70000>, /* Supply 0 */ + <70000>, /* Supply 1 */ + <70000>; /* Supply 2 */ + clock-latency-ns = <300000>; + }; + + /* OR */ + + entry00 { + opp-hz = <1000000000>; + opp-microvolt = <970000 975000 985000>, /* Supply 0 */ + <960000 965000 975000>, /* Supply 1 */ + <960000 965000 975000>; /* Supply 2 */ + opp-microamp = <70000>, /* Supply 0 */ + <0>, /* Supply 1 doesn't need this */ + <70000>; /* Supply 2 */ + clock-latency-ns = <300000>; + }; + }; +};
On Tue, May 19, 2015 at 10:41 PM, Viresh Kumar viresh.kumar@linaro.org wrote:
Current OPP (Operating performance point) DT bindings are proven to be insufficient at multiple instances.
The shortcomings we are trying to solve here:
Getting clock/voltage/current rails sharing information between CPUs. Shared by all cores vs independent clock per core vs shared clock per cluster.
Support for specifying current levels along with voltages.
Support for multiple regulators.
Support for turbo modes.
Other per OPP settings: transition latencies, disabled status, etc.?
Expandability of OPPs in future.
This patch introduces new bindings "operating-points-v2" to get these problems solved. Refer to the bindings for more details.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
This looks good to me:
Reviewed-by: Rob Herring robh@kernel.org
As I've mentioned before, I would like to see some users' acks on this as well.
Rob
Documentation/devicetree/bindings/power/opp.txt | 379 +++++++++++++++++++++++- 1 file changed, 375 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 74499e5033fc..d132e2927b21 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -1,8 +1,19 @@ -* Generic OPP Interface +Generic OPP (Operating Performance Points) Bindings +----------------------------------------------------
-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. +Devices work at voltage-current-frequency combinations and some implementations +have the liberty of choosing these. These combinations are called Operating +Performance Points aka OPPs. This document defines bindings for these OPPs +applicable across wide range of devices. For illustration purpose, this document +uses CPU as a device.
+This document contain multiple versions of OPP binding and only one of them +should be used per device.
+Binding 1: operating-points +============================
+This binding only supports voltage-frequency pairs.
Properties:
- operating-points: An array of 2-tuples items, and each item consists
@@ -23,3 +34,363 @@ cpu@0 { 198000 850000 >; };
+Binding 2: operating-points-v2 +============================
+* Property: operating-points-v2
+Devices supporting OPPs must set their "operating-points-v2" property with +phandle to a OPP descriptor in their DT node. The OPP core will use this phandle +to find the operating points for the device.
+* OPP Descriptor Node
+This describes the OPPs belonging to a device. This node can have following +properties:
+Required properties: +- compatible: Allow OPPs to express their compatibility. It should be:
- "operating-points-v2".
+- OPP nodes: One or more OPP nodes describing voltage-current-frequency
- combinations. Their name isn't significant but their phandle can be used to
- reference an OPP.
+Optional properties: +- opp-shared: Indicates that device nodes using this OPP descriptor's phandle
- switch their DVFS state together, i.e. they share clock/voltage/current lines.
- Missing property means devices have independent clock/voltage/current lines,
- but they share OPP tables.
+* OPP Node
+This defines voltage-current-frequency combinations along with other related +properties.
+Required properties: +- opp-hz: Frequency in Hz
+Optional properties: +- opp-microvolt: voltage in micro Volts.
- A single regulator's voltage is specified with an array of size one or three.
- Single entry is for target voltage and three entries are for <target min max>
- voltages.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node.
+- opp-microamp: The maximum current drawn by the device in microamperes
- considering system specific parameters (such as transients, process, aging,
- maximum operating temperature range etc.) as necessary. This may be used to
- set the most efficient regulator operating mode.
- Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node. If this property isn't required
- for few regulators, then this should be marked as zero for them. If it isn't
- required for any regulator, then this property need not be present.
+- clock-latency-ns: Specifies the maximum possible transition latency (in
- nanoseconds) for switching to this OPP from any other OPP.
+- turbo-mode: Marks the OPP to be used only for turbo modes. Turbo mode is
- available on some platforms, where the device can run over its operating
- frequency for a short duration of time limited by the device's power, current
- and thermal limits.
+- status: Marks the node enabled/disabled.
+Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
+/ {
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 = "operating-points-v2";
opp-shared;
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000 975000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
};
entry01 {
opp-hz = <1100000000>;
opp-microvolt = <980000 1000000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
entry02 {
opp-hz = <1200000000>;
opp-microvolt = <1025000>;
clock-latency-ns = <290000>;
turbo-mode;
};
};
+};
+Example 2: Single cluster, Quad-core Qualcom-krait, switches DVFS states +independently.
+/ {
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 = <&cpu0_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 = <&cpu0_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 = <&cpu0_opp>;
};
};
cpu0_opp: opp0 {
compatible = "operating-points-v2";
/*
* Missing opp-shared property means CPUs switch DVFS states
* independently.
*/
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000 975000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
};
entry01 {
opp-hz = <1100000000>;
opp-microvolt = <980000 1000000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
entry02 {
opp-hz = <1200000000>;
opp-microvolt = <1025000>;
opp-microamp = <90000;
lock-latency-ns = <290000>;
turbo-mode;
};
};
+};
+Example 3: Dual-cluster, Dual-core per cluster. CPUs within a cluster switch +DVFS state together.
+/ {
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 = <&cluster0_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 = <&cluster0_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 = <&cluster1_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 = <&cluster1_opp>;
};
};
cluster0_opp: opp0 {
compatible = "operating-points-v2";
opp-shared;
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000 975000 985000>;
opp-microamp = <70000>;
clock-latency-ns = <300000>;
};
entry01 {
opp-hz = <1100000000>;
opp-microvolt = <980000 1000000 1010000>;
opp-microamp = <80000>;
clock-latency-ns = <310000>;
};
entry02 {
opp-hz = <1200000000>;
opp-microvolt = <1025000>;
opp-microamp = <90000>;
clock-latency-ns = <290000>;
turbo-mode;
};
};
cluster1_opp: opp1 {
compatible = "operating-points-v2";
opp-shared;
entry10 {
opp-hz = <1300000000>;
opp-microvolt = <1045000 1050000 1055000>;
opp-microamp = <95000>;
clock-latency-ns = <400000>;
};
entry11 {
opp-hz = <1400000000>;
opp-microvolt = <1075000>;
opp-microamp = <100000>;
clock-latency-ns = <400000>;
};
entry12 {
opp-hz = <1500000000>;
opp-microvolt = <1010000 1100000 1110000>;
opp-microamp = <95000>;
clock-latency-ns = <400000>;
turbo-mode;
};
};
+};
+Example 4: Handling multiple regulators
+/ {
cpus {
cpu@0 {
compatible = "arm,cortex-a7";
...
opp-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
operating-points-v2 = <&cpu0_opp>;
};
};
cpu0_opp: opp0 {
compatible = "operating-points-v2";
opp-shared;
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000>, /* Supply 0 */
<960000>, /* Supply 1 */
<960000>; /* Supply 2 */
opp-microamp = <70000>, /* Supply 0 */
<70000>, /* Supply 1 */
<70000>; /* Supply 2 */
clock-latency-ns = <300000>;
};
/* OR */
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000 975000 985000>, /* Supply 0 */
<960000 965000 975000>, /* Supply 1 */
<960000 965000 975000>; /* Supply 2 */
opp-microamp = <70000>, /* Supply 0 */
<70000>, /* Supply 1 */
<70000>; /* Supply 2 */
clock-latency-ns = <300000>;
};
/* OR */
entry00 {
opp-hz = <1000000000>;
opp-microvolt = <970000 975000 985000>, /* Supply 0 */
<960000 965000 975000>, /* Supply 1 */
<960000 965000 975000>; /* Supply 2 */
opp-microamp = <70000>, /* Supply 0 */
<0>, /* Supply 1 doesn't need this */
<70000>; /* Supply 2 */
clock-latency-ns = <300000>;
};
};
+};
2.4.0
-- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20-05-15, 08:27, Rob Herring wrote:
This looks good to me:
Reviewed-by: Rob Herring robh@kernel.org
Thanks a lot !!
As I've mentioned before, I would like to see some users' acks on this as well.
Sure. Stephen and Nishanth have already approved 2/3 and I hope they will Ack other two as well today.
$subject: (Rafael usually hand fixes it.. but might be nice of us not to save him that effort) PM / OPP: Additional binding definition to address ...
we are not really redefining the old definitions..
On 05/19/2015 10:41 PM, Viresh Kumar wrote:
Current OPP (Operating performance point) DT bindings are proven to be
Nit Pick: s/DT/device tree ; s/are/have been
insufficient at multiple instances.
insufficient due to the inflexible nature of the original bindings. Over time, we have realized that Operating Performance Point definitions and usage is varied depending on the SoC and a "single size (just frequency, voltage) fits all" model which the original bindings attempted and failed.
The shortcomings we are trying to solve here:
The proposed next generation of the bindings addresses by providing a expandable binding for OPPs and introduces the following common shortcomings seen with the original bindings:
[...]
Documentation/devicetree/bindings/power/opp.txt | 379 +++++++++++++++++++++++- 1 file changed, 375 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 74499e5033fc..d132e2927b21 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -1,8 +1,19 @@ -* Generic OPP Interface +Generic OPP (Operating Performance Points) Bindings +---------------------------------------------------- -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. +Devices work at voltage-current-frequency combinations and some implementations +have the liberty of choosing these. These combinations are called Operating +Performance Points aka OPPs. This document defines bindings for these OPPs +applicable across wide range of devices. For illustration purpose, this document +uses CPU as a device.
+This document contain multiple versions of OPP binding and only one of them +should be used per device.
Will be nice to repeat this message in the commit message as well.. folks just doing git log should probably not freak out that the current dtbs will stop working once the implementation is merged in - it might help deal with some of the concerns of folks not aware of the mailing thread discussions.
+Binding 1: operating-points +============================
+This binding only supports voltage-frequency pairs. Properties:
- operating-points: An array of 2-tuples items, and each item consists
@@ -23,3 +34,363 @@ cpu@0 { 198000 850000
;
};
Extra EOLs? maybe just stop at 2 EOLs to separate the sections.?
+Binding 2: operating-points-v2 +============================
+* Property: operating-points-v2
+Devices supporting OPPs must set their "operating-points-v2" property with +phandle to a OPP descriptor in their DT node. The OPP core will use this phandle +to find the operating points for the device.
I would suggest adding a link to how future vendor specific extension docs might look like - maybe this is probably not the time to discuss this.. but anyways.. we could make some statement to the effect of "SoC vendor specfic extensions are documented as Documentation/devicetree/bindings/power/<vendor>,opp.txt and should clearly indicate that the extensions are permitted only under the operating-points-v2 compatible description."
+* OPP Descriptor Node
+This describes the OPPs belonging to a device. This node can have following +properties:
+Required properties: +- compatible: Allow OPPs to express their compatibility. It should be:
- "operating-points-v2".
+- OPP nodes: One or more OPP nodes describing voltage-current-frequency
- combinations. Their name isn't significant but their phandle can be used to
- reference an OPP.
What if this was generated data (say using an overlay)? does it have to be "required" or just "optional" :)
+Optional properties: +- opp-shared: Indicates that device nodes using this OPP descriptor's phandle
- switch their DVFS state together, i.e. they share clock/voltage/current lines.
- Missing property means devices have independent clock/voltage/current lines,
- but they share OPP tables.
+* OPP Node
+This defines voltage-current-frequency combinations along with other related +properties.
+Required properties: +- opp-hz: Frequency in Hz
I am just being nit picky -> should we keep Heinrich Hertz respected[2] and name it opp-Hz ? No strong opinions either way.
different angle: How about just target-freq-Hz? just drop the "opp" prefix for properties of an OPP node? No strong feelings here. (some folks did have variations of a few Hz based on clock tree - example with a crystal frequency of 19.2MHz you'd probably get 1001MHz exact, with a 26MHz crystal, you'd get 1000MHz -> ofcourse round-rate is supposed to help with that... but anyways.. why not say we are trying to indicate target frequency? I do recollect during initial days of OPP (pre-dt-adoption days) folks did complain about this - we kinda worked around this with round-rated handling - but we might as well anticipate it.
[...]
+Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
Thanks for adding the examples - My customer support team especially will appreciate having such examples ;).
+/ {
- 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>;
I agree with Mike[1] here -> why not move clocks and supply to cpu0_opp? " It seems wrong to me that the clock and supply data is owned by the cpu node, and not the opp descriptor. Everything about the opp transition should belong to a provider node. Then the cpu simply needs to consume that via a phandle. "
I am not sure if we discussed that point further OR if we kinda got hooked on to the "should it be in kernel" point of debate in V4.
clock-names = "cpu";
opp-supply = <&cpu_supply0>;
operating-points-v2 = <&cpu0_opp>;
};
- };
[...]
[1] http://marc.info/?l=linux-pm&m=143146696029140&w=2 [2] http://marc.info/?l=linux-kernel&m=143135047525313&w=2
On 21-05-15, 00:27, Nishanth Menon wrote:
+This describes the OPPs belonging to a device. This node can have following +properties:
+Required properties: +- compatible: Allow OPPs to express their compatibility. It should be:
- "operating-points-v2".
+- OPP nodes: One or more OPP nodes describing voltage-current-frequency
- combinations. Their name isn't significant but their phandle can be used to
- reference an OPP.
What if this was generated data (say using an overlay)?
Sorry I am not aware of this, can you explain a bit how this is done ? Are you talking about something like fdtput here ?
does it have to be "required" or just "optional" :)
This has to be required (by the parser, kernel in our case).
+Required properties: +- opp-hz: Frequency in Hz
I am just being nit picky -> should we keep Heinrich Hertz respected[2] and name it opp-Hz ? No strong opinions either way.
Documentation/devicetree/booting-without-of.txt:
4) Note about node and property names and character set -------------------------------------------------------
While Open Firmware provides more flexible usage of 8859-1, this specification enforces more strict rules. Nodes and properties should be comprised only of ASCII characters 'a' to 'z', '0' to '9', ',', '.', '_', '+', '#', '?', and '-'. Node names additionally allow uppercase characters 'A' to 'Z' (property names should be lowercase. The fact that vendors like Apple don't respect this rule is irrelevant here). Additionally, node and property names should always begin with a character in the range 'a' to 'z' (or 'A' to 'Z' for node names).
different angle: How about just target-freq-Hz? just drop the "opp" prefix for properties of an OPP node? No strong feelings here. (some folks did have variations of a few Hz based on clock tree - example with a crystal frequency of 19.2MHz you'd probably get 1001MHz exact, with a 26MHz crystal, you'd get 1000MHz -> ofcourse round-rate is supposed to help with that... but anyways.. why not say we are trying to indicate target frequency? I do recollect during initial days of OPP (pre-dt-adoption days) folks did complain about this - we kinda worked around this with round-rated handling - but we might as well anticipate it.
Rob suggested opp- prefix and it looks good to me, lets see what others have to say :)
Thanks for adding the examples - My customer support team especially will appreciate having such examples ;).
I can understand that :)
I agree with Mike[1] here -> why not move clocks and supply to cpu0_opp? " It seems wrong to me that the clock and supply data is owned by the cpu node, and not the opp descriptor. Everything about the opp transition should belong to a provider node. Then the cpu simply needs to consume that via a phandle. "
I am not sure if we discussed that point further OR if we kinda got hooked on to the "should it be in kernel" point of debate in V4.
I did send a reply to that, but no one replied after that. Probably you want to reply on that ?
Thanks for your detailed review.
On 05/21/2015 12:46 AM, Viresh Kumar wrote:
On 21-05-15, 00:27, Nishanth Menon wrote:
+This describes the OPPs belonging to a device. This node can have following +properties:
+Required properties: +- compatible: Allow OPPs to express their compatibility. It should be:
- "operating-points-v2".
+- OPP nodes: One or more OPP nodes describing voltage-current-frequency
- combinations. Their name isn't significant but their phandle can be used to
- reference an OPP.
What if this was generated data (say using an overlay)?
Sorry I am not aware of this, can you explain a bit how this is done ? Are you talking about something like fdtput here ?
Honestly, have'nt thought out all the details in yet :)... just thinking.. since overlays can come in later in the system, we could boot up a very bare minimum kernel, have device tree populated runtime (say based on data incoming from a power management microcontroller based description of the capabilities.. I know PSCI or some other form will eventually deal with it.. just wondering about scaling options)... basic node description made in original dtb, and rest of the data populated dynamically(by some magic mechanism yet to be defined) prior to consumer of that data crunching away.. mostly to deal with paper spins that many of us in SoC vendor world work with..
does it have to be "required" or just "optional" :)
This has to be required (by the parser, kernel in our case).
+Required properties: +- opp-hz: Frequency in Hz
I am just being nit picky -> should we keep Heinrich Hertz respected[2] and name it opp-Hz ? No strong opinions either way.
Documentation/devicetree/booting-without-of.txt:
hehe.. thanks on the reminder to RTFM. [...]
different angle: How about just target-freq-Hz? just drop the "opp" prefix for properties of an OPP node? No strong feelings here. (some folks did have variations of a few Hz based on clock tree - example with a crystal frequency of 19.2MHz you'd probably get 1001MHz exact, with a 26MHz crystal, you'd get 1000MHz -> ofcourse round-rate is supposed to help with that... but anyways.. why not say we are trying to indicate target frequency? I do recollect during initial days of OPP (pre-dt-adoption days) folks did complain about this - we kinda worked around this with round-rated handling - but we might as well anticipate it.
Rob suggested opp- prefix and it looks good to me, lets see what others have to say :)
OK.
I agree with Mike[1] here -> why not move clocks and supply to cpu0_opp? " It seems wrong to me that the clock and supply data is owned by the cpu node, and not the opp descriptor. Everything about the opp transition should belong to a provider node. Then the cpu simply needs to consume that via a phandle. "
I am not sure if we discussed that point further OR if we kinda got hooked on to the "should it be in kernel" point of debate in V4.
I did send a reply to that, but no one replied after that. Probably you want to reply on that ?
I assume you meant [1] which in turn pointed to [2]. Thanks, will do so.
[1] http://marc.info/?l=linux-pm&m=143150734506159&w=2 [2] https://lists.linaro.org/pipermail/linaro-kernel/2014-December/019505.html
On Thu, May 21, 2015 at 12:46 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
On 21-05-15, 00:27, Nishanth Menon wrote:
+This describes the OPPs belonging to a device. This node can have following +properties:
[...]
different angle: How about just target-freq-Hz? just drop the "opp" prefix for properties of an OPP node? No strong feelings here. (some folks did have variations of a few Hz based on clock tree - example with a crystal frequency of 19.2MHz you'd probably get 1001MHz exact, with a 26MHz crystal, you'd get 1000MHz -> ofcourse round-rate is supposed to help with that... but anyways.. why not say we are trying to indicate target frequency? I do recollect during initial days of OPP (pre-dt-adoption days) folks did complain about this - we kinda worked around this with round-rated handling - but we might as well anticipate it.
Rob suggested opp- prefix and it looks good to me, lets see what others have to say :)
You can decide the color of the bike shed.
Thanks for adding the examples - My customer support team especially will appreciate having such examples ;).
I can understand that :)
I agree with Mike[1] here -> why not move clocks and supply to cpu0_opp? " It seems wrong to me that the clock and supply data is owned by the cpu node, and not the opp descriptor. Everything about the opp transition should belong to a provider node. Then the cpu simply needs to consume that via a phandle. "
I am not sure if we discussed that point further OR if we kinda got hooked on to the "should it be in kernel" point of debate in V4.
I did send a reply to that, but no one replied after that. Probably you want to reply on that ?
Clock and regulator bindings describe connections in the h/w. OPPs are not a h/w block, but rather properties of the h/w. The connections here are to the cpu's.
Rob
On 22-05-15, 10:55, Rob Herring wrote:
Rob suggested opp- prefix and it looks good to me, lets see what others have to say :)
You can decide the color of the bike shed.
I like that color more :)
On 21-05-15, 00:27, Nishanth Menon wrote:
I would suggest adding a link to how future vendor specific extension docs might look like - maybe this is probably not the time to discuss this.. but anyways.. we could make some statement to the effect of "SoC vendor specfic extensions are documented as Documentation/devicetree/bindings/power/<vendor>,opp.txt and should clearly indicate that the extensions are permitted only under the operating-points-v2 compatible description."
Will this work ?:
If required, this can be extended for SoC vendor specfic bindings. Such bindings should be documented as Documentation/devicetree/bindings/power/<vendor>-opp.txt and should have a compatible description like: "operating-points-v2-<vendor>".
On some platforms (Like Qualcomm's SoCs), it is not decided until runtime on what OPPs to use. The OPP tables can be fixed at compile time, but which table to use is found out only after reading some efuses (sort of an prom) and knowing characteristics of the SoC.
To support such platform we need to pass multiple OPP tables per device and hardware should be able to choose one and only one table out of those.
Update OPP-v2 bindings to support that.
Acked-by: Nishanth Menon nm@ti.com Reviewed-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Documentation/devicetree/bindings/power/opp.txt | 52 +++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index d132e2927b21..7f30d9b07db7 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -46,6 +46,9 @@ Devices supporting OPPs must set their "operating-points-v2" property with phandle to a OPP descriptor in their DT node. The OPP core will use this phandle to find the operating points for the device.
+Devices may want to choose OPP tables at runtime and so can provide a list of +phandles here. But only *one* of them should be chosen at runtime. +
* OPP Descriptor Node
@@ -61,6 +64,9 @@ This describes the OPPs belonging to a device. This node can have following reference an OPP.
Optional properties: +- opp-name: Name of the OPP table, to uniquely identify it if more than one OPP + table is supplied in "operating-points-v2" property of device. + - opp-shared: Indicates that device nodes using this OPP descriptor's phandle switch their DVFS state together, i.e. they share clock/voltage/current lines. Missing property means devices have independent clock/voltage/current lines, @@ -394,3 +400,49 @@ Example 4: Handling multiple regulators }; }; }; + +Example 5: Multiple OPP tables + +/ { + cpus { + cpu@0 { + compatible = "arm,cortex-a7"; + ... + + opp-supply = <&cpu_supply> + operating-points-v2 = <&cpu0_opp0>, <&cpu0_opp1>; + }; + }; + + cpu0_opp0: opp0 { + compatible = "operating-points-v2"; + opp-name = "opp-slow"; + opp-shared; + + entry00 { + opp-hz = <600000000>; + ... + }; + + entry01 { + opp-hz = <800000000>; + ... + }; + }; + + cpu0_opp1: opp1 { + compatible = "operating-points-v2"; + opp-name = "opp-fast"; + opp-shared; + + entry10 { + opp-hz = <1000000000>; + ... + }; + + entry11 { + opp-hz = <1100000000>; + ... + }; + }; +};
On 05/19/2015 10:41 PM, Viresh Kumar wrote: Thinking a little more..
+Example 5: Multiple OPP tables
+/ {
- cpus {
cpu@0 {
compatible = "arm,cortex-a7";
...
opp-supply = <&cpu_supply>
operating-points-v2 = <&cpu0_opp0>, <&cpu0_opp1>;
};
- };
- cpu0_opp0: opp0 {
Maybe we should rename these as cpu0_opp_table_slow opp_table-fast
compatible = "operating-points-v2";
opp-name = "opp-slow";
just name = "slow" ?
opp-shared;
entry00 {
rename these as opp0, opp01 etc? these are the actual OPP description, while, what we call "cpu0_opp0" is actually an OPP table choice we have.
opp-hz = <600000000>;
...
};
entry01 {
opp-hz = <800000000>;
...
};
- };
- cpu0_opp1: opp1 {
compatible = "operating-points-v2";
opp-name = "opp-fast";
opp-shared;
entry10 {
opp-hz = <1000000000>;
...
};
entry11 {
opp-hz = <1100000000>;
...
};
- };
+};
On 21-05-15, 00:34, Nishanth Menon wrote:
- cpu0_opp0: opp0 {
Maybe we should rename these as cpu0_opp_table_slow opp_table-fast
compatible = "operating-points-v2";
opp-name = "opp-slow";
just name = "slow" ?
opp-shared;
entry00 {
rename these as opp0, opp01 etc? these are the actual OPP description, while, what we call "cpu0_opp0" is actually an OPP table choice we have.
What about this now: +Example 5: Multiple OPP tables + +/ { + cpus { + cpu@0 { + compatible = "arm,cortex-a7"; + ... + + opp-supply = <&cpu_supply> + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; + }; + }; + + cpu0_opp_table_slow: opp_table_slow { + compatible = "operating-points-v2"; + opp-name = "slow"; + opp-shared; + + opp00 { + opp-hz = <600000000>; + ... + }; + + opp01 { + opp-hz = <800000000>; + ... + }; + }; + + cpu0_opp_table_fast: opp_table_fast { + compatible = "operating-points-v2"; + opp-name = "fast"; + opp-shared; + + opp10 { + opp-hz = <1000000000>; + ... + }; + + opp11 { + opp-hz = <1100000000>; + ... + }; + }; +};
May I carry your Ack or you want to give it again?
On few platforms, for power efficiency, we want the device to be configured for a specific OPP while we put the device in suspend state.
Add an optional property in operating-points-v2 bindings for that.
Suggested-by: Nishanth Menon nm@ti.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Documentation/devicetree/bindings/power/opp.txt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 7f30d9b07db7..880f5e14b6e2 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -72,6 +72,8 @@ This describes the OPPs belonging to a device. This node can have following Missing property means devices have independent clock/voltage/current lines, but they share OPP tables.
+- opp-suspend: Phandle of the OPP to set while device is suspended. +
* OPP Node
@@ -143,9 +145,10 @@ Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
cpu0_opp: opp0 { compatible = "operating-points-v2"; + opp-suspend = <&suspend_opp>; opp-shared;
- entry00 { + suspend-opp: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>; @@ -217,13 +220,14 @@ independently.
cpu0_opp: opp0 { compatible = "operating-points-v2"; + opp-suspend = <&suspend_opp>;
/* * Missing opp-shared property means CPUs switch DVFS states * independently. */
- entry00 { + suspend-opp: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>; @@ -296,9 +300,10 @@ DVFS state together.
cluster0_opp: opp0 { compatible = "operating-points-v2"; + opp-suspend = <&suspend_opp0>; opp-shared;
- entry00 { + suspend-opp0: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>; @@ -321,9 +326,10 @@ DVFS state together.
cluster1_opp: opp1 { compatible = "operating-points-v2"; + opp-suspend = <&suspend_opp1>; opp-shared;
- entry10 { + suspend-opp1: entry10 { opp-hz = <1300000000>; opp-microvolt = <1045000 1050000 1055000>; opp-microamp = <95000>;
On 05/19/2015 10:41 PM, Viresh Kumar wrote:
On few platforms, for power efficiency, we want the device to be configured for a specific OPP while we put the device in suspend state.
Add an optional property in operating-points-v2 bindings for that.
Why not just mark it as a property of an OPP rather than a phandle? just thinking that this might setup a precedence for future needs like "shutdown OPP" or "Reboot OPP" or something different that some other SoC might have..
Suggested-by: Nishanth Menon nm@ti.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Documentation/devicetree/bindings/power/opp.txt | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 7f30d9b07db7..880f5e14b6e2 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -72,6 +72,8 @@ This describes the OPPs belonging to a device. This node can have following Missing property means devices have independent clock/voltage/current lines, but they share OPP tables. +- opp-suspend: Phandle of the OPP to set while device is suspended.
- OPP Node
@@ -143,9 +145,10 @@ Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. cpu0_opp: opp0 { compatible = "operating-points-v2";
opp-shared;opp-suspend = <&suspend_opp>;
entry00 {
suspend-opp: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>;
@@ -217,13 +220,14 @@ independently. cpu0_opp: opp0 { compatible = "operating-points-v2";
opp-suspend = <&suspend_opp>;
/* * Missing opp-shared property means CPUs switch DVFS states * independently. */
entry00 {
suspend-opp: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>;
@@ -296,9 +300,10 @@ DVFS state together. cluster0_opp: opp0 { compatible = "operating-points-v2";
opp-shared;opp-suspend = <&suspend_opp0>;
entry00 {
suspend-opp0: entry00 { opp-hz = <1000000000>; opp-microvolt = <970000 975000 985000>; opp-microamp = <70000>;
@@ -321,9 +326,10 @@ DVFS state together. cluster1_opp: opp1 { compatible = "operating-points-v2";
opp-shared;opp-suspend = <&suspend_opp1>;
entry10 {
suspend-opp1: entry10 { opp-hz = <1300000000>; opp-microvolt = <1045000 1050000 1055000>; opp-microamp = <95000>;
On 21-05-15, 00:32, Nishanth Menon wrote:
Why not just mark it as a property of an OPP rather than a phandle? just thinking that this might setup a precedence for future needs like "shutdown OPP" or "Reboot OPP" or something different that some other SoC might have..
AFAIU, a property should be present in the OPP if and only if any OPP can set it. But in this case, only one OPP from the entire list can set it. So, its more a property of the list rather than every OPP within it. And then there wouldn't be any need to code that would check bugs in dtbs where multiple OPPs have it set.
On 05/21/2015 12:49 AM, Viresh Kumar wrote:
On 21-05-15, 00:32, Nishanth Menon wrote:
Why not just mark it as a property of an OPP rather than a phandle? just thinking that this might setup a precedence for future needs like "shutdown OPP" or "Reboot OPP" or something different that some other SoC might have..
AFAIU, a property should be present in the OPP if and only if any OPP can set it. But in this case, only one OPP from the entire list can set it. So, its more a property of the list rather than every OPP within it. And then there wouldn't be any need to code that would check bugs in dtbs where multiple OPPs have it set.
True.. fair enough.
Acked-by: Nishanth Menon nm@ti.com
linaro-kernel@lists.linaro.org