[PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

Power-domains need to express their active states in DT and the devices
within the power-domain need to express their dependency on those active
states. The power-domains can use the OPP tables without any
modifications to the bindings.

Add a new property "power-domain-opp", which will contain phandle to the
OPP node of the parent power domain. This is required for devices which
have dependency on the configured active state of the power domain for
their working.

For some platforms the actual frequency and voltages of the power
domains are managed by the firmware and are so hidden from the high
level operating system. The "opp-hz" property is relaxed a bit to
contain indexes instead of actual frequency values to support such
platforms.

Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 63725498bd20..6e30cae2a936 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
 properties.
 
 Required properties:
-- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
+- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
+  cases the exact frequency in Hz may be hidden from the OS by the firmware and
+  this field may contain values that represent the frequency in a firmware
+  dependent way, for example an index of an array in the firmware.
 
 Optional properties:
 - opp-microvolt: voltage in micro Volts.
@@ -154,6 +157,13 @@ properties.
 
 - status: Marks the node enabled/disabled.
 
+- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
+  parent power-domain should be configured to the OPP whose node is pointed by
+  the phandle, in order to configure the device for the OPP node that contains
+  this property. The order in which the device and power domain should be
+  configured is implementation defined. The OPP table of a device can set this
+  property only if the device node contains "power-domains" property.
+
 Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
 
 / {
@@ -528,3 +538,65 @@ Example 5: opp-supported-hw
 		};
 	};
 };
+
+Example 7: Power domains with their own OPP tables:
+(example: For 1GHz device require domain state 1 and for 1.1 & 1.2 GHz device require state 2)
+
+/ {
+	domain_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		/*
+		 * NOTE: Actual frequency is managed by firmware and is hidden
+		 * from HLOS, so we simply use index in the opp-hz field to
+		 * select the OPP.
+		 */
+		domain_opp_1: opp-1 {
+			opp-hz = /bits/ 64 <1>;
+			opp-microvolt = <975000 970000 985000>;
+		};
+		domain_opp_2: opp-2 {
+			opp-hz = /bits/ 64 <2>;
+			opp-microvolt = <1075000 1000000 1085000>;
+		};
+	};
+
+	foo_domain: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+		operating-points-v2 = <&domain_opp_table>;
+	}
+
+	cpu0_opp_table: opp_table1 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			power-domain-opp = <&domain_opp_1>;
+		};
+		opp-1100000000 {
+			opp-hz = /bits/ 64 <1100000000>;
+			power-domain-opp = <&domain_opp_2>;
+		};
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			power-domain-opp = <&domain_opp_2>;
+		};
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			compatible = "arm,cortex-a9";
+			reg = <0>;
+			clocks = <&clk_controller 0>;
+			clock-names = "cpu";
+			operating-points-v2 = <&cpu0_opp_table>;
+			power-domains = <&foo_domain>;
+		};
+	};
+};
-- 
2.12.0.432.g71c3a4f4ba37

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

[PATCH V6 2/9] PM / Domains: Allow OPP table to be used for power-domains

[Viresh Kumar]

Update the power-domain bindings to allow "operating-points-v2" to be
present within the power-domain's provider node.

Also allow consumer devices that don't use OPP tables, to specify the
parent power-domain's OPP node in their "power-domain-opp" property.

