[02/16] PM / OPP: Add 'opp-microvolt-triplets' binding

Message ID 254dd353453a495bb0921fa23e430428484f36fa.1441972771.git.viresh.kumar@linaro.org
State New
Headers show

Commit Message

Viresh Kumar Sept. 11, 2015, 12:01 p.m.
If 'opp-microvolt' is used to specify values for multiple regulators,
then we need this additional information to know if the values passed
should be treated as <target> or <target min max>.

This is because, DT doesn't differentiate between these two styles:

prop = <x>, <y>, <z>;
prop = <x y z>;

Cc: devicetree@vger.kernel.org
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Rob Herring Sept. 14, 2015, 8:30 p.m. | #1
On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> If 'opp-microvolt' is used to specify values for multiple regulators,
> then we need this additional information to know if the values passed
> should be treated as <target> or <target min max>.

Can't you determine this implicitly from # of cells / # of regulators
being either 1 or 3?

> 
> This is because, DT doesn't differentiate between these two styles:
> 
> prop = <x>, <y>, <z>;
> prop = <x y z>;
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 8759bc4783ed..719603b87353 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -76,6 +76,16 @@ This describes the OPPs belonging to a device. This node can have following
>    present here is important, as that should match the order in which values are
>    present in 'opp-microvolt' and 'opp-microamp' properties.
>  
> +- opp-microvolt-triplets: This is a required property, only if multiple supplies
> +  are available for the device. Otherwise it is ignored.
> +
> +  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
> +  power-supply. 'opp-microvolt-triplets' property is used to find the form in
> +  which 'opp-microvolt' is present.
> +
> +  If present, then 'opp-microvolt' must be present in <target min max> form,
> +  else in <target> form.

This implies that bindings currently with triplets are wrong since they
will be missing opp-microvolt-triplets.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar Sept. 15, 2015, 3:30 a.m. | #2
[+Cc Mark, I thought I cc'd him earlier, but no, I cc'd him only for
the first patch]

On 14-09-15, 15:30, Rob Herring wrote:
> On 09/11/2015 07:01 AM, Viresh Kumar wrote:
> > If 'opp-microvolt' is used to specify values for multiple regulators,
> > then we need this additional information to know if the values passed
> > should be treated as <target> or <target min max>.
> 
> Can't you determine this implicitly from # of cells / # of regulators
> being either 1 or 3?

I thought the #<name>-cells property is used to pass arguments along with the
phandle, so something like this:

supply0: regulator@f8000000 {
        regulator-cells = 1;
        ...
}

cpu@1 {
        cpu-supply = <&supply0 XYZ>
}

But if we can define something like:

supply0: regulator@f8000000 {
        regulator-cells or microvolt-cells = 1 or 3;
        ...
}

And then do:

cpu@1 {
        cpu-supply = <&supply0>
        operating-points-v2 = "&opp-table";
}

opp-table: table {

        ...

        opp0 {
                opp-hz = ...;
                ...
                opp-microvolt = <one or three values here>
        }
}

then it will be very simple. Also, this would mean that with multiple
regulators, we can have one regulator supporting single microvolts
value and other supporting tar/min/max values..

> > 
> > This is because, DT doesn't differentiate between these two styles:
> > 
> > prop = <x>, <y>, <z>;
> > prop = <x y z>;
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/opp/opp.txt | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> > index 8759bc4783ed..719603b87353 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -76,6 +76,16 @@ This describes the OPPs belonging to a device. This node can have following
> >    present here is important, as that should match the order in which values are
> >    present in 'opp-microvolt' and 'opp-microamp' properties.
> >  
> > +- opp-microvolt-triplets: This is a required property, only if multiple supplies
> > +  are available for the device. Otherwise it is ignored.
> > +
> > +  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
> > +  power-supply. 'opp-microvolt-triplets' property is used to find the form in
> > +  which 'opp-microvolt' is present.
> > +
> > +  If present, then 'opp-microvolt' must be present in <target min max> form,
> > +  else in <target> form.
> 
> This implies that bindings currently with triplets are wrong since they
> will be missing opp-microvolt-triplets.

Yeah, the bindings are incomplete, but there are no users yet, which
need the triplet thing. So, its all working just fine.
Mark Brown Sept. 19, 2015, 3:39 p.m. | #3
On Tue, Sep 15, 2015 at 09:00:27AM +0530, Viresh Kumar wrote:

> [+Cc Mark, I thought I cc'd him earlier, but no, I cc'd him only for
> the first patch]

I'm reading this on a plane so have no other context and to be honest
I'm struggling to understand what is being discussed here.  It would be
really helpful if you were to describe in words what proposed bindings
are intended to do as well as presenting examples, the examples by
themselves require the reader to reverse engineer what the semantics are
intended to be.

> But if we can define something like:

> supply0: regulator@f8000000 {
>         regulator-cells or microvolt-cells = 1 or 3;
>         ...
> }

As far as I can tell this is proposing adding something to the regulator
binding specifying if users must present either a single value or a
min/target/max triplet.  This is obviously problematic since regulators
can be shared - the needs of one user may not match the needs of another
user, and of course most users should not be specifying voltages at all
in the device tree in the first place.

Patch hide | download patch | download mbox

diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 8759bc4783ed..719603b87353 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -76,6 +76,16 @@  This describes the OPPs belonging to a device. This node can have following
   present here is important, as that should match the order in which values are
   present in 'opp-microvolt' and 'opp-microamp' properties.
 
+- opp-microvolt-triplets: This is a required property, only if multiple supplies
+  are available for the device. Otherwise it is ignored.
+
+  'opp-microvolt' can be present in two forms: <target> or <target min max>, per
+  power-supply. 'opp-microvolt-triplets' property is used to find the form in
+  which 'opp-microvolt' is present.
+
+  If present, then 'opp-microvolt' must be present in <target min max> form,
+  else in <target> form.
+
 - OPP nodes: One or more OPP nodes describing voltage-current-frequency
   combinations. Their name isn't significant but their phandle can be used to
   reference an OPP.
@@ -403,6 +413,8 @@  Example 4: Handling multiple regulators
 
 		/* OR */
 
+		opp-microvolt-triplets;
+
 		opp00 {
 			opp-hz = /bits/ 64 <1000000000>;
 			opp-microvolt = <970000 975000 985000>, /* Supply 0 */
@@ -416,6 +428,8 @@  Example 4: Handling multiple regulators
 
 		/* OR */
 
+		opp-microvolt-triplets;
+
 		opp00 {
 			opp-hz = /bits/ 64 <1000000000>;
 			opp-microvolt = <970000 975000 985000>, /* Supply 0 */