| Message ID | 263c128844f5a3c9280c8be71f6c9eb1869a5188.1433434659.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, Jun 4, 2015 at 11:20 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On some platforms (Like Qualcomm's SoCs), it is not decided until > runtime on what OPPs to use. The OPP tables can be fixed at compile > time, but which table to use is found out only after reading some efuses > (sort of an prom) and knowing characteristics of the SoC. > > To support such platform we need to pass multiple OPP tables per device > and hardware should be able to choose one and only one table out of > those. > > Update OPP-v2 bindings to support that. > > Acked-by: Nishanth Menon <nm@ti.com> > Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > Documentation/devicetree/bindings/power/opp.txt | 52 +++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt > index 259bf00edf7d..2938c52dbf84 100644 > --- a/Documentation/devicetree/bindings/power/opp.txt > +++ b/Documentation/devicetree/bindings/power/opp.txt > @@ -45,6 +45,9 @@ 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. > > +Devices may want to choose OPP tables at runtime and so can provide a list of > +phandles here. But only *one* of them should be chosen at runtime. > + > If required, this can be extended for SoC vendor specfic 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>". > @@ -63,6 +66,9 @@ This describes the OPPs belonging to a device. This node can have following > reference an OPP. > > Optional properties: > +- opp-name: Name of the OPP table, to uniquely identify it if more than one OPP > + table is supplied in "operating-points-v2" property of device. > + > - 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, > @@ -396,3 +402,49 @@ Example 4: Handling multiple regulators > }; > }; > }; > + > +Example 5: Multiple OPP tables > + > +/ { > + cpus { > + cpu@0 { > + compatible = "arm,cortex-a7"; > + ... > + > + cpu-supply = <&cpu_supply> > + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; You've made a fundamental change here in that this can now be a list of phandles. There should be some description on what a list means (merge the tables?, select one?). I think this needs to have a defined order and the platform should know what that is. For example, if you read the efuses and decide you need the "slow" table, you know to pick the first entry. Then you don't need opp-name. Does that work for QCom? 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
On 17-06-15, 08:23, Rob Herring wrote: > > + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; > > You've made a fundamental change here in that this can now be a list > of phandles. There should be some description on what a list means > (merge the tables?, select one?). Did you miss the description I wrote few lines earlier or are you asking for something else? This is what I wrote earlier: > > +Devices may want to choose OPP tables at runtime and so can provide a list of > > +phandles here. But only *one* of them should be chosen at runtime. So, clearly only ONE of the tables should be used. > I think this needs to have a defined order and the platform should > know what that is. For example, if you read the efuses and decide you > need the "slow" table, you know to pick the first entry. Then you > don't need opp-name. Does that work for QCom? Why forcing on the order here? For example, consider a case where the platform can have four tables, A B C D. Now DT is free to pass all four or just a subset of those. Like, for some boards table B doesn't stand valid. And so it may wanna pass just A C D. And so keeping these tables in order is going to break for sure. Flexibility is probably better in this case.
On Wed, Jun 17, 2015 at 8:33 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 17-06-15, 08:23, Rob Herring wrote: >> > + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; >> >> You've made a fundamental change here in that this can now be a list >> of phandles. There should be some description on what a list means >> (merge the tables?, select one?). > > Did you miss the description I wrote few lines earlier or are you > asking for something else? This is what I wrote earlier: > >> > +Devices may want to choose OPP tables at runtime and so can provide a list of >> > +phandles here. But only *one* of them should be chosen at runtime. > > So, clearly only ONE of the tables should be used. Yes, never mind. > >> I think this needs to have a defined order and the platform should >> know what that is. For example, if you read the efuses and decide you >> need the "slow" table, you know to pick the first entry. Then you >> don't need opp-name. Does that work for QCom? > > Why forcing on the order here? For example, consider a case where the > platform can have four tables, A B C D. Now DT is free to pass all > four or just a subset of those. Like, for some boards table B doesn't > stand valid. And so it may wanna pass just A C D. And so keeping these > tables in order is going to break for sure. Flexibility is probably > better in this case. Defined order is a key part of DT bindings. We solve the variable length problem with name lists associated with variable length property like: operating-point-names = "slow", "fast"; I'm not a fan of doing this if we can avoid it, but we should at least follow the same pattern. Don't send me a patch with that yet, I want to hear from Stephen. You can also use "status" to disable specific tables rather than removing from the list. 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
On 17-06-15, 08:47, Rob Herring wrote: > Defined order is a key part of DT bindings. We solve the variable > length problem with name lists associated with variable length > property like: > > operating-point-names = "slow", "fast"; I agree that we should have used this.. > I'm not a fan of doing this if we can avoid it, but we should at least > follow the same pattern. Don't send me a patch with that yet, I want > to hear from Stephen. Good that you told me to stop :) > You can also use "status" to disable specific tables rather than > removing from the list. Hmmm, right. That's far better.
On 06/17/2015 06:47 AM, Rob Herring wrote: > On Wed, Jun 17, 2015 at 8:33 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: >> On 17-06-15, 08:23, Rob Herring wrote: >>>> + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; >>> You've made a fundamental change here in that this can now be a list >>> of phandles. There should be some description on what a list means >>> (merge the tables?, select one?). >> Did you miss the description I wrote few lines earlier or are you >> asking for something else? This is what I wrote earlier: >> >>>> +Devices may want to choose OPP tables at runtime and so can provide a list of >>>> +phandles here. But only *one* of them should be chosen at runtime. >> So, clearly only ONE of the tables should be used. > Yes, never mind. > >>> I think this needs to have a defined order and the platform should >>> know what that is. For example, if you read the efuses and decide you >>> need the "slow" table, you know to pick the first entry. Then you >>> don't need opp-name. Does that work for QCom? >> Why forcing on the order here? For example, consider a case where the >> platform can have four tables, A B C D. Now DT is free to pass all >> four or just a subset of those. Like, for some boards table B doesn't >> stand valid. And so it may wanna pass just A C D. And so keeping these >> tables in order is going to break for sure. Flexibility is probably >> better in this case. > Defined order is a key part of DT bindings. We solve the variable > length problem with name lists associated with variable length > property like: > > operating-point-names = "slow", "fast"; > > I'm not a fan of doing this if we can avoid it, but we should at least > follow the same pattern. Don't send me a patch with that yet, I want > to hear from Stephen. > > You can also use "status" to disable specific tables rather than > removing from the list. > An operating-point(s?)-names property seems ok... but doesn't that mean that every CPU that uses the OPP has to have the same list of operating-point-names? It would make sense to me if the operating points were called something different depending on *which* CPU is using them, but in this case the only name for the operating point is "slow" or "fast", etc. In reality we've assigned them names like speedX-binY-vZ so that we know which speed bin, voltage bin, and version they're part of. Maybe OPP node properties like qcom,speed-bin = <u32>, qcom,pvs-bin = <u32>, etc. would be better? It would make the parsing code slightly more complicated because it needs to look for 2 or 3 properties instead of a name property though. Or would having different node names be acceptable? That would avoid this list of strings. At the least, operating-points-names will be required on qcom platforms. A fixed ordering known to the platform would mean that we know exactly how many voltage bins and speed bins and how many voltage bins per speed bin are used for a particular SoC, which we've avoided knowing so far.
On 17-06-15, 18:30, Stephen Boyd wrote: > An operating-point(s?)-names property seems ok... but doesn't that mean > that every CPU that uses the OPP has to have the same list of > operating-point-names? Why do you think so? For me the operating-points-v2-names property will be present in CPU node (as there is no OPP node which can have it) and so every CPU is free to choose what it wants to. > It would make sense to me if the operating points > were called something different depending on *which* CPU is using them, > but in this case the only name for the operating point is "slow" or > "fast", etc. I am completely confused now. :) The problem you stated now was there with the current state of bindings. The name is embedded into the OPP table node and so is fixed for all the CPUs. Moving it to the CPU node will give all CPUs a chance to name it whatever they want to. And the same list has to be replicated to all CPUs sharing the clock rails. > In reality we've assigned them names like speedX-binY-vZ so that we know > which speed bin, voltage bin, and version they're part of. Maybe OPP > node properties like qcom,speed-bin = <u32>, qcom,pvs-bin = <u32>, etc. > would be better? Lets see, only if we can't get the generic stuff for this. > At the least, operating-points-names will be required on qcom platforms. > A fixed ordering known to the platform would mean that we know exactly > how many voltage bins and speed bins and how many voltage bins per speed > bin are used for a particular SoC, which we've avoided knowing so far. What are we referring to fixed ordering? If we have both a list of phandles to OPP tables and a list of names, they can be rearranged in whatever fashion we want. Isn't it?
On 06/18, Viresh Kumar wrote: > On 17-06-15, 18:30, Stephen Boyd wrote: > > An operating-point(s?)-names property seems ok... but doesn't that mean > > that every CPU that uses the OPP has to have the same list of > > operating-point-names? > > Why do you think so? For me the operating-points-v2-names property > will be present in CPU node (as there is no OPP node which can have > it) and so every CPU is free to choose what it wants to. Yes. > > > It would make sense to me if the operating points > > were called something different depending on *which* CPU is using them, > > but in this case the only name for the operating point is "slow" or > > "fast", etc. > > I am completely confused now. :) > As am I. > The problem you stated now was there with the current state of > bindings. The name is embedded into the OPP table node and so is fixed > for all the CPUs. Moving it to the CPU node will give all CPUs a > chance to name it whatever they want to. And the same list has to be > replicated to all CPUs sharing the clock rails. > Yes I don't see how the name will be different for any CPU, hence my complaint/question about duplicate names in each CPU. I guess it isn't any worse than clock-names though so I'm fine with it. > > In reality we've assigned them names like speedX-binY-vZ so that we know > > which speed bin, voltage bin, and version they're part of. Maybe OPP > > node properties like qcom,speed-bin = <u32>, qcom,pvs-bin = <u32>, etc. > > would be better? > > Lets see, only if we can't get the generic stuff for this. > > > At the least, operating-points-names will be required on qcom platforms. > > A fixed ordering known to the platform would mean that we know exactly > > how many voltage bins and speed bins and how many voltage bins per speed > > bin are used for a particular SoC, which we've avoided knowing so far. > > What are we referring to fixed ordering? If we have both a list of > phandles to OPP tables and a list of names, they can be rearranged in > whatever fashion we want. Isn't it? > This is a reply to Rob's question about fixed ordering without a names property. That's unlikely to work out for qcom chips.
On 19-06-15, 11:44, Stephen Boyd wrote: > On 06/18, Viresh Kumar wrote: > > The problem you stated now was there with the current state of > > bindings. The name is embedded into the OPP table node and so is fixed > > for all the CPUs. Moving it to the CPU node will give all CPUs a > > chance to name it whatever they want to. And the same list has to be > > replicated to all CPUs sharing the clock rails. > > > > Yes I don't see how the name will be different for any CPU, hence > my complaint/question about duplicate names in each CPU. I guess > it isn't any worse than clock-names though so I'm fine with it. So what I wrote about the string being same for all CPUs, is valid only to CPUs sharing clock line and hence OPPs. If there are CPUs with independent lines, like in Krait, then the CPUs are free to choose whatever name they want for the OPP tables, even if they are sharing the same tables.
diff --git a/Documentation/devicetree/bindings/power/opp.txt b/Documentation/devicetree/bindings/power/opp.txt index 259bf00edf7d..2938c52dbf84 100644 --- a/Documentation/devicetree/bindings/power/opp.txt +++ b/Documentation/devicetree/bindings/power/opp.txt @@ -45,6 +45,9 @@ 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. +Devices may want to choose OPP tables at runtime and so can provide a list of +phandles here. But only *one* of them should be chosen at runtime. + If required, this can be extended for SoC vendor specfic 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>". @@ -63,6 +66,9 @@ This describes the OPPs belonging to a device. This node can have following reference an OPP. Optional properties: +- opp-name: Name of the OPP table, to uniquely identify it if more than one OPP + table is supplied in "operating-points-v2" property of device. + - 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, @@ -396,3 +402,49 @@ Example 4: Handling multiple regulators }; }; }; + +Example 5: Multiple OPP tables + +/ { + cpus { + cpu@0 { + compatible = "arm,cortex-a7"; + ... + + cpu-supply = <&cpu_supply> + operating-points-v2 = <&cpu0_opp_table_slow>, <&cpu0_opp_table_fast>; + }; + }; + + cpu0_opp_table_slow: opp_table_slow { + compatible = "operating-points-v2"; + opp-name = "slow"; + opp-shared; + + opp00 { + opp-hz = <600000000>; + ... + }; + + opp01 { + opp-hz = <800000000>; + ... + }; + }; + + cpu0_opp_table_fast: opp_table_fast { + compatible = "operating-points-v2"; + opp-name = "fast"; + opp-shared; + + opp10 { + opp-hz = <1000000000>; + ... + }; + + opp11 { + opp-hz = <1100000000>; + ... + }; + }; +};