| Message ID | CAKohpokUXvhqt+z4EPC9k=O8ZnQOqNSGGi8w6w4shYtbvv8mTw@mail.gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Dec 9, 2014 at 9:51 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 4 December 2014 at 16:44, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> The shortcomings we are trying to solve here: >> >> - Some kind of compatibility string to probe the right cpufreq driver for >> platforms, when multiple drivers are available. For example: how to choose >> between cpufreq-dt and arm_big_little drivers. >> >> - Getting clock sharing information between CPUs. Single shared clock vs. >> independent clock per core vs. shared clock per cluster. >> >> - Support for turbo modes >> >> - Other per OPP settings: transition latencies, disabled status, etc.? > > Some updates on the structure of bindings which I got up to with help of Arnd > and Rob over IRC, have got better examples to show how things would look > like: > > diff --git a/Documentation/devicetree/bindings/power/opp.txt > b/Documentation/devicetree/bindings/power/opp.txt > index 74499e5033fc..8ae574b84650 100644 > --- a/Documentation/devicetree/bindings/power/opp.txt > +++ b/Documentation/devicetree/bindings/power/opp.txt > @@ -1,9 +1,292 @@ > -* Generic OPP Interface > +Generic OPP (Operating Performance Points) Interface > +---------------------------------------------------- > > SoCs have a standard set of tuples consisting of frequency and > voltage pairs that the device will support per voltage domain. These > are called Operating Performance Points or OPPs. > > +This documents defines OPP bindings with its required/optional properties. > +OPPs can be defined for any device, this file uses CPU device as an example to > +illustrate how to define OPPs. > + > +opp nodes and opp-lists > + > +- opp-listN: > + List of nodes defining performance points. Following belong to the nodes > + within the opp-lists. > + > + Required properties: > + - frequency-kHz: Frequency in kHz s/kHz/khz/ > + - voltage-uV: voltage in micro Volts -microvolt is more consistent with regulator binding. The names are a bit redundant. perhaps opp-khz and opp-microvolt instead. > + > + Optional properties: > + - turbo-mode: Marks the volt-freq pair as turbo pair. > + - status: Marks the node enabled/disabled. > + > +- oppN: > + Operating performance point node per device. Devices using it should have its > + phandle in their "operating-points-v2" property. > + > + Required properties: > + - compatible: allow OPPs to express their compatibility > + - opp-list: phandle to opp-list defined above. > + > + Optional properties: > + - clocks: Tuple of clock providers > + - clock-names: Clock names > + - opp-supply: phandle to the parent supply/regulator node > + - voltage-tolerance: Specify the CPU voltage tolerance in percentage. > + - clock-latency: Specify the possible maximum transition latency for clock, > + in unit of nanoseconds. > + > +Example 1: Simple case of dual-core cortex A9-single cluster, sharing > clock line. > + > +/ { > + cpus { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cpu@0 { > + compatible = "arm,cortex-a9"; > + reg = <0>; > + next-level-cache = <&L2>; > + operating-points-v2 = <&opp0>; > + > + opp0: opp0 { I don't really like having this under cpu0 when it applies to all cpus. I would move it out of /cpus. > + compatible = "linux,cpu-dvfs"; This should not be linux specific. Probably not cpu specific either. > + clocks = <&clk-controller 0>; > + clock-names = "cpu"; clocks are an input to the cpu, not really an opp. You could have a an OPP which uses a different parent clock, but that is most likely a switch within the clock controller rather than 2 clock inputs to the cpu. I think the clock binding for cpus should stand on its own independent of OPPs. > + opp-supply = <&cpu-supply0>; Same comment as clocks applies here. > + voltage-tolerance = <2>; /* percentage */ > + clock-latency = <300000>; These could be per entry. I'm not sure it is worth the savings to not just always specify them per entry. We should append units (-us) to clock-latency unless there is a good reason to maintain compatibility. > + opp-list = <&opplist0>; With the above changes, having this list is unnecessary. So, what I envision is like this: /cpus { cpu@0 { clocks = <...>; cpu-supply = <...>; operating-point-v2 = <&cpu0-opp>; }; cpu@1 { clocks = <...>; cpu-supply = <...>; operating-point-v2 = <&cpu1-opp>; }; }; cpu0-opp : opp0 { compatible = "operating-point-table"; entry0 { opp-khz = <500000>; opp-microvolt = <900000>; }; entry1 { opp-khz = <1000000>; opp-microvolt = <1000000>; turbo-mode; }; }; cpu1-opp : opp1 { compatible = "operating-point-table"; ... }; We need to also consider if this all works for other non-cpu OPPs like GPUs or DRAM/bus. Rob -- 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 29 December 2014 at 22:35, Rob Herring <robherring2@gmail.com> wrote: >> + - frequency-kHz: Frequency in kHz > > s/kHz/khz/ > >> + - voltage-uV: voltage in micro Volts > > -microvolt is more consistent with regulator binding. > > The names are a bit redundant. perhaps opp-khz and opp-microvolt instead. All accepted. >> +Example 1: Simple case of dual-core cortex A9-single cluster, sharing >> clock line. >> + >> +/ { >> + cpus { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + cpu@0 { >> + compatible = "arm,cortex-a9"; >> + reg = <0>; >> + next-level-cache = <&L2>; >> + operating-points-v2 = <&opp0>; >> + >> + opp0: opp0 { > > I don't really like having this under cpu0 when it applies to all > cpus. I would move it out of /cpus. That's how I had it initially, but then Arnd didn't like it much. >> + compatible = "linux,cpu-dvfs"; > > This should not be linux specific. Probably not cpu specific either. This was just an example of one of the bindings and there will be others as well. 'linux' here doesn't mean linux specific, but just that its first used by Linux. That's what my understanding is atleast. >> + clocks = <&clk-controller 0>; >> + clock-names = "cpu"; > > clocks are an input to the cpu, not really an opp. You could have a an > OPP which uses a different parent clock, but that is most likely a > switch within the clock controller rather than 2 clock inputs to the > cpu. > > I think the clock binding for cpus should stand on its own independent of OPPs. > >> + opp-supply = <&cpu-supply0>; > > Same comment as clocks applies here. Both will be kept in the cpu node where they were initially. >> + voltage-tolerance = <2>; /* percentage */ >> + clock-latency = <300000>; > > These could be per entry. I'm not sure it is worth the savings to not > just always specify them per entry. Done. > We should append units (-us) to clock-latency unless there is a good > reason to maintain compatibility. So you meant something like this: clock-latency-us = <300000>; right? >> + opp-list = <&opplist0>; > > With the above changes, having this list is unnecessary. It might be for the use case I mentioned earlier about something like Krait. > So, what I envision is like this: > > /cpus { > cpu@0 { > clocks = <...>; > cpu-supply = <...>; > operating-point-v2 = <&cpu0-opp>; > }; > cpu@1 { > clocks = <...>; > cpu-supply = <...>; > operating-point-v2 = <&cpu1-opp>; > }; > }; Looks fine.. > cpu0-opp : opp0 { > compatible = "operating-point-table"; > entry0 { > opp-khz = <500000>; > opp-microvolt = <900000>; > }; > entry1 { > opp-khz = <1000000>; > opp-microvolt = <1000000>; > turbo-mode; > }; > }; > cpu1-opp : opp1 { > compatible = "operating-point-table"; > ... > }; What about something like Krait which wants to use exactly same bindings for all CPUs but want to specify they are controlled separately. So I had it like: cpu0-opp : opp0 { compatible = "operating-point-table"; opp-list = <&opplist0>; opplist0: opp-list0 { entry0 { opp-khz = <500000>; opp-microvolt = <900000>; }; entry1 { opp-khz = <1000000>; opp-microvolt = <1000000>; turbo-mode; }; }; }; cpu1-opp : opp1 { compatible = "operating-point-table"; opp-list = <&opplist0>; }; > We need to also consider if this all works for other non-cpu OPPs like > GPUs or DRAM/bus. Do you any input here? Or if you know somebody who can give inputs about them? -- 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 31 December 2014 at 10:17, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 29 December 2014 at 22:35, Rob Herring <robherring2@gmail.com> wrote: >>> + - frequency-kHz: Frequency in kHz >> >> s/kHz/khz/ >> >>> + - voltage-uV: voltage in micro Volts >> >> -microvolt is more consistent with regulator binding. >> >> The names are a bit redundant. perhaps opp-khz and opp-microvolt instead. > > All accepted. > >>> +Example 1: Simple case of dual-core cortex A9-single cluster, sharing >>> clock line. >>> + >>> +/ { >>> + cpus { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + cpu@0 { >>> + compatible = "arm,cortex-a9"; >>> + reg = <0>; >>> + next-level-cache = <&L2>; >>> + operating-points-v2 = <&opp0>; >>> + >>> + opp0: opp0 { >> >> I don't really like having this under cpu0 when it applies to all >> cpus. I would move it out of /cpus. > > That's how I had it initially, but then Arnd didn't like it much. > >>> + compatible = "linux,cpu-dvfs"; >> >> This should not be linux specific. Probably not cpu specific either. > > This was just an example of one of the bindings and there will be others > as well. > > 'linux' here doesn't mean linux specific, but just that its first used by > Linux. That's what my understanding is atleast. > >>> + clocks = <&clk-controller 0>; >>> + clock-names = "cpu"; >> >> clocks are an input to the cpu, not really an opp. You could have a an >> OPP which uses a different parent clock, but that is most likely a >> switch within the clock controller rather than 2 clock inputs to the >> cpu. >> >> I think the clock binding for cpus should stand on its own independent of OPPs. >> >>> + opp-supply = <&cpu-supply0>; >> >> Same comment as clocks applies here. > > Both will be kept in the cpu node where they were initially. > >>> + voltage-tolerance = <2>; /* percentage */ >>> + clock-latency = <300000>; >> >> These could be per entry. I'm not sure it is worth the savings to not >> just always specify them per entry. > > Done. > >> We should append units (-us) to clock-latency unless there is a good >> reason to maintain compatibility. > > So you meant something like this: > > clock-latency-us = <300000>; > > right? > >>> + opp-list = <&opplist0>; >> >> With the above changes, having this list is unnecessary. > > It might be for the use case I mentioned earlier about something > like Krait. > >> So, what I envision is like this: >> >> /cpus { >> cpu@0 { >> clocks = <...>; >> cpu-supply = <...>; >> operating-point-v2 = <&cpu0-opp>; >> }; >> cpu@1 { >> clocks = <...>; >> cpu-supply = <...>; >> operating-point-v2 = <&cpu1-opp>; >> }; >> }; > > Looks fine.. > >> cpu0-opp : opp0 { >> compatible = "operating-point-table"; >> entry0 { >> opp-khz = <500000>; >> opp-microvolt = <900000>; >> }; >> entry1 { >> opp-khz = <1000000>; >> opp-microvolt = <1000000>; >> turbo-mode; >> }; >> }; >> cpu1-opp : opp1 { >> compatible = "operating-point-table"; >> ... >> }; > > What about something like Krait which wants to use exactly same > bindings for all CPUs but want to specify they are controlled separately. > > So I had it like: > > cpu0-opp : opp0 { > compatible = "operating-point-table"; > opp-list = <&opplist0>; > > opplist0: opp-list0 { > entry0 { > opp-khz = <500000>; > opp-microvolt = <900000>; > }; > entry1 { > opp-khz = <1000000>; > opp-microvolt = <1000000>; > turbo-mode; > }; > }; > }; > > cpu1-opp : opp1 { > compatible = "operating-point-table"; > opp-list = <&opplist0>; > }; Gentle reminder, so that we can close this long standing issue soon.. -- 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
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 74499e5033fc..8ae574b84650 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -1,9 +1,292 @@ -* Generic OPP Interface +Generic OPP (Operating Performance Points) Interface +---------------------------------------------------- SoCs have a standard set of tuples consisting of frequency and voltage pairs that the device will support per voltage domain. These are called Operating Performance Points or OPPs. +This documents defines OPP bindings with its required/optional properties. +OPPs can be defined for any device, this file uses CPU device as an example to +illustrate how to define OPPs. + +opp nodes and opp-lists + +- opp-listN: + List of nodes defining performance points. Following belong to the nodes + within the opp-lists. + + Required properties: + - frequency-kHz: Frequency in kHz + - voltage-uV: voltage in micro Volts + + Optional properties: + - turbo-mode: Marks the volt-freq pair as turbo pair. + - status: Marks the node enabled/disabled. + +- oppN: + Operating performance point node per device. Devices using it should have its + phandle in their "operating-points-v2" property. + + Required properties: + - compatible: allow OPPs to express their compatibility + - opp-list: phandle to opp-list defined above. + + Optional properties: + - clocks: Tuple of clock providers + - clock-names: Clock names + - opp-supply: phandle to the parent supply/regulator node + - voltage-tolerance: Specify the CPU voltage tolerance in percentage. + - clock-latency: Specify the possible maximum transition latency for clock, + in unit of nanoseconds. + +Example 1: Simple case of dual-core cortex A9-single cluster, sharing clock line. + +/ { + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + compatible = "arm,cortex-a9"; + reg = <0>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp0>; + + opp0: opp0 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 0>; + clock-names = "cpu"; + opp-supply = <&cpu-supply0>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + + opplist0: opp-list0 { + entry00 { + frequency-kHz = <1000000>; + voltage-uV = <975000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1100000>; + voltage-uV = <1000000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1200000>; + voltage-uV = <1025000>; + status = "okay"; + turbo-mode; + }; + }; + }; + }; + + cpu@1 { + compatible = "arm,cortex-a9"; + reg = <1>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp0>; + }; + }; +}; + +Example 2: Quad-core krait (All CPUs have independent clock lines but have same set of OPPs) + +/ { + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + compatible = "qcom,krait"; + reg = <0>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp0>; + + opp0: opp0 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 0>; + clock-names = "cpu"; + opp-supply = <&cpu-supply0>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + + opplist0: opp-list0 { + entry00 { + frequency-kHz = <1000000>; + voltage-uV = <975000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1100000>; + voltage-uV = <1000000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1200000>; + voltage-uV = <1025000>; + status = "okay"; + turbo-mode; + }; + }; + }; + }; + + cpu@1 { + compatible = "qcom,krait"; + reg = <1>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp1>; + + opp1: opp1 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 1>; + clock-names = "cpu"; + opp-supply = <&cpu-supply1>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + }; + }; + + cpu@2 { + compatible = "qcom,krait"; + reg = <2>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp2>; + + opp2: opp2 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 2>; + clock-names = "cpu"; + opp-supply = <&cpu-supply2>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + }; + }; + + cpu@3 { + compatible = "qcom,krait"; + reg = <3>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp3>; + + opp3: opp3 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 3>; + clock-names = "cpu"; + opp-supply = <&cpu-supply3>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + }; + }; + }; +}; + +Example 3: Multi-cluster system with separate clock line per cluster. + +/ { + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu@0 { + compatible = "arm,cortex-a7"; + reg = <0>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp0>; + + opp0: opp0 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 0>; + clock-names = "cpu"; + opp-supply = <&cpu-supply0>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <300000>; + opp-list = <&opplist0>; + + opplist0: opp-list0 { + entry00 { + frequency-kHz = <1000000>; + voltage-uV = <975000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1100000>; + voltage-uV = <1000000>; + status = "okay"; + }; + entry01 { + frequency-kHz = <1200000>; + voltage-uV = <1025000>; + status = "okay"; + turbo-mode; + }; + }; + }; + }; + + cpu@1 { + compatible = "arm,cortex-a7"; + reg = <1>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp0>; + }; + + cpu@100 { + compatible = "arm,cortex-a15"; + reg = <100>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp1>; + + opp1: opp1 { + compatible = "linux,cpu-dvfs"; + clocks = <&clk-controller 1>; + clock-names = "cpu"; + opp-supply = <&cpu-supply1>; + voltage-tolerance = <2>; /* percentage */ + clock-latency = <400000>; + opp-list = <&opplist1>; + + opplist1: opp-list1 { + entry10 { + frequency-kHz = <1300000>; + voltage-uV = <1050000>; + status = "okay"; + }; + entry11 { + frequency-kHz = <1400000>; + voltage-uV = <1075000>; + status = "disabled"; + }; + entry12 { + frequency-kHz = <1500000>; + voltage-uV = <1100000>; + status = "okay"; + turbo-mode; + }; + }; + }; + }; + + cpu@101 { + compatible = "arm,cortex-a15"; + reg = <101>; + next-level-cache = <&L2>; + operating-points-v2 = <&opp1>; + }; + }; +}; + + + +Deprecated Bindings +------------------- + -- To unsubscribe from this list: send the line "unsubscribe devicetree" in