diff mbox

[v4,2/2] dt: power: st: Provide bindings for ST's OPPs

Message ID 20150803034642.GV899@linux
State New
Headers show

Commit Message

Viresh Kumar Aug. 3, 2015, 3:46 a.m. UTC 
On 31-07-15, 09:37, Stephen Boyd wrote:
> For qcom platforms, the frequency is almost always constant.
> There may be some tables where we have a couple higher
> frequencies than others because the speed bin is different.
> Otherwise the voltage/current is changing based on the silicon
> characteristics. So the biggest duplication is the frequency
> property.
> 
> As far as I know there isn't any algorithm to generate the
> voltage values. It's all hand tuned tables based on the silicon
> characterization, so we're left to store these tables in DT and
> pick the right one at runtime. With regards to the table
> explosion, on qcom platforms we haven't worried that we have ~40
> tables, but I'm not opposed to expressing it in a smaller set of
> nodes, tables, etc. if that's what's desired.
> 
> Do we need vendor specific properties for that though? Or do we
> need some sort of extended frequency/voltage properties that are
> arrays of values that we can index into based on some silicon
> characteristics? I like the name based approach because it's
> simple. Use this OPP table because it's called
> x-y-z-characteristics and be done. Cramming the tables into less
> lines may save us some typing and dtb space, but I'm not sure
> what else it does.

What about something like this:

Comments

Lee Jones Aug. 10, 2015, 1:22 p.m. UTC | #1 
On Mon, 03 Aug 2015, Viresh Kumar wrote:

> On 31-07-15, 09:37, Stephen Boyd wrote:
> > For qcom platforms, the frequency is almost always constant.
> > There may be some tables where we have a couple higher
> > frequencies than others because the speed bin is different.
> > Otherwise the voltage/current is changing based on the silicon
> > characteristics. So the biggest duplication is the frequency
> > property.
> > 
> > As far as I know there isn't any algorithm to generate the
> > voltage values. It's all hand tuned tables based on the silicon
> > characterization, so we're left to store these tables in DT and
> > pick the right one at runtime. With regards to the table
> > explosion, on qcom platforms we haven't worried that we have ~40
> > tables, but I'm not opposed to expressing it in a smaller set of
> > nodes, tables, etc. if that's what's desired.
> > 
> > Do we need vendor specific properties for that though? Or do we
> > need some sort of extended frequency/voltage properties that are
> > arrays of values that we can index into based on some silicon
> > characteristics? I like the name based approach because it's
> > simple. Use this OPP table because it's called
> > x-y-z-characteristics and be done. Cramming the tables into less
> > lines may save us some typing and dtb space, but I'm not sure
> > what else it does.
> 
> What about something like this:
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 0cb44dc21f97..bad7a8299b9c 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -74,6 +74,8 @@ This describes the OPPs belonging to a device. This node can have following
>    reference an OPP.
>  
>  Optional properties:
> +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> +  can support.

This isn't very generic.

I'm guessing some vendors my have quite a few ways to differentiate
between board versions/revisions/cuts etc.

How about another array where a vendor can choose to identify a piece
of hardware however they see fit.

Example 1 (simple version):

/* Version 1 */
opp-version = <1>;