Also note that the "operating-points-v2" property is extended to support
an array for the power domain providers.

Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 .../devicetree/bindings/power/power_domain.txt     | 106 +++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 14bd9e945ff6..730af0afc09a 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -40,6 +40,10 @@ 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 for a power domain provider.
+  If the provider provides a single power domain, then this shall contain a
+  single phandle. Refer to ../opp/opp.txt for more information.
+
 Example:
 
 	power: power-controller@12340000 {
@@ -120,4 +124,106 @@ 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:
+- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
+  parent power-domain should be configured to the OPP whose node is pointed by
+  the phandle, in order to use the device that contains this property.
+
+
+Example:
+- Device with parent power domain with two active states represented by OPP
+  table.
+
+	domain_opp_table: opp_table {
+		compatible = "operating-points-v2";
+
+		/*
+		 * NOTE: Actual frequency is managed by firmware and is hidden
+		 * from HLOS, so we simply use index in the opp-hz field to
+		 * select the OPP.
+		 */
+		domain_opp_1: opp-1 {
+			opp-hz = /bits/ 64 <1>;
+			opp-microvolt = <975000 970000 985000>;
+		};
+		domain_opp_2: opp-2 {
+			opp-hz = /bits/ 64 <2>;
+			opp-microvolt = <1075000 1000000 1085000>;
+		};
+	};
+
+
+	parent: power-controller@12340000 {
+		compatible = "foo,power-controller";
+		reg = <0x12340000 0x1000>;
+		#power-domain-cells = <0>;
+		operating-points-v2 = <&domain_opp_table>;
+	};
+
+	leaky-device@12350000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12350000 0x1000>;
+		power-domains = <&parent>;
+		power-domain-opp = <&domain_opp_2>;
+	};
+
+- OPP table for domain provider that provides two domains.
+
+	domain0_opp_table: opp_table0 {
+		compatible = "operating-points-v2";
+
+		/*
+		 * NOTE: Actual frequency is managed by firmware and is hidden
+		 * from HLOS, so we simply use index in the opp-hz field to
+		 * select the OPP.
+		 */
+		domain0_opp_1: opp-1 {
+			opp-hz = /bits/ 64 <1>;
+			opp-microvolt = <975000 970000 985000>;
+		};
+		domain0_opp_2: opp-2 {
+			opp-hz = /bits/ 64 <2>;
+			opp-microvolt = <1075000 1000000 1085000>;
+		};
+	};
+
+	domain1_opp_table: opp_table1 {
+		compatible = "operating-points-v2";
+
+		/*
+		 * NOTE: Actual frequency is managed by firmware and is hidden
+		 * from HLOS, so we simply use index in the opp-hz field to
+		 * select the OPP.
+		 */
+		domain1_opp_1: opp-1 {
+			opp-hz = /bits/ 64 <1>;
+			opp-microvolt = <975000 970000 985000>;
+		};
+		domain1_opp_2: opp-2 {
+			opp-hz = /bits/ 64 <2>;
+			opp-microvolt = <1075000 1000000 1085000>;
+		};
+	};
+
+	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>;
+		power-domain-opp = <&domain0_opp_2>;
+	};
+
+	leaky-device1@12350000 {
+		compatible = "foo,i-leak-current";
+		reg = <0x12350000 0x1000>;
+		power-domains = <&parent 1>;
+		power-domain-opp = <&domain1_opp_2>;
+	};
+
 [1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
-- 
2.12.0.432.g71c3a4f4ba37

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Rob Herring]

On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
> Power-domains need to express their active states in DT and the devices
> within the power-domain need to express their dependency on those active
> states. The power-domains can use the OPP tables without any
> modifications to the bindings.
> 
> Add a new property "power-domain-opp", which will contain phandle to the
> OPP node of the parent power domain. This is required for devices which
> have dependency on the configured active state of the power domain for
> their working.
> 
> For some platforms the actual frequency and voltages of the power
> domains are managed by the firmware and are so hidden from the high
> level operating system. The "opp-hz" property is relaxed a bit to
> contain indexes instead of actual frequency values to support such
> platforms.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>  1 file changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 63725498bd20..6e30cae2a936 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>  properties.
>  
>  Required properties:
> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
> +  this field may contain values that represent the frequency in a firmware
> +  dependent way, for example an index of an array in the firmware.

Not really sure OPP binding makes sense here. What about all the other 
properties. We expose voltage, but not freq?

>  
>  Optional properties:
>  - opp-microvolt: voltage in micro Volts.
> @@ -154,6 +157,13 @@ properties.
>  
>  - status: Marks the node enabled/disabled.
>  
> +- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
> +  parent power-domain should be configured to the OPP whose node is pointed by
> +  the phandle, in order to configure the device for the OPP node that contains
> +  this property. The order in which the device and power domain should be
> +  configured is implementation defined. The OPP table of a device can set this
> +  property only if the device node contains "power-domains" property.
> +

I don't even know what to say on this. The continual evolution of 
OPP bindings continues. This seems like further abuse of DT 
power-domains (being a region in a chip that can be powergated) with 
Linux PM domains.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Sudeep Holla]

On 28/04/17 21:48, Rob Herring wrote:
> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>> Power-domains need to express their active states in DT and the devices
>> within the power-domain need to express their dependency on those active
>> states. The power-domains can use the OPP tables without any
>> modifications to the bindings.
>>
>> Add a new property "power-domain-opp", which will contain phandle to the
>> OPP node of the parent power domain. This is required for devices which
>> have dependency on the configured active state of the power domain for
>> their working.
>>
>> For some platforms the actual frequency and voltages of the power
>> domains are managed by the firmware and are so hidden from the high
>> level operating system. The "opp-hz" property is relaxed a bit to
>> contain indexes instead of actual frequency values to support such
>> platforms.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>  1 file changed, 73 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>> index 63725498bd20..6e30cae2a936 100644
>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>  properties.
>>  
>>  Required properties:
>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>> +  this field may contain values that represent the frequency in a firmware
>> +  dependent way, for example an index of an array in the firmware.
> 
> Not really sure OPP binding makes sense here. What about all the other 
> properties. We expose voltage, but not freq?
> 

