* [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains @ 2017-12-18 10:21 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Nishanth Menon, Rafael J. Wysocki, Stephen Boyd, Viresh Kumar Cc: Viresh Kumar, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel Hi, Now that the performance state of PM domains are supported by the kernel (merged in linux-next), I am trying once again to define the bindings which we dropped until the code is merged first. Summary: Power-domains can also have their active states and this patchset enhances the OPP binding to define those. The power domains can use the OPP bindings mostly as is. Though there are some changes required to support special cases: - Allow "operating-points-v2" to contain multiple phandles for power domain providers providing multiple domains. - A new property "required-opp" is added for the devices to specify the minimum required OPP of the master domain or any other type of device. - Allow some of the OPP properties to accept magic values (firmware dependent) as the OS doesn't know the real freq/voltage values. V7->V8: - V7 1/2 divided into two patches. 1/3 is unchanged from V7. - 2/3 renamed the property from "power-domain-opp" to "required-opp", as suggested by Rob. - Added Ulf's reviewed-by for 1/3 and 3/3. -- viresh Viresh Kumar (3): OPP: Allow OPP table to be used for power-domains OPP: Introduce "required-opp" property OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Documentation/devicetree/bindings/opp/opp.txt | 19 +++++++ .../devicetree/bindings/power/power_domain.txt | 65 ++++++++++++++++++++++ 2 files changed, 84 insertions(+) -- 2.15.0.194.g9af6a3dea062 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains @ 2017-12-18 10:21 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Nishanth Menon, Rafael J. Wysocki, Stephen Boyd, Viresh Kumar Cc: Viresh Kumar, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Hi, Now that the performance state of PM domains are supported by the kernel (merged in linux-next), I am trying once again to define the bindings which we dropped until the code is merged first. Summary: Power-domains can also have their active states and this patchset enhances the OPP binding to define those. The power domains can use the OPP bindings mostly as is. Though there are some changes required to support special cases: - Allow "operating-points-v2" to contain multiple phandles for power domain providers providing multiple domains. - A new property "required-opp" is added for the devices to specify the minimum required OPP of the master domain or any other type of device. - Allow some of the OPP properties to accept magic values (firmware dependent) as the OS doesn't know the real freq/voltage values. V7->V8: - V7 1/2 divided into two patches. 1/3 is unchanged from V7. - 2/3 renamed the property from "power-domain-opp" to "required-opp", as suggested by Rob. - Added Ulf's reviewed-by for 1/3 and 3/3. -- viresh Viresh Kumar (3): OPP: Allow OPP table to be used for power-domains OPP: Introduce "required-opp" property OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Documentation/devicetree/bindings/opp/opp.txt | 19 +++++++ .../devicetree/bindings/power/power_domain.txt | 65 ++++++++++++++++++++++ 2 files changed, 84 insertions(+) -- 2.15.0.194.g9af6a3dea062 -- 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V8 1/3] OPP: Allow OPP table to be used for power-domains 2017-12-18 10:21 ` Viresh Kumar (?) @ 2017-12-18 10:21 ` Viresh Kumar 2017-12-21 22:06 ` Rob Herring -1 siblings, 1 reply; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki Cc: Viresh Kumar, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel Power-domains can also have their active states and this patch enhances the OPP binding to define those. The power domains can use the OPP bindings as is, with one additional change to Allow "operating-points-v2" property to contain multiple phandles for power domain providers providing multiple domains. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Documentation/devicetree/bindings/opp/opp.txt | 5 +++++ Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 9d733af26be7..a3953a1bb1a1 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -45,6 +45,11 @@ Devices supporting OPPs must set their "operating-points-v2" property with phandle to a OPP table in their DT node. The OPP core will use this phandle to find the operating points for the device. +This can contain more than one phandle for power domain providers that provide +multiple power domains. That is, one phandle for each power domain. If only one +phandle is available, then the same OPP table will be used for all power domains +provided by the power domain provider. + If required, this can be extended for SoC vendor specific 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>". diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt index 14bd9e945ff6..61549840ab3b 100644 --- a/Documentation/devicetree/bindings/power/power_domain.txt +++ b/Documentation/devicetree/bindings/power/power_domain.txt @@ -40,6 +40,12 @@ phandle arguments (so called PM domain specifiers) of length specified by the domain's idle states. In the absence of this property, the domain would be considered as capable of being powered-on or powered-off. +- operating-points-v2 : Phandles to the OPP tables of power domains provided by + a power domain provider. If the provider provides a single power domain only + or all the power domains provided by the provider have identical OPP tables, + then this shall contain a single phandle. Refer to ../opp/opp.txt for more + information. + Example: power: power-controller@12340000 { -- 2.15.0.194.g9af6a3dea062 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH V8 1/3] OPP: Allow OPP table to be used for power-domains @ 2017-12-21 22:06 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-21 22:06 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On Mon, Dec 18, 2017 at 03:51:28PM +0530, Viresh Kumar wrote: > Power-domains can also have their active states and this patch enhances > the OPP binding to define those. The power domains can use the OPP > bindings as is, with one additional change to Allow > "operating-points-v2" property to contain multiple phandles for power > domain providers providing multiple domains. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 5 +++++ > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++ > 2 files changed, 11 insertions(+) Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 1/3] OPP: Allow OPP table to be used for power-domains @ 2017-12-21 22:06 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-21 22:06 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Mon, Dec 18, 2017 at 03:51:28PM +0530, Viresh Kumar wrote: > Power-domains can also have their active states and this patch enhances > the OPP binding to define those. The power domains can use the OPP > bindings as is, with one additional change to Allow > "operating-points-v2" property to contain multiple phandles for power > domain providers providing multiple domains. > > Reviewed-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 5 +++++ > Documentation/devicetree/bindings/power/power_domain.txt | 6 ++++++ > 2 files changed, 11 insertions(+) Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> -- 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH V8 2/3] OPP: Introduce "required-opp" property @ 2017-12-18 10:21 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki Cc: Viresh Kumar, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel Devices have inter-dependencies some times. For example a device that needs to run at 800 MHz, needs another device (e.g. Its power domain) to be configured at a particular operating performance point. This patch introduces a new property "required-opp" which can be present directly in a device's node (if it doesn't need to change its OPPs), or in device's OPP nodes. More details on the property can be seen in the binding itself. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Documentation/devicetree/bindings/opp/opp.txt | 8 +++ .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index a3953a1bb1a1..4e4f30288c8b 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -159,6 +159,14 @@ properties. - status: Marks the node enabled/disabled. +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt index 61549840ab3b..f824763b202e 100644 --- a/Documentation/devicetree/bindings/power/power_domain.txt +++ b/Documentation/devicetree/bindings/power/power_domain.txt @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located inside a PM domain with index 0 of a power controller represented by a node with the label "power". +Optional properties: +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + +Example: +- OPP table for domain provider that provides two domains. + + domain0_opp_table: opp_table0 { + compatible = "operating-points-v2"; + + domain0_opp_0: opp-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain0_opp_1: opp-1100000000 { + opp-hz = /bits/ 64 <1100000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + domain1_opp_table: opp_table1 { + compatible = "operating-points-v2"; + + domain1_opp_0: opp-1200000000 { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain1_opp_1: opp-1300000000 { + opp-hz = /bits/ 64 <1300000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + parent: power-controller@12340000 { + compatible = "foo,power-controller"; + reg = <0x12340000 0x1000>; + #power-domain-cells = <1>; + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; + }; + + leaky-device0@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 0>; + required-opp = <&domain0_opp_0>; + }; + + leaky-device1@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 1>; + required-opp = <&domain1_opp_1>; + }; + [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt -- 2.15.0.194.g9af6a3dea062 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH V8 2/3] OPP: Introduce "required-opp" property @ 2017-12-18 10:21 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki Cc: Viresh Kumar, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Devices have inter-dependencies some times. For example a device that needs to run at 800 MHz, needs another device (e.g. Its power domain) to be configured at a particular operating performance point. This patch introduces a new property "required-opp" which can be present directly in a device's node (if it doesn't need to change its OPPs), or in device's OPP nodes. More details on the property can be seen in the binding itself. Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- Documentation/devicetree/bindings/opp/opp.txt | 8 +++ .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index a3953a1bb1a1..4e4f30288c8b 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -159,6 +159,14 @@ properties. - status: Marks the node enabled/disabled. +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt index 61549840ab3b..f824763b202e 100644 --- a/Documentation/devicetree/bindings/power/power_domain.txt +++ b/Documentation/devicetree/bindings/power/power_domain.txt @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located inside a PM domain with index 0 of a power controller represented by a node with the label "power". +Optional properties: +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + +Example: +- OPP table for domain provider that provides two domains. + + domain0_opp_table: opp_table0 { + compatible = "operating-points-v2"; + + domain0_opp_0: opp-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain0_opp_1: opp-1100000000 { + opp-hz = /bits/ 64 <1100000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + domain1_opp_table: opp_table1 { + compatible = "operating-points-v2"; + + domain1_opp_0: opp-1200000000 { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain1_opp_1: opp-1300000000 { + opp-hz = /bits/ 64 <1300000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + parent: power-controller@12340000 { + compatible = "foo,power-controller"; + reg = <0x12340000 0x1000>; + #power-domain-cells = <1>; + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; + }; + + leaky-device0@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 0>; + required-opp = <&domain0_opp_0>; + }; + + leaky-device1@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&parent 1>; + required-opp = <&domain1_opp_1>; + }; + [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt -- 2.15.0.194.g9af6a3dea062 -- 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 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH V8 2/3] OPP: Introduce "required-opp" property 2017-12-18 10:21 ` Viresh Kumar (?) @ 2017-12-20 8:23 ` Ulf Hansson 2017-12-20 8:26 ` Viresh Kumar -1 siblings, 1 reply; 41+ messages in thread From: Ulf Hansson @ 2017-12-20 8:23 UTC (permalink / raw) To: Viresh Kumar Cc: Kevin Hilman, Rob Herring, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 18 December 2017 at 11:21, Viresh Kumar <viresh.kumar@linaro.org> wrote: > Devices have inter-dependencies some times. For example a device that > needs to run at 800 MHz, needs another device (e.g. Its power domain) to > be configured at a particular operating performance point. > > This patch introduces a new property "required-opp" which can be present > directly in a device's node (if it doesn't need to change its OPPs), or > in device's OPP nodes. More details on the property can be seen in the > binding itself. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 8 +++ > .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ > 2 files changed, 67 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index a3953a1bb1a1..4e4f30288c8b 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -159,6 +159,14 @@ properties. > > - status: Marks the node enabled/disabled. > > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. > > / { > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt > index 61549840ab3b..f824763b202e 100644 > --- a/Documentation/devicetree/bindings/power/power_domain.txt > +++ b/Documentation/devicetree/bindings/power/power_domain.txt > @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located > inside a PM domain with index 0 of a power controller represented by a node > with the label "power". > > +Optional properties: > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > +Example: > +- OPP table for domain provider that provides two domains. > + > + domain0_opp_table: opp_table0 { > + compatible = "operating-points-v2"; > + > + domain0_opp_0: opp-1000000000 { > + opp-hz = /bits/ 64 <1000000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain0_opp_1: opp-1100000000 { > + opp-hz = /bits/ 64 <1100000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + domain1_opp_table: opp_table1 { > + compatible = "operating-points-v2"; > + > + domain1_opp_0: opp-1200000000 { > + opp-hz = /bits/ 64 <1200000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain1_opp_1: opp-1300000000 { > + opp-hz = /bits/ 64 <1300000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + parent: power-controller@12340000 { Nitpick: Could you please change "parent" to "power", to be consistent with earlier examples. > + compatible = "foo,power-controller"; > + reg = <0x12340000 0x1000>; > + #power-domain-cells = <1>; > + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; > + }; > + > + leaky-device0@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 0>; > + required-opp = <&domain0_opp_0>; > + }; > + > + leaky-device1@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 1>; > + required-opp = <&domain1_opp_1>; > + }; > + > [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt > -- > 2.15.0.194.g9af6a3dea062 > Besides the nitpick, feel free to add: Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 2/3] OPP: Introduce "required-opp" property 2017-12-20 8:23 ` Ulf Hansson @ 2017-12-20 8:26 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-20 8:26 UTC (permalink / raw) To: Ulf Hansson Cc: Kevin Hilman, Rob Herring, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, devicetree, linux-kernel On 20-12-17, 09:23, Ulf Hansson wrote: > Nitpick: Could you please change "parent" to "power", to be consistent > with earlier examples. Sure. Thanks for the review. -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 2/3] OPP: Introduce "required-opp" property @ 2017-12-21 22:26 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-21 22:26 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On Mon, Dec 18, 2017 at 03:51:29PM +0530, Viresh Kumar wrote: > Devices have inter-dependencies some times. For example a device that > needs to run at 800 MHz, needs another device (e.g. Its power domain) to > be configured at a particular operating performance point. > > This patch introduces a new property "required-opp" which can be present > directly in a device's node (if it doesn't need to change its OPPs), or > in device's OPP nodes. More details on the property can be seen in the > binding itself. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 8 +++ > .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ > 2 files changed, 67 insertions(+) A couple of nits, otherwise: Reviewed-by: Rob Herring <robh@kernel.org> > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index a3953a1bb1a1..4e4f30288c8b 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -159,6 +159,14 @@ properties. > > - status: Marks the node enabled/disabled. > > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. > > / { > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt > index 61549840ab3b..f824763b202e 100644 > --- a/Documentation/devicetree/bindings/power/power_domain.txt > +++ b/Documentation/devicetree/bindings/power/power_domain.txt > @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located > inside a PM domain with index 0 of a power controller represented by a node > with the label "power". > > +Optional properties: > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > +Example: > +- OPP table for domain provider that provides two domains. > + > + domain0_opp_table: opp_table0 { opp-table0 > + compatible = "operating-points-v2"; > + > + domain0_opp_0: opp-1000000000 { > + opp-hz = /bits/ 64 <1000000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain0_opp_1: opp-1100000000 { > + opp-hz = /bits/ 64 <1100000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + domain1_opp_table: opp_table1 { opp-table1 > + compatible = "operating-points-v2"; > + > + domain1_opp_0: opp-1200000000 { > + opp-hz = /bits/ 64 <1200000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain1_opp_1: opp-1300000000 { > + opp-hz = /bits/ 64 <1300000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + parent: power-controller@12340000 { > + compatible = "foo,power-controller"; > + reg = <0x12340000 0x1000>; > + #power-domain-cells = <1>; > + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; > + }; > + > + leaky-device0@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 0>; > + required-opp = <&domain0_opp_0>; > + }; > + > + leaky-device1@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 1>; > + required-opp = <&domain1_opp_1>; > + }; > + > [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt > -- > 2.15.0.194.g9af6a3dea062 > ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 2/3] OPP: Introduce "required-opp" property @ 2017-12-21 22:26 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-21 22:26 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Mon, Dec 18, 2017 at 03:51:29PM +0530, Viresh Kumar wrote: > Devices have inter-dependencies some times. For example a device that > needs to run at 800 MHz, needs another device (e.g. Its power domain) to > be configured at a particular operating performance point. > > This patch introduces a new property "required-opp" which can be present > directly in a device's node (if it doesn't need to change its OPPs), or > in device's OPP nodes. More details on the property can be seen in the > binding itself. > > Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 8 +++ > .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ > 2 files changed, 67 insertions(+) A couple of nits, otherwise: Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index a3953a1bb1a1..4e4f30288c8b 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -159,6 +159,14 @@ properties. > > - status: Marks the node enabled/disabled. > > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. > > / { > diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt > index 61549840ab3b..f824763b202e 100644 > --- a/Documentation/devicetree/bindings/power/power_domain.txt > +++ b/Documentation/devicetree/bindings/power/power_domain.txt > @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located > inside a PM domain with index 0 of a power controller represented by a node > with the label "power". > > +Optional properties: > +- required-opp: This contains phandle to an OPP node in another device's OPP > + table. It may contain an array of phandles, where each phandle points to an > + OPP of a different device. It should not contain multiple phandles to the OPP > + nodes in the same OPP table. This specifies the minimum required OPP of the > + device(s), whose OPP's phandle is present in this property, for the > + functioning of the current device at the current OPP (where this property is > + present). > + > +Example: > +- OPP table for domain provider that provides two domains. > + > + domain0_opp_table: opp_table0 { opp-table0 > + compatible = "operating-points-v2"; > + > + domain0_opp_0: opp-1000000000 { > + opp-hz = /bits/ 64 <1000000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain0_opp_1: opp-1100000000 { > + opp-hz = /bits/ 64 <1100000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + domain1_opp_table: opp_table1 { opp-table1 > + compatible = "operating-points-v2"; > + > + domain1_opp_0: opp-1200000000 { > + opp-hz = /bits/ 64 <1200000000>; > + opp-microvolt = <975000 970000 985000>; > + }; > + domain1_opp_1: opp-1300000000 { > + opp-hz = /bits/ 64 <1300000000>; > + opp-microvolt = <1000000 980000 1010000>; > + }; > + }; > + > + parent: power-controller@12340000 { > + compatible = "foo,power-controller"; > + reg = <0x12340000 0x1000>; > + #power-domain-cells = <1>; > + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; > + }; > + > + leaky-device0@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 0>; > + required-opp = <&domain0_opp_0>; > + }; > + > + leaky-device1@12350000 { > + compatible = "foo,i-leak-current"; > + reg = <0x12350000 0x1000>; > + power-domains = <&parent 1>; > + required-opp = <&domain1_opp_1>; > + }; > + > [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt > -- > 2.15.0.194.g9af6a3dea062 > -- 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 2/3] OPP: Introduce "required-opp" property 2017-12-18 10:21 ` Viresh Kumar ` (2 preceding siblings ...) (?) @ 2017-12-22 5:28 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-22 5:28 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael J. Wysocki Cc: linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On 18-12-17, 15:51, Viresh Kumar wrote: > Devices have inter-dependencies some times. For example a device that > needs to run at 800 MHz, needs another device (e.g. Its power domain) to > be configured at a particular operating performance point. > > This patch introduces a new property "required-opp" which can be present > directly in a device's node (if it doesn't need to change its OPPs), or > in device's OPP nodes. More details on the property can be seen in the > binding itself. This is slightly updated based on Rob and Ulf's comments. Will apply it to the OPP tree with below diff. -- viresh -------------------------8<------------------------- Subject: [PATCH] OPP: Introduce "required-opp" property Devices have inter-dependencies some times. For example a device that needs to run at 800 MHz, needs another device (e.g. Its power domain) to be configured at a particular operating performance point. This patch introduces a new property "required-opp" which can be present directly in a device's node (if it doesn't need to change its OPPs), or in device's OPP nodes. More details on the property can be seen in the binding itself. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Changes: - s/opp_table/opp-table for node names. - s/parent/power for domain alias. Documentation/devicetree/bindings/opp/opp.txt | 8 +++ .../devicetree/bindings/power/power_domain.txt | 59 ++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index a3953a1bb1a1..4e4f30288c8b 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -159,6 +159,14 @@ properties. - status: Marks the node enabled/disabled. +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt index 61549840ab3b..f3355313c020 100644 --- a/Documentation/devicetree/bindings/power/power_domain.txt +++ b/Documentation/devicetree/bindings/power/power_domain.txt @@ -126,4 +126,63 @@ The node above defines a typical PM domain consumer device, which is located inside a PM domain with index 0 of a power controller represented by a node with the label "power". +Optional properties: +- required-opp: This contains phandle to an OPP node in another device's OPP + table. It may contain an array of phandles, where each phandle points to an + OPP of a different device. It should not contain multiple phandles to the OPP + nodes in the same OPP table. This specifies the minimum required OPP of the + device(s), whose OPP's phandle is present in this property, for the + functioning of the current device at the current OPP (where this property is + present). + +Example: +- OPP table for domain provider that provides two domains. + + domain0_opp_table: opp-table0 { + compatible = "operating-points-v2"; + + domain0_opp_0: opp-1000000000 { + opp-hz = /bits/ 64 <1000000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain0_opp_1: opp-1100000000 { + opp-hz = /bits/ 64 <1100000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + domain1_opp_table: opp-table1 { + compatible = "operating-points-v2"; + + domain1_opp_0: opp-1200000000 { + opp-hz = /bits/ 64 <1200000000>; + opp-microvolt = <975000 970000 985000>; + }; + domain1_opp_1: opp-1300000000 { + opp-hz = /bits/ 64 <1300000000>; + opp-microvolt = <1000000 980000 1010000>; + }; + }; + + power: power-controller@12340000 { + compatible = "foo,power-controller"; + reg = <0x12340000 0x1000>; + #power-domain-cells = <1>; + operating-points-v2 = <&domain0_opp_table>, <&domain1_opp_table>; + }; + + leaky-device0@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&power 0>; + required-opp = <&domain0_opp_0>; + }; + + leaky-device1@12350000 { + compatible = "foo,i-leak-current"; + reg = <0x12350000 0x1000>; + power-domains = <&power 1>; + required-opp = <&domain1_opp_1>; + }; + [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt ^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-18 10:21 ` Viresh Kumar ` (2 preceding siblings ...) (?) @ 2017-12-18 10:21 ` Viresh Kumar 2017-12-26 20:29 ` Rob Herring -1 siblings, 1 reply; 41+ messages in thread From: Viresh Kumar @ 2017-12-18 10:21 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Viresh Kumar, Nishanth Menon, Stephen Boyd Cc: Viresh Kumar, Rafael Wysocki, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On some platforms the exact frequency or voltage may be hidden from the OS by the firmware. Allow such configurations to pass magic values in the "opp-hz" or the "opp-microvolt" properties, which should be interpreted in a platform dependent way. Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 4e4f30288c8b..00a3bdbd0f1f 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -167,6 +167,12 @@ properties. functioning of the current device at the current OPP (where this property is present). + +On some platforms the exact frequency or voltage may be hidden from the OS by +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain +magic values that represent the frequency or voltage in a firmware dependent +way, for example an index of an array in the firmware. + Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together. / { -- 2.15.0.194.g9af6a3dea062 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-26 20:29 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-26 20:29 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > On some platforms the exact frequency or voltage may be hidden from the > OS by the firmware. Allow such configurations to pass magic values in > the "opp-hz" or the "opp-microvolt" properties, which should be > interpreted in a platform dependent way. > > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index 4e4f30288c8b..00a3bdbd0f1f 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -167,6 +167,12 @@ properties. > functioning of the current device at the current OPP (where this property is > present). > > + > +On some platforms the exact frequency or voltage may be hidden from the OS by > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > +magic values that represent the frequency or voltage in a firmware dependent > +way, for example an index of an array in the firmware. I'm still not convinced this is a good idea. If you have firmware partially managing things, then I think we should have platform specific bindings or drivers. This is complex enough I'm not taking silence from Stephen as an okay. Rob ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-26 20:29 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-26 20:29 UTC (permalink / raw) To: Viresh Kumar Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > On some platforms the exact frequency or voltage may be hidden from the > OS by the firmware. Allow such configurations to pass magic values in > the "opp-hz" or the "opp-microvolt" properties, which should be > interpreted in a platform dependent way. > > Reviewed-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > Documentation/devicetree/bindings/opp/opp.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt > index 4e4f30288c8b..00a3bdbd0f1f 100644 > --- a/Documentation/devicetree/bindings/opp/opp.txt > +++ b/Documentation/devicetree/bindings/opp/opp.txt > @@ -167,6 +167,12 @@ properties. > functioning of the current device at the current OPP (where this property is > present). > > + > +On some platforms the exact frequency or voltage may be hidden from the OS by > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > +magic values that represent the frequency or voltage in a firmware dependent > +way, for example an index of an array in the firmware. I'm still not convinced this is a good idea. If you have firmware partially managing things, then I think we should have platform specific bindings or drivers. This is complex enough I'm not taking silence from Stephen as an okay. 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-26 20:29 ` Rob Herring @ 2017-12-27 8:56 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-27 8:56 UTC (permalink / raw) To: Rob Herring Cc: ulf.hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On 26-12-17, 14:29, Rob Herring wrote: > On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > > +On some platforms the exact frequency or voltage may be hidden from the OS by > > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > > +magic values that represent the frequency or voltage in a firmware dependent > > +way, for example an index of an array in the firmware. > > I'm still not convinced this is a good idea. You were kind-of a few days back :) lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com So here is the deal: - I proposed "domain-performance-state" property for this stuff initially. - But Kevin didn't like that and proposed reusing "opp-hz" and "opp-microvolt", which we all agreed to multiple times.. - And we are back to the same discussion now and its painful and time killing for all of us. TBH, I don't have too strong preferences about any of the suggestions you guys have and I need you guys to tell me what binding changes to do here and I will do that. > If you have firmware > partially managing things, then I think we should have platform specific > bindings or drivers. What about the initial idea then, like "performance-state" for the power domains ? All platforms will anyway replicate that binding only. > This is complex enough I'm not taking silence from Stephen as an okay. Sure, but I am not sure how to make him speak :) -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-27 8:56 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-27 8:56 UTC (permalink / raw) To: Rob Herring Cc: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 26-12-17, 14:29, Rob Herring wrote: > On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > > +On some platforms the exact frequency or voltage may be hidden from the OS by > > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > > +magic values that represent the frequency or voltage in a firmware dependent > > +way, for example an index of an array in the firmware. > > I'm still not convinced this is a good idea. You were kind-of a few days back :) lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org So here is the deal: - I proposed "domain-performance-state" property for this stuff initially. - But Kevin didn't like that and proposed reusing "opp-hz" and "opp-microvolt", which we all agreed to multiple times.. - And we are back to the same discussion now and its painful and time killing for all of us. TBH, I don't have too strong preferences about any of the suggestions you guys have and I need you guys to tell me what binding changes to do here and I will do that. > If you have firmware > partially managing things, then I think we should have platform specific > bindings or drivers. What about the initial idea then, like "performance-state" for the power domains ? All platforms will anyway replicate that binding only. > This is complex enough I'm not taking silence from Stephen as an okay. Sure, but I am not sure how to make him speak :) -- viresh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-27 8:56 ` Viresh Kumar @ 2017-12-27 21:54 ` Rob Herring -1 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-27 21:54 UTC (permalink / raw) To: Viresh Kumar Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 26-12-17, 14:29, Rob Herring wrote: >> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > >> > +On some platforms the exact frequency or voltage may be hidden from the OS by >> > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain >> > +magic values that represent the frequency or voltage in a firmware dependent >> > +way, for example an index of an array in the firmware. >> >> I'm still not convinced this is a good idea. > > You were kind-of a few days back :) > > lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com Yeah, well that was before Stephen said anything. > So here is the deal: > > - I proposed "domain-performance-state" property for this stuff > initially. > - But Kevin didn't like that and proposed reusing "opp-hz" and > "opp-microvolt", which we all agreed to multiple times.. > - And we are back to the same discussion now and its painful and time > killing for all of us. There's bigger issues than where we put magic values as I raised in the other patch. > TBH, I don't have too strong preferences about any of the suggestions > you guys have and I need you guys to tell me what binding changes to > do here and I will do that. > >> If you have firmware >> partially managing things, then I think we should have platform specific >> bindings or drivers. > > What about the initial idea then, like "performance-state" for the > power domains ? All platforms will anyway replicate that binding only. I don't really know. I don't really care either. I'll probably go along with what everyone agrees to, but the only one I see any agreement from is Ulf. Also, it is pretty vague as to what platforms will use this. You claimed you can support QCom scenarios, but there's really no evidence that that is true. What I don't want to see is this merged and then we need something more yet again in a few months for another platform. Rob ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-27 21:54 ` Rob Herring 0 siblings, 0 replies; 41+ messages in thread From: Rob Herring @ 2017-12-27 21:54 UTC (permalink / raw) To: Viresh Kumar Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 26-12-17, 14:29, Rob Herring wrote: >> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > >> > +On some platforms the exact frequency or voltage may be hidden from the OS by >> > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain >> > +magic values that represent the frequency or voltage in a firmware dependent >> > +way, for example an index of an array in the firmware. >> >> I'm still not convinced this is a good idea. > > You were kind-of a few days back :) > > lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com Yeah, well that was before Stephen said anything. > So here is the deal: > > - I proposed "domain-performance-state" property for this stuff > initially. > - But Kevin didn't like that and proposed reusing "opp-hz" and > "opp-microvolt", which we all agreed to multiple times.. > - And we are back to the same discussion now and its painful and time > killing for all of us. There's bigger issues than where we put magic values as I raised in the other patch. > TBH, I don't have too strong preferences about any of the suggestions > you guys have and I need you guys to tell me what binding changes to > do here and I will do that. > >> If you have firmware >> partially managing things, then I think we should have platform specific >> bindings or drivers. > > What about the initial idea then, like "performance-state" for the > power domains ? All platforms will anyway replicate that binding only. I don't really know. I don't really care either. I'll probably go along with what everyone agrees to, but the only one I see any agreement from is Ulf. Also, it is pretty vague as to what platforms will use this. You claimed you can support QCom scenarios, but there's really no evidence that that is true. What I don't want to see is this merged and then we need something more yet again in a few months for another platform. Rob ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-27 21:54 ` Rob Herring @ 2017-12-28 4:37 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-28 4:37 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 27-12-17, 15:54, Rob Herring wrote: > On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 26-12-17, 14:29, Rob Herring wrote: > >> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > > > >> > +On some platforms the exact frequency or voltage may be hidden from the OS by > >> > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > >> > +magic values that represent the frequency or voltage in a firmware dependent > >> > +way, for example an index of an array in the firmware. > >> > >> I'm still not convinced this is a good idea. > > > > You were kind-of a few days back :) > > > > lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com > > Yeah, well that was before Stephen said anything. > > > So here is the deal: > > > > - I proposed "domain-performance-state" property for this stuff > > initially. > > - But Kevin didn't like that and proposed reusing "opp-hz" and > > "opp-microvolt", which we all agreed to multiple times.. > > - And we are back to the same discussion now and its painful and time > > killing for all of us. > > There's bigger issues than where we put magic values as I raised in > the other patch. > > > TBH, I don't have too strong preferences about any of the suggestions > > you guys have and I need you guys to tell me what binding changes to > > do here and I will do that. > > > >> If you have firmware > >> partially managing things, then I think we should have platform specific > >> bindings or drivers. > > > > What about the initial idea then, like "performance-state" for the > > power domains ? All platforms will anyway replicate that binding only. > > I don't really know. I don't really care either. I'll probably go > along with what everyone agrees to, but the only one I see any > agreement from is Ulf. Also, it is pretty vague as to what platforms > will use this. You claimed you can support QCom scenarios, but there's > really no evidence that that is true. Well, I sent out the code few days back based on these bindings and everyone can see how these bindings will get used now. > What I don't want to see is this > merged and then we need something more yet again in a few months for > another platform. Sure, I get your concerns. So what we need now is: - Stephen to start responding and clarify all the doubts he had as being silent isn't helping. - Or Rajendra to post patches which can prove that this is usable. The last time I had a chat with him, he confirmed that he will post patches after 4.15-rc1 and he should have posted them by now, but he didn't :( -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-28 4:37 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-28 4:37 UTC (permalink / raw) To: Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 27-12-17, 15:54, Rob Herring wrote: > On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 26-12-17, 14:29, Rob Herring wrote: > >> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: > > > >> > +On some platforms the exact frequency or voltage may be hidden from the OS by > >> > +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain > >> > +magic values that represent the frequency or voltage in a firmware dependent > >> > +way, for example an index of an array in the firmware. > >> > >> I'm still not convinced this is a good idea. > > > > You were kind-of a few days back :) > > > > lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com > > Yeah, well that was before Stephen said anything. > > > So here is the deal: > > > > - I proposed "domain-performance-state" property for this stuff > > initially. > > - But Kevin didn't like that and proposed reusing "opp-hz" and > > "opp-microvolt", which we all agreed to multiple times.. > > - And we are back to the same discussion now and its painful and time > > killing for all of us. > > There's bigger issues than where we put magic values as I raised in > the other patch. > > > TBH, I don't have too strong preferences about any of the suggestions > > you guys have and I need you guys to tell me what binding changes to > > do here and I will do that. > > > >> If you have firmware > >> partially managing things, then I think we should have platform specific > >> bindings or drivers. > > > > What about the initial idea then, like "performance-state" for the > > power domains ? All platforms will anyway replicate that binding only. > > I don't really know. I don't really care either. I'll probably go > along with what everyone agrees to, but the only one I see any > agreement from is Ulf. Also, it is pretty vague as to what platforms > will use this. You claimed you can support QCom scenarios, but there's > really no evidence that that is true. Well, I sent out the code few days back based on these bindings and everyone can see how these bindings will get used now. > What I don't want to see is this > merged and then we need something more yet again in a few months for > another platform. Sure, I get your concerns. So what we need now is: - Stephen to start responding and clarify all the doubts he had as being silent isn't helping. - Or Rajendra to post patches which can prove that this is usable. The last time I had a chat with him, he confirmed that he will post patches after 4.15-rc1 and he should have posted them by now, but he didn't :( -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-28 4:37 ` Viresh Kumar @ 2017-12-29 0:32 ` Stephen Boyd -1 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2017-12-29 0:32 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 12/28, Viresh Kumar wrote: > On 27-12-17, 15:54, Rob Herring wrote: > > > > I don't really know. I don't really care either. I'll probably go > > along with what everyone agrees to, but the only one I see any > > agreement from is Ulf. Also, it is pretty vague as to what platforms > > will use this. You claimed you can support QCom scenarios, but there's > > really no evidence that that is true. > > Well, I sent out the code few days back based on these bindings and everyone can > see how these bindings will get used now. > > > What I don't want to see is this > > merged and then we need something more yet again in a few months for > > another platform. > > Sure, I get your concerns. > > So what we need now is: > > - Stephen to start responding and clarify all the doubts he had as being silent > isn't helping. What can I reply to specifically? From what I can tell, the patches have changed to this 'opp-required' thing in the past week and the position of 'generic OPP layer interprets magic values' doesn't look to have changed. Is that the summary? I can look deeply at the patches tomorrow. > > - Or Rajendra to post patches which can prove that this is usable. The last time > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > and he should have posted them by now, but he didn't :( > I'd prefer either that, or some tree here that we can look at. I said this last time, I'm having a really hard time seeing how everything is going to work. The little drip of code, then DT binding, then maybe a small change in dts files, then maybe some code that uses the new APIs, etc. is pretty annoying. From my perspective you've chopped the problem up into pieces that don't stand on their own and then started sending patches for some parts without showing the overall result. It's like we're being taken on a ride in your development workflow, and we don't get to see what's coming around the corner, and the only assumption I can make is that you don't know either. I'm actually confused how any of the code is even tested or used. It looks like things are getting merged without any users, for what exactly I'm not sure. Please, please, get an end-to-end solution going and actually use the code from day one on a real device that can use it. Sorry for the rant, but my inbox keeps filling with patches for this series and I have no idea what's going on. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-29 0:32 ` Stephen Boyd 0 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2017-12-29 0:32 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 12/28, Viresh Kumar wrote: > On 27-12-17, 15:54, Rob Herring wrote: > > > > I don't really know. I don't really care either. I'll probably go > > along with what everyone agrees to, but the only one I see any > > agreement from is Ulf. Also, it is pretty vague as to what platforms > > will use this. You claimed you can support QCom scenarios, but there's > > really no evidence that that is true. > > Well, I sent out the code few days back based on these bindings and everyone can > see how these bindings will get used now. > > > What I don't want to see is this > > merged and then we need something more yet again in a few months for > > another platform. > > Sure, I get your concerns. > > So what we need now is: > > - Stephen to start responding and clarify all the doubts he had as being silent > isn't helping. What can I reply to specifically? From what I can tell, the patches have changed to this 'opp-required' thing in the past week and the position of 'generic OPP layer interprets magic values' doesn't look to have changed. Is that the summary? I can look deeply at the patches tomorrow. > > - Or Rajendra to post patches which can prove that this is usable. The last time > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > and he should have posted them by now, but he didn't :( > I'd prefer either that, or some tree here that we can look at. I said this last time, I'm having a really hard time seeing how everything is going to work. The little drip of code, then DT binding, then maybe a small change in dts files, then maybe some code that uses the new APIs, etc. is pretty annoying. From my perspective you've chopped the problem up into pieces that don't stand on their own and then started sending patches for some parts without showing the overall result. It's like we're being taken on a ride in your development workflow, and we don't get to see what's coming around the corner, and the only assumption I can make is that you don't know either. I'm actually confused how any of the code is even tested or used. It looks like things are getting merged without any users, for what exactly I'm not sure. Please, please, get an end-to-end solution going and actually use the code from day one on a real device that can use it. Sorry for the rant, but my inbox keeps filling with patches for this series and I have no idea what's going on. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-29 0:32 ` Stephen Boyd @ 2017-12-29 4:58 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-29 4:58 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 28-12-17, 16:32, Stephen Boyd wrote: > On 12/28, Viresh Kumar wrote: > > So what we need now is: > > > > - Stephen to start responding and clarify all the doubts he had as being silent > > isn't helping. > > What can I reply to specifically? I explained in detail how this stuff is going to get used last time you replied. Did you get a chance to look at that ? What am I supposed to do when you don't reply back to the clarifications I provide ? https://marc.info/?l=linux-kernel&m=151202516128980&w=2 > From what I can tell, the > patches have changed to this 'opp-required' thing in the past > week and the position of 'generic OPP layer interprets magic > values' doesn't look to have changed. Is that the summary? I can > look deeply at the patches tomorrow. Kind of yeah, because you didn't reply to my explanation on how the magic values are going to get used and so they never changed. Again, I don't have problems adding new property for performance-state thing or leave it for the platform, but I was sticking around the magic values because Kevin was strongly in favor of that earlier. > > - Or Rajendra to post patches which can prove that this is usable. The last time > > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > > and he should have posted them by now, but he didn't :( > > > > I'd prefer either that, or some tree here that we can look at. I am quite sure Rajendra can help here, he had been testing this stuff on *real* hardware for ages now with me. This is what he shared with me earlier, based on what we have merged in the kernel today.. https://github.com/rrnayak/linux/commits/genpd-performance-state > I said this last time, I'm having a really hard time seeing how > everything is going to work. The little drip of code, then DT > binding, then maybe a small change in dts files, then maybe some > code that uses the new APIs, etc. is pretty annoying. From my > perspective you've chopped the problem up into pieces that don't > stand on their own and then started sending patches for some > parts without showing the overall result. It's like we're being > taken on a ride in your development workflow, and we don't get to > see what's coming around the corner, and the only assumption I > can make is that you don't know either. > > I'm actually confused how any of the code is even tested or used. > It looks like things are getting merged without any users, for > what exactly I'm not sure. Please, please, get an end-to-end > solution going and actually use the code from day one on a real > device that can use it. There is just too much code, specially Qcom specific, and I can't fit all that in a single series really. Its going to be more annoying for people to see that. I used to keep some Qcom test code in the earlier series which got merged and was told by Rajendra that the Qcom stuff will get posted after 4.15-rc1, but that didn't happen. I can't post final code for that as it touches lots of things and its Qcom who needs to upstream it. Now how much test code can I keep supplying every time ? I have already posted the code that will use these bindings few days back: https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar@linaro.org The only missing part left now (after bindings and above series) is, again, platform specific Qcom code to use it. Below is some dummy code to complete the story for you: DT changes that shows two devices, mmc and dsp, using the performances states of domain: foo: foo-power-domain@09000000 { compatible = "foo,genpd"; #power-domain-cells = <0>; operating-points-v2 = <&domain_opp_table>; }; domain_opp_table: domain_opp_table { compatible = "operating-points-v2"; domain_opp_1: opp00 { opp-hz = /bits/ 64 <1>; //This is performances state }; domain_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; domain_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; mmc@f7112000 { compatible = "***"; *** power-domains = <&foo>; operating-points-v2 = <&mmc_opp_table>; }; mmc_opp_table: mmc-opp-table { compatible = "operating-points-v2"; opp-shared; opp00 { opp-hz = /bits/ 64 <208000000>; required-opp = <&domain_opp_1>; }; opp01 { opp-hz = /bits/ 64 <432000000>; required-opp = <&domain_opp_2>; }; opp02 { opp-hz = /bits/ 64 <729000000>; required-opp = <&domain_opp_2>; }; opp03 { opp-hz = /bits/ 64 <960000000>; required-opp = <&domain_opp_3>; }; }; dsp@f8152000 { compatible = "***"; *** power-domains = <&foo>; required-opp = <&domain_opp_2>; }; Platform specific power domain driver: static int foo_set_performance(struct generic_pm_domain *genpd, unsigned int state) { /* Set the state here */ return 0; } static unsigned int foo_get_performance(struct generic_pm_domain *genpd, struct dev_pm_opp *opp) { /* * Simply return freq value as we passed the state in opp-hz. * * If we choose to use platform-specific bindings instead of opp-hz, * then only this routine requires to change to read the DT and provide * the value from platform specific binding. */ return dev_pm_opp_get_freq(opp); } static const struct of_device_id pm_domain_of_match[] __initconst = { { .compatible = "foo,genpd", }, { }, }; static int __init genpd_foo_init(void) { struct device_node *np; struct generic_pm_domain *pd; for_each_matching_node_and_match(np, pm_domain_of_match, NULL) { pd = kzalloc(sizeof(*pd), GFP_KERNEL); if (!pd) return -ENOMEM; pd->name = kstrdup_const(np->full_name, GFP_KERNEL); if (!pd->name) { of_node_put(np); return -ENOMEM; } ... pd->set_performance_state = foo_set_performance; pd->get_performance_state = foo_get_performance; pm_genpd_init(pd, NULL, false); of_genpd_add_provider_simple(np, pd); } return dev_pm_domain_attach(dev, false); } device_initcall(genpd_foo_init); There is nothing left after this from my end for performance-state stuff. That's all. I don't mind rant from any of you or others (we are all good friends after all), but please please provide feedback. Its going to waste more time if you don't reply :) Hope you have a very happy new year !! -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2017-12-29 4:58 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2017-12-29 4:58 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 28-12-17, 16:32, Stephen Boyd wrote: > On 12/28, Viresh Kumar wrote: > > So what we need now is: > > > > - Stephen to start responding and clarify all the doubts he had as being silent > > isn't helping. > > What can I reply to specifically? I explained in detail how this stuff is going to get used last time you replied. Did you get a chance to look at that ? What am I supposed to do when you don't reply back to the clarifications I provide ? https://marc.info/?l=linux-kernel&m=151202516128980&w=2 > From what I can tell, the > patches have changed to this 'opp-required' thing in the past > week and the position of 'generic OPP layer interprets magic > values' doesn't look to have changed. Is that the summary? I can > look deeply at the patches tomorrow. Kind of yeah, because you didn't reply to my explanation on how the magic values are going to get used and so they never changed. Again, I don't have problems adding new property for performance-state thing or leave it for the platform, but I was sticking around the magic values because Kevin was strongly in favor of that earlier. > > - Or Rajendra to post patches which can prove that this is usable. The last time > > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > > and he should have posted them by now, but he didn't :( > > > > I'd prefer either that, or some tree here that we can look at. I am quite sure Rajendra can help here, he had been testing this stuff on *real* hardware for ages now with me. This is what he shared with me earlier, based on what we have merged in the kernel today.. https://github.com/rrnayak/linux/commits/genpd-performance-state > I said this last time, I'm having a really hard time seeing how > everything is going to work. The little drip of code, then DT > binding, then maybe a small change in dts files, then maybe some > code that uses the new APIs, etc. is pretty annoying. From my > perspective you've chopped the problem up into pieces that don't > stand on their own and then started sending patches for some > parts without showing the overall result. It's like we're being > taken on a ride in your development workflow, and we don't get to > see what's coming around the corner, and the only assumption I > can make is that you don't know either. > > I'm actually confused how any of the code is even tested or used. > It looks like things are getting merged without any users, for > what exactly I'm not sure. Please, please, get an end-to-end > solution going and actually use the code from day one on a real > device that can use it. There is just too much code, specially Qcom specific, and I can't fit all that in a single series really. Its going to be more annoying for people to see that. I used to keep some Qcom test code in the earlier series which got merged and was told by Rajendra that the Qcom stuff will get posted after 4.15-rc1, but that didn't happen. I can't post final code for that as it touches lots of things and its Qcom who needs to upstream it. Now how much test code can I keep supplying every time ? I have already posted the code that will use these bindings few days back: https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar@linaro.org The only missing part left now (after bindings and above series) is, again, platform specific Qcom code to use it. Below is some dummy code to complete the story for you: DT changes that shows two devices, mmc and dsp, using the performances states of domain: foo: foo-power-domain@09000000 { compatible = "foo,genpd"; #power-domain-cells = <0>; operating-points-v2 = <&domain_opp_table>; }; domain_opp_table: domain_opp_table { compatible = "operating-points-v2"; domain_opp_1: opp00 { opp-hz = /bits/ 64 <1>; //This is performances state }; domain_opp_2: opp01 { opp-hz = /bits/ 64 <2>; }; domain_opp_3: opp02 { opp-hz = /bits/ 64 <3>; }; }; mmc@f7112000 { compatible = "***"; *** power-domains = <&foo>; operating-points-v2 = <&mmc_opp_table>; }; mmc_opp_table: mmc-opp-table { compatible = "operating-points-v2"; opp-shared; opp00 { opp-hz = /bits/ 64 <208000000>; required-opp = <&domain_opp_1>; }; opp01 { opp-hz = /bits/ 64 <432000000>; required-opp = <&domain_opp_2>; }; opp02 { opp-hz = /bits/ 64 <729000000>; required-opp = <&domain_opp_2>; }; opp03 { opp-hz = /bits/ 64 <960000000>; required-opp = <&domain_opp_3>; }; }; dsp@f8152000 { compatible = "***"; *** power-domains = <&foo>; required-opp = <&domain_opp_2>; }; Platform specific power domain driver: static int foo_set_performance(struct generic_pm_domain *genpd, unsigned int state) { /* Set the state here */ return 0; } static unsigned int foo_get_performance(struct generic_pm_domain *genpd, struct dev_pm_opp *opp) { /* * Simply return freq value as we passed the state in opp-hz. * * If we choose to use platform-specific bindings instead of opp-hz, * then only this routine requires to change to read the DT and provide * the value from platform specific binding. */ return dev_pm_opp_get_freq(opp); } static const struct of_device_id pm_domain_of_match[] __initconst = { { .compatible = "foo,genpd", }, { }, }; static int __init genpd_foo_init(void) { struct device_node *np; struct generic_pm_domain *pd; for_each_matching_node_and_match(np, pm_domain_of_match, NULL) { pd = kzalloc(sizeof(*pd), GFP_KERNEL); if (!pd) return -ENOMEM; pd->name = kstrdup_const(np->full_name, GFP_KERNEL); if (!pd->name) { of_node_put(np); return -ENOMEM; } ... pd->set_performance_state = foo_set_performance; pd->get_performance_state = foo_get_performance; pm_genpd_init(pd, NULL, false); of_genpd_add_provider_simple(np, pd); } return dev_pm_domain_attach(dev, false); } device_initcall(genpd_foo_init); There is nothing left after this from my end for performance-state stuff. That's all. I don't mind rant from any of you or others (we are all good friends after all), but please please provide feedback. Its going to waste more time if you don't reply :) Hope you have a very happy new year !! -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-29 4:58 ` Viresh Kumar @ 2018-01-05 22:19 ` Stephen Boyd -1 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-05 22:19 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 12/29, Viresh Kumar wrote: > On 28-12-17, 16:32, Stephen Boyd wrote: > > On 12/28, Viresh Kumar wrote: > > > > So what we need now is: > > > > > > - Stephen to start responding and clarify all the doubts he had as being silent > > > isn't helping. > > > > What can I reply to specifically? > > I explained in detail how this stuff is going to get used last time you replied. > Did you get a chance to look at that ? What am I supposed to do when you don't > reply back to the clarifications I provide ? > > https://marc.info/?l=linux-kernel&m=151202516128980&w=2 > > > From what I can tell, the > > patches have changed to this 'opp-required' thing in the past > > week and the position of 'generic OPP layer interprets magic > > values' doesn't look to have changed. Is that the summary? I can > > look deeply at the patches tomorrow. > > Kind of yeah, because you didn't reply to my explanation on how the magic values > are going to get used and so they never changed. Again, I don't have problems > adding new property for performance-state thing or leave it for the platform, > but I was sticking around the magic values because Kevin was strongly in favor > of that earlier. Could you please point to Kevin's comments and also include the reasoning behind magic values in the commit text for the patch? It would be very helpful to know why something is done a certain way. The two to three line commit text in this patch is not helpful right now. > > > > - Or Rajendra to post patches which can prove that this is usable. The last time > > > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > > > and he should have posted them by now, but he didn't :( > > > > > > > I'd prefer either that, or some tree here that we can look at. > > I am quite sure Rajendra can help here, he had been testing this stuff on *real* > hardware for ages now with me. This is what he shared with me earlier, based on > what we have merged in the kernel today.. > > https://github.com/rrnayak/linux/commits/genpd-performance-state Thanks for the pointer. That whole matching devices with of_match_device() is not looking good. I don't see how we're going to convince each driver to move to using the OPP framework to set a performance state + clk frequency when they're going to want/need to have certain clks and regulators in hand to do something besides set the frequency or voltage. Probably we're going to have a hybrid approach, where some drivers can just set rate and voltage through OPP because it's fairly well fixed (think CPU or GPU frequency scaling), while other drivers are going to set frequency and voltage/performance state based on some calculation of their required frequency (think of display panels or even the uart baud rate or i2c bus frequency requirements). For these other drivers, I don't really want to see the OPP framework proxy all the clk and regulator calls into the appropriate framework by wrapping clk_round_rate(), regulator_set_load(), etc. with opp_*() APIs. This becomes especially annoying if OPP framework is grabbing and holding these clk and regulator references in the core, instead of letting the driver figure that out and tell the OPP framework what resources it should operate on. So really, I'm hoping that OPP framework "stays away" and acts as a data hole, sometimes, where we can look up the performance state for a particular frequency, but also have it do everything to set some particular performance state and frequency, etc. if the user wants that. And also OPP framework hopefully doesn't force a rigid set of frequencies that a particular clk can be set to, so that we can calculate frequencies for things like display panels based on height and width of the panel, or uart baud rate which is entirely use-case driven, and then ask OPP framework to tell us what the performance state of a particular domain would be if we are within some frequency range. Because sometimes we really can't determine every possible frequency that a clk can be running at, but we can figure out the maximum frequency that a clk can be running at for a particular voltage/performance state to support it. > > > I said this last time, I'm having a really hard time seeing how > > everything is going to work. The little drip of code, then DT > > binding, then maybe a small change in dts files, then maybe some > > code that uses the new APIs, etc. is pretty annoying. From my > > perspective you've chopped the problem up into pieces that don't > > stand on their own and then started sending patches for some > > parts without showing the overall result. It's like we're being > > taken on a ride in your development workflow, and we don't get to > > see what's coming around the corner, and the only assumption I > > can make is that you don't know either. > > > > I'm actually confused how any of the code is even tested or used. > > It looks like things are getting merged without any users, for > > what exactly I'm not sure. Please, please, get an end-to-end > > solution going and actually use the code from day one on a real > > device that can use it. > > There is just too much code, specially Qcom specific, and I can't fit all that > in a single series really. Its going to be more annoying for people to see that. > I used to keep some Qcom test code in the earlier series which got merged and > was told by Rajendra that the Qcom stuff will get posted after 4.15-rc1, but > that didn't happen. I can't post final code for that as it touches lots of > things and its Qcom who needs to upstream it. Now how much test code can I keep > supplying every time ? I'm not asking for test code. A git tree pointer in cover letter is sufficient, with the full and complete solution to the problem. Then only a few patches out of the tree sent to the list is fine if you want to chunk it up into sub-topics. That way the list and reviewers aren't overloaded, but if someone wants to review the full picture they can do that easily. > > I have already posted the code that will use these bindings few days back: > > https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar@linaro.org > > The only missing part left now (after bindings and above series) is, again, > platform specific Qcom code to use it. Below is some dummy code to complete the > story for you: > > DT changes that shows two devices, mmc and dsp, using the performances states of > domain: > > foo: foo-power-domain@09000000 { > compatible = "foo,genpd"; > #power-domain-cells = <0>; > operating-points-v2 = <&domain_opp_table>; > }; > > domain_opp_table: domain_opp_table { > compatible = "operating-points-v2"; > > domain_opp_1: opp00 { > opp-hz = /bits/ 64 <1>; //This is performances state > }; > domain_opp_2: opp01 { > opp-hz = /bits/ 64 <2>; > }; > domain_opp_3: opp02 { > opp-hz = /bits/ 64 <3>; > }; > }; > > mmc@f7112000 { > compatible = "***"; > > *** > > > power-domains = <&foo>; > operating-points-v2 = <&mmc_opp_table>; > }; > > > mmc_opp_table: mmc-opp-table { > compatible = "operating-points-v2"; > opp-shared; > > opp00 { > opp-hz = /bits/ 64 <208000000>; > required-opp = <&domain_opp_1>; It can have multiple phandles though, right? Makes me think it should have been called 'required-opps' instead. > }; > opp01 { > opp-hz = /bits/ 64 <432000000>; > required-opp = <&domain_opp_2>; > }; > opp02 { > opp-hz = /bits/ 64 <729000000>; > required-opp = <&domain_opp_2>; > }; > opp03 { > opp-hz = /bits/ 64 <960000000>; > required-opp = <&domain_opp_3>; > }; > }; > > dsp@f8152000 { > compatible = "***"; > > *** > > > power-domains = <&foo>; > required-opp = <&domain_opp_2>; > }; > > > > > Platform specific power domain driver: > > static int foo_set_performance(struct generic_pm_domain *genpd, > unsigned int state) > { > /* Set the state here */ > > return 0; > } > > static unsigned int foo_get_performance(struct generic_pm_domain *genpd, > struct dev_pm_opp *opp) > { > /* > * Simply return freq value as we passed the state in opp-hz. > * > * If we choose to use platform-specific bindings instead of opp-hz, > * then only this routine requires to change to read the DT and provide > * the value from platform specific binding. If we wanted to change this function to do a platform specific thing, will we somehow get a way to access the DT node of the opp passed into this function? I don't see another way to do it. Also, I don't see how the foo_get_performance function will use the genpd pointer passed here. Maybe that's never used? Finally, I would think that a "getter" like this function would be informing the framework about what the current performance state is, not translating an OPP into a performance state. So the whole thing looks like a misnomer, and probably should be called something like xlate_opp_performance. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-05 22:19 ` Stephen Boyd 0 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-05 22:19 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 12/29, Viresh Kumar wrote: > On 28-12-17, 16:32, Stephen Boyd wrote: > > On 12/28, Viresh Kumar wrote: > > > > So what we need now is: > > > > > > - Stephen to start responding and clarify all the doubts he had as being silent > > > isn't helping. > > > > What can I reply to specifically? > > I explained in detail how this stuff is going to get used last time you replied. > Did you get a chance to look at that ? What am I supposed to do when you don't > reply back to the clarifications I provide ? > > https://marc.info/?l=linux-kernel&m=151202516128980&w=2 > > > From what I can tell, the > > patches have changed to this 'opp-required' thing in the past > > week and the position of 'generic OPP layer interprets magic > > values' doesn't look to have changed. Is that the summary? I can > > look deeply at the patches tomorrow. > > Kind of yeah, because you didn't reply to my explanation on how the magic values > are going to get used and so they never changed. Again, I don't have problems > adding new property for performance-state thing or leave it for the platform, > but I was sticking around the magic values because Kevin was strongly in favor > of that earlier. Could you please point to Kevin's comments and also include the reasoning behind magic values in the commit text for the patch? It would be very helpful to know why something is done a certain way. The two to three line commit text in this patch is not helpful right now. > > > > - Or Rajendra to post patches which can prove that this is usable. The last time > > > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > > > and he should have posted them by now, but he didn't :( > > > > > > > I'd prefer either that, or some tree here that we can look at. > > I am quite sure Rajendra can help here, he had been testing this stuff on *real* > hardware for ages now with me. This is what he shared with me earlier, based on > what we have merged in the kernel today.. > > https://github.com/rrnayak/linux/commits/genpd-performance-state Thanks for the pointer. That whole matching devices with of_match_device() is not looking good. I don't see how we're going to convince each driver to move to using the OPP framework to set a performance state + clk frequency when they're going to want/need to have certain clks and regulators in hand to do something besides set the frequency or voltage. Probably we're going to have a hybrid approach, where some drivers can just set rate and voltage through OPP because it's fairly well fixed (think CPU or GPU frequency scaling), while other drivers are going to set frequency and voltage/performance state based on some calculation of their required frequency (think of display panels or even the uart baud rate or i2c bus frequency requirements). For these other drivers, I don't really want to see the OPP framework proxy all the clk and regulator calls into the appropriate framework by wrapping clk_round_rate(), regulator_set_load(), etc. with opp_*() APIs. This becomes especially annoying if OPP framework is grabbing and holding these clk and regulator references in the core, instead of letting the driver figure that out and tell the OPP framework what resources it should operate on. So really, I'm hoping that OPP framework "stays away" and acts as a data hole, sometimes, where we can look up the performance state for a particular frequency, but also have it do everything to set some particular performance state and frequency, etc. if the user wants that. And also OPP framework hopefully doesn't force a rigid set of frequencies that a particular clk can be set to, so that we can calculate frequencies for things like display panels based on height and width of the panel, or uart baud rate which is entirely use-case driven, and then ask OPP framework to tell us what the performance state of a particular domain would be if we are within some frequency range. Because sometimes we really can't determine every possible frequency that a clk can be running at, but we can figure out the maximum frequency that a clk can be running at for a particular voltage/performance state to support it. > > > I said this last time, I'm having a really hard time seeing how > > everything is going to work. The little drip of code, then DT > > binding, then maybe a small change in dts files, then maybe some > > code that uses the new APIs, etc. is pretty annoying. From my > > perspective you've chopped the problem up into pieces that don't > > stand on their own and then started sending patches for some > > parts without showing the overall result. It's like we're being > > taken on a ride in your development workflow, and we don't get to > > see what's coming around the corner, and the only assumption I > > can make is that you don't know either. > > > > I'm actually confused how any of the code is even tested or used. > > It looks like things are getting merged without any users, for > > what exactly I'm not sure. Please, please, get an end-to-end > > solution going and actually use the code from day one on a real > > device that can use it. > > There is just too much code, specially Qcom specific, and I can't fit all that > in a single series really. Its going to be more annoying for people to see that. > I used to keep some Qcom test code in the earlier series which got merged and > was told by Rajendra that the Qcom stuff will get posted after 4.15-rc1, but > that didn't happen. I can't post final code for that as it touches lots of > things and its Qcom who needs to upstream it. Now how much test code can I keep > supplying every time ? I'm not asking for test code. A git tree pointer in cover letter is sufficient, with the full and complete solution to the problem. Then only a few patches out of the tree sent to the list is fine if you want to chunk it up into sub-topics. That way the list and reviewers aren't overloaded, but if someone wants to review the full picture they can do that easily. > > I have already posted the code that will use these bindings few days back: > > https://lkml.kernel.org/r/cover.1513926033.git.viresh.kumar@linaro.org > > The only missing part left now (after bindings and above series) is, again, > platform specific Qcom code to use it. Below is some dummy code to complete the > story for you: > > DT changes that shows two devices, mmc and dsp, using the performances states of > domain: > > foo: foo-power-domain@09000000 { > compatible = "foo,genpd"; > #power-domain-cells = <0>; > operating-points-v2 = <&domain_opp_table>; > }; > > domain_opp_table: domain_opp_table { > compatible = "operating-points-v2"; > > domain_opp_1: opp00 { > opp-hz = /bits/ 64 <1>; //This is performances state > }; > domain_opp_2: opp01 { > opp-hz = /bits/ 64 <2>; > }; > domain_opp_3: opp02 { > opp-hz = /bits/ 64 <3>; > }; > }; > > mmc@f7112000 { > compatible = "***"; > > *** > > > power-domains = <&foo>; > operating-points-v2 = <&mmc_opp_table>; > }; > > > mmc_opp_table: mmc-opp-table { > compatible = "operating-points-v2"; > opp-shared; > > opp00 { > opp-hz = /bits/ 64 <208000000>; > required-opp = <&domain_opp_1>; It can have multiple phandles though, right? Makes me think it should have been called 'required-opps' instead. > }; > opp01 { > opp-hz = /bits/ 64 <432000000>; > required-opp = <&domain_opp_2>; > }; > opp02 { > opp-hz = /bits/ 64 <729000000>; > required-opp = <&domain_opp_2>; > }; > opp03 { > opp-hz = /bits/ 64 <960000000>; > required-opp = <&domain_opp_3>; > }; > }; > > dsp@f8152000 { > compatible = "***"; > > *** > > > power-domains = <&foo>; > required-opp = <&domain_opp_2>; > }; > > > > > Platform specific power domain driver: > > static int foo_set_performance(struct generic_pm_domain *genpd, > unsigned int state) > { > /* Set the state here */ > > return 0; > } > > static unsigned int foo_get_performance(struct generic_pm_domain *genpd, > struct dev_pm_opp *opp) > { > /* > * Simply return freq value as we passed the state in opp-hz. > * > * If we choose to use platform-specific bindings instead of opp-hz, > * then only this routine requires to change to read the DT and provide > * the value from platform specific binding. If we wanted to change this function to do a platform specific thing, will we somehow get a way to access the DT node of the opp passed into this function? I don't see another way to do it. Also, I don't see how the foo_get_performance function will use the genpd pointer passed here. Maybe that's never used? Finally, I would think that a "getter" like this function would be informing the framework about what the current performance state is, not translating an OPP into a performance state. So the whole thing looks like a misnomer, and probably should be called something like xlate_opp_performance. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2018-01-05 22:19 ` Stephen Boyd @ 2018-01-08 4:16 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-08 4:16 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 05-01-18, 14:19, Stephen Boyd wrote: > On 12/29, Viresh Kumar wrote: > Could you please point to Kevin's comments and also include the https://lkml.kernel.org/r/m2r30i4w35.fsf@baylibre.com > reasoning behind magic values in the commit text for the patch? > It would be very helpful to know why something is done a certain > way. The two to three line commit text in this patch is not > helpful right now. Sure. > > https://github.com/rrnayak/linux/commits/genpd-performance-state > > Thanks for the pointer. That whole matching devices with > of_match_device() is not looking good. Yeah, I agree, but that was done because we didn't had these bindings then. Once these bindings are in place, we wouldn't require the of_match_device() thingy. > I don't see how we're going to convince each driver to move to > using the OPP framework to set a performance state + clk > frequency when they're going to want/need to have certain clks > and regulators in hand to do something besides set the frequency > or voltage. Probably we're going to have a hybrid approach, where > some drivers can just set rate and voltage through OPP because > it's fairly well fixed (think CPU or GPU frequency scaling), > while other drivers are going to set frequency and > voltage/performance state based on some calculation of their > required frequency (think of display panels or even the uart baud > rate or i2c bus frequency requirements). The OPP framework doesn't (and shouldn't) force drivers to move to the dev_pm_opp_set_rate() API. That is optional and is provided to make user code simpler. So, if a driver wants to handle everything by itself, it will just use the OPP core as "provider" for the data it needs and do everything by itself. > For these other drivers, I don't really want to see the OPP > framework proxy all the clk and regulator calls into the > appropriate framework by wrapping clk_round_rate(), > regulator_set_load(), etc. with opp_*() APIs. This becomes > especially annoying if OPP framework is grabbing and holding > these clk and regulator references in the core, instead of > letting the driver figure that out and tell the OPP framework > what resources it should operate on. Sure. > So really, I'm hoping that OPP framework "stays away" and acts as > a data hole, sometimes, where we can look up the performance > state for a particular frequency, but also have it do everything > to set some particular performance state and frequency, etc. if > the user wants that. That's how I see it as well. > And also OPP framework hopefully doesn't > force a rigid set of frequencies that a particular clk can be set > to, so that we can calculate frequencies for things like display > panels based on height and width of the panel, or uart baud rate > which is entirely use-case driven, and then ask OPP framework to > tell us what the performance state of a particular domain would > be if we are within some frequency range. Because sometimes we > really can't determine every possible frequency that a clk can be > running at, but we can figure out the maximum frequency that a > clk can be running at for a particular voltage/performance state > to support it. That makes sense to me. We can do that once we have someone who wants to use OPP core for such devices. > I'm not asking for test code. A git tree pointer in cover letter > is sufficient, with the full and complete solution to the > problem. Then only a few patches out of the tree sent to the list > is fine if you want to chunk it up into sub-topics. That way the > list and reviewers aren't overloaded, but if someone wants to > review the full picture they can do that easily. That's fine. > > mmc_opp_table: mmc-opp-table { > > compatible = "operating-points-v2"; > > opp-shared; > > > > opp00 { > > opp-hz = /bits/ 64 <208000000>; > > required-opp = <&domain_opp_1>; > > It can have multiple phandles though, right? Makes me think it > should have been called 'required-opps' instead. Okay. > > static unsigned int foo_get_performance(struct generic_pm_domain *genpd, > > struct dev_pm_opp *opp) > > { > > /* > > * Simply return freq value as we passed the state in opp-hz. > > * > > * If we choose to use platform-specific bindings instead of opp-hz, > > * then only this routine requires to change to read the DT and provide > > * the value from platform specific binding. > > If we wanted to change this function to do a platform specific > thing, will we somehow get a way to access the DT node of the opp > passed into this function? Yes, we can do that. The OPP core already stores pointer to the node in the OPP structure, we will just need another API to expose that. > Also, I don't see how the foo_get_performance function will use > the genpd pointer passed here. Maybe that's never used? It depends actually and I think its better to pass it anyway. What if a single driver is handling multiple genpds and wants to do things differently for them? It should know which genpd it is called for, looks like basic requirement to me. > Finally, I would think that a "getter" like this function would > be informing the framework about what the current performance > state is, not translating an OPP into a performance state. So the > whole thing looks like a misnomer, and probably should be called > something like xlate_opp_performance. Sure, I can name it anything. So the question still stands, are we all okay for using magic values or we want platform specific properties ? -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-08 4:16 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-08 4:16 UTC (permalink / raw) To: Stephen Boyd Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 05-01-18, 14:19, Stephen Boyd wrote: > On 12/29, Viresh Kumar wrote: > Could you please point to Kevin's comments and also include the https://lkml.kernel.org/r/m2r30i4w35.fsf@baylibre.com > reasoning behind magic values in the commit text for the patch? > It would be very helpful to know why something is done a certain > way. The two to three line commit text in this patch is not > helpful right now. Sure. > > https://github.com/rrnayak/linux/commits/genpd-performance-state > > Thanks for the pointer. That whole matching devices with > of_match_device() is not looking good. Yeah, I agree, but that was done because we didn't had these bindings then. Once these bindings are in place, we wouldn't require the of_match_device() thingy. > I don't see how we're going to convince each driver to move to > using the OPP framework to set a performance state + clk > frequency when they're going to want/need to have certain clks > and regulators in hand to do something besides set the frequency > or voltage. Probably we're going to have a hybrid approach, where > some drivers can just set rate and voltage through OPP because > it's fairly well fixed (think CPU or GPU frequency scaling), > while other drivers are going to set frequency and > voltage/performance state based on some calculation of their > required frequency (think of display panels or even the uart baud > rate or i2c bus frequency requirements). The OPP framework doesn't (and shouldn't) force drivers to move to the dev_pm_opp_set_rate() API. That is optional and is provided to make user code simpler. So, if a driver wants to handle everything by itself, it will just use the OPP core as "provider" for the data it needs and do everything by itself. > For these other drivers, I don't really want to see the OPP > framework proxy all the clk and regulator calls into the > appropriate framework by wrapping clk_round_rate(), > regulator_set_load(), etc. with opp_*() APIs. This becomes > especially annoying if OPP framework is grabbing and holding > these clk and regulator references in the core, instead of > letting the driver figure that out and tell the OPP framework > what resources it should operate on. Sure. > So really, I'm hoping that OPP framework "stays away" and acts as > a data hole, sometimes, where we can look up the performance > state for a particular frequency, but also have it do everything > to set some particular performance state and frequency, etc. if > the user wants that. That's how I see it as well. > And also OPP framework hopefully doesn't > force a rigid set of frequencies that a particular clk can be set > to, so that we can calculate frequencies for things like display > panels based on height and width of the panel, or uart baud rate > which is entirely use-case driven, and then ask OPP framework to > tell us what the performance state of a particular domain would > be if we are within some frequency range. Because sometimes we > really can't determine every possible frequency that a clk can be > running at, but we can figure out the maximum frequency that a > clk can be running at for a particular voltage/performance state > to support it. That makes sense to me. We can do that once we have someone who wants to use OPP core for such devices. > I'm not asking for test code. A git tree pointer in cover letter > is sufficient, with the full and complete solution to the > problem. Then only a few patches out of the tree sent to the list > is fine if you want to chunk it up into sub-topics. That way the > list and reviewers aren't overloaded, but if someone wants to > review the full picture they can do that easily. That's fine. > > mmc_opp_table: mmc-opp-table { > > compatible = "operating-points-v2"; > > opp-shared; > > > > opp00 { > > opp-hz = /bits/ 64 <208000000>; > > required-opp = <&domain_opp_1>; > > It can have multiple phandles though, right? Makes me think it > should have been called 'required-opps' instead. Okay. > > static unsigned int foo_get_performance(struct generic_pm_domain *genpd, > > struct dev_pm_opp *opp) > > { > > /* > > * Simply return freq value as we passed the state in opp-hz. > > * > > * If we choose to use platform-specific bindings instead of opp-hz, > > * then only this routine requires to change to read the DT and provide > > * the value from platform specific binding. > > If we wanted to change this function to do a platform specific > thing, will we somehow get a way to access the DT node of the opp > passed into this function? Yes, we can do that. The OPP core already stores pointer to the node in the OPP structure, we will just need another API to expose that. > Also, I don't see how the foo_get_performance function will use > the genpd pointer passed here. Maybe that's never used? It depends actually and I think its better to pass it anyway. What if a single driver is handling multiple genpds and wants to do things differently for them? It should know which genpd it is called for, looks like basic requirement to me. > Finally, I would think that a "getter" like this function would > be informing the framework about what the current performance > state is, not translating an OPP into a performance state. So the > whole thing looks like a misnomer, and probably should be called > something like xlate_opp_performance. Sure, I can name it anything. So the question still stands, are we all okay for using magic values or we want platform specific properties ? -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2018-01-08 4:16 ` Viresh Kumar @ 2018-01-10 2:54 ` Stephen Boyd -1 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-10 2:54 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 01/08, Viresh Kumar wrote: > On 05-01-18, 14:19, Stephen Boyd wrote: > > On 12/29, Viresh Kumar wrote: > > > Could you please point to Kevin's comments and also include the > > https://lkml.kernel.org/r/m2r30i4w35.fsf@baylibre.com Ok. That thread was months ago. Shocked I can't remember! My read of Kevin's comments lead me to think he's saying that a generic 'domain-performance-state' property is worse than putting the numbers directly inside of the opp table with a comment above it. Now that's all fine, but now that we have required-opps binding we sort of have the domain-performance-state property again, but it's a phandle instead of a raw state number. So we have required-opps = <&perf_state>; but what was proposed before was domain-performance-state = <1>; or Kevin's opp-table = <100000 1>; >From what I can tell, the domain-performance-state proposal was at the same abstraction level as the OPP itself, but now that's moved to be inside of the power domain OPPs. So now we're talking about describing some power domains performance levels with the OPP binding, for a power domain, not describing a performance state for some unknown power domain that's associated with a device's OPP table. The power domain is not running at any sort of frequency for us. It's using some particular voltage, but we may or may not know what that voltage is depending on the platform. This makes me see it as power_domain_opp { low: point0 { opp-microvolt = <950000> }; medium: point1 { opp-microvolt = <975000> }; high: point2 { opp-microvolt = <1000000> }; }; for some sort of power domain that deals with voltages. This makes perfect sense. The power domain needs to be at some voltage and the binding says that. Honestly, the whole required-opps thing works great here and it's all wonderful, and it looks like can even be used for other OPP linking in the future, like connecting frequencies between CPUs and caches. Where it starts to break down is when the voltage isn't known to the user, just some number that we've agreed means "low", "medium", "high" or whatever. Again, the binding looks similar: power_domain_opp { low: point0 { qcom,corner = <0>; }; medium: point1 { qcom,corner = <10>; }; high: point2 { qcom,corner = <250>; }; }; but now the number is only meaningful to the power domain driver. What we really care about is associating some firmware specific information via the phandle to some frequency that a device is using. Behind the scenes of the firmware, that number is really being translated into some voltage, like opp-microvolt = <950000>, but we don't know what that is and it could vary at runtime. It all feels really close, and it totally works for the non-magic value parts that we have to deal with, but I'm not convinced that we should stick the firmware specific information into some generic OPP property just so we don't have to review the binding again. Hopefully there's some other reason why we shouldn't come up with a firmware specific binding for this piece of information. > > > * If we choose to use platform-specific bindings instead of opp-hz, > > > * then only this routine requires to change to read the DT and provide > > > * the value from platform specific binding. > > > > If we wanted to change this function to do a platform specific > > thing, will we somehow get a way to access the DT node of the opp > > passed into this function? > > Yes, we can do that. The OPP core already stores pointer to the node > in the OPP structure, we will just need another API to expose that. Great! > > > Also, I don't see how the foo_get_performance function will use > > the genpd pointer passed here. Maybe that's never used? > > It depends actually and I think its better to pass it anyway. What if > a single driver is handling multiple genpds and wants to do things > differently for them? It should know which genpd it is called for, > looks like basic requirement to me. Ok, fair enough. > > > Finally, I would think that a "getter" like this function would > > be informing the framework about what the current performance > > state is, not translating an OPP into a performance state. So the > > whole thing looks like a misnomer, and probably should be called > > something like xlate_opp_performance. > > Sure, I can name it anything. Alright. Sounds like my read of the code is correct. > > So the question still stands, are we all okay for using magic values > or we want platform specific properties ? > Kevin? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-10 2:54 ` Stephen Boyd 0 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-10 2:54 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 01/08, Viresh Kumar wrote: > On 05-01-18, 14:19, Stephen Boyd wrote: > > On 12/29, Viresh Kumar wrote: > > > Could you please point to Kevin's comments and also include the > > https://lkml.kernel.org/r/m2r30i4w35.fsf@baylibre.com Ok. That thread was months ago. Shocked I can't remember! My read of Kevin's comments lead me to think he's saying that a generic 'domain-performance-state' property is worse than putting the numbers directly inside of the opp table with a comment above it. Now that's all fine, but now that we have required-opps binding we sort of have the domain-performance-state property again, but it's a phandle instead of a raw state number. So we have required-opps = <&perf_state>; but what was proposed before was domain-performance-state = <1>; or Kevin's opp-table = <100000 1>; >From what I can tell, the domain-performance-state proposal was at the same abstraction level as the OPP itself, but now that's moved to be inside of the power domain OPPs. So now we're talking about describing some power domains performance levels with the OPP binding, for a power domain, not describing a performance state for some unknown power domain that's associated with a device's OPP table. The power domain is not running at any sort of frequency for us. It's using some particular voltage, but we may or may not know what that voltage is depending on the platform. This makes me see it as power_domain_opp { low: point0 { opp-microvolt = <950000> }; medium: point1 { opp-microvolt = <975000> }; high: point2 { opp-microvolt = <1000000> }; }; for some sort of power domain that deals with voltages. This makes perfect sense. The power domain needs to be at some voltage and the binding says that. Honestly, the whole required-opps thing works great here and it's all wonderful, and it looks like can even be used for other OPP linking in the future, like connecting frequencies between CPUs and caches. Where it starts to break down is when the voltage isn't known to the user, just some number that we've agreed means "low", "medium", "high" or whatever. Again, the binding looks similar: power_domain_opp { low: point0 { qcom,corner = <0>; }; medium: point1 { qcom,corner = <10>; }; high: point2 { qcom,corner = <250>; }; }; but now the number is only meaningful to the power domain driver. What we really care about is associating some firmware specific information via the phandle to some frequency that a device is using. Behind the scenes of the firmware, that number is really being translated into some voltage, like opp-microvolt = <950000>, but we don't know what that is and it could vary at runtime. It all feels really close, and it totally works for the non-magic value parts that we have to deal with, but I'm not convinced that we should stick the firmware specific information into some generic OPP property just so we don't have to review the binding again. Hopefully there's some other reason why we shouldn't come up with a firmware specific binding for this piece of information. > > > * If we choose to use platform-specific bindings instead of opp-hz, > > > * then only this routine requires to change to read the DT and provide > > > * the value from platform specific binding. > > > > If we wanted to change this function to do a platform specific > > thing, will we somehow get a way to access the DT node of the opp > > passed into this function? > > Yes, we can do that. The OPP core already stores pointer to the node > in the OPP structure, we will just need another API to expose that. Great! > > > Also, I don't see how the foo_get_performance function will use > > the genpd pointer passed here. Maybe that's never used? > > It depends actually and I think its better to pass it anyway. What if > a single driver is handling multiple genpds and wants to do things > differently for them? It should know which genpd it is called for, > looks like basic requirement to me. Ok, fair enough. > > > Finally, I would think that a "getter" like this function would > > be informing the framework about what the current performance > > state is, not translating an OPP into a performance state. So the > > whole thing looks like a misnomer, and probably should be called > > something like xlate_opp_performance. > > Sure, I can name it anything. Alright. Sounds like my read of the code is correct. > > So the question still stands, are we all okay for using magic values > or we want platform specific properties ? > Kevin? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2018-01-10 2:54 ` Stephen Boyd @ 2018-01-10 5:37 ` Viresh Kumar -1 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-10 5:37 UTC (permalink / raw) To: Rob Herring, Stephen Boyd Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 09-01-18, 18:54, Stephen Boyd wrote: > My read of Kevin's comments lead me to think he's saying that a > generic 'domain-performance-state' property is worse than putting > the numbers directly inside of the opp table with a comment above > it. Now that's all fine, but now that we have required-opps > binding we sort of have the domain-performance-state property > again, but it's a phandle instead of a raw state number. > > So we have > > required-opps = <&perf_state>; > > but what was proposed before was > > domain-performance-state = <1>; > > or Kevin's > > opp-table = <100000 1>; His concern was also on what will we do if "frequency" or other OPP properties aren't known tomorrow by the kernel but the firmware? In Qcom case, its just the voltage (corner) today, but it can very well be other properties tomorrow. Are we going to add more platform specific bindings then ? And this is the main reason why I have been aligned towards using something like this patch. If we drop the magic-values idea and hence this patch, then we can either add a "domain-performance-state" property, which will only be used by the power domains or leave it for the platforms to add something like "qcom,corner". All we are doing here is putting a voltage (corner) value, unknown to the kernel, in a new property instead of "opp-microvolt". But the above question still remains, what about other properties that may need magic values in future. Honestly speaking, I am not sure what's the right thing to do here. I will do whatever you and Rob incline for. -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-10 5:37 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-10 5:37 UTC (permalink / raw) To: Rob Herring, Stephen Boyd Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 09-01-18, 18:54, Stephen Boyd wrote: > My read of Kevin's comments lead me to think he's saying that a > generic 'domain-performance-state' property is worse than putting > the numbers directly inside of the opp table with a comment above > it. Now that's all fine, but now that we have required-opps > binding we sort of have the domain-performance-state property > again, but it's a phandle instead of a raw state number. > > So we have > > required-opps = <&perf_state>; > > but what was proposed before was > > domain-performance-state = <1>; > > or Kevin's > > opp-table = <100000 1>; His concern was also on what will we do if "frequency" or other OPP properties aren't known tomorrow by the kernel but the firmware? In Qcom case, its just the voltage (corner) today, but it can very well be other properties tomorrow. Are we going to add more platform specific bindings then ? And this is the main reason why I have been aligned towards using something like this patch. If we drop the magic-values idea and hence this patch, then we can either add a "domain-performance-state" property, which will only be used by the power domains or leave it for the platforms to add something like "qcom,corner". All we are doing here is putting a voltage (corner) value, unknown to the kernel, in a new property instead of "opp-microvolt". But the above question still remains, what about other properties that may need magic values in future. Honestly speaking, I am not sure what's the right thing to do here. I will do whatever you and Rob incline for. -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2018-01-10 5:37 ` Viresh Kumar @ 2018-01-13 0:46 ` Stephen Boyd -1 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-13 0:46 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 01/10, Viresh Kumar wrote: > On 09-01-18, 18:54, Stephen Boyd wrote: > > My read of Kevin's comments lead me to think he's saying that a > > generic 'domain-performance-state' property is worse than putting > > the numbers directly inside of the opp table with a comment above > > it. Now that's all fine, but now that we have required-opps > > binding we sort of have the domain-performance-state property > > again, but it's a phandle instead of a raw state number. > > > > So we have > > > > required-opps = <&perf_state>; > > > > but what was proposed before was > > > > domain-performance-state = <1>; > > > > or Kevin's > > > > opp-table = <100000 1>; > > His concern was also on what will we do if "frequency" or other OPP > properties aren't known tomorrow by the kernel but the firmware? In > Qcom case, its just the voltage (corner) today, but it can very well > be other properties tomorrow. Are we going to add more platform > specific bindings then ? Yes, we would add more bindings. > And this is the main reason why I have been > aligned towards using something like this patch. Once we exceed the number of properties that can fit into the existing voltage and frequency properties we'll only be able to make it work by adding a platform specific property. That's one concern, but it's a future concern so it's not a real problem yet. If you can clearly describe in the commit text why we shouldn't use platform specific properties it would be helpful. > > If we drop the magic-values idea and hence this patch, then we can > either add a "domain-performance-state" property, which will only be > used by the power domains or leave it for the platforms to add > something like "qcom,corner". > > All we are doing here is putting a voltage (corner) value, unknown to > the kernel, in a new property instead of "opp-microvolt". But the > above question still remains, what about other properties that may > need magic values in future. > > Honestly speaking, I am not sure what's the right thing to do here. I > will do whatever you and Rob incline for. > Hopefully Rob and Kevin can reply here. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-13 0:46 ` Stephen Boyd 0 siblings, 0 replies; 41+ messages in thread From: Stephen Boyd @ 2018-01-13 0:46 UTC (permalink / raw) To: Viresh Kumar Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Rafael Wysocki, linux-pm, Vincent Guittot, Rajendra Nayak, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 01/10, Viresh Kumar wrote: > On 09-01-18, 18:54, Stephen Boyd wrote: > > My read of Kevin's comments lead me to think he's saying that a > > generic 'domain-performance-state' property is worse than putting > > the numbers directly inside of the opp table with a comment above > > it. Now that's all fine, but now that we have required-opps > > binding we sort of have the domain-performance-state property > > again, but it's a phandle instead of a raw state number. > > > > So we have > > > > required-opps = <&perf_state>; > > > > but what was proposed before was > > > > domain-performance-state = <1>; > > > > or Kevin's > > > > opp-table = <100000 1>; > > His concern was also on what will we do if "frequency" or other OPP > properties aren't known tomorrow by the kernel but the firmware? In > Qcom case, its just the voltage (corner) today, but it can very well > be other properties tomorrow. Are we going to add more platform > specific bindings then ? Yes, we would add more bindings. > And this is the main reason why I have been > aligned towards using something like this patch. Once we exceed the number of properties that can fit into the existing voltage and frequency properties we'll only be able to make it work by adding a platform specific property. That's one concern, but it's a future concern so it's not a real problem yet. If you can clearly describe in the commit text why we shouldn't use platform specific properties it would be helpful. > > If we drop the magic-values idea and hence this patch, then we can > either add a "domain-performance-state" property, which will only be > used by the power domains or leave it for the platforms to add > something like "qcom,corner". > > All we are doing here is putting a voltage (corner) value, unknown to > the kernel, in a new property instead of "opp-microvolt". But the > above question still remains, what about other properties that may > need magic values in future. > > Honestly speaking, I am not sure what's the right thing to do here. I > will do whatever you and Rob incline for. > Hopefully Rob and Kevin can reply here. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values 2017-12-28 4:37 ` Viresh Kumar @ 2018-01-02 6:05 ` Rajendra Nayak -1 siblings, 0 replies; 41+ messages in thread From: Rajendra Nayak @ 2018-01-02 6:05 UTC (permalink / raw) To: Viresh Kumar, Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 12/28/2017 10:07 AM, Viresh Kumar wrote: > On 27-12-17, 15:54, Rob Herring wrote: >> On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >>> On 26-12-17, 14:29, Rob Herring wrote: >>>> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: >>> >>>>> +On some platforms the exact frequency or voltage may be hidden from the OS by >>>>> +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain >>>>> +magic values that represent the frequency or voltage in a firmware dependent >>>>> +way, for example an index of an array in the firmware. >>>> >>>> I'm still not convinced this is a good idea. >>> >>> You were kind-of a few days back :) >>> >>> lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com >> >> Yeah, well that was before Stephen said anything. >> >>> So here is the deal: >>> >>> - I proposed "domain-performance-state" property for this stuff >>> initially. >>> - But Kevin didn't like that and proposed reusing "opp-hz" and >>> "opp-microvolt", which we all agreed to multiple times.. >>> - And we are back to the same discussion now and its painful and time >>> killing for all of us. >> >> There's bigger issues than where we put magic values as I raised in >> the other patch. >> >>> TBH, I don't have too strong preferences about any of the suggestions >>> you guys have and I need you guys to tell me what binding changes to >>> do here and I will do that. >>> >>>> If you have firmware >>>> partially managing things, then I think we should have platform specific >>>> bindings or drivers. >>> >>> What about the initial idea then, like "performance-state" for the >>> power domains ? All platforms will anyway replicate that binding only. >> >> I don't really know. I don't really care either. I'll probably go >> along with what everyone agrees to, but the only one I see any >> agreement from is Ulf. Also, it is pretty vague as to what platforms >> will use this. You claimed you can support QCom scenarios, but there's >> really no evidence that that is true. > > Well, I sent out the code few days back based on these bindings and everyone can > see how these bindings will get used now. > >> What I don't want to see is this >> merged and then we need something more yet again in a few months for >> another platform. > > Sure, I get your concerns. > > So what we need now is: > > - Stephen to start responding and clarify all the doubts he had as being silent > isn't helping. > > - Or Rajendra to post patches which can prove that this is usable. The last time > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > and he should have posted them by now, but he didn't :( I would want to reiterate what I have been saying for a while, that for these patches to be usable on any qualcomm platform completely we need support to associate multiple power-domains to a single device which is missing today. The last time this came up during a discussion at connect, I believe the understanding was to get this (performance state support) merged *after* we decide how to support multiple powerdomains per device. What I have been testing with these patches is to move a single user (MMC, which BTW does not have to put requests on multiple powerdomains) to use this solution on a db820c (msm8996) device. Getting this merged now can open up issues for other devices (which can't move to this solution) since MMC alone would put requests to pull a *common* rail up/down while others can't. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-02 6:05 ` Rajendra Nayak 0 siblings, 0 replies; 41+ messages in thread From: Rajendra Nayak @ 2018-01-02 6:05 UTC (permalink / raw) To: Viresh Kumar, Rob Herring Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 12/28/2017 10:07 AM, Viresh Kumar wrote: > On 27-12-17, 15:54, Rob Herring wrote: >> On Wed, Dec 27, 2017 at 2:56 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >>> On 26-12-17, 14:29, Rob Herring wrote: >>>> On Mon, Dec 18, 2017 at 03:51:30PM +0530, Viresh Kumar wrote: >>> >>>>> +On some platforms the exact frequency or voltage may be hidden from the OS by >>>>> +the firmware and the "opp-hz" or the "opp-microvolt" properties may contain >>>>> +magic values that represent the frequency or voltage in a firmware dependent >>>>> +way, for example an index of an array in the firmware. >>>> >>>> I'm still not convinced this is a good idea. >>> >>> You were kind-of a few days back :) >>> >>> lkml.kernel.org/r/CAL_JsqK-qtAaM_Ou5NtxcWR3F_q=8rMPJUm-VqGtKhbtWe5SAQ@mail.gmail.com >> >> Yeah, well that was before Stephen said anything. >> >>> So here is the deal: >>> >>> - I proposed "domain-performance-state" property for this stuff >>> initially. >>> - But Kevin didn't like that and proposed reusing "opp-hz" and >>> "opp-microvolt", which we all agreed to multiple times.. >>> - And we are back to the same discussion now and its painful and time >>> killing for all of us. >> >> There's bigger issues than where we put magic values as I raised in >> the other patch. >> >>> TBH, I don't have too strong preferences about any of the suggestions >>> you guys have and I need you guys to tell me what binding changes to >>> do here and I will do that. >>> >>>> If you have firmware >>>> partially managing things, then I think we should have platform specific >>>> bindings or drivers. >>> >>> What about the initial idea then, like "performance-state" for the >>> power domains ? All platforms will anyway replicate that binding only. >> >> I don't really know. I don't really care either. I'll probably go >> along with what everyone agrees to, but the only one I see any >> agreement from is Ulf. Also, it is pretty vague as to what platforms >> will use this. You claimed you can support QCom scenarios, but there's >> really no evidence that that is true. > > Well, I sent out the code few days back based on these bindings and everyone can > see how these bindings will get used now. > >> What I don't want to see is this >> merged and then we need something more yet again in a few months for >> another platform. > > Sure, I get your concerns. > > So what we need now is: > > - Stephen to start responding and clarify all the doubts he had as being silent > isn't helping. > > - Or Rajendra to post patches which can prove that this is usable. The last time > I had a chat with him, he confirmed that he will post patches after 4.15-rc1 > and he should have posted them by now, but he didn't :( I would want to reiterate what I have been saying for a while, that for these patches to be usable on any qualcomm platform completely we need support to associate multiple power-domains to a single device which is missing today. The last time this came up during a discussion at connect, I believe the understanding was to get this (performance state support) merged *after* we decide how to support multiple powerdomains per device. What I have been testing with these patches is to move a single user (MMC, which BTW does not have to put requests on multiple powerdomains) to use this solution on a db820c (msm8996) device. Getting this merged now can open up issues for other devices (which can't move to this solution) since MMC alone would put requests to pull a *common* rail up/down while others can't. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-02 6:33 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-02 6:33 UTC (permalink / raw) To: Rajendra Nayak Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm, Vincent Guittot, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On 02-01-18, 11:35, Rajendra Nayak wrote: > I would want to reiterate what I have been saying for a while, that for these patches > to be usable on any qualcomm platform completely we need support to associate > multiple power-domains to a single device which is missing today. Sure, but I had the understanding based on our last communication that you are going to go with partial support for now and multiple domain thing will be done later on. > What I have been testing with these patches is to move a single user (MMC, which BTW does not > have to put requests on multiple powerdomains) to use this solution on a db820c (msm8996) device. Right. > Getting this merged now can open up issues for other devices (which can't move to this solution) > since MMC alone would put requests to pull a *common* rail up/down while others can't. Even on that I thought that you will artificially vote for a high requirement for those devices from some platform code initially, and remove that code once all the device drivers (you need) are updated to post their own requirements. -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values @ 2018-01-02 6:33 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-02 6:33 UTC (permalink / raw) To: Rajendra Nayak Cc: Rob Herring, Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon, Stephen Boyd, Rafael Wysocki, linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, Sudeep Holla, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 02-01-18, 11:35, Rajendra Nayak wrote: > I would want to reiterate what I have been saying for a while, that for these patches > to be usable on any qualcomm platform completely we need support to associate > multiple power-domains to a single device which is missing today. Sure, but I had the understanding based on our last communication that you are going to go with partial support for now and multiple domain thing will be done later on. > What I have been testing with these patches is to move a single user (MMC, which BTW does not > have to put requests on multiple powerdomains) to use this solution on a db820c (msm8996) device. Right. > Getting this merged now can open up issues for other devices (which can't move to this solution) > since MMC alone would put requests to pull a *common* rail up/down while others can't. Even on that I thought that you will artificially vote for a high requirement for those devices from some platform code initially, and remove that code once all the device drivers (you need) are updated to post their own requirements. -- viresh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains @ 2018-01-03 7:20 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-03 7:20 UTC (permalink / raw) To: ulf.hansson, Kevin Hilman, robh+dt, Nishanth Menon, Rafael J. Wysocki, Stephen Boyd, Viresh Kumar Cc: linux-pm, Vincent Guittot, rnayak, sudeep.holla, devicetree, linux-kernel On 18-12-17, 15:51, Viresh Kumar wrote: > Hi, > > Now that the performance state of PM domains are supported by the kernel > (merged in linux-next), I am trying once again to define the bindings > which we dropped until the code is merged first. > > Summary: > > Power-domains can also have their active states and this patchset > enhances the OPP binding to define those. > > The power domains can use the OPP bindings mostly as is. Though there > are some changes required to support special cases: > > - Allow "operating-points-v2" to contain multiple phandles for power > domain providers providing multiple domains. > > - A new property "required-opp" is added for the devices to specify the > minimum required OPP of the master domain or any other type of device. > > - Allow some of the OPP properties to accept magic values (firmware > dependent) as the OS doesn't know the real freq/voltage values. > > V7->V8: > - V7 1/2 divided into two patches. 1/3 is unchanged from V7. > - 2/3 renamed the property from "power-domain-opp" to "required-opp", as > suggested by Rob. > - Added Ulf's reviewed-by for 1/3 and 3/3. > > -- > viresh > > Viresh Kumar (3): > OPP: Allow OPP table to be used for power-domains > OPP: Introduce "required-opp" property > OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Discussions are still going on for the last commit, though the first two are already Acked by Rob and Ulf and are quite independent of the third one. Any objections to getting the first two merged for 4.16-rc1 ? I will send them to Rafael on Friday if no one objects. -- viresh ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains @ 2018-01-03 7:20 ` Viresh Kumar 0 siblings, 0 replies; 41+ messages in thread From: Viresh Kumar @ 2018-01-03 7:20 UTC (permalink / raw) To: ulf.hansson-QSEj5FYQhm4dnm+yROfE0A, Kevin Hilman, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, Nishanth Menon, Rafael J. Wysocki, Stephen Boyd, Viresh Kumar Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA, Vincent Guittot, rnayak-sgV2jX0FEOL9JmXXK+q4OQ, sudeep.holla-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 18-12-17, 15:51, Viresh Kumar wrote: > Hi, > > Now that the performance state of PM domains are supported by the kernel > (merged in linux-next), I am trying once again to define the bindings > which we dropped until the code is merged first. > > Summary: > > Power-domains can also have their active states and this patchset > enhances the OPP binding to define those. > > The power domains can use the OPP bindings mostly as is. Though there > are some changes required to support special cases: > > - Allow "operating-points-v2" to contain multiple phandles for power > domain providers providing multiple domains. > > - A new property "required-opp" is added for the devices to specify the > minimum required OPP of the master domain or any other type of device. > > - Allow some of the OPP properties to accept magic values (firmware > dependent) as the OS doesn't know the real freq/voltage values. > > V7->V8: > - V7 1/2 divided into two patches. 1/3 is unchanged from V7. > - 2/3 renamed the property from "power-domain-opp" to "required-opp", as > suggested by Rob. > - Added Ulf's reviewed-by for 1/3 and 3/3. > > -- > viresh > > Viresh Kumar (3): > OPP: Allow OPP table to be used for power-domains > OPP: Introduce "required-opp" property > OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Discussions are still going on for the last commit, though the first two are already Acked by Rob and Ulf and are quite independent of the third one. Any objections to getting the first two merged for 4.16-rc1 ? I will send them to Rafael on Friday if no one objects. -- viresh -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2018-01-13 0:46 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-18 10:21 [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2017-12-18 10:21 ` Viresh Kumar 2017-12-18 10:21 ` [PATCH V8 1/3] " Viresh Kumar 2017-12-21 22:06 ` Rob Herring 2017-12-21 22:06 ` Rob Herring 2017-12-18 10:21 ` [PATCH V8 2/3] OPP: Introduce "required-opp" property Viresh Kumar 2017-12-18 10:21 ` Viresh Kumar 2017-12-20 8:23 ` Ulf Hansson 2017-12-20 8:26 ` Viresh Kumar 2017-12-21 22:26 ` Rob Herring 2017-12-21 22:26 ` Rob Herring 2017-12-22 5:28 ` Viresh Kumar 2017-12-18 10:21 ` [PATCH V8 3/3] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values Viresh Kumar 2017-12-26 20:29 ` Rob Herring 2017-12-26 20:29 ` Rob Herring 2017-12-27 8:56 ` Viresh Kumar 2017-12-27 8:56 ` Viresh Kumar 2017-12-27 21:54 ` Rob Herring 2017-12-27 21:54 ` Rob Herring 2017-12-28 4:37 ` Viresh Kumar 2017-12-28 4:37 ` Viresh Kumar 2017-12-29 0:32 ` Stephen Boyd 2017-12-29 0:32 ` Stephen Boyd 2017-12-29 4:58 ` Viresh Kumar 2017-12-29 4:58 ` Viresh Kumar 2018-01-05 22:19 ` Stephen Boyd 2018-01-05 22:19 ` Stephen Boyd 2018-01-08 4:16 ` Viresh Kumar 2018-01-08 4:16 ` Viresh Kumar 2018-01-10 2:54 ` Stephen Boyd 2018-01-10 2:54 ` Stephen Boyd 2018-01-10 5:37 ` Viresh Kumar 2018-01-10 5:37 ` Viresh Kumar 2018-01-13 0:46 ` Stephen Boyd 2018-01-13 0:46 ` Stephen Boyd 2018-01-02 6:05 ` Rajendra Nayak 2018-01-02 6:05 ` Rajendra Nayak 2018-01-02 6:33 ` Viresh Kumar 2018-01-02 6:33 ` Viresh Kumar 2018-01-03 7:20 ` [PATCH V8 0/3] OPP: Allow OPP table to be used for power-domains Viresh Kumar 2018-01-03 7:20 ` Viresh Kumar
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.