Example 2 (using the kernel's versioning):

/* 2.6.32-rc1 */
opp-version = <2 6 32 1>;

Example 3 (using ST's versioning):

/* Major 2, Minor 0, Cut 2, All substrates */
opp-version = <2 0 2 0xff>;

Qcom (or anyone else wanting to use names to identify their revisions)
can continue to use their node name method, as it doesn't break any
convention.

>  - opp-shared: Indicates that device nodes using this OPP Table Node's phandle
>    switch their DVFS state together, i.e. they share clock/voltage/current lines.
>    Missing property means devices have independent clock/voltage/current lines,
> @@ -100,6 +102,9 @@ properties.
>    Entries for multiple regulators must be present in the same order as
>    regulators are specified in device's DT node.
>  
> +  If used with 'opp-cuts', then the number of entries present here must match
> +  the number of strings present in 'opp-cuts'.
> +
>  - opp-microamp: The maximum current drawn by the device in microamperes
>    considering system specific parameters (such as transients, process, aging,
>    maximum operating temperature range etc.) as necessary. This may be used to
>
Viresh Kumar Aug. 11, 2015, 8 a.m. UTC | #2 
On 10-08-15, 14:22, Lee Jones wrote:
> >  Optional properties:
> > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > +  can support.
> 
> This isn't very generic.
> 
> I'm guessing some vendors my have quite a few ways to differentiate
> between board versions/revisions/cuts etc.
> 
> How about another array where a vendor can choose to identify a piece
> of hardware however they see fit.
> 
> Example 1 (simple version):
> 
> /* Version 1 */
> opp-version = <1>;
> 
> Example 2 (using the kernel's versioning):
> 
> /* 2.6.32-rc1 */
> opp-version = <2 6 32 1>;
> 
> Example 3 (using ST's versioning):
> 
> /* Major 2, Minor 0, Cut 2, All substrates */
> opp-version = <2 0 2 0xff>;

But how will we parse this with generic code ?
Lee Jones Aug. 11, 2015, 9:30 a.m. UTC | #3 
On Tue, 11 Aug 2015, Viresh Kumar wrote:

> On 10-08-15, 14:22, Lee Jones wrote:
> > >  Optional properties:
> > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > +  can support.
> > 
> > This isn't very generic.
> > 
> > I'm guessing some vendors my have quite a few ways to differentiate
> > between board versions/revisions/cuts etc.
> > 
> > How about another array where a vendor can choose to identify a piece
> > of hardware however they see fit.
> > 
> > Example 1 (simple version):
> > 
> > /* Version 1 */
> > opp-version = <1>;
> > 
> > Example 2 (using the kernel's versioning):
> > 
> > /* 2.6.32-rc1 */
> > opp-version = <2 6 32 1>;
> > 
> > Example 3 (using ST's versioning):
> > 
> > /* Major 2, Minor 0, Cut 2, All substrates */
> > opp-version = <2 0 2 0xff>;
> 
> But how will we parse this with generic code ?

Why would you want to?
Viresh Kumar Aug. 11, 2015, 10:09 a.m. UTC | #4 
On 11-08-15, 10:30, Lee Jones wrote:
> On Tue, 11 Aug 2015, Viresh Kumar wrote:
> 
> > On 10-08-15, 14:22, Lee Jones wrote:
> > > >  Optional properties:
> > > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > > +  can support.
> > > 
> > > This isn't very generic.
> > > 
> > > I'm guessing some vendors my have quite a few ways to differentiate
> > > between board versions/revisions/cuts etc.
> > > 
> > > How about another array where a vendor can choose to identify a piece
> > > of hardware however they see fit.
> > > 
> > > Example 1 (simple version):
> > > 
> > > /* Version 1 */
> > > opp-version = <1>;
> > > 
> > > Example 2 (using the kernel's versioning):
> > > 
> > > /* 2.6.32-rc1 */
> > > opp-version = <2 6 32 1>;
> > > 
> > > Example 3 (using ST's versioning):
> > > 
> > > /* Major 2, Minor 0, Cut 2, All substrates */
> > > opp-version = <2 0 2 0xff>;
> > 
> > But how will we parse this with generic code ?
> 
> Why would you want to?

So that individual platforms don't need to reinvent the wheel ?
Lee Jones Aug. 11, 2015, 11:54 a.m. UTC | #5 
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 10:30, Lee Jones wrote:
> > On Tue, 11 Aug 2015, Viresh Kumar wrote:
> > 
> > > On 10-08-15, 14:22, Lee Jones wrote:
> > > > >  Optional properties:
> > > > > +- opp-cuts: One or more strings, describing the versions of hardware the OPPs
> > > > > +  can support.
> > > > 
> > > > This isn't very generic.
> > > > 
> > > > I'm guessing some vendors my have quite a few ways to differentiate
> > > > between board versions/revisions/cuts etc.
> > > > 
> > > > How about another array where a vendor can choose to identify a piece
> > > > of hardware however they see fit.
> > > > 
> > > > Example 1 (simple version):
> > > > 
> > > > /* Version 1 */
> > > > opp-version = <1>;
> > > > 
> > > > Example 2 (using the kernel's versioning):
> > > > 
> > > > /* 2.6.32-rc1 */
> > > > opp-version = <2 6 32 1>;
> > > > 
> > > > Example 3 (using ST's versioning):
> > > > 
> > > > /* Major 2, Minor 0, Cut 2, All substrates */
> > > > opp-version = <2 0 2 0xff>;
> > > 
> > > But how will we parse this with generic code ?
> > 
> > Why would you want to?
> 
> So that individual platforms don't need to reinvent the wheel ?

The framework does not need to parse this information.  It is used
solely by the platform driver, whose job it is to decide which OPPs
are appropriate for the running platform.

Back to my question; how would you like arbitrary version information,
which means different things to different vendors to be used in
shared/generic/framework code?
Viresh Kumar Aug. 11, 2015, 12:01 p.m. UTC | #6 
On 11-08-15, 12:54, Lee Jones wrote:
> The framework does not need to parse this information.  It is used
> solely by the platform driver, whose job it is to decide which OPPs
> are appropriate for the running platform.

The OPP layer needs to parse OPP nodes in DT. But for doing that it
needs to know which OPPs to pick from the table as, in cases like
yours or qcom, not all OPPs might be available.

One of the ways to do that is:
- the platform reads its efuses (or whatever) and encodes the
  information into a string.
- This string should match with the strings present (somewhere) in the
  OPP table. That location can be like what I proposed few mails back.
- Then the *generic* OPP code can parse only those OPP nodes which
  match with that string.

This way, we can avoid pushing the platform code to parse OPP tables.

> Back to my question; how would you like arbitrary version information,
> which means different things to different vendors to be used in
> shared/generic/framework code?

Exactly like I wrote above. The platform just needs to give a string
that should match with the OPPs ..
Lee Jones Aug. 11, 2015, 1:27 p.m. UTC | #7 
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 12:54, Lee Jones wrote:
> > The framework does not need to parse this information.  It is used
> > solely by the platform driver, whose job it is to decide which OPPs
> > are appropriate for the running platform.
> 
> The OPP layer needs to parse OPP nodes in DT. But for doing that it
> needs to know which OPPs to pick from the table as, in cases like
> yours or qcom, not all OPPs might be available.
> 
> One of the ways to do that is:
> - the platform reads its efuses (or whatever) and encodes the
>   information into a string.
> - This string should match with the strings present (somewhere) in the
>   OPP table. That location can be like what I proposed few mails back.
> - Then the *generic* OPP code can parse only those OPP nodes which
>   match with that string.
> 
> This way, we can avoid pushing the platform code to parse OPP tables.

Okay, so what you're saying is that you've already made the decision
to create a separate node for every OPP permutation, despite the fact
that I've told you this could lead to more nodes than anyone would
care to successfully write or maintain?

Perhaps an example might help explain the issue.

Using the current driver, we need to place the following in DT and the
driver does the rest:

opp-list {
	opp1 {
		opp-hz = <1500000000>;
		st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
		st,substrate = <0xff>;
		st,cuts = <0xff>;
	};
	opp0 {
		opp-hz = <1200000000>;
		st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
		st,substrate = <0xff>;
		st,cuts = <0x2>;
	};
};

However, what you're suggesting, even for this very simple example
(imagine what this would look like with 5 or more frequencies where
two or more of them were only appropriate to run on particular
variants) requires this to be broken out to:

opp-list {
	pcode0-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1110000>;
		};
	};

	pcode0-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1200000>;
		};
	};

	pcode1-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1150000>;
		};
	};

	pcode1-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1200000>;
		};
	};

	pcode2-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1100000>;
		};
	};

	pcode2-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1200000>;
		};
	};

	pcode3-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1080000>;
		};
	};

	pcode3-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1200000>;
		};
	};

	pcode4-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1040000>;
		};

	pcode4-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1170000>;
		};
	};

	pcode5-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <1020000>;
		};
	};

	pcode5-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1140000>;
		};
	};

	pcode6-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <980000>;
		};
	};

	pcode6-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1100000>;
		};
	};

	pcode7-cut2-allsubstrates {
		opp0 {
			opp-hz = <1200000000>;
			opp-microvolt = <930000>;
		};
	};

	pcode7-allcuts-allsubstrates {
		opp0 {
			opp-hz = <1500000000>;
			opp-microvolt = <1070000>;
		};
	};
};