I completely agree with that and I have been pushing this to be
represented as just regulators[0]. Mark B seem to dislike that
idea [1]

-- 
Regards,
Sudeep

[0] http://www.spinics.net/lists/devicetree/msg174725.html
[1] http://www.spinics.net/lists/devicetree/msg175113.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Kevin Hilman]

Sudeep Holla <sudeep.holla@arm.com> writes:

> On 28/04/17 21:48, Rob Herring wrote:
>> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>>> Power-domains need to express their active states in DT and the devices
>>> within the power-domain need to express their dependency on those active
>>> states. The power-domains can use the OPP tables without any
>>> modifications to the bindings.
>>>
>>> Add a new property "power-domain-opp", which will contain phandle to the
>>> OPP node of the parent power domain. This is required for devices which
>>> have dependency on the configured active state of the power domain for
>>> their working.
>>>
>>> For some platforms the actual frequency and voltages of the power
>>> domains are managed by the firmware and are so hidden from the high
>>> level operating system. The "opp-hz" property is relaxed a bit to
>>> contain indexes instead of actual frequency values to support such
>>> platforms.
>>>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> ---
>>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>>  1 file changed, 73 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>> index 63725498bd20..6e30cae2a936 100644
>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>>  properties.
>>>  
>>>  Required properties:
>>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>>> +  this field may contain values that represent the frequency in a firmware
>>> +  dependent way, for example an index of an array in the firmware.
>> 
>> Not really sure OPP binding makes sense here. What about all the other 
>> properties. We expose voltage, but not freq?
>> 
>
> I completely agree with that and I have been pushing this to be
> represented as just regulators[0]. Mark B seem to dislike that
> idea [1]

And Mark is right, because what's being described is not (simply) a
voltage regultor.  While it might be "just" voltage on some SoCs (for
now), it is clearly about performance (a.k.a. OPP) on others.

Kevin

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Kevin Hilman]

Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>> Power-domains need to express their active states in DT and the devices
>> within the power-domain need to express their dependency on those active
>> states. The power-domains can use the OPP tables without any
>> modifications to the bindings.
>> 
>> Add a new property "power-domain-opp", which will contain phandle to the
>> OPP node of the parent power domain. This is required for devices which
>> have dependency on the configured active state of the power domain for
>> their working.
>> 
>> For some platforms the actual frequency and voltages of the power
>> domains are managed by the firmware and are so hidden from the high
>> level operating system. The "opp-hz" property is relaxed a bit to
>> contain indexes instead of actual frequency values to support such
>> platforms.
>> 
>> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>  1 file changed, 73 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>> index 63725498bd20..6e30cae2a936 100644
>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>  properties.
>>  
>>  Required properties:
>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>> +  this field may contain values that represent the frequency in a firmware
>> +  dependent way, for example an index of an array in the firmware.
>
> Not really sure OPP binding makes sense here.

I think OPP makes perfect sense here, because microcontroller firmware
is managaging OPPs in hardware.  We just may not know the exact voltage
and/or frequency (and the firmware/hardware may even be doing AVS for
micro-adjustments.)

> What about all the other properties. We expose voltage, but not freq?

I had the same question.  Seems the same comment about an abstract
"index" is needed for voltage also.

>>  
>>  Optional properties:
>>  - opp-microvolt: voltage in micro Volts.
>> @@ -154,6 +157,13 @@ properties.
>>  
>>  - status: Marks the node enabled/disabled.
>>  
>> +- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
>> +  parent power-domain should be configured to the OPP whose node is pointed by
>> +  the phandle, in order to configure the device for the OPP node that contains
>> +  this property. The order in which the device and power domain should be
>> +  configured is implementation defined. The OPP table of a device can set this
>> +  property only if the device node contains "power-domains" property.
>> +

I do understand the need to map a device OPP to a parent power-domain
OPP, but I really don't like another phandle.

First, just because a device OPP changes does not mean that a
power-domain OPP has to change.  What really needs to be specified is a
minimum requirement, not an exact OPP.  IOW, if a device changes OPP,
the power-domain OPP has to be *at least* an OPP that can guarantee that
level of performance, but could also be a more performant OPP, right?

