Hi Guys,
This is rebased over following series that adds debugfs support to OPP core: http://marc.info/?i=cover.1441354424.git.viresh.kumar%40linaro.org
This series extends V2 bindings support further to make it usable to most of the platforms.
[1-2] update the bindings a bit to get them working for multiple regulators case.
[3-4] cleanups. [5-7] Multiple regulator support [8-16] OPP transition support, so that the user drivers can directly ask to switch device to a particular OPP, instead of them dealing with the complexity of handling clocks and voltages.
I have also got cpufreq-dt driver updated to work with the new bindings, but holded-off those changes to keep this series smaller. Those were another Nine patches.
For curious developers/reviewers, all required code (debugfs, this and cpufreq-dt) is pushed to:
https://git.linaro.org/people/viresh.kumar/linux.git opp/multi-regulator-v1
Please help in getting this reviewed :)
Viresh Kumar (16): PM / OPP: Add 'supply-names' binding PM / OPP: Add 'opp-microvolt-triplets' binding PM / OPP: Improve debug print messages with pr_fmt PM / OPP: Rename routines specific to old bindings with _v1 PM / OPP: Parse all power-supply related bindings together PM / OPP: Create separate structure for regulator/supplies PM / OPP: Add multiple regulators support PM / OPP: get/put regulators from OPP core PM / OPP: Disable OPPs that aren't supported by the regulators PM / OPP: Introduce dev_pm_opp_get_max_volt_latency() PM / OPP: Introduce dev_pm_opp_get_max_transition_latency() PM / OPP: Parse clock and voltage tolerance for v1 bindings PM / OPP: Manage device clk as well PM / OPP: Add dev_pm_opp_set_regulator() to specify regulator PM / OPP: Add dev_pm_opp_set_rate() PM / OPP: don't print error message for deferred probing
Documentation/devicetree/bindings/opp/opp.txt | 40 +- drivers/base/power/opp/core.c | 637 +++++++++++++++++++++++--- drivers/base/power/opp/cpu.c | 8 +- drivers/base/power/opp/debugfs.c | 52 ++- drivers/base/power/opp/opp.h | 44 +- include/linux/pm_opp.h | 25 + 6 files changed, 722 insertions(+), 84 deletions(-)
Regulators already have stable DT bindings, wherein the consumer (of supplies) will have following for each regulator/supply.
<name>-supply: <phandle to the regulator node>;
Current OPP bindings extend above, by transforming it into a list of phandles. But we missed the <name> string, which is used to identify the regulator.
And looking from regulators perspective, having two different ways of specifying regulators doesn't seem like a step forward, it also means we have to update every single device binding. And things will become complex.
Another way to support multiple regulators per device (in OPP V2 bindings) is to leave regulator consumer bindings as is, and create a 'supply-names' property in the opp-table node, which will contain a list of strings. The names in this list shall match 'name' from the '<name>-supply' strings present in the device node.
The strings in this list also specify the order in which values must be present in 'opp-microvolt' and 'opp-microamp' properties.
Cc: Mark Brown broonie@kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 0cb44dc21f97..8759bc4783ed 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following - compatible: Allow OPPs to express their compatibility. It should be: "operating-points-v2".
+- supply-names: This is a required property, only if multiple supplies are + available for the device. Otherwise it is optional. + + This list is used to pass names of all the device supplies. The order of names + present here is important, as that should match the order in which values are + present in 'opp-microvolt' and 'opp-microamp' properties. + - 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. @@ -97,8 +104,8 @@ properties. Single entry is for target voltage and three entries are for <target min max> voltages.
- Entries for multiple regulators must be present in the same order as - regulators are specified in device's DT node. + Entries for multiple regulators must be present in the same order as their + names are present in 'supply-names' property of the opp-table.
- opp-microamp: The maximum current drawn by the device in microamperes considering system specific parameters (such as transients, process, aging, @@ -107,10 +114,12 @@ properties.
Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as - regulators are specified in device's DT node. If this property isn't required - for few regulators, then this should be marked as zero for them. If it isn't - required for any regulator, then this property need not be present. + Entries for multiple regulators must be present in the same order as their + names are present in 'supply-names' property of the opp-table. + + If this property isn't required for few regulators, then this should be marked + as zero for them. If it isn't required for any regulator, then this property + need not be present.
- clock-latency-ns: Specifies the maximum possible transition latency (in nanoseconds) for switching to this OPP from any other OPP. @@ -369,13 +378,16 @@ Example 4: Handling multiple regulators compatible = "arm,cortex-a7"; ...
- cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>; + vcc0-supply = <&cpu_supply0>; + vcc1-supply = <&cpu_supply1>; + vcc2-supply = <&cpu_supply2>; operating-points-v2 = <&cpu0_opp_table>; }; };
cpu0_opp_table: opp_table0 { compatible = "operating-points-v2"; + supply-names = "vcc0", "vcc1", "vcc2"; opp-shared;
opp00 {
On 09/11/2015 07:01 AM, Viresh Kumar wrote:
Regulators already have stable DT bindings, wherein the consumer (of supplies) will have following for each regulator/supply.
<name>-supply: <phandle to the regulator node>;
Current OPP bindings extend above, by transforming it into a list of phandles. But we missed the <name> string, which is used to identify the regulator.
And looking from regulators perspective, having two different ways of specifying regulators doesn't seem like a step forward, it also means we have to update every single device binding. And things will become complex.
Another way to support multiple regulators per device (in OPP V2 bindings) is to leave regulator consumer bindings as is, and create a 'supply-names' property in the opp-table node, which will contain a list of strings. The names in this list shall match 'name' from the '<name>-supply' strings present in the device node.
The strings in this list also specify the order in which values must be present in 'opp-microvolt' and 'opp-microamp' properties.
Cc: Mark Brown broonie@kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 0cb44dc21f97..8759bc4783ed 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following
- compatible: Allow OPPs to express their compatibility. It should be: "operating-points-v2".
+- supply-names: This is a required property, only if multiple supplies are
- available for the device. Otherwise it is optional.
- This list is used to pass names of all the device supplies. The order of names
- present here is important, as that should match the order in which values are
- present in 'opp-microvolt' and 'opp-microamp' properties.
What if we have a 2nd device and supply rail? For example, what if the L2$ has a separate rail from the cores but is linked to the OPPs.
- 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.
@@ -97,8 +104,8 @@ properties. Single entry is for target voltage and three entries are for <target min max> voltages.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node.
- Entries for multiple regulators must be present in the same order as their
- names are present in 'supply-names' property of the opp-table.
- opp-microamp: The maximum current drawn by the device in microamperes considering system specific parameters (such as transients, process, aging,
@@ -107,10 +114,12 @@ properties. Should only be set if opp-microvolt is set for the OPP.
- Entries for multiple regulators must be present in the same order as
- regulators are specified in device's DT node. If this property isn't required
- for few regulators, then this should be marked as zero for them. If it isn't
- required for any regulator, then this property need not be present.
- Entries for multiple regulators must be present in the same order as their
- names are present in 'supply-names' property of the opp-table.
- If this property isn't required for few regulators, then this should be marked
- as zero for them. If it isn't required for any regulator, then this property
- need not be present.
Remind me of when do we have multiple regulators for a cpu? The number of supplies should be defined by the cpu binding as function of how many supply rails a cpu has.
- clock-latency-ns: Specifies the maximum possible transition latency (in nanoseconds) for switching to this OPP from any other OPP.
@@ -369,13 +378,16 @@ Example 4: Handling multiple regulators compatible = "arm,cortex-a7"; ...
cpu-supply = <&cpu_supply0>, <&cpu_supply1>, <&cpu_supply2>;
vcc0-supply = <&cpu_supply0>;vcc1-supply = <&cpu_supply1>;vcc2-supply = <&cpu_supply2>;
This may just be an example, but a CA7 doesn't have 3 supply rails.
Rob
On 14-09-15, 15:22, Rob Herring wrote:
What if we have a 2nd device and supply rail? For example, what if the L2$ has a separate rail from the cores but is linked to the OPPs.
Right, so that is the case with the Mediatek SoC as well, AFAIR. How do we plan to treat L2 devices? For example, in the mediatek cpufreq driver, they change L2's supplies together with CPUs.
One way to get that done, with such very closely bound devices is to add two regulators:
cpu-supply = ...; l2-supply = ...;
And then a property in OPP table:
supply-name = "cpu", "l2";
And maybe fix the order in which the supplies which be updated, based on the order in which entries are present in the above property.
Any other way you suggest for doing that ?
Remind me of when do we have multiple regulators for a cpu?
I haven't seen that yet, but its more like what I explained above. i.e. one for the CPU and other one for the L2 cache.
But, these bindings do apply for other devices as well, where it should be very much possible.
Rob,
On 15-09-15, 08:17, Viresh Kumar wrote:
On 14-09-15, 15:22, Rob Herring wrote:
What if we have a 2nd device and supply rail? For example, what if the L2$ has a separate rail from the cores but is linked to the OPPs.
Right, so that is the case with the Mediatek SoC as well, AFAIR. How do we plan to treat L2 devices? For example, in the mediatek cpufreq driver, they change L2's supplies together with CPUs.
One way to get that done, with such very closely bound devices is to add two regulators:
cpu-supply = ...; l2-supply = ...;
And then a property in OPP table:
supply-name = "cpu", "l2";
And maybe fix the order in which the supplies which be updated, based on the order in which entries are present in the above property.
Any other way you suggest for doing that ?
Remind me of when do we have multiple regulators for a cpu?
I haven't seen that yet, but its more like what I explained above. i.e. one for the CPU and other one for the L2 cache.
But, these bindings do apply for other devices as well, where it should be very much possible.
Not sure if you still have any objections to this patch?
On 09/14, Rob Herring wrote:
On 09/11/2015 07:01 AM, Viresh Kumar wrote:
Regulators already have stable DT bindings, wherein the consumer (of supplies) will have following for each regulator/supply.
<name>-supply: <phandle to the regulator node>;
Current OPP bindings extend above, by transforming it into a list of phandles. But we missed the <name> string, which is used to identify the regulator.
And looking from regulators perspective, having two different ways of specifying regulators doesn't seem like a step forward, it also means we have to update every single device binding. And things will become complex.
Another way to support multiple regulators per device (in OPP V2 bindings) is to leave regulator consumer bindings as is, and create a 'supply-names' property in the opp-table node, which will contain a list of strings. The names in this list shall match 'name' from the '<name>-supply' strings present in the device node.
The strings in this list also specify the order in which values must be present in 'opp-microvolt' and 'opp-microamp' properties.
Cc: Mark Brown broonie@kernel.org Cc: devicetree@vger.kernel.org Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Documentation/devicetree/bindings/opp/opp.txt | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt index 0cb44dc21f97..8759bc4783ed 100644 --- a/Documentation/devicetree/bindings/opp/opp.txt +++ b/Documentation/devicetree/bindings/opp/opp.txt @@ -69,6 +69,13 @@ This describes the OPPs belonging to a device. This node can have following
- compatible: Allow OPPs to express their compatibility. It should be: "operating-points-v2".
+- supply-names: This is a required property, only if multiple supplies are
- available for the device. Otherwise it is optional.
- This list is used to pass names of all the device supplies. The order of names
- present here is important, as that should match the order in which values are
- present in 'opp-microvolt' and 'opp-microamp' properties.
What if we have a 2nd device and supply rail? For example, what if the L2$ has a separate rail from the cores but is linked to the OPPs.
I'm lost why we need this property at all. What happened to using
opp-microvolt-0 = <1 2 3>; opp-microvolt-1 = <1>; opp-microvolt-2 = <3 4 5>; etc.
That seems to avoid any problem with 3 vs. 1 element properties combined into one large array. Having supply-names seems too brittle and would tie us to a particular OPP user's decision to call supplies by some name.
Also, I've seen devices that are split across two power domains. These devices aren't CPUs, but they are other devices including L2 caches. So we're going to need either multiple regulator support or multiple "power domain at a particular performance levels" support somehow.
On 15-10-15, 17:22, Stephen Boyd wrote:
I'm lost why we need this property at all. What happened to using
opp-microvolt-0 = <1 2 3>; opp-microvolt-1 = <1>; opp-microvolt-2 = <3 4 5>; etc.
Perhaps you are confusing this with the bindings we came up for picking right voltage levels based on the cuts/version of the hardware we are running on. The problem that Lee Jones mentioned and that can be used in your case as well.
That seems to avoid any problem with 3 vs. 1 element properties combined into one large array.
That's not the problem I was trying to solve here.
Having supply-names seems too brittle and would tie us to a particular OPP user's decision to call supplies by some name.
No. The name has to match the <name>-supply property present in the device's node, that's why we need this property :)
Also, I've seen devices that are split across two power domains. These devices aren't CPUs, but they are other devices including L2 caches. So we're going to need either multiple regulator support or multiple "power domain at a particular performance levels" support somehow.
Right, that's a good example of why we need multi-regulator support :)
On 10/16, Viresh Kumar wrote:
On 15-10-15, 17:22, Stephen Boyd wrote:
I'm lost why we need this property at all. What happened to using
opp-microvolt-0 = <1 2 3>; opp-microvolt-1 = <1>; opp-microvolt-2 = <3 4 5>; etc.
Perhaps you are confusing this with the bindings we came up for picking right voltage levels based on the cuts/version of the hardware we are running on. The problem that Lee Jones mentioned and that can be used in your case as well.
Isn't that what this patch series is for?
That seems to avoid any problem with 3 vs. 1 element properties combined into one large array.
That's not the problem I was trying to solve here.
What problem are you trying to solve then?
Having supply-names seems too brittle and would tie us to a particular OPP user's decision to call supplies by some name.
No. The name has to match the <name>-supply property present in the device's node, that's why we need this property :)
Why does it need to match? Sorry I'm totally lost now.
On 16-10-15, 12:16, Stephen Boyd wrote:
On 10/16, Viresh Kumar wrote:
On 15-10-15, 17:22, Stephen Boyd wrote:
I'm lost why we need this property at all. What happened to using
opp-microvolt-0 = <1 2 3>; opp-microvolt-1 = <1>; opp-microvolt-2 = <3 4 5>; etc.
Perhaps you are confusing this with the bindings we came up for picking right voltage levels based on the cuts/version of the hardware we are running on. The problem that Lee Jones mentioned and that can be used in your case as well.
Isn't that what this patch series is for?
Hehe, no.
Okay here is the problem statement:
We have two supplies for a device and the device node will have something like:
name1-supply = <&supply1>; name2-supply = <&supply2>;
And the OPP node needs to have voltages for both of them:
opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;
Where XYZ(1) are for supply1 and XYZ(2) are for supply2.
Now we need to identify the supplies for which the values are present here and their order as well. How do we do that?
The way I am suggesting is to add a property in opp node which will keep "name1" and "name2" in it.
On 17-10-15, 09:40, Viresh Kumar wrote:
Hehe, no.
Okay here is the problem statement:
We have two supplies for a device and the device node will have something like:
name1-supply = <&supply1>; name2-supply = <&supply2>;
And the OPP node needs to have voltages for both of them:
opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;
Where XYZ(1) are for supply1 and XYZ(2) are for supply2.
Now we need to identify the supplies for which the values are present here and their order as well. How do we do that?
The way I am suggesting is to add a property in opp node which will keep "name1" and "name2" in it.
Any more comments? Acks ? I want to close this series :)
On Sat, Oct 17, 2015 at 09:40:55AM +0530, Viresh Kumar wrote:
Okay here is the problem statement:
We have two supplies for a device and the device node will have something like:
name1-supply = <&supply1>; name2-supply = <&supply2>;
And the OPP node needs to have voltages for both of them:
opp-microvolt = <X1 Y1 Z1>, <X2 Y2 Z2>;
Where XYZ(1) are for supply1 and XYZ(2) are for supply2.
Now we need to identify the supplies for which the values are present here and their order as well. How do we do that?
The way I am suggesting is to add a property in opp node which will keep "name1" and "name2" in it.
When we start doing this we also start having to worry about things like the sequencing of the updates between the various supplies and end up in full on power sequencing (or at least baking some sequencing into the DT which will doubtless need extending at some point). I'm not sure that's a place we want to end up just yet, I think it's safer to just have a little bit of code in the kernel that glues things together in the cases where this is needed.
On 23-10-15, 01:39, Mark Brown wrote:
When we start doing this we also start having to worry about things like the sequencing of the updates between the various supplies and end up in full on power sequencing (or at least baking some sequencing into the DT which will doubtless need extending at some point).
Absolutely.
I'm not sure that's a place we want to end up just yet, I think it's safer to just have a little bit of code in the kernel that glues things together in the cases where this is needed.
So you are effectively saying that we shouldn't go ahead with multi regulator support in OPP library, right?
I went ahead with it as it came as a requirement (specially from Qcom).
To the problem of sequencing, maybe we can just support that for the simple case, where supplies will be programmed in the order in which they are present in the property I added in this patch. And not try to solve problem for the complex cases, if we feel it is getting ugly.
@Stephen ?
On Tue, Oct 27, 2015 at 01:49:17PM +0530, Viresh Kumar wrote:
On 23-10-15, 01:39, Mark Brown wrote:
I'm not sure that's a place we want to end up just yet, I think it's safer to just have a little bit of code in the kernel that glues things together in the cases where this is needed.
So you are effectively saying that we shouldn't go ahead with multi regulator support in OPP library, right?
Well, I think things like libraries for getting the data tables out of DT are fine but I'm not convinced that trying to avoid having any device specific code at all is sufficiently clear yet - as far as I know we're mostly looking at a fairly small subset of devices still and with things like sequencing in the mix it's a bit worrying to me to be putting it all into an ABI intended to be used with no knowledge of the platform.
On 10/28, Mark Brown wrote:
On Tue, Oct 27, 2015 at 01:49:17PM +0530, Viresh Kumar wrote:
On 23-10-15, 01:39, Mark Brown wrote:
I'm not sure that's a place we want to end up just yet, I think it's safer to just have a little bit of code in the kernel that glues things together in the cases where this is needed.
So you are effectively saying that we shouldn't go ahead with multi regulator support in OPP library, right?
Well, I think things like libraries for getting the data tables out of DT are fine but I'm not convinced that trying to avoid having any device specific code at all is sufficiently clear yet - as far as I know we're mostly looking at a fairly small subset of devices still and with things like sequencing in the mix it's a bit worrying to me to be putting it all into an ABI intended to be used with no knowledge of the platform.
Agreed. Looking at the current oppv2 binding I'm confused how it even works. It shows cpu-supply = <&phandle1>, <&phandle2>, etc. which doesn't work with the regulator bindings design. It seems that Viresh figured that out when implementing support for multiple regulators, so the binding was changed to put names in the opp tables and then use that to get regulators in the opp core. Urgh.
I think I understand the goal here though. The goal is to move all the clk_get()/regulator_get() and clock and regulator API interactions into the OPP framework so that we can say "go to OPP at index 4" from the cpufreq core and the OPP framework takes care of actually doing the transition. Given that doing such a transition could be very machine specific, we probably ought to make the OPP framework flexible enough to let us decide how to do that.
Perhaps we need to expand on the compatible string in the opp node to have compatible = "vendor,cool-transition", "operating-points-v2"? Then we can plug in vendor specific drivers that handle the frequency/voltage transition and they'll know that the fourth column in the voltage entry corresponds to this particular device's foo-supply and in what order to change voltages. A generic driver could exist for the simple case of one regulator and one clock that matches on "operating-points-v2".
Or we can avoid doing any clk_get()/regulator_get() stuff inside the OPP framework, and let OPP consumers tell the OPP framework about each clock and regulator it wants the framework to manage on the consumer's behalf. It could even tell the framework which regulator corresponds to which voltage column and in what order to change the voltages so that the OPP framework doesn't need to know these device specific details. Then the consumer can still say go to OPP level 4 and that all works without introducing some new DT ABI.
[Removed few people and LKML from CC as its not really about DT change anymore but OPP]
Hi Guys,
On 15 September 2015 at 01:52, Rob Herring robh@kernel.org wrote:
Remind me of when do we have multiple regulators for a cpu? The number of supplies should be defined by the cpu binding as function of how many supply rails a cpu has.
This thread died long back and we never worked out support for multiple regulators into the OPP core.
But, Dave and Nishanth (from TI) pinged me last week about it. According to them, their platform has got two regulators for the CPU device and they want OPP layer to have this support.
I was expecting them to reignite this thread, but don't know what happened.
And so, I am starting it here.
@Dave/Nishanth: Can you please explain the use-case here please? So that we can get convinced that you really need OPP-core to support multiple regulators..
-- viresh
On 03/01/2016 12:45 AM, Viresh Kumar wrote:
[Removed few people and LKML from CC as its not really about DT change anymore but OPP]
Hi Guys,
On 15 September 2015 at 01:52, Rob Herring robh@kernel.org wrote:
Remind me of when do we have multiple regulators for a cpu? The number of supplies should be defined by the cpu binding as function of how many supply rails a cpu has.
This thread died long back and we never worked out support for multiple regulators into the OPP core.
But, Dave and Nishanth (from TI) pinged me last week about it. According to them, their platform has got two regulators for the CPU device and they want OPP layer to have this support.
I was expecting them to reignite this thread, but don't know what happened.
Thanks a ton viresh bringing this back up again.
And so, I am starting it here.
@Dave/Nishanth: Can you please explain the use-case here please? So that we can get convinced that you really need OPP-core to support multiple regulators..
For controlling leakage and switching frequency of the transistors, traditional controls are usually around the voltage rail alone. This is not entirely accurate. many SoC vendors including TI [1] and others[2] use Body Biasing to control leakage. in fact this is already modeled as a regulator for controlling the SoC internal LDO for TI devices in Linux kernel (drivers/regulator/ti-abb-regulator.c).
What this means is something as simple as follows:
Typical usage: VDD --- | | / |/ --| |\ V | | GND
Here we provide voltage optimized for the switching frequency required for operation.
With Body Biasing: (simplified diagram) VDD VBB (Body Bias) --- --- | | | | / | |/ | --| ----/ |\ V | | GND
This implies that for an OPP, we have the triplet {frequency, VDD(SMPS voltage), VBB(Bias voltage) } - Multi regulator is key for all OMAP generation SoCs since OMAP3 to function for frequencies -> DRA7 for example mandates ABB for all OPPs.
For some reason, this entire thread slipped out attention :( - Thanks viresh for bringing it back to focus.
[1] http://www.ti.com/product/DM3730/technicaldocuments including http://www.ti.com/product/am5728?keyMatch=am5728 (TI) OMAP3, OMAP4, OMAP5, DRA7, AM57xx SoCs all have this
[2] http://www.techdesignforums.com/practice/guides/dvfs-body-back-bias/ (ST Micro for example)
On Tue, Mar 01, 2016 at 12:15:48PM +0530, Viresh Kumar wrote:
[Removed few people and LKML from CC as its not really about DT change anymore but OPP]
Please don't take things off-list unless there is a really strong reason to do so. Sending things to the list ensures that everyone gets a chance to read and comment on things.
I'm really not sure what I can say here that I've not already said repeatedly, what's new here?
On 02-03-16, 11:50, Mark Brown wrote:
Please don't take things off-list unless there is a really strong reason to do so. Sending things to the list ensures that everyone gets a chance to read and comment on things.
Okay. I thought keeping that on linux-pm would be sufficient for interested folks to follow.
I'm really not sure what I can say here that I've not already said repeatedly, what's new here?
Yeah, you have raised some genuine concerns earlier, like the sequence in which the voltages of multiple supplies should be changed, and if we should rely on some DT sequence for that.
I started it again, because we have a real case now (not like the virtual case earlier) where a platform has multiple supplies for a device (CPU). I just wanted to hear from all of you on how should we go ahead and solve it.
TI guys had some crazy ideas, like using OPP-notifiers to update the second regulator, etc and I really hated it :)
On Wed, Mar 02, 2016 at 03:58:31PM +0530, Viresh Kumar wrote:
On 02-03-16, 11:50, Mark Brown wrote:
I'm really not sure what I can say here that I've not already said repeatedly, what's new here?
Yeah, you have raised some genuine concerns earlier, like the sequence in which the voltages of multiple supplies should be changed, and if we should rely on some DT sequence for that.
I started it again, because we have a real case now (not like the virtual case earlier) where a platform has multiple supplies for a device (CPU). I just wanted to hear from all of you on how should we go ahead and solve it.
TI guys had some crazy ideas, like using OPP-notifiers to update the second regulator, etc and I really hated it :)
My main thought with this stuff is that if it's really complicated to write generic code that might be a sign that it's sensible to write a custom driver. We can always writee something later if we figure out some general patterns.
On Wed, Mar 2, 2016 at 5:24 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Mar 02, 2016 at 03:58:31PM +0530, Viresh Kumar wrote:
On 02-03-16, 11:50, Mark Brown wrote:
I'm really not sure what I can say here that I've not already said repeatedly, what's new here?
Yeah, you have raised some genuine concerns earlier, like the sequence in which the voltages of multiple supplies should be changed, and if we should rely on some DT sequence for that.
I started it again, because we have a real case now (not like the virtual case earlier) where a platform has multiple supplies for a device (CPU). I just wanted to hear from all of you on how should we go ahead and solve it.
TI guys had some crazy ideas, like using OPP-notifiers to update the second regulator, etc and I really hated it :)
Ummm... We prefer not to do this via notifier solution.. yeah doing that around notifiers is indeed a crazier alternative
My main thought with this stuff is that if it's really complicated to write generic code that might be a sign that it's sensible to write a custom driver. We can always writee something later if we figure out some general patterns.
We did propose to go via voltage domain[1] to actually do an OPP transition and provide hooks that can implement required regulator set for the SoC. After seeing dev_pm_opp_set_rate is available, it does not make sense to go down the voltagedomain path anymore.
We do have a real need to control multiple regulators around an OPP change. this logic is valid for more than CPU device -> it is valid for devices like DSP, GPU etc in the SoC as well across multiple families in the OMAP generation SoCs. The logic by itself it not too complicated. a voltage plane for a device has two regulators that need to be controlled in tandem for a given target frequency. by allowing dev_pm_opp_set_rate to control "n" regulators it actually does simplify the problem down for a generic solution -> DVFS for devices triggered from cpufreq or via devfreq.
[1] http://marc.info/?l=linux-omap&m=139275579731801&w=2
--- Regards, Nishanth Menon
On Wed, Mar 02, 2016 at 09:26:03AM -0600, Nishanth Menon wrote:
We do have a real need to control multiple regulators around an OPP change. this logic is valid for more than CPU device -> it is valid for devices like DSP, GPU etc in the SoC as well across multiple families in the OMAP generation SoCs. The logic by itself it not too complicated. a voltage plane for a device has two regulators that need to be controlled in tandem for a given target frequency. by allowing dev_pm_opp_set_rate to control "n" regulators it actually does simplify the problem down for a generic solution -> DVFS for devices triggered from cpufreq or via devfreq.
That's n regulators that need to be changed in arbitrary orders (together with some other number of clocks).
On Wed, Mar 2, 2016 at 9:30 AM, Mark Brown broonie@kernel.org wrote:
On Wed, Mar 02, 2016 at 09:26:03AM -0600, Nishanth Menon wrote:
We do have a real need to control multiple regulators around an OPP change. this logic is valid for more than CPU device -> it is valid for devices like DSP, GPU etc in the SoC as well across multiple families in the OMAP generation SoCs. The logic by itself it not too complicated. a voltage plane for a device has two regulators that need to be controlled in tandem for a given target frequency. by allowing dev_pm_opp_set_rate to control "n" regulators it actually does simplify the problem down for a generic solution -> DVFS for devices triggered from cpufreq or via devfreq.
That's n regulators that need to be changed in arbitrary orders (together with some other number of clocks).
I agree, the "n" regulators that control supplies to a device cannot obviously be transitioned in random order and ofcourse we want to describe what supplies are present in the hardware in dts, not how the supplies need to be transitioned. There will be a sequencing required based on platfiorm for these n regulators (and or optimization of voltages) - similar to requirement for voltage Vs frequency scale needs.
One alternative I can think of is to reintroduce voltage domain driver back in (we will get rid of the clock notifier tieups etc).. this voltage domain mechanism will handle all the required sequencing based on platform and any optimizations or quirks needed, then, _set_opp_voltage would see if a voltagedomain is present, if yes, it will invoke that instead of a single regulator. obviously two things take place with this approach: a) dev_pm_opp_set_rate still remain the single entry point for all OPP transition (I think this is a good idea) b) a level of abstraction introduced by complex drivers via the voltage domain framework ( the original proposed framework had ability already to handle 1-n regulators based on platform voltage domain drivers).
What other alternatives can folks suggest we go towards?
--- Regards, Nishanth Menon
On Fri, Sep 11, 2015 at 05:31:57PM +0530, Viresh Kumar wrote:
+- supply-names: This is a required property, only if multiple supplies are
- available for the device. Otherwise it is optional.
- This list is used to pass names of all the device supplies. The order of names
- present here is important, as that should match the order in which values are
- present in 'opp-microvolt' and 'opp-microamp' properties.
If we were going to add something like this (which I'm really not that thrilled about due to their encouragement of bad practice as previously discussed) it seems wrong to add it at the level of a specific binding rather than as a general facility. I think I'm not entirely sold on this use case though, I'll follow up to a later message in this thread...
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(+)
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 */
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
[+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.
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.
With debug options on, it is difficult to locate OPP core's debug prints. Fix this by prefixing OPP debug prints with KBUILD_MODNAME.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 2 ++ drivers/base/power/opp/cpu.c | 3 +++ 2 files changed, 5 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 8a72f0e586e8..113fdd0ce6d9 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -11,6 +11,8 @@ * published by the Free Software Foundation. */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include <linux/errno.h> #include <linux/err.h> #include <linux/slab.h> diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index 3d946b508a13..9ac691341fb0 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -10,6 +10,9 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include <linux/cpu.h> #include <linux/cpufreq.h> #include <linux/err.h>
Clearly distinguish routines based on what version of bindings they parse. We have already postfixed routines properly with _v2 for new bindings. Postfix the older ones now with _v1.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 113fdd0ce6d9..6bebc503d727 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -718,7 +718,7 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, }
/** - * _opp_add_dynamic() - Allocate a dynamic OPP. + * _opp_add_v1() - Allocate a OPP based on v1 bindings. * @dev: device for which we do this operation * @freq: Frequency in Hz for this OPP * @u_volt: Voltage in uVolts for this OPP @@ -744,8 +744,8 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, * Duplicate OPPs (both freq and volt are same) and !opp->available * -ENOMEM Memory allocation failure */ -static int _opp_add_dynamic(struct device *dev, unsigned long freq, - long u_volt, bool dynamic) +static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, + bool dynamic) { struct device_opp *dev_opp; struct dev_pm_opp *new_opp; @@ -949,7 +949,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) */ int dev_pm_opp_add(struct device *dev, unsigned long freq, unsigned long u_volt) { - return _opp_add_dynamic(dev, freq, u_volt, true); + return _opp_add_v1(dev, freq, u_volt, true); } EXPORT_SYMBOL_GPL(dev_pm_opp_add);
@@ -1251,7 +1251,7 @@ static int _of_add_opp_table_v1(struct device *dev) unsigned long freq = be32_to_cpup(val++) * 1000; unsigned long volt = be32_to_cpup(val++);
- if (_opp_add_dynamic(dev, freq, volt, false)) + if (_opp_add_v1(dev, freq, volt, false)) dev_warn(dev, "%s: Failed to add OPP %ld\n", __func__, freq); nr -= 2;
Move all DT parsing for the power supplies to a single function, rather than keeping them at separate places. This will help manage things properly.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6bebc503d727..463417b6838c 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -787,9 +787,10 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, }
/* TODO: Support multiple regulators */ -static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev) +static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) { u32 microvolt[3] = {0}; + u32 val; int count, ret;
count = of_property_count_u32_elems(opp->np, "opp-microvolt"); @@ -815,6 +816,9 @@ static int opp_get_microvolt(struct dev_pm_opp *opp, struct device *dev) opp->u_volt_min = microvolt[1]; opp->u_volt_max = microvolt[2];
+ if (!of_property_read_u32(opp->np, "opp-microamp", &val)) + opp->u_amp = val; + return 0; }
@@ -879,13 +883,10 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) if (!of_property_read_u32(np, "clock-latency-ns", &val)) new_opp->clock_latency_ns = val;
- ret = opp_get_microvolt(new_opp, dev); + ret = opp_parse_supplies(new_opp, dev); if (ret) goto free_opp;
- if (!of_property_read_u32(new_opp->np, "opp-microamp", &val)) - new_opp->u_amp = val; - ret = _opp_add(dev, new_opp, dev_opp); if (ret) goto free_opp;
Support for multiple regulators will be added later, until then move all power-supply related information in a separate structure.
To make allocating/freeing memory for these supply structures easy, allocate memory for them along with opp.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 57 ++++++++++++++++++++++++++-------------- drivers/base/power/opp/debugfs.c | 9 ++++--- drivers/base/power/opp/opp.h | 29 ++++++++++++++------ 3 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 463417b6838c..14e5fa10be2d 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -127,7 +127,7 @@ unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) if (IS_ERR_OR_NULL(tmp_opp) || !tmp_opp->available) pr_err("%s: Invalid parameters\n", __func__); else - v = tmp_opp->u_volt; + v = tmp_opp->supplies[0].u_volt;
return v; } @@ -491,13 +491,14 @@ struct device_list_opp *_add_list_dev(const struct device *dev, /** * _add_device_opp() - Find device OPP table or allocate a new one * @dev: device for which we do this operation + * @supply_count: Number of supplies available for each OPP. * * It tries to find an existing table first, if it couldn't find one, it * allocates a new OPP table and returns that. * * Return: valid device_opp pointer if success, else NULL. */ -static struct device_opp *_add_device_opp(struct device *dev) +static struct device_opp *_add_device_opp(struct device *dev, int supply_count) { struct device_opp *dev_opp; struct device_list_opp *list_dev; @@ -515,6 +516,7 @@ static struct device_opp *_add_device_opp(struct device *dev) if (!dev_opp) return NULL;
+ dev_opp->supply_count = supply_count; INIT_LIST_HEAD(&dev_opp->dev_list);
list_dev = _add_list_dev(dev, dev_opp); @@ -652,24 +654,31 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) } EXPORT_SYMBOL_GPL(dev_pm_opp_remove);
-static struct dev_pm_opp *_allocate_opp(struct device *dev, - struct device_opp **dev_opp) +static struct dev_pm_opp * +_allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count) { struct dev_pm_opp *opp; + size_t size = sizeof(*opp); + + /* Memory for the OPP and its supplies is allocated together */ + size += supply_count * sizeof(*opp->supplies);
/* allocate new OPP node */ - opp = kzalloc(sizeof(*opp), GFP_KERNEL); + opp = kzalloc(size, GFP_KERNEL); if (!opp) return NULL;
INIT_LIST_HEAD(&opp->node);
- *dev_opp = _add_device_opp(dev); + *dev_opp = _add_device_opp(dev, supply_count); if (!*dev_opp) { kfree(opp); return NULL; }
+ /* Supplies points to the memory right after the opp */ + opp->supplies = (struct opp_supply *)(opp + 1); + return opp; }
@@ -699,10 +708,12 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
/* Duplicate OPPs */ dev_warn(dev, "%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n", - __func__, opp->rate, opp->u_volt, opp->available, - new_opp->rate, new_opp->u_volt, new_opp->available); + __func__, opp->rate, opp->supplies[0].u_volt, + opp->available, new_opp->rate, + new_opp->supplies[0].u_volt, new_opp->available);
- return opp->available && new_opp->u_volt == opp->u_volt ? + return opp->available && + opp->supplies[0].u_volt == new_opp->supplies[0].u_volt ? 0 : -EEXIST; }
@@ -754,7 +765,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, /* Hold our list modification lock here */ mutex_lock(&dev_opp_list_lock);
- new_opp = _allocate_opp(dev, &dev_opp); + new_opp = _allocate_opp(dev, &dev_opp, 1); if (!new_opp) { ret = -ENOMEM; goto unlock; @@ -762,7 +773,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */ new_opp->rate = freq; - new_opp->u_volt = u_volt; + new_opp->supplies[0].u_volt = u_volt; new_opp->available = true; new_opp->dynamic = dynamic;
@@ -789,6 +800,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, /* TODO: Support multiple regulators */ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) { + struct opp_supply *supply = &opp->supplies[0]; u32 microvolt[3] = {0}; u32 val; int count, ret; @@ -812,12 +824,12 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) return -EINVAL; }
- opp->u_volt = microvolt[0]; - opp->u_volt_min = microvolt[1]; - opp->u_volt_max = microvolt[2]; + supply->u_volt = microvolt[0]; + supply->u_volt_min = microvolt[1]; + supply->u_volt_max = microvolt[2];
if (!of_property_read_u32(opp->np, "opp-microamp", &val)) - opp->u_amp = val; + supply->u_amp = val;
return 0; } @@ -826,6 +838,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) * @dev: device for which we do this operation * @np: device node + * @supply_count: Number of supplies available for each OPP * * This function adds an opp definition to the opp list and returns status. The * opp can be controlled using dev_pm_opp_enable/disable functions and may be @@ -845,10 +858,12 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) * -ENOMEM Memory allocation failure * -EINVAL Failed parsing the OPP node */ -static int _opp_add_static_v2(struct device *dev, struct device_node *np) +static int _opp_add_static_v2(struct device *dev, struct device_node *np, + int supply_count) { struct device_opp *dev_opp; struct dev_pm_opp *new_opp; + struct opp_supply *supply; u64 rate; u32 val; int ret; @@ -856,7 +871,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) /* Hold our list modification lock here */ mutex_lock(&dev_opp_list_lock);
- new_opp = _allocate_opp(dev, &dev_opp); + new_opp = _allocate_opp(dev, &dev_opp, supply_count); if (!new_opp) { ret = -ENOMEM; goto unlock; @@ -906,9 +921,10 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np)
mutex_unlock(&dev_opp_list_lock);
+ supply = &new_opp->supplies[0]; pr_debug("%s: turbo:%d rate:%lu uv:%lu uvmin:%lu uvmax:%lu latency:%lu\n", - __func__, new_opp->turbo, new_opp->rate, new_opp->u_volt, - new_opp->u_volt_min, new_opp->u_volt_max, + __func__, new_opp->turbo, new_opp->rate, supply->u_volt, + supply->u_volt_min, supply->u_volt_max, new_opp->clock_latency_ns);
/* @@ -1195,7 +1211,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) for_each_available_child_of_node(opp_np, np) { count++;
- ret = _opp_add_static_v2(dev, np); + /* Todo: Add support for multiple supplies */ + ret = _opp_add_static_v2(dev, np, 1); if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c index 865cbfa24640..e6ba29c04513 100644 --- a/drivers/base/power/opp/debugfs.c +++ b/drivers/base/power/opp/debugfs.c @@ -34,6 +34,7 @@ void opp_debug_remove_one(struct dev_pm_opp *opp) int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp) { struct dentry *pdentry = dev_opp->dentry; + struct opp_supply *supply = &opp->supplies[0]; struct dentry *d; char name[15];
@@ -59,18 +60,18 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp) return -ENOMEM;
if (!debugfs_create_u32("u_volt_target", S_IRUGO, d, - (u32 *)&opp->u_volt)) + (u32 *)&supply->u_volt)) return -ENOMEM;
if (!debugfs_create_u32("u_volt_min", S_IRUGO, d, - (u32 *)&opp->u_volt_min)) + (u32 *)&supply->u_volt_min)) return -ENOMEM;
if (!debugfs_create_u32("u_volt_max", S_IRUGO, d, - (u32 *)&opp->u_volt_max)) + (u32 *)&supply->u_volt_max)) return -ENOMEM;
- if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&opp->u_amp)) + if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&supply->u_amp)) return -ENOMEM;
if (!debugfs_create_u32("clock_latency_ns", S_IRUGO, d, diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index 0e4c27fb0c4a..a7a6917d6fbd 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -39,6 +39,22 @@ */
/** + * struct opp_supply - Per power-supply structure + * @u_volt: Target voltage in microvolts corresponding to this OPP + * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP + * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP + * @u_amp: Maximum current drawn by the device in microamperes + * + * This structure stores the OPP power-supply information for a given device. + */ +struct opp_supply { + unsigned long u_volt; + unsigned long u_volt_min; + unsigned long u_volt_max; + unsigned long u_amp; +}; + +/** * struct dev_pm_opp - Generic OPP description structure * @node: opp list node. The nodes are maintained throughout the lifetime * of boot. It is expected only an optimal set of OPPs are @@ -52,10 +68,7 @@ * @available: true/false - marks if this OPP as available or not * @turbo: true if turbo (boost) OPP * @rate: Frequency in hertz - * @u_volt: Target voltage in microvolts corresponding to this OPP - * @u_volt_min: Minimum voltage in microvolts corresponding to this OPP - * @u_volt_max: Maximum voltage in microvolts corresponding to this OPP - * @u_amp: Maximum current drawn by the device in microamperes + * @supplies: Array of power-supplies for the device. * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's * frequency from any other OPP's frequency. * @dev_opp: points back to the device_opp struct this opp belongs to @@ -73,10 +86,8 @@ struct dev_pm_opp { bool turbo; unsigned long rate;
- unsigned long u_volt; - unsigned long u_volt_min; - unsigned long u_volt_max; - unsigned long u_amp; + struct opp_supply *supplies; + unsigned long clock_latency_ns;
struct device_opp *dev_opp; @@ -123,6 +134,7 @@ struct device_list_opp { * @dev_list: list of devices that share these OPPs * @opp_list: list of opps * @np: struct device_node pointer for opp's DT node. + * @supply_count: Number of power-supplies * @shared_opp: OPP is shared between multiple devices. * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. @@ -145,6 +157,7 @@ struct device_opp {
struct device_node *np; unsigned long clock_latency_ns_max; + unsigned int supply_count; bool shared_opp; struct dev_pm_opp *suspend_opp;
This adds support to parse multiple regulators or power-supplies in OPP core. This doesn't use those values yet.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 197 +++++++++++++++++++++++++++++++-------- drivers/base/power/opp/debugfs.c | 53 +++++++---- drivers/base/power/opp/opp.h | 4 + 3 files changed, 201 insertions(+), 53 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 14e5fa10be2d..d6e945ec6467 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -502,21 +502,27 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count) { struct device_opp *dev_opp; struct device_list_opp *list_dev; + size_t size;
/* Check for existing list for 'dev' first */ dev_opp = _find_device_opp(dev); if (!IS_ERR(dev_opp)) return dev_opp;
+ /* Allocate size for supply-names with dev_opp */ + size = sizeof(*dev_opp) + supply_count * sizeof(*dev_opp->supply_names); + /* * Allocate a new device OPP table. In the infrequent case where a new * device is needed to be added, we pay this penalty. */ - dev_opp = kzalloc(sizeof(*dev_opp), GFP_KERNEL); + dev_opp = kzalloc(size, GFP_KERNEL); if (!dev_opp) return NULL;
dev_opp->supply_count = supply_count; + dev_opp->supply_names = (const char **)(dev_opp + 1); + INIT_LIST_HEAD(&dev_opp->dev_list);
list_dev = _add_list_dev(dev, dev_opp); @@ -525,6 +531,13 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count) return NULL; }
+ /* + * Initialize supply-name as dev-name for single supplies. This is + * required for the debugfs code. + */ + if (supply_count == 1) + *dev_opp->supply_names = dev_name(dev); + srcu_init_notifier_head(&dev_opp->srcu_head); INIT_LIST_HEAD(&dev_opp->opp_list);
@@ -682,6 +695,23 @@ _allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count) return opp; }
+static bool _supplies_match(struct device_opp *dev_opp, + struct dev_pm_opp *old_opp, + struct dev_pm_opp *new_opp) +{ + struct opp_supply *old = old_opp->supplies; + struct opp_supply *new = new_opp->supplies; + int i; + + for (i = 0; i < dev_opp->supply_count; i++) + if (old[i].u_volt != new[i].u_volt || + old[i].u_volt_min != new[i].u_volt_min || + old[i].u_volt_max != new[i].u_volt_max) + return false; + + return true; +} + static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, struct device_opp *dev_opp) { @@ -712,9 +742,8 @@ static int _opp_add(struct device *dev, struct dev_pm_opp *new_opp, opp->available, new_opp->rate, new_opp->supplies[0].u_volt, new_opp->available);
- return opp->available && - opp->supplies[0].u_volt == new_opp->supplies[0].u_volt ? - 0 : -EEXIST; + return opp->available && _supplies_match(dev_opp, opp, new_opp) + ? 0 : -EEXIST; }
new_opp->dev_opp = dev_opp; @@ -797,41 +826,99 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, return ret; }
-/* TODO: Support multiple regulators */ -static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) +/* returns the number of entries used from microvolt */ +static void opp_parse_single_supply(struct opp_supply *supply, bool triplet, + u32 *microvolt, u32 *microamp) { - struct opp_supply *supply = &opp->supplies[0]; - u32 microvolt[3] = {0}; - u32 val; - int count, ret; + if (triplet) { + supply->u_volt = microvolt[0]; + supply->u_volt_min = microvolt[1]; + supply->u_volt_max = microvolt[2]; + } else { + supply->u_volt = microvolt[0]; + supply->u_volt_min = microvolt[0]; + supply->u_volt_max = microvolt[0]; + } + + supply->u_amp = *microamp; +} + +static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, + int supply_count, bool triplets) +{ + struct opp_supply *supply = opp->supplies; + int i, vcount, icount, ret, step; + u32 *microvolt, *microamp;
- count = of_property_count_u32_elems(opp->np, "opp-microvolt"); - if (!count) + vcount = of_property_count_u32_elems(opp->np, "opp-microvolt"); + if (!vcount) return 0;
- /* There can be one or three elements here */ - if (count != 1 && count != 3) { - dev_err(dev, "%s: Invalid number of elements in opp-microvolt property (%d)\n", - __func__, count); - return -EINVAL; - } + icount = of_property_count_u32_elems(opp->np, "opp-microamp"); + if (!icount) + return 0; + + /* Allocate memory for volt/amp */ + microvolt = kcalloc(vcount, sizeof(*microvolt), GFP_KERNEL); + microamp = kcalloc(icount, sizeof(*microamp), GFP_KERNEL); + if (!microvolt || !microamp) + return -ENOMEM;
ret = of_property_read_u32_array(opp->np, "opp-microvolt", microvolt, - count); + vcount); if (ret) { dev_err(dev, "%s: error parsing opp-microvolt: %d\n", __func__, ret); - return -EINVAL; + ret = -EINVAL; + goto free_microvolt; }
- supply->u_volt = microvolt[0]; - supply->u_volt_min = microvolt[1]; - supply->u_volt_max = microvolt[2]; + ret = of_property_read_u32_array(opp->np, "opp-microamp", microamp, + icount); + if (ret) { + dev_err(dev, "%s: error parsing opp-microamp: %d\n", __func__, + ret); + ret = -EINVAL; + goto free_microvolt; + }
- if (!of_property_read_u32(opp->np, "opp-microamp", &val)) - supply->u_amp = val; + /* + * For single supply, "opp-microvolt-triplets" is not mandatory and we + * need to find it ourselves. + */ + if (supply_count == 1) { + if (vcount == 1) { + triplets = false; + } else if (vcount == 3) { + triplets = true; + } else { + dev_err(dev, "%s: opp-microvolt property should have 1 or 3 elements (%d)\n", + __func__, vcount); + ret = -EINVAL; + goto free_microvolt; + } + }
- return 0; + step = triplets ? 3 : 1; + + /* microvolt sanity check */ + if ((vcount != supply_count * step) || (icount != supply_count)) { + dev_err(dev, "%s: Invalid number of elements in opp-microvolt/amp property (v=%d i=%d c=%d t=%d)\n", + __func__, vcount, icount, supply_count * step, + triplets); + ret = -EINVAL; + goto free_microvolt; + } + + for (i = 0; i < supply_count; i++) + opp_parse_single_supply(supply + i, triplets, + microvolt + step * i, microamp + i); + +free_microvolt: + kfree(microamp); + kfree(microvolt); + + return ret; }
/** @@ -839,6 +926,8 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) * @dev: device for which we do this operation * @np: device node * @supply_count: Number of supplies available for each OPP + * @triplets: If true, microvolt property should be in form <target min max>, + * else <target>. * * This function adds an opp definition to the opp list and returns status. The * opp can be controlled using dev_pm_opp_enable/disable functions and may be @@ -859,7 +948,7 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) * -EINVAL Failed parsing the OPP node */ static int _opp_add_static_v2(struct device *dev, struct device_node *np, - int supply_count) + int supply_count, bool triplets) { struct device_opp *dev_opp; struct dev_pm_opp *new_opp; @@ -898,7 +987,7 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np, if (!of_property_read_u32(np, "clock-latency-ns", &val)) new_opp->clock_latency_ns = val;
- ret = opp_parse_supplies(new_opp, dev); + ret = opp_parse_supplies(new_opp, dev, supply_count, triplets); if (ret) goto free_opp;
@@ -1165,6 +1254,10 @@ void dev_pm_opp_of_remove_table(struct device *dev)
/* Find if dev_opp manages a single device */ if (list_is_singular(&dev_opp->dev_list)) { + /* Free dev_opp if no OPPs are added yet */ + if (list_empty(&dev_opp->opp_list)) + _remove_device_opp(dev_opp); + /* Free static OPPs */ list_for_each_entry_safe(opp, tmp, &dev_opp->opp_list, node) { if (!opp->dynamic) @@ -1197,7 +1290,9 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) { struct device_node *np; struct device_opp *dev_opp; - int ret = 0, count = 0; + const char **name; + int ret = 0, count, supply_count, string_count; + bool triplets;
dev_opp = _managed_opp(opp_np); if (dev_opp) { @@ -1207,12 +1302,44 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) return ret; }
+ triplets = of_property_read_bool(opp_np, "opp-microvolt-triplets"); + string_count = of_property_count_strings(opp_np, "supply-names"); + + /* Fallback to single power-supply if multiple aren't present */ + if (string_count <= 0) { + supply_count = 1; + string_count = 0; + } else { + supply_count = string_count; + } + + /* + * We need to add dev_opp before adding any OPPs, so that supply_names + * are valid while the OPPs are getting added. + */ + dev_opp = _add_device_opp(dev, supply_count); + if (!dev_opp) + return -ENOMEM; + + /* Parse supply names */ + name = dev_opp->supply_names; + for (count = 0; count < string_count; count++, name++) { + /* Parse supply names */ + ret = of_property_read_string_index(opp_np, "supply-names", + count, name); + if (ret) { + dev_err(dev, "%s: read supply names (%s) error (%d)\n", + __func__, opp_np->name, ret); + goto free_table; + } + } + /* We have opp-list node now, iterate over it and add OPPs */ + count = 0; for_each_available_child_of_node(opp_np, np) { count++;
- /* Todo: Add support for multiple supplies */ - ret = _opp_add_static_v2(dev, np, 1); + ret = _opp_add_static_v2(dev, np, supply_count, triplets); if (ret) { dev_err(dev, "%s: Failed to add OPP, %d\n", __func__, ret); @@ -1221,12 +1348,8 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) }
/* There should be one of more OPP defined */ - if (WARN_ON(!count)) - return -ENOENT; - - dev_opp = _find_device_opp(dev); - if (WARN_ON(IS_ERR(dev_opp))) { - ret = PTR_ERR(dev_opp); + if (WARN_ON(!count)) { + ret = -ENOENT; goto free_table; }
diff --git a/drivers/base/power/opp/debugfs.c b/drivers/base/power/opp/debugfs.c index e6ba29c04513..de2083d69297 100644 --- a/drivers/base/power/opp/debugfs.c +++ b/drivers/base/power/opp/debugfs.c @@ -31,12 +31,44 @@ void opp_debug_remove_one(struct dev_pm_opp *opp) debugfs_remove_recursive(opp->dentry); }
+int opp_debug_create_supplies(struct dev_pm_opp *opp, + struct device_opp *dev_opp, struct dentry *dentry) +{ + struct opp_supply *supply = opp->supplies; + char name[NAME_MAX]; + const char **supply_name = dev_opp->supply_names; + int i; + + for (i = 0; i < dev_opp->supply_count; i++, supply_name++) { + snprintf(name, sizeof(name), "%s_u_volt_target", *supply_name); + if (!debugfs_create_u32(name, S_IRUGO, dentry, + (u32 *)&supply->u_volt)) + return -ENOMEM; + + snprintf(name, sizeof(name), "%s_u_volt_min", *supply_name); + if (!debugfs_create_u32(name, S_IRUGO, dentry, + (u32 *)&supply->u_volt_min)) + return -ENOMEM; + + snprintf(name, sizeof(name), "%s_u_volt_max", *supply_name); + if (!debugfs_create_u32(name, S_IRUGO, dentry, + (u32 *)&supply->u_volt_max)) + return -ENOMEM; + } + + if (!debugfs_create_u32("u_amp", S_IRUGO, dentry, + (u32 *)&supply->u_amp)) + return -ENOMEM; + + return 0; +} + int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp) { struct dentry *pdentry = dev_opp->dentry; - struct opp_supply *supply = &opp->supplies[0]; struct dentry *d; char name[15]; + int ret;
/* Rate is unique to each OPP, use it to give opp-name */ sprintf(name, "opp:%lu", opp->rate); @@ -59,25 +91,14 @@ int opp_debug_create_one(struct dev_pm_opp *opp, struct device_opp *dev_opp) if (!debugfs_create_u32("rate_hz", S_IRUGO, d, (u32 *)&opp->rate)) return -ENOMEM;
- if (!debugfs_create_u32("u_volt_target", S_IRUGO, d, - (u32 *)&supply->u_volt)) - return -ENOMEM; - - if (!debugfs_create_u32("u_volt_min", S_IRUGO, d, - (u32 *)&supply->u_volt_min)) - return -ENOMEM; - - if (!debugfs_create_u32("u_volt_max", S_IRUGO, d, - (u32 *)&supply->u_volt_max)) - return -ENOMEM; - - if (!debugfs_create_u32("u_amp", S_IRUGO, d, (u32 *)&supply->u_amp)) - return -ENOMEM; - if (!debugfs_create_u32("clock_latency_ns", S_IRUGO, d, (u32 *)&opp->clock_latency_ns)) return -ENOMEM;
+ ret = opp_debug_create_supplies(opp, dev_opp, d); + if (ret) + return ret; + opp->dentry = d; return 0; } diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index a7a6917d6fbd..16575268f6ce 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -135,6 +135,7 @@ struct device_list_opp { * @opp_list: list of opps * @np: struct device_node pointer for opp's DT node. * @supply_count: Number of power-supplies + * @supply_names: Array of strings containing names of the power-supplies * @shared_opp: OPP is shared between multiple devices. * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. @@ -157,7 +158,10 @@ struct device_opp {
struct device_node *np; unsigned long clock_latency_ns_max; + unsigned int supply_count; + const char **supply_names; + bool shared_opp; struct dev_pm_opp *suspend_opp;
When the 'supply_names' property is present, the OPP core can request regulators for them. All the regulators from the 'supply_names' property must be present, in order to move forward.
If an error occurs, undo all allocations and return the appropriate error. This also helps in returning error on -EPROBE_DEFER case, where the driver must try again at a later point of time.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 30 +++++++++++++++++++++++++++--- drivers/base/power/opp/opp.h | 3 +++ 2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index d6e945ec6467..42ea5a332ddc 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -509,8 +509,10 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count) if (!IS_ERR(dev_opp)) return dev_opp;
- /* Allocate size for supply-names with dev_opp */ - size = sizeof(*dev_opp) + supply_count * sizeof(*dev_opp->supply_names); + /* Allocate memory for supply-names & regulators with dev_opp */ + size = sizeof(*dev_opp); + size += supply_count * sizeof(*dev_opp->supply_names); + size += supply_count * sizeof(*dev_opp->regulators);
/* * Allocate a new device OPP table. In the infrequent case where a new @@ -521,7 +523,8 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count) return NULL;
dev_opp->supply_count = supply_count; - dev_opp->supply_names = (const char **)(dev_opp + 1); + dev_opp->regulators = (struct regulator **)(dev_opp + 1); + dev_opp->supply_names = (const char **)(dev_opp->regulators + supply_count);
INIT_LIST_HEAD(&dev_opp->dev_list);
@@ -566,10 +569,15 @@ static void _kfree_device_rcu(struct rcu_head *head) static void _remove_device_opp(struct device_opp *dev_opp) { struct device_list_opp *list_dev; + int count;
if (!list_empty(&dev_opp->opp_list)) return;
+ /* Release regulators */ + for (count = dev_opp->supply_count - 1; count >= 0; count--) + regulator_put(dev_opp->regulators[count]); + list_dev = list_first_entry(&dev_opp->dev_list, struct device_list_opp, node);
@@ -1293,6 +1301,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) const char **name; int ret = 0, count, supply_count, string_count; bool triplets; + struct regulator *reg;
dev_opp = _managed_opp(opp_np); if (dev_opp) { @@ -1334,6 +1343,21 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) } }
+ /* Get supply regulators */ + name = dev_opp->supply_names; + for (count = 0; count < supply_count; count++, name++) { + /* Parse regulators */ + reg = regulator_get_optional(dev, *name); + if (IS_ERR(reg)) { + ret = PTR_ERR(reg); + if (ret != -EPROBE_DEFER) + dev_err(dev, "%s: no regulator (%s) found: %d\n", + __func__, *name, ret); + goto free_table; + } + dev_opp->regulators[count] = reg; + } + /* We have opp-list node now, iterate over it and add OPPs */ count = 0; for_each_available_child_of_node(opp_np, np) { diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index 16575268f6ce..830c4b654a51 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -19,6 +19,7 @@ #include <linux/list.h> #include <linux/limits.h> #include <linux/pm_opp.h> +#include <linux/regulator/consumer.h> #include <linux/rculist.h> #include <linux/rcupdate.h>
@@ -135,6 +136,7 @@ struct device_list_opp { * @opp_list: list of opps * @np: struct device_node pointer for opp's DT node. * @supply_count: Number of power-supplies + * @regulators: Array of regulators * @supply_names: Array of strings containing names of the power-supplies * @shared_opp: OPP is shared between multiple devices. * @dentry: debugfs dentry pointer of the real device directory (not links). @@ -160,6 +162,7 @@ struct device_opp { unsigned long clock_latency_ns_max;
unsigned int supply_count; + struct regulator **regulators; const char **supply_names;
bool shared_opp;
Disable any OPPs where the connected regulators aren't able to provide the specified voltage.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 42ea5a332ddc..04fe181b8132 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -834,6 +834,34 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, return ret; }
+static bool opp_supported_by_regulators(struct dev_pm_opp *opp, + struct device_opp *dev_opp) +{ + struct opp_supply *supply; + struct regulator *reg; + int count; + + for (count = 0; count < dev_opp->supply_count; count++) { + supply = opp->supplies + count; + reg = dev_opp->regulators[count]; + + /* Regulator may not be available for device */ + if (IS_ERR(reg)) + continue; + + if (!regulator_is_supported_voltage(reg, supply->u_volt_min, + supply->u_volt_max)) { + pr_warn("%s: OPP minuV: %lu maxuV: %lu, not supported by regulator: %s\n", + __func__, supply->u_volt_min, + supply->u_volt_max, + dev_opp->supply_names[count]); + return false; + } + } + + return true; +} + /* returns the number of entries used from microvolt */ static void opp_parse_single_supply(struct opp_supply *supply, bool triplet, u32 *microvolt, u32 *microamp) @@ -1016,6 +1044,12 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np, if (new_opp->clock_latency_ns > dev_opp->clock_latency_ns_max) dev_opp->clock_latency_ns_max = new_opp->clock_latency_ns;
+ if (!opp_supported_by_regulators(new_opp, dev_opp)) { + new_opp->available = false; + dev_warn(dev, "%s: OPP not supported by regulators (%lu)\n", + __func__, new_opp->rate); + } + mutex_unlock(&dev_opp_list_lock);
supply = &new_opp->supplies[0];
In few use cases (like: cpufreq), it is desired to get the maximum voltage latency for changing OPPs. Add support for that.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 59 +++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_opp.h | 6 +++++ 2 files changed, 65 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 04fe181b8132..e272e738a8d0 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -226,6 +226,65 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev) EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
/** + * dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds + * @dev: device for which we do this operation + * + * Return: This function returns the max voltage latency in nanoseconds. + * + * Locking: This function takes rcu_read_lock(). + */ +unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) +{ + struct device_opp *dev_opp; + struct dev_pm_opp *opp; + struct opp_supply *supply; + struct regulator *reg; + unsigned long latency_ns = 0; + unsigned long min_uV, max_uV; + int count, ret; + + rcu_read_lock(); + + dev_opp = _find_device_opp(dev); + if (IS_ERR(dev_opp)) + goto unlock; + + /* Sum max latencies for all supplies */ + for (count = 0; count < dev_opp->supply_count; count++) { + min_uV = ~0; + max_uV = 0; + + reg = dev_opp->regulators[count]; + + /* Regulator may not be available for device */ + if (IS_ERR(reg)) + continue; + + list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) { + if (!opp->available) + continue; + + supply = opp->supplies + count; + + if (supply->u_volt_min < min_uV) + min_uV = supply->u_volt_min; + if (supply->u_volt_max > max_uV) + max_uV = supply->u_volt_max; + } + + ret = regulator_set_voltage_time(reg, min_uV, max_uV); + if (ret > 0) + latency_ns += ret * 1000; + } + +unlock: + rcu_read_unlock(); + + return latency_ns; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_volt_latency); + +/** * dev_pm_opp_get_suspend_opp() - Get suspend opp * @dev: device for which we do this operation * diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 9a2e50337af9..fd58e2074721 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -34,6 +34,7 @@ bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp);
int dev_pm_opp_get_opp_count(struct device *dev); unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev); +unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev); struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, @@ -81,6 +82,11 @@ static inline unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev) return 0; }
+static inline unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) +{ + return 0; +} + static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev) { return NULL;
In few use cases (like: cpufreq), it is desired to get the maximum latency for changing OPPs. Add support for that.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 17 +++++++++++++++++ include/linux/pm_opp.h | 6 ++++++ 2 files changed, 23 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index e272e738a8d0..d0678e804a75 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -285,6 +285,23 @@ unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_volt_latency);
/** + * dev_pm_opp_get_max_transition_latency() - Get max transition latency in + * nanoseconds + * @dev: device for which we do this operation + * + * Return: This function returns the max transition latency, in nanoseconds, to + * switch from one OPP to other. + * + * Locking: This function takes rcu_read_lock(). + */ +unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev) +{ + return dev_pm_opp_get_max_volt_latency(dev) + + dev_pm_opp_get_max_clock_latency(dev); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_transition_latency); + +/** * dev_pm_opp_get_suspend_opp() - Get suspend opp * @dev: device for which we do this operation * diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index fd58e2074721..e36f347ff32b 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -35,6 +35,7 @@ bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp); int dev_pm_opp_get_opp_count(struct device *dev); unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev); unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev); +unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev); struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev);
struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev, @@ -87,6 +88,11 @@ static inline unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev) return 0; }
+static inline unsigned long dev_pm_opp_get_max_transition_latency(struct device *dev) +{ + return 0; +} + static inline struct dev_pm_opp *dev_pm_opp_get_suspend_opp(struct device *dev) { return NULL;
V2 bindings have better support for that and doesn't need special care. To use callbacks, like dev_pm_opp_get_max_{transition|volt}_latency(), irrespective of the bindings, the core needs to know clock/voltage tolerance for the earlier bindings.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 29 ++++++++++++++++++++++++++--- drivers/base/power/opp/opp.h | 5 +++++ 2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index d0678e804a75..9633f2fc3481 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -568,16 +568,19 @@ struct device_list_opp *_add_list_dev(const struct device *dev, * _add_device_opp() - Find device OPP table or allocate a new one * @dev: device for which we do this operation * @supply_count: Number of supplies available for each OPP. + * @opp_v2: true if parsing v2 bindings, else false. * * It tries to find an existing table first, if it couldn't find one, it * allocates a new OPP table and returns that. * * Return: valid device_opp pointer if success, else NULL. */ -static struct device_opp *_add_device_opp(struct device *dev, int supply_count) +static struct device_opp *_add_device_opp(struct device *dev, int supply_count, + bool opp_v2) { struct device_opp *dev_opp; struct device_list_opp *list_dev; + struct device_node *np; size_t size;
/* Check for existing list for 'dev' first */ @@ -598,6 +601,22 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count) if (!dev_opp) return NULL;
+ if (!opp_v2) { + /* + * Only required for backward compatibility with v1 bindings. + */ + np = of_node_get(dev->of_node); + if (np) { + u32 val; + + if (!of_property_read_u32(np, "clock-latency", &val)) + dev_opp->clock_latency_ns_max = val; + of_property_read_u32(np, "voltage-tolerance", + &dev_opp->voltage_tolerance_v1); + of_node_put(np); + } + } + dev_opp->supply_count = supply_count; dev_opp->regulators = (struct regulator **)(dev_opp + 1); dev_opp->supply_names = (const char **)(dev_opp->regulators + supply_count); @@ -767,7 +786,7 @@ _allocate_opp(struct device *dev, struct device_opp **dev_opp, int supply_count)
INIT_LIST_HEAD(&opp->node);
- *dev_opp = _add_device_opp(dev, supply_count); + *dev_opp = _add_device_opp(dev, supply_count, false); if (!*dev_opp) { kfree(opp); return NULL; @@ -873,6 +892,7 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt, { struct device_opp *dev_opp; struct dev_pm_opp *new_opp; + unsigned long tol; int ret;
/* Hold our list modification lock here */ @@ -886,7 +906,10 @@ static int _opp_add_v1(struct device *dev, unsigned long freq, long u_volt,
/* populate the opp table */ new_opp->rate = freq; + tol = u_volt * dev_opp->voltage_tolerance_v1 / 100; new_opp->supplies[0].u_volt = u_volt; + new_opp->supplies[0].u_volt_min = u_volt - tol; + new_opp->supplies[0].u_volt_max = u_volt + tol; new_opp->available = true; new_opp->dynamic = dynamic;
@@ -1436,7 +1459,7 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) * We need to add dev_opp before adding any OPPs, so that supply_names * are valid while the OPPs are getting added. */ - dev_opp = _add_device_opp(dev, supply_count); + dev_opp = _add_device_opp(dev, supply_count, true); if (!dev_opp) return -ENOMEM;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index 830c4b654a51..55205cbe59dc 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -142,6 +142,8 @@ struct device_list_opp { * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. * + * @voltage_tolerance_v1: In percentage, for v1 bindings only. + * * This is an internal data structure maintaining the link to opps attached to * a device. This structure is not meant to be shared to users as it is * meant for book keeping and private to OPP library. @@ -173,6 +175,9 @@ struct device_opp { struct dentry *dentry; char dentry_name[NAME_MAX]; #endif + + /* For backward compatibility with v1 bindings */ + unsigned int voltage_tolerance_v1; };
/* Routines internal to opp core */
OPP core has got almost everything now to manage device's OPP transitions, the only thing left is device's clk. Get that as well.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 9 +++++++++ drivers/base/power/opp/opp.h | 3 +++ 2 files changed, 12 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 9633f2fc3481..a04dcacb8a07 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -636,6 +636,11 @@ static struct device_opp *_add_device_opp(struct device *dev, int supply_count, if (supply_count == 1) *dev_opp->supply_names = dev_name(dev);
+ /* Find clk for the device */ + dev_opp->clk = clk_get(dev, NULL); + if (IS_ERR(dev_opp->clk)) + dev_dbg(dev, "%s: Couldn't find clock\n", __func__); + srcu_init_notifier_head(&dev_opp->srcu_head); INIT_LIST_HEAD(&dev_opp->opp_list);
@@ -669,6 +674,10 @@ static void _remove_device_opp(struct device_opp *dev_opp) if (!list_empty(&dev_opp->opp_list)) return;
+ /* Release clk */ + if (!IS_ERR(dev_opp->clk)) + clk_put(dev_opp->clk); + /* Release regulators */ for (count = dev_opp->supply_count - 1; count >= 0; count--) regulator_put(dev_opp->regulators[count]); diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index 55205cbe59dc..e36ee6296f1a 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -14,6 +14,7 @@ #ifndef __DRIVER_OPP_H__ #define __DRIVER_OPP_H__
+#include <linux/clk.h> #include <linux/device.h> #include <linux/kernel.h> #include <linux/list.h> @@ -135,6 +136,7 @@ struct device_list_opp { * @dev_list: list of devices that share these OPPs * @opp_list: list of opps * @np: struct device_node pointer for opp's DT node. + * @clk: struct clk pointer * @supply_count: Number of power-supplies * @regulators: Array of regulators * @supply_names: Array of strings containing names of the power-supplies @@ -163,6 +165,7 @@ struct device_opp { struct device_node *np; unsigned long clock_latency_ns_max;
+ struct clk *clk; unsigned int supply_count; struct regulator **regulators; const char **supply_names;
The new OPP V2 bindings have a way to find supplies for a particular device. But the old V1 bindings doesn't have any API to do it today.
This is required in order to move the complexity of switching OPPs (i.e. changing clock and voltages), into the OPP core, rather then keeping that in individual drivers.
This patch adds another API, to be used only for V1 bindings, which first finds the regulator for the dev_opp and then disables all OPPs that aren't supported by the regulator.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 74 ++++++++++++++++++++++++++++++++++++++++++- include/linux/pm_opp.h | 6 ++++ 2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index a04dcacb8a07..4ee0911b97ea 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -970,6 +970,73 @@ static bool opp_supported_by_regulators(struct dev_pm_opp *opp, return true; }
+/** + * dev_pm_opp_set_regulator() - Set regulator for the device OPP + * @dev: Device for which the regulator has to be set. + * @id: String used to find regulator. + * + * This is only required for the V1 bindings, as regulator is automatically + * found for the V2 bindings. OPP core finds a regulator with name <reg>-supply, + * and then get/put it automatically. The name of the debugfs files isn't + * changed however, to keep it simple. They are named as <dev_name>_*. + */ +int dev_pm_opp_set_regulator(struct device *dev, const char *id) +{ + struct device_opp *dev_opp; + struct dev_pm_opp *opp; + struct regulator *reg; + int ret = 0; + + rcu_read_lock(); + + dev_opp = _find_device_opp(dev); + if (IS_ERR(dev_opp)) { + ret = PTR_ERR(dev_opp); + dev_err(dev, "%s: no device opp found: %d\n", __func__, ret); + goto unlock; + } + + /* Do we already have a regulator attached to this dev_opp? */ + if (!IS_ERR_OR_NULL(*dev_opp->regulators)) { + dev_err(dev, "%s: can't add (%s), regulator already present\n", + __func__, id); + ret = -EINVAL; + goto unlock; + } + + reg = regulator_get_optional(dev, id); + + /* + * Few platforms may not have regulators for the device, and we need to + * save error number in that case as well. + */ + *dev_opp->regulators = reg; + + if (IS_ERR(reg)) { + ret = PTR_ERR(reg); + if (ret != -EPROBE_DEFER) + dev_err(dev, "%s: no regulator (%s) found: %d\n", + __func__, id, ret); + goto unlock; + } + + list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) { + /* Disable OPPs, that aren't supported by the regulator */ + if (opp_supported_by_regulators(opp, dev_opp)) + continue; + + opp->available = false; + dev_warn(dev, "%s: disabled OPP (%lu), not supported by regulator (%s)\n", + __func__, opp->rate, id); + } + +unlock: + rcu_read_unlock(); + + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_set_regulator); + /* returns the number of entries used from microvolt */ static void opp_parse_single_supply(struct opp_supply *supply, bool triplet, u32 *microvolt, u32 *microamp) @@ -1493,9 +1560,14 @@ static int _of_add_opp_table_v2(struct device *dev, struct device_node *opp_np) if (IS_ERR(reg)) { ret = PTR_ERR(reg); if (ret != -EPROBE_DEFER) + dev_err(dev, "%s: deferring, no regulator (%s) found: %d\n", + __func__, *name, ret); + /* Regulator may not be compulsory for the device */ + if (!string_count) dev_err(dev, "%s: no regulator (%s) found: %d\n", __func__, *name, ret); - goto free_table; + else + goto free_table; } dev_opp->regulators[count] = reg; } diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index e36f347ff32b..e8aee03b974a 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -57,6 +57,7 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq); int dev_pm_opp_disable(struct device *dev, unsigned long freq);
struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev); +int dev_pm_opp_set_regulator(struct device *dev, const char *id); #else static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) { @@ -141,6 +142,11 @@ static inline struct srcu_notifier_head *dev_pm_opp_get_notifier( { return ERR_PTR(-EINVAL); } + +static inline int dev_pm_opp_set_regulator(struct device *dev, const char *id) +{ + return -EINVAL; +} #endif /* CONFIG_PM_OPP */
#if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
This adds a routine, dev_pm_opp_set_rate(), responsible for configuring power-supplies and clock source for on an OPP.
The OPP is found by matching against the target_freq passed to the routine. This shall replace similar code present in most of the OPP users and help simplify them a lot.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 152 ++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_opp.h | 7 ++ 2 files changed, 159 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 4ee0911b97ea..9fbc38222ca0 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -523,6 +523,158 @@ struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev, } EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);
+/* + * _set_opp_voltages() - Configure device supplies for an OPP + * @dev: device for which we do this operation + * @dev_opp: dev_opp for the device + * @opp: opp for getting supply voltage levels + * + * This is used to configure all the power supplies, used by the device, to the + * voltage levels specified by a particular OPP. + */ +static int _set_opp_voltages(struct device *dev, struct device_opp *dev_opp, + struct dev_pm_opp *opp) +{ + struct opp_supply *supply; + struct regulator *reg; + const char *name; + int count, ret; + + /* Sum max latencies for all supplies */ + for (count = 0; count < dev_opp->supply_count; count++) { + supply = &opp->supplies[count]; + reg = dev_opp->regulators[count]; + name = dev_opp->supply_names[count]; + + /* Regulator may not be available for device */ + if (IS_ERR(reg)) { + dev_dbg(dev, "%s: skipping %s regulator: %ld\n", + __func__, name, PTR_ERR(reg)); + continue; + } + + dev_dbg(dev, "%s: Supply: %s, voltages (mV): %lu %lu %lu\n", + __func__, name, supply->u_volt_min, supply->u_volt, + supply->u_volt_max); + + ret = regulator_set_voltage_triplet(reg, supply->u_volt_min, + supply->u_volt, + supply->u_volt_max); + if (ret) { + dev_err(dev, "%s: failed to set voltage for %s regulator (%lu %lu %lu mV): %d\n", + __func__, name, supply->u_volt_min, + supply->u_volt, supply->u_volt_max, ret); + return ret; + } + } + + return 0; +} + +/** + * dev_pm_opp_set_rate() - Configure new OPP based on frequency + * @dev: device for which we do this operation + * @target_freq: frequency to achieve + * + * This configures the power-supplies and clock source to the levels specified + * by the OPP corresponding to the target_freq. It takes all necessary locks + * required for the operation and the caller doesn't need to care about the rcu + * locks. + */ +int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) +{ + struct device_opp *dev_opp; + struct dev_pm_opp *old_opp, *opp; + struct clk *clk; + unsigned long freq, old_freq; + int ret; + + if (unlikely(!dev || !target_freq)) { + pr_err("%s: Invalid arguments dev=%p, freq=%lu\n", __func__, + dev, target_freq); + return -EINVAL; + } + + dev_opp = _find_device_opp(dev); + if (IS_ERR(dev_opp)) { + dev_err(dev, "%s: device opp doesn't exist\n", __func__); + return PTR_ERR(dev_opp); + } + + clk = dev_opp->clk; + if (IS_ERR(clk)) { + dev_err(dev, "%s: No clock available for the device\n", + __func__); + return -EINVAL; + } + + freq = clk_round_rate(clk, target_freq); + if ((long)freq <= 0) + freq = target_freq; + + old_freq = clk_get_rate(clk); + + /* Return early if nothing to do */ + if (old_freq == freq) { + dev_dbg(dev, "%s: old/new frequencies (%lu Hz) are same, nothing to do\n", + __func__, freq); + return 0; + } + + rcu_read_lock(); + + old_opp = dev_pm_opp_find_freq_ceil(dev, &old_freq); + opp = dev_pm_opp_find_freq_ceil(dev, &freq); + + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); + dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n", + __func__, freq, ret); + goto unlock; + } + + /* Scaling up? Scale voltage before frequency */ + if (freq > old_freq) { + ret = _set_opp_voltages(dev, dev_opp, opp); + if (ret) + goto restore_voltage; + } + + /* Change frequency */ + + dev_dbg(dev, "%s: switching OPP: %lu Hz --> %lu Hz\n", + __func__, old_freq, freq); + + ret = clk_set_rate(clk, freq); + if (ret) { + dev_err(dev, "%s: failed to set clock rate: %d\n", __func__, + ret); + goto restore_voltage; + } + + /* Scaling down? Scale voltage after frequency */ + if (freq < old_freq) { + ret = _set_opp_voltages(dev, dev_opp, opp); + if (ret) + goto restore_freq; + } + + goto unlock; + +restore_freq: + if (clk_set_rate(clk, old_freq)) + dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n", + __func__, old_freq); +restore_voltage: + /* This shouldn't harm if the voltages weren't updated earlier */ + _set_opp_voltages(dev, dev_opp, old_opp); +unlock: + rcu_read_unlock(); + + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_set_rate); + /* List-dev Helpers */ static void _kfree_list_dev_rcu(struct rcu_head *head) { diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index e8aee03b974a..b0f71c0f333a 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -58,6 +58,7 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq);
struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev); int dev_pm_opp_set_regulator(struct device *dev, const char *id); +int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq); #else static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) { @@ -147,6 +148,12 @@ static inline int dev_pm_opp_set_regulator(struct device *dev, const char *id) { return -EINVAL; } + +static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) +{ + return -EINVAL; +} + #endif /* CONFIG_PM_OPP */
#if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
dev_pm_opp_of_add_table() can fail with a return value of -EPROBE_DEFER and in such a case we should skip printing the error message, as its not really an error.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/cpu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/base/power/opp/cpu.c b/drivers/base/power/opp/cpu.c index 9ac691341fb0..f259a994fd16 100644 --- a/drivers/base/power/opp/cpu.c +++ b/drivers/base/power/opp/cpu.c @@ -198,8 +198,9 @@ int dev_pm_opp_of_cpumask_add_table(cpumask_var_t cpumask)
ret = dev_pm_opp_of_add_table(cpu_dev); if (ret) { - pr_err("%s: couldn't find opp table for cpu:%d, %d\n", - __func__, cpu, ret); + if (ret != -EPROBE_DEFER) + pr_err("%s: couldn't find opp table for cpu:%d, %d\n", + __func__, cpu, ret);
/* Free all other OPPs */ dev_pm_opp_of_cpumask_remove_table(cpumask);
linaro-kernel@lists.linaro.org
-
Mark Brown -
Nishanth Menon -
Rob Herring -
Stephen Boyd -
Viresh Kumar