These will soon multiply once you start providing more complex
examples.  And how do you plan on handling this in the framework?  Can
the driver submit more than one variations of the string?  In the
current example the driver would need to submit four strings to
provide all acceptable variations; "pcodeX-cutY-substrateZ",
"pcodeX-allcuts-substrateZ", "pcodeX-cutY-allsubstrates" and
"pcodeX-allcuts-allsubstrates"
Viresh Kumar Aug. 11, 2015, 2:28 p.m. UTC | #8 
On 11-08-15, 14:27, Lee Jones wrote:
> Okay, so what you're saying is that you've already made the decision
> to create a separate node for every OPP permutation,

Absolutely not.

> despite the fact
> that I've told you this could lead to more nodes than anyone would
> care to successfully write or maintain?

I have enough fear of yours and then I have to see you in another
month as well. I wouldn't dare to disobey your command SIR :)

> Perhaps an example might help explain the issue.
> 
> Using the current driver, we need to place the following in DT and the
> driver does the rest:
> 
> opp-list {
> 	opp1 {
> 		opp-hz = <1500000000>;
> 		st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> 		st,substrate = <0xff>;
> 		st,cuts = <0xff>;
> 	};
> 	opp0 {
> 		opp-hz = <1200000000>;
> 		st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> 		st,substrate = <0xff>;
> 		st,cuts = <0x2>;
> 	};
> };