Also, the parent power-domain driver will have a list of all its
devices, and be able to get OPPs from those devices.

IMO, we should do the first (few) implementations of this feature from
the power-domain driver itself, and not try to figure out how to define
this for everyone in DT until we have a better handle on it (pun
intended) ;)

> I don't even know what to say on this. The continual evolution of 
> OPP bindings continues. This seems like further abuse of DT 
> power-domains (being a region in a chip that can be powergated) with 
> Linux PM domains.

Agreed.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

On 06-05-17, 11:58, Kevin Hilman wrote:
> Rob Herring <robh@kernel.org> writes:
> 
> > On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
> >> Power-domains need to express their active states in DT and the devices
> >> within the power-domain need to express their dependency on those active
> >> states. The power-domains can use the OPP tables without any
> >> modifications to the bindings.
> >> 
> >> Add a new property "power-domain-opp", which will contain phandle to the
> >> OPP node of the parent power domain. This is required for devices which
> >> have dependency on the configured active state of the power domain for
> >> their working.
> >> 
> >> For some platforms the actual frequency and voltages of the power
> >> domains are managed by the firmware and are so hidden from the high
> >> level operating system. The "opp-hz" property is relaxed a bit to
> >> contain indexes instead of actual frequency values to support such
> >> platforms.
> >> 
> >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
> >>  1 file changed, 73 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> >> index 63725498bd20..6e30cae2a936 100644
> >> --- a/Documentation/devicetree/bindings/opp/opp.txt
> >> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> >> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
> >>  properties.
> >>  
> >>  Required properties:
> >> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
> >> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
> >> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
> >> +  this field may contain values that represent the frequency in a firmware
> >> +  dependent way, for example an index of an array in the firmware.
> >
> > Not really sure OPP binding makes sense here.
> 
> I think OPP makes perfect sense here, because microcontroller firmware
> is managaging OPPs in hardware.  We just may not know the exact voltage
> and/or frequency (and the firmware/hardware may even be doing AVS for
> micro-adjustments.)

Yes, AVS is being done for the Qcom SoC as well.

> > What about all the other properties. We expose voltage, but not freq?
> 
> I had the same question.  Seems the same comment about an abstract
> "index" is needed for voltage also.

Why should we do that? Here are the cases that I had in mind while writing this:

- DT only contains the performance-index and nothing else (i.e. voltages aren't
  exposed).

  We wouldn't be required to fill the microvolt property as it is optional.

- DT contains both performance-index and voltages.

  The microvolts property will contain the actual voltages and opp-hz will
  contain the index.

I don't see why would we like to put some index value in the microvolts
property. We are setting the index value in the opp-hz property to avoid adding
extra fields and making sure opp-hz is still the unique property for the nodes.

> >>  
> >>  Optional properties:
> >>  - opp-microvolt: voltage in micro Volts.
> >> @@ -154,6 +157,13 @@ properties.
> >>  
> >>  - status: Marks the node enabled/disabled.
> >>  
> >> +- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
> >> +  parent power-domain should be configured to the OPP whose node is pointed by
> >> +  the phandle, in order to configure the device for the OPP node that contains
> >> +  this property. The order in which the device and power domain should be
> >> +  configured is implementation defined. The OPP table of a device can set this
> >> +  property only if the device node contains "power-domains" property.
> >> +
> 
> I do understand the need to map a device OPP to a parent power-domain
> OPP, but I really don't like another phandle.
> 
> First, just because a device OPP changes does not mean that a
> power-domain OPP has to change.  What really needs to be specified is a
> minimum requirement, not an exact OPP.  IOW, if a device changes OPP,
> the power-domain OPP has to be *at least* an OPP that can guarantee that
> level of performance, but could also be a more performant OPP, right?

Right and that's how the code is interpreting it right now. Yes, the description
above should have been more clear on that though.

> Also, the parent power-domain driver will have a list of all its
> devices, and be able to get OPPs from those devices.
> 
> IMO, we should do the first (few) implementations of this feature from
> the power-domain driver itself, and not try to figure out how to define
> this for everyone in DT until we have a better handle on it (pun
> intended) ;)

Hmm, I am not sure how things are going to work in that case. The opp-hz value
read from the phandle is passed to the QoS framework in this series, which makes
sure that we select the highest requested performance point for a particular
power-domain. The index value is required to be present with the OPP framework
to make it all work, at least based on the way I have designed it for now.

-- 
viresh

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Rajendra Nayak]

On 05/08/2017 09:45 AM, Viresh Kumar wrote:
> On 06-05-17, 11:58, Kevin Hilman wrote:
>> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>>
>>> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>>>> Power-domains need to express their active states in DT and the devices
>>>> within the power-domain need to express their dependency on those active
>>>> states. The power-domains can use the OPP tables without any
>>>> modifications to the bindings.
>>>>
>>>> Add a new property "power-domain-opp", which will contain phandle to the
>>>> OPP node of the parent power domain. This is required for devices which
>>>> have dependency on the configured active state of the power domain for
>>>> their working.
>>>>
>>>> For some platforms the actual frequency and voltages of the power
>>>> domains are managed by the firmware and are so hidden from the high
>>>> level operating system. The "opp-hz" property is relaxed a bit to
>>>> contain indexes instead of actual frequency values to support such
>>>> platforms.
>>>>
>>>> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>>>  1 file changed, 73 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>>> index 63725498bd20..6e30cae2a936 100644
>>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>>>  properties.
>>>>  
>>>>  Required properties:
>>>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>>>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>>>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>>>> +  this field may contain values that represent the frequency in a firmware
>>>> +  dependent way, for example an index of an array in the firmware.
>>>
>>> Not really sure OPP binding makes sense here.
>>
>> I think OPP makes perfect sense here, because microcontroller firmware
>> is managaging OPPs in hardware.  We just may not know the exact voltage
>> and/or frequency (and the firmware/hardware may even be doing AVS for
>> micro-adjustments.)
> 
> Yes, AVS is being done for the Qcom SoC as well.
> 
>>> What about all the other properties. We expose voltage, but not freq?
>>
>> I had the same question.  Seems the same comment about an abstract
>> "index" is needed for voltage also.
> 
> Why should we do that? Here are the cases that I had in mind while writing this:
> 
> - DT only contains the performance-index and nothing else (i.e. voltages aren't
>   exposed).
> 
>   We wouldn't be required to fill the microvolt property as it is optional.

So the performance-index is specified in opp-hz property?
What if the microcontroller firmware maps the performance-index to voltage but
expects linux to scale the frequency? There is no way to specify a performance-index
*and* a frequency for a OPP now I guess?

> 
> - DT contains both performance-index and voltages.
> 
>   The microvolts property will contain the actual voltages and opp-hz will
>   contain the index.

So this is for cases where the performance-index maps to a freq managed by the
microcontroller and voltages managed by linux? I have a case of exact opposite
and I don't see now how to handle it now with these bindings.

> 
> I don't see why would we like to put some index value in the microvolts
> property. We are setting the index value in the opp-hz property to avoid adding
> extra fields and making sure opp-hz is still the unique property for the nodes.

Maybe to handle the case like what I described above?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

I had a long chat with Rajendra offline and clarified few things..

On 08-05-17, 11:06, Rajendra Nayak wrote:
> On 05/08/2017 09:45 AM, Viresh Kumar wrote:
> > On 06-05-17, 11:58, Kevin Hilman wrote:

> >> I had the same question.  Seems the same comment about an abstract
> >> "index" is needed for voltage also.
> > 
> > Why should we do that? Here are the cases that I had in mind while writing this:
> > 
> > - DT only contains the performance-index and nothing else (i.e. voltages aren't
> >   exposed).
> > 
> >   We wouldn't be required to fill the microvolt property as it is optional.
> 
> So the performance-index is specified in opp-hz property?

Yes, but in the OPP table of the power-domain and not the device. The
device can still have its own OPP table with normal freq/voltage
values (for a separate regulator).

> What if the microcontroller firmware maps the performance-index to voltage but
> expects linux to scale the frequency?

As you clarified on the chat, you were talking about the device here.
It isn't a problem as we will have two separate tables here, one for
the device and one for the domain.

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

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