Nothing is fixed as of now but this is what I am thinking of:

	cpu0_opp_table: opp_table0 {
		compatible = "operating-points-v2";
                opp-cuts = "10", "3c", "f0";
		supply-names = "vcc0", "vcc1", "vcc2";
		opp-shared;

		opp00 {
			opp-hz = /bits/ 64 <1000000000>;
			clock-latency-ns = <300000>;
			opp-microvolt-10 = <970000>;
			opp-microvolt-3c = <950000>;
			opp-microvolt-f0 = <930000>;
		};

		/* OR */

		opp00 {
			opp-hz = /bits/ 64 <1000000000>;
			clock-latency-ns = <300000>;
                        opp-microvolt = <970000>, <950000>, <930000>;
		};
        };

And then the platform code needs to tell OPP layer:
"Use OPPs for cut f0 for device X", and OPP layer will store that
somewhere.

And then it will only initialize OPPs after matching this string with
the values.

Out of the earlier two options, I may prefer the first one. As we will
be soon adding support for multiple regulators, and a single regulator
can have min/max/target values.. So, a single list will become too
long.

But, something like this should be generic enough to capture most of
the cases.

@Stephen/Rob ??
Lee Jones Aug. 11, 2015, 3:17 p.m. UTC | #9 
On Tue, 11 Aug 2015, Viresh Kumar wrote:
> On 11-08-15, 14:27, Lee Jones wrote:
> > Okay, so what you're saying is that you've already made the decision
> > to create a separate node for every OPP permutation,
> 
> Absolutely not.
> 
> > despite the fact
> > that I've told you this could lead to more nodes than anyone would
> > care to successfully write or maintain?
> 
> I have enough fear of yours and then I have to see you in another
> month as well. I wouldn't dare to disobey your command SIR :)

Funny guy! ;)

> > Perhaps an example might help explain the issue.
> > 
> > Using the current driver, we need to place the following in DT and the
> > driver does the rest:
> > 
> > opp-list {
> > 	opp1 {
> > 		opp-hz = <1500000000>;
> > 		st,avs = <1200 1200 1200 1200 1170 1140 1100 1070>;
> > 		st,substrate = <0xff>;
> > 		st,cuts = <0xff>;
> > 	};
> > 	opp0 {
> > 		opp-hz = <1200000000>;
> > 		st,avs = <1110 1150 1100 1080 1040 1020 980 930>;
> > 		st,substrate = <0xff>;
> > 		st,cuts = <0x2>;
> > 	};
> > };
> 
> Nothing is fixed as of now but this is what I am thinking of:
> 
> 	cpu0_opp_table: opp_table0 {
> 		compatible = "operating-points-v2";
>                 opp-cuts = "10", "3c", "f0";
> 		supply-names = "vcc0", "vcc1", "vcc2";
> 		opp-shared;
> 
> 		opp00 {
> 			opp-hz = /bits/ 64 <1000000000>;
> 			clock-latency-ns = <300000>;
> 			opp-microvolt-10 = <970000>;
> 			opp-microvolt-3c = <950000>;
> 			opp-microvolt-f0 = <930000>;
> 		};
> 
> 		/* OR */
> 
> 		opp00 {
> 			opp-hz = /bits/ 64 <1000000000>;
> 			clock-latency-ns = <300000>;
>                         opp-microvolt = <970000>, <950000>, <930000>;
> 		};
>         };
> 
> And then the platform code needs to tell OPP layer:
> "Use OPPs for cut f0 for device X", and OPP layer will store that
> somewhere.
> 
> And then it will only initialize OPPs after matching this string with
> the values.
> 
> Out of the earlier two options, I may prefer the first one. As we will
> be soon adding support for multiple regulators, and a single regulator
> can have min/max/target values.. So, a single list will become too
> long.

This would work if we only had a single variable to contend with, but
what I showed you in my previous example is that we have 3 variables
to consider; cut (version), pcode and substrate.

Using the two (simple) examples I provided, how would your suggestion
look in our case?

> But, something like this should be generic enough to capture most of
> the cases.
> 
> @Stephen/Rob ??
>
Viresh Kumar Aug. 12, 2015, 11:08 a.m. UTC | #10 
On 11-08-15, 16:17, Lee Jones wrote:
> This would work if we only had a single variable to contend with, but
> what I showed you in my previous example is that we have 3 variables
> to consider; cut (version), pcode and substrate.
> 
> Using the two (simple) examples I provided, how would your suggestion
> look in our case?

So the solution I gave is for picking the microvolt based on pcode.
The other two (cut, substrate) aren't about picking microvolt, but if
the OPP is available or not. Right?

If these terms are generic enough, then we can add something similar
to what you have added..

@Stephen ?
Lee Jones Aug. 26, 2015, 12:06 p.m. UTC | #11 
On Wed, 12 Aug 2015, Viresh Kumar wrote:

> On 11-08-15, 16:17, Lee Jones wrote:
> > This would work if we only had a single variable to contend with, but
> > what I showed you in my previous example is that we have 3 variables
> > to consider; cut (version), pcode and substrate.
> > 
> > Using the two (simple) examples I provided, how would your suggestion
> > look in our case?
> 
> So the solution I gave is for picking the microvolt based on pcode.
> The other two (cut, substrate) aren't about picking microvolt, but if
> the OPP is available or not. Right?

'pcode', 'cut' and 'substrate' all determine whether a given set of
OPPs an be used on the running platform.  I do not believe that you
can differentiate between them. 

> If these terms are generic enough, then we can add something similar
> to what you have added..

If it makes it easier, you can treat them as version numbers 2.2.1
<pcode.cut.substrate>, but I don't see how this can help.  Obviously
this becomes more difficult when you add wild cards to the OPPs, where
a particular OPP would be suitable for all cuts for example.

If you still think you can come up with a generic method to lay out
CPUFreq OPP nodes that will satisfy all vendors and not be a mass of
10's of separate nodes, then great.  Again, I'm struggling to see how
that might be possible.

What I believe we shouldn't do, is have this blocked forever for the
sake of adding a couple of vendor properties however.
Viresh Kumar Sept. 2, 2015, 8:06 a.m. UTC | #12 
On 26-08-15, 13:06, Lee Jones wrote:
> On Wed, 12 Aug 2015, Viresh Kumar wrote:
> 
> > On 11-08-15, 16:17, Lee Jones wrote:
> > > This would work if we only had a single variable to contend with, but
> > > what I showed you in my previous example is that we have 3 variables
> > > to consider; cut (version), pcode and substrate.
> > > 
> > > Using the two (simple) examples I provided, how would your suggestion
> > > look in our case?
> > 
> > So the solution I gave is for picking the microvolt based on pcode.
> > The other two (cut, substrate) aren't about picking microvolt, but if
> > the OPP is available or not. Right?
> 
> 'pcode', 'cut' and 'substrate' all determine whether a given set of
> OPPs an be used on the running platform.  I do not believe that you
> can differentiate between them. 
> 
> > If these terms are generic enough, then we can add something similar
> > to what you have added..
> 
> If it makes it easier, you can treat them as version numbers 2.2.1
> <pcode.cut.substrate>, but I don't see how this can help.  Obviously
> this becomes more difficult when you add wild cards to the OPPs, where
> a particular OPP would be suitable for all cuts for example.
> 
> If you still think you can come up with a generic method to lay out
> CPUFreq OPP nodes that will satisfy all vendors and not be a mass of
> 10's of separate nodes, then great.  Again, I'm struggling to see how
> that might be possible.
> 
> What I believe we shouldn't do, is have this blocked forever for the
> sake of adding a couple of vendor properties however.

I agree and can understand the pain you are feeling..

@Rob/Stephen: Please close this thread soon and let Lee get his work
done :)
Rob Herring Sept. 2, 2015, 6:58 p.m. UTC | #13 
On Wed, Sep 2, 2015 at 3:06 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 26-08-15, 13:06, Lee Jones wrote:
>> On Wed, 12 Aug 2015, Viresh Kumar wrote:
>>
>> > On 11-08-15, 16:17, Lee Jones wrote:
>> > > This would work if we only had a single variable to contend with, but
>> > > what I showed you in my previous example is that we have 3 variables
>> > > to consider; cut (version), pcode and substrate.
>> > >
>> > > Using the two (simple) examples I provided, how would your suggestion
>> > > look in our case?
>> >
>> > So the solution I gave is for picking the microvolt based on pcode.
>> > The other two (cut, substrate) aren't about picking microvolt, but if
>> > the OPP is available or not. Right?
>>
>> 'pcode', 'cut' and 'substrate' all determine whether a given set of
>> OPPs an be used on the running platform.  I do not believe that you
>> can differentiate between them.
>>
>> > If these terms are generic enough, then we can add something similar
>> > to what you have added..
>>
>> If it makes it easier, you can treat them as version numbers 2.2.1
>> <pcode.cut.substrate>, but I don't see how this can help.  Obviously
>> this becomes more difficult when you add wild cards to the OPPs, where
>> a particular OPP would be suitable for all cuts for example.
>>
>> If you still think you can come up with a generic method to lay out
>> CPUFreq OPP nodes that will satisfy all vendors and not be a mass of
>> 10's of separate nodes, then great.  Again, I'm struggling to see how
>> that might be possible.
>>
>> What I believe we shouldn't do, is have this blocked forever for the
>> sake of adding a couple of vendor properties however.
>
> I agree and can understand the pain you are feeling..
>
> @Rob/Stephen: Please close this thread soon and let Lee get his work
> done :)

What do you expect here? It is your job to close it. Ultimately, this
will be your problem to deal with. If you have 10 different vendors
doing selection of OPPs in 10 different ways you will not be able to
change that easily later. Maybe if you can't come up with something
common, then this should just not go into DT. You can always look at
how to do this in a common way and move from the kernel to DT later.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 0cb44dc21f97..bad7a8299b9c 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -74,6 +74,8 @@  This describes the OPPs belonging to a device. This node can have following
   reference an OPP.
 
 Optional properties:
+- opp-cuts: One or more strings, describing the versions of hardware the OPPs
+  can support.
 - opp-shared: Indicates that device nodes using this OPP Table Node's phandle
   switch their DVFS state together, i.e. they share clock/voltage/current lines.
   Missing property means devices have independent clock/voltage/current lines,
@@ -100,6 +102,9 @@  properties.
   Entries for multiple regulators must be present in the same order as
   regulators are specified in device's DT node.
 
+  If used with 'opp-cuts', then the number of entries present here must match
+  the number of strings present in 'opp-cuts'.
+
 - opp-microamp: The maximum current drawn by the device in microamperes
   considering system specific parameters (such as transients, process, aging,
   maximum operating temperature range etc.) as necessary. This may be used to