On 03-05-17, 12:29, Sudeep Holla wrote:
> 
> 
> On 28/04/17 21:48, Rob Herring wrote:
> > On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
> >> Power-domains need to express their active states in DT and the devices
> >> within the power-domain need to express their dependency on those active
> >> states. The power-domains can use the OPP tables without any
> >> modifications to the bindings.
> >>
> >> Add a new property "power-domain-opp", which will contain phandle to the
> >> OPP node of the parent power domain. This is required for devices which
> >> have dependency on the configured active state of the power domain for
> >> their working.
> >>
> >> For some platforms the actual frequency and voltages of the power
> >> domains are managed by the firmware and are so hidden from the high
> >> level operating system. The "opp-hz" property is relaxed a bit to
> >> contain indexes instead of actual frequency values to support such
> >> platforms.
> >>
> >> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> >> ---
> >>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
> >>  1 file changed, 73 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> >> index 63725498bd20..6e30cae2a936 100644
> >> --- a/Documentation/devicetree/bindings/opp/opp.txt
> >> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> >> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
> >>  properties.
> >>  
> >>  Required properties:
> >> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
> >> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
> >> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
> >> +  this field may contain values that represent the frequency in a firmware
> >> +  dependent way, for example an index of an array in the firmware.
> > 
> > Not really sure OPP binding makes sense here. What about all the other 
> > properties. We expose voltage, but not freq?
> > 
> 
> I completely agree with that and I have been pushing this to be
> represented as just regulators[0]. Mark B seem to dislike that
> idea [1]

Just as an update, Rajendra confirmed (offline) that for some of the
implementations, the microcontroller handles both frequency and
voltages of a device. So it isn't just a regulator anymore and as me
and Kevin were saying, we need a complete OPP here.

-- 
viresh

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Sudeep Holla]

On 06/05/17 10:39, Kevin Hilman wrote:
> Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> writes:
> 
>> On 28/04/17 21:48, Rob Herring wrote:
>>> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>>>> Power-domains need to express their active states in DT and the devices
>>>> within the power-domain need to express their dependency on those active
>>>> states. The power-domains can use the OPP tables without any
>>>> modifications to the bindings.
>>>>
>>>> Add a new property "power-domain-opp", which will contain phandle to the
>>>> OPP node of the parent power domain. This is required for devices which
>>>> have dependency on the configured active state of the power domain for
>>>> their working.
>>>>
>>>> For some platforms the actual frequency and voltages of the power
>>>> domains are managed by the firmware and are so hidden from the high
>>>> level operating system. The "opp-hz" property is relaxed a bit to
>>>> contain indexes instead of actual frequency values to support such
>>>> platforms.
>>>>
>>>> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>>>  1 file changed, 73 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>>> index 63725498bd20..6e30cae2a936 100644
>>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>>>  properties.
>>>>  
>>>>  Required properties:
>>>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>>>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>>>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>>>> +  this field may contain values that represent the frequency in a firmware
>>>> +  dependent way, for example an index of an array in the firmware.
>>>
>>> Not really sure OPP binding makes sense here. What about all the other 
>>> properties. We expose voltage, but not freq?
>>>
>>
>> I completely agree with that and I have been pushing this to be
>> represented as just regulators[0]. Mark B seem to dislike that
>> idea [1]
> 
> And Mark is right, because what's being described is not (simply) a
> voltage regultor.  While it might be "just" voltage on some SoCs (for
> now), it is clearly about performance (a.k.a. OPP) on others.
> 

Agreed. What I was against in this particular case was it was just
voltage for the domain and the devices had their own OPP with clocks
described which looks really weird when both are represented as OPPs.

I am fine with OPP representation in all such cases provide the bindings
are well defined especially if they are hierarchical, what takes
precedence, ...etc.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Sudeep Holla]

On 08/05/17 08:13, Viresh Kumar wrote:
> On 03-05-17, 12:29, Sudeep Holla wrote:
>>
>>
>> On 28/04/17 21:48, Rob Herring wrote:
>>> On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>>>> Power-domains need to express their active states in DT and the devices
>>>> within the power-domain need to express their dependency on those active
>>>> states. The power-domains can use the OPP tables without any
>>>> modifications to the bindings.
>>>>
>>>> Add a new property "power-domain-opp", which will contain phandle to the
>>>> OPP node of the parent power domain. This is required for devices which
>>>> have dependency on the configured active state of the power domain for
>>>> their working.
>>>>
>>>> For some platforms the actual frequency and voltages of the power
>>>> domains are managed by the firmware and are so hidden from the high
>>>> level operating system. The "opp-hz" property is relaxed a bit to
>>>> contain indexes instead of actual frequency values to support such
>>>> platforms.
>>>>
>>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>>>>  1 file changed, 73 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>>>> index 63725498bd20..6e30cae2a936 100644
>>>> --- a/Documentation/devicetree/bindings/opp/opp.txt
>>>> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>>>> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>>>>  properties.
>>>>  
>>>>  Required properties:
>>>> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>>>> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>>>> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>>>> +  this field may contain values that represent the frequency in a firmware
>>>> +  dependent way, for example an index of an array in the firmware.
>>>
>>> Not really sure OPP binding makes sense here. What about all the other 
>>> properties. We expose voltage, but not freq?
>>>
>>
>> I completely agree with that and I have been pushing this to be
>> represented as just regulators[0]. Mark B seem to dislike that
>> idea [1]
> 
> Just as an update, Rajendra confirmed (offline) that for some of the
> implementations, the microcontroller handles both frequency and
> voltages of a device. So it isn't just a regulator anymore and as me
> and Kevin were saying, we need a complete OPP here.
> 

Yes, I followed the thread and figured that out. But Rajendra also
raised "What if the microcontroller firmware maps the performance-index
to voltage but expects linux to scale the frequency? There is no way to
specify a performance-index *and* a frequency for a OPP now I guess? So
this needs to be addressd now IIUC.

So as Kevin pointed out, we need to experiment and look at all
possibilities before finalizing the bindings. Better to have examples
for all these and describe how bindings are be used including how to
distinguish between these use-case from the bindings if it's not implicit.

-- 
Regards,
Sudeep

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

On 08-05-17, 14:57, Sudeep Holla wrote:
> Yes, I followed the thread and figured that out. But Rajendra also
> raised "What if the microcontroller firmware maps the performance-index
> to voltage but expects linux to scale the frequency? There is no way to
> specify a performance-index *and* a frequency for a OPP now I guess? So
> this needs to be addressd now IIUC.

No, he misunderstood it. He was saying that the domain needs a performance-index
and the device needs freq-scaling, how do we do that? He thought that there will
be just one OPP table for the device here, but we will actually have two and
that would work.

> So as Kevin pointed out, we need to experiment and look at all
> possibilities before finalizing the bindings. Better to have examples
> for all these and describe how bindings are be used including how to
> distinguish between these use-case from the bindings if it's not implicit.

Yeah, I have some doubts on how we are going to implement that and looking for
more input from him.

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

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Kevin Hilman]

Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:

> On 06-05-17, 11:58, Kevin Hilman wrote:
>> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>> 
>> > On Wed, Apr 26, 2017 at 04:27:05PM +0530, Viresh Kumar wrote:
>> >> Power-domains need to express their active states in DT and the devices
>> >> within the power-domain need to express their dependency on those active
>> >> states. The power-domains can use the OPP tables without any
>> >> modifications to the bindings.
>> >> 
>> >> Add a new property "power-domain-opp", which will contain phandle to the
>> >> OPP node of the parent power domain. This is required for devices which
>> >> have dependency on the configured active state of the power domain for
>> >> their working.
>> >> 
>> >> For some platforms the actual frequency and voltages of the power
>> >> domains are managed by the firmware and are so hidden from the high
>> >> level operating system. The "opp-hz" property is relaxed a bit to
>> >> contain indexes instead of actual frequency values to support such
>> >> platforms.
>> >> 
>> >> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> >> ---
>> >>  Documentation/devicetree/bindings/opp/opp.txt | 74 ++++++++++++++++++++++++++-
>> >>  1 file changed, 73 insertions(+), 1 deletion(-)
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
>> >> index 63725498bd20..6e30cae2a936 100644
>> >> --- a/Documentation/devicetree/bindings/opp/opp.txt
>> >> +++ b/Documentation/devicetree/bindings/opp/opp.txt
>> >> @@ -77,7 +77,10 @@ This defines voltage-current-frequency combinations along with other related
>> >>  properties.
>> >>  
>> >>  Required properties:
>> >> -- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer.
>> >> +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. In some
>> >> +  cases the exact frequency in Hz may be hidden from the OS by the firmware and
>> >> +  this field may contain values that represent the frequency in a firmware
>> >> +  dependent way, for example an index of an array in the firmware.
>> >
>> > Not really sure OPP binding makes sense here.
>> 
>> I think OPP makes perfect sense here, because microcontroller firmware
>> is managaging OPPs in hardware.  We just may not know the exact voltage
>> and/or frequency (and the firmware/hardware may even be doing AVS for
>> micro-adjustments.)
>
> Yes, AVS is being done for the Qcom SoC as well.
>
>> > What about all the other properties. We expose voltage, but not freq?
>> 
>> I had the same question.  Seems the same comment about an abstract
>> "index" is needed for voltage also.
>
> Why should we do that?

For starters, because the lack of it looks very strange upon first read
(notice that both Rob and I pointed that out), and because you didn't
explain why in the first place, it draws attention.

> Here are the cases that I had in mind while writing this:
>
> - DT only contains the performance-index and nothing else (i.e. voltages aren't
>   exposed).
>
>   We wouldn't be required to fill the microvolt property as it is optional.
>
> - DT contains both performance-index and voltages.
>
>   The microvolts property will contain the actual voltages and opp-hz will
>   contain the index.
>
> I don't see why would we like to put some index value in the microvolts
> property. We are setting the index value in the opp-hz property to avoid adding
> extra fields and making sure opp-hz is still the unique property for the nodes.

What about the case where firmware wants exact frequencies, and
microvolts property is just an index?

The point is, you have a very specific SoC and use-case in mind, but the
goal of a binding change like this is to make something that could be
generically useful.

>> >>  
>> >>  Optional properties:
>> >>  - opp-microvolt: voltage in micro Volts.
>> >> @@ -154,6 +157,13 @@ properties.
>> >>  
>> >>  - status: Marks the node enabled/disabled.
>> >>  
>> >> +- power-domain-opp: Phandle to the OPP node of the parent power-domain. The
>> >> +  parent power-domain should be configured to the OPP whose node is pointed by
>> >> +  the phandle, in order to configure the device for the OPP node that contains
>> >> +  this property. The order in which the device and power domain should be
>> >> +  configured is implementation defined. The OPP table of a device can set this
>> >> +  property only if the device node contains "power-domains" property.
>> >> +
>> 
>> I do understand the need to map a device OPP to a parent power-domain
>> OPP, but I really don't like another phandle.
>> 
>> First, just because a device OPP changes does not mean that a
>> power-domain OPP has to change.  What really needs to be specified is a
>> minimum requirement, not an exact OPP.  IOW, if a device changes OPP,
>> the power-domain OPP has to be *at least* an OPP that can guarantee that
>> level of performance, but could also be a more performant OPP, right?
>
> Right and that's how the code is interpreting it right now. Yes, the description
> above should have been more clear on that though.
>
>> Also, the parent power-domain driver will have a list of all its
>> devices, and be able to get OPPs from those devices.
>> 
>> IMO, we should do the first (few) implementations of this feature from
>> the power-domain driver itself, and not try to figure out how to define
>> this for everyone in DT until we have a better handle on it (pun
>> intended) ;)
>
> Hmm, I am not sure how things are going to work in that case. The opp-hz value
> read from the phandle is passed to the QoS framework in this series, which makes
> sure that we select the highest requested performance point for a particular
> power-domain. The index value is required to be present with the OPP framework
> to make it all work, at least based on the way I have designed it for now.

IMO, this kind of dependency isn't the job of the OPP framework, it's
the job of the power-domain governor.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V6 1/9] PM / OPP: Introduce "power-domain-opp" property

[Viresh Kumar]

On 12 May 2017 at 20:29, Kevin Hilman <khilman@baylibre.com> wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:

>> Why should we do that?
>
> For starters, because the lack of it looks very strange upon first read
> (notice that both Rob and I pointed that out), and because you didn't
> explain why in the first place, it draws attention.

:)

>> I don't see why would we like to put some index value in the microvolts
>> property. We are setting the index value in the opp-hz property to avoid adding
>> extra fields and making sure opp-hz is still the unique property for the nodes.
>
> What about the case where firmware wants exact frequencies, and
> microvolts property is just an index?
>
> The point is, you have a very specific SoC and use-case in mind, but the
> goal of a binding change like this is to make something that could be
> generically useful.

I agree, but I am not sure of having such a case in very near future at least.
Wouldn't it be wise to not touch opp-microvolt for now and update it only
when needed? Its not a big change anyway..

>> Hmm, I am not sure how things are going to work in that case. The opp-hz value
>> read from the phandle is passed to the QoS framework in this series, which makes
>> sure that we select the highest requested performance point for a particular
>> power-domain. The index value is required to be present with the OPP framework
>> to make it all work, at least based on the way I have designed it for now.
>
> IMO, this kind of dependency isn't the job of the OPP framework, it's
> the job of the power-domain governor.

Okay. So the way it will work with the current suggestions is:

- OPP framework gets DVFS update request for device X
- OPP framework finds that the device has a power-domain and so it asks
  the power-domain framework to set the device in a particular state
  corresponding to the OPP (if we are going to a higher OPP).
- If the power-domain supports state selection, it does that or returns error.
  (Actually we can optimize this by asking the genpd initially if
state selection
   is possible, only then OPP core calls the genpd API).
- The genpd API will manage a list of all devices in the domain (which it
  already does) and also the states selected for them. It finds the max of
  the requested states and selects that.
- Note that the QoS framework isn't there in the picture anymore.

Will that be fine ?

--
viresh