Hi Rafael/Stephen,
The bindings are reviewed now and here is the code to parse them. The first patch is a minor cleanup and other two contain the real stuff.
Rebased over: v4.4-rc1 + OPP debugfs support patch. Tested-on: Exynos 5250, dual core A15.
V1->V2: - Fixed locking - NUL terminate strings instead of sprintf - Remove NULL checkers for the routines - constify 'versions' - s/EINVAL/EBUSY - updated comments over routines - Use of_property_read_u32_index() instead of allocating arrays - remove dev_opp for failures
Viresh Kumar (3): PM / OPP: Add missing doc comments PM / OPP: Parse 'opp-supported-hw' binding PM / OPP: Parse 'opp-<prop>-<name>' bindings
drivers/base/power/opp/core.c | 301 +++++++++++++++++++++++++++++++++++++++--- drivers/base/power/opp/opp.h | 11 +- include/linux/pm_opp.h | 22 +++ 3 files changed, 318 insertions(+), 16 deletions(-)
Few doc-style comments were missing, add them. Rearrange another one to match the sequence within the structure.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Acked-by: Pavel Machek pavel@ucw.cz Reviewed-by: Stephen Boyd sboyd@codeaurora.org --- drivers/base/power/opp/opp.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index a6bd8d2c2b47..b8880c7f8be1 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -51,8 +51,8 @@ extern struct mutex dev_opp_list_lock; * are protected by the dev_opp_list_lock for integrity. * IMPORTANT: the opp nodes should be maintained in increasing * order. - * @dynamic: not-created from static DT entries. * @available: true/false - marks if this OPP as available or not + * @dynamic: not-created from static DT entries. * @turbo: true if turbo (boost) OPP * @suspend: true if suspend OPP * @rate: Frequency in hertz @@ -126,7 +126,9 @@ 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. + * @clock_latency_ns_max: Max clock latency in nanoseconds. * @shared_opp: OPP is shared between multiple devices. + * @suspend_opp: Pointer to OPP to be used during device suspend. * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. *
On Thursday, November 19, 2015 09:13:56 AM Viresh Kumar wrote:
Few doc-style comments were missing, add them. Rearrange another one to match the sequence within the structure.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Acked-by: Pavel Machek pavel@ucw.cz Reviewed-by: Stephen Boyd sboyd@codeaurora.org
drivers/base/power/opp/opp.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index a6bd8d2c2b47..b8880c7f8be1 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -51,8 +51,8 @@ extern struct mutex dev_opp_list_lock;
are protected by the dev_opp_list_lock for integrity.IMPORTANT: the opp nodes should be maintained in increasingorder.
- @dynamic: not-created from static DT entries.
- @available: true/false - marks if this OPP as available or not
- @dynamic: not-created from static DT entries.
- @turbo: true if turbo (boost) OPP
- @suspend: true if suspend OPP
- @rate: Frequency in hertz
@@ -126,7 +126,9 @@ 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.
- @clock_latency_ns_max: Max clock latency in nanoseconds.
- @shared_opp: OPP is shared between multiple devices.
- @suspend_opp: Pointer to OPP to be used during device suspend.
- @dentry: debugfs dentry pointer of the real device directory (not links).
- @dentry_name: Name of the real dentry.
Queued up for 4.5, thanks!
ACKs are needed for the other two.
Thanks, Rafael
OPP bindings allow a platform to enable OPPs based on the version of the hardware they are used for.
Add support to the OPP-core to parse these bindings, by introducing dev_pm_opp_{set|put}_supported_hw() APIs.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 142 ++++++++++++++++++++++++++++++++++++++++++ drivers/base/power/opp/opp.h | 5 ++ include/linux/pm_opp.h | 13 ++++ 3 files changed, 160 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6aa172be6e8e..5449bae74a44 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -559,6 +559,9 @@ static void _remove_device_opp(struct device_opp *dev_opp) if (!list_empty(&dev_opp->opp_list)) return;
+ if (dev_opp->supported_hw) + return; + list_dev = list_first_entry(&dev_opp->dev_list, struct device_list_opp, node);
@@ -834,6 +837,139 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) }
/** + * dev_pm_opp_set_supported_hw() - Set supported platforms + * @dev: Device for which supported-hw has to be set. + * @versions: Array of hierarchy of versions to match. + * @count: Number of elements in the array. + * + * This is required only for the V2 bindings, and it enables a platform to + * specify the hierarchy of versions it supports. OPP layer will then enable + * OPPs, which are available for those versions, based on its 'opp-supported-hw' + * property. + * + * Locking: The internal device_opp and opp structures are RCU protected. + * Hence this function internally uses RCU updater strategy with mutex locks + * to keep the integrity of the internal data structures. Callers should ensure + * that this function is *NOT* called under RCU protection or in contexts where + * mutex cannot be locked. + */ +int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions, + unsigned int count) +{ + struct device_opp *dev_opp; + int ret = 0; + + /* Hold our list modification lock here */ + mutex_lock(&dev_opp_list_lock); + + dev_opp = _add_device_opp(dev); + if (!dev_opp) { + ret = -ENOMEM; + goto unlock; + } + + /* Do we already have a version hierarchy associated with dev_opp? */ + if (dev_opp->supported_hw) { + dev_err(dev, "%s: Already have supported hardware list\n", + __func__); + ret = -EBUSY; + goto err; + } + + dev_opp->supported_hw = kmemdup(versions, count * sizeof(*versions), + GFP_KERNEL); + if (!dev_opp->supported_hw) { + ret = -ENOMEM; + goto err; + } + + dev_opp->supported_hw_count = count; + mutex_unlock(&dev_opp_list_lock); + return 0; + +err: + _remove_device_opp(dev_opp); +unlock: + mutex_unlock(&dev_opp_list_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_set_supported_hw); + +/** + * dev_pm_opp_put_supported_hw() - Releases resources blocked for supported hw + * @dev: Device for which supported-hw has to be set. + * + * This is required only for the V2 bindings, and is called for a matching + * dev_pm_opp_set_supported_hw(). Until this is called, the device_opp structure + * will not be freed. + * + * Locking: The internal device_opp and opp structures are RCU protected. + * Hence this function internally uses RCU updater strategy with mutex locks + * to keep the integrity of the internal data structures. Callers should ensure + * that this function is *NOT* called under RCU protection or in contexts where + * mutex cannot be locked. + */ +void dev_pm_opp_put_supported_hw(struct device *dev) +{ + struct device_opp *dev_opp; + + /* Hold our list modification lock here */ + mutex_lock(&dev_opp_list_lock); + + /* Check for existing list for 'dev' first */ + dev_opp = _find_device_opp(dev); + if (IS_ERR(dev_opp)) { + dev_err(dev, "Failed to find dev_opp: %ld\n", PTR_ERR(dev_opp)); + goto unlock; + } + + if (!dev_opp->supported_hw) { + dev_err(dev, "%s: Doesn't have supported hardware list\n", + __func__); + goto unlock; + } + + kfree(dev_opp->supported_hw); + dev_opp->supported_hw = NULL; + dev_opp->supported_hw_count = 0; + + /* Try freeing device_opp if this was the last blocking resource */ + _remove_device_opp(dev_opp); + +unlock: + mutex_unlock(&dev_opp_list_lock); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_put_supported_hw); + +static bool _opp_is_supported(struct device *dev, struct device_opp *dev_opp, + struct device_node *np) +{ + unsigned int count = dev_opp->supported_hw_count; + u32 version; + int ret; + + if (!dev_opp->supported_hw) + return true; + + while (count--) { + ret = of_property_read_u32_index(np, "opp-supported-hw", count, + &version); + if (ret) { + dev_warn(dev, "%s: failed to read opp-supported-hw property at index %d: %d\n", + __func__, count, ret); + return false; + } + + /* Both of these are bitwise masks of the versions */ + if (!(version & dev_opp->supported_hw[count])) + return false; + } + + return true; +} + +/** * _opp_add_static_v2() - Allocate static OPPs (As per 'v2' DT bindings) * @dev: device for which we do this operation * @np: device node @@ -879,6 +1015,12 @@ static int _opp_add_static_v2(struct device *dev, struct device_node *np) goto free_opp; }
+ /* Check if the OPP supports hardware's hierarchy of versions or not */ + if (!_opp_is_supported(dev, dev_opp, np)) { + dev_dbg(dev, "OPP not supported by hardware: %llu\n", rate); + goto free_opp; + } + /* * Rate is defined as an unsigned long in clk API, and so casting * explicitly to its type. Must be fixed once rate is 64 bit diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index b8880c7f8be1..70f4564a6ab9 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -129,6 +129,8 @@ struct device_list_opp { * @clock_latency_ns_max: Max clock latency in nanoseconds. * @shared_opp: OPP is shared between multiple devices. * @suspend_opp: Pointer to OPP to be used during device suspend. + * @supported_hw: Array of version number to support. + * @supported_hw_count: Number of elements in supported_hw array. * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. * @@ -153,6 +155,9 @@ struct device_opp { bool shared_opp; struct dev_pm_opp *suspend_opp;
+ unsigned int *supported_hw; + unsigned int supported_hw_count; + #ifdef CONFIG_DEBUG_FS struct dentry *dentry; char dentry_name[NAME_MAX]; diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 9a2e50337af9..3a85110242f0 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -55,6 +55,9 @@ 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_supported_hw(struct device *dev, const u32 *versions, + unsigned int count); +void dev_pm_opp_put_supported_hw(struct device *dev); #else static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) { @@ -129,6 +132,16 @@ static inline struct srcu_notifier_head *dev_pm_opp_get_notifier( { return ERR_PTR(-EINVAL); } + +static inline int dev_pm_opp_set_supported_hw(struct device *dev, + const u32 *versions, + unsigned int count) +{ + return -EINVAL; +} + +static inline void dev_pm_opp_put_supported_hw(struct device *dev) {} + #endif /* CONFIG_PM_OPP */
#if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
On 11/19, Viresh Kumar wrote:
OPP bindings allow a platform to enable OPPs based on the version of the hardware they are used for.
Add support to the OPP-core to parse these bindings, by introducing dev_pm_opp_{set|put}_supported_hw() APIs.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
drivers/base/power/opp/core.c | 142 ++++++++++++++++++++++++++++++++++++++++++ drivers/base/power/opp/opp.h | 5 ++ include/linux/pm_opp.h | 13 ++++ 3 files changed, 160 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6aa172be6e8e..5449bae74a44 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -559,6 +559,9 @@ static void _remove_device_opp(struct device_opp *dev_opp) if (!list_empty(&dev_opp->opp_list)) return;
- if (dev_opp->supported_hw)
return;- list_dev = list_first_entry(&dev_opp->dev_list, struct device_list_opp, node);
@@ -834,6 +837,139 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev) } /**
- dev_pm_opp_set_supported_hw() - Set supported platforms
- @dev: Device for which supported-hw has to be set.
- @versions: Array of hierarchy of versions to match.
- @count: Number of elements in the array.
- This is required only for the V2 bindings, and it enables a platform to
- specify the hierarchy of versions it supports. OPP layer will then enable
- OPPs, which are available for those versions, based on its 'opp-supported-hw'
- property.
- Locking: The internal device_opp and opp structures are RCU protected.
- Hence this function internally uses RCU updater strategy with mutex locks
- to keep the integrity of the internal data structures. Callers should ensure
- that this function is *NOT* called under RCU protection or in contexts where
- mutex cannot be locked.
- */
+int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
unsigned int count)+{
- struct device_opp *dev_opp;
- int ret = 0;
- /* Hold our list modification lock here */
- mutex_lock(&dev_opp_list_lock);
- dev_opp = _add_device_opp(dev);
So this function will publish an opp to the list...
- if (!dev_opp) {
ret = -ENOMEM;goto unlock;- }
- /* Do we already have a version hierarchy associated with dev_opp? */
- if (dev_opp->supported_hw) {
dev_err(dev, "%s: Already have supported hardware list\n",__func__);ret = -EBUSY;goto err;- }
- dev_opp->supported_hw = kmemdup(versions, count * sizeof(*versions),
GFP_KERNEL);
And then we're going to modify said opp here under the mutex lock.
- if (!dev_opp->supported_hw) {
ret = -ENOMEM;goto err;- }
- dev_opp->supported_hw_count = count;
So we've properly handled the concurrent writer case (which is probably not even real), but we have improperly handled the case where a reader is running in parallel to the writer. We should only list_add_rcu the pointer once we're done modifying the pointer we created. Otherwise a reader can come along and see the half initialized structure, which is not good.
I'm worried that the RCU locking is messed up in other places in this file now too. Hopefully not, but it's going to require an audit.
On 25-11-15, 12:51, Stephen Boyd wrote:
+int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions,
unsigned int count)+{
- struct device_opp *dev_opp;
- int ret = 0;
- /* Hold our list modification lock here */
- mutex_lock(&dev_opp_list_lock);
- dev_opp = _add_device_opp(dev);
So this function will publish an opp to the list...
Not an opp but opp-dev.
- if (!dev_opp) {
ret = -ENOMEM;goto unlock;- }
- /* Do we already have a version hierarchy associated with dev_opp? */
- if (dev_opp->supported_hw) {
dev_err(dev, "%s: Already have supported hardware list\n",__func__);ret = -EBUSY;goto err;- }
- dev_opp->supported_hw = kmemdup(versions, count * sizeof(*versions),
GFP_KERNEL);And then we're going to modify said opp here under the mutex lock.
opp-dev ..
- if (!dev_opp->supported_hw) {
ret = -ENOMEM;goto err;- }
- dev_opp->supported_hw_count = count;
So we've properly handled the concurrent writer case (which is probably not even real), but we have improperly handled the case where a reader is running in parallel to the writer. We should only list_add_rcu the pointer once we're done modifying the pointer we created. Otherwise a reader can come along and see the half initialized structure, which is not good.
This function will be called, from some platform code, before the OPP table is initialized. It isn't useful to call it after the OPPs are added for the device. So there wouldn't be any concurrent reader.
I'm worried that the RCU locking is messed up in other places in this file now too. Hopefully not, but it's going to require an audit.
I do share your concern.. But this stuff should be okay. Even the other patch as well.
On 27-11-15, 10:15, Viresh Kumar wrote:
- dev_opp->supported_hw = kmemdup(versions, count * sizeof(*versions),
GFP_KERNEL);And then we're going to modify said opp here under the mutex lock.
opp-dev ..
- if (!dev_opp->supported_hw) {
ret = -ENOMEM;goto err;- }
- dev_opp->supported_hw_count = count;
So we've properly handled the concurrent writer case (which is probably not even real), but we have improperly handled the case where a reader is running in parallel to the writer. We should only list_add_rcu the pointer once we're done modifying the pointer we created. Otherwise a reader can come along and see the half initialized structure, which is not good.
This function will be called, from some platform code, before the OPP table is initialized. It isn't useful to call it after the OPPs are added for the device. So there wouldn't be any concurrent reader.
Since these functions are *only* going to be called before any OPPs are added for the device, and hence ruling out any concurrent readers, maybe we can guarantee that with this:
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 5449bae74a44..ec74d98afe75 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -876,6 +876,9 @@ int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions, goto err; }
+ /* Make sure there are no concurrent readers while updating dev_opp */ + WARN_ON(!list_empty(&dev_opp->opp_list)); + dev_opp->supported_hw = kmemdup(versions, count * sizeof(*versions), GFP_KERNEL); if (!dev_opp->supported_hw) { @@ -924,6 +927,9 @@ void dev_pm_opp_put_supported_hw(struct device *dev) goto unlock; }
+ /* Make sure there are no concurrent readers while updating dev_opp */ + WARN_ON(!list_empty(&dev_opp->opp_list)); + if (!dev_opp->supported_hw) { dev_err(dev, "%s: Doesn't have supported hardware list\n", __func__);
I don't really want to create a duplicate dev_opp here and then replace that in the list, because we know that we have just created it.
Over that, if a reference to dev_opp is used somewhere else, lets say within the pm_opp structure, then updating all OPPs at such times would be really hard.
Lets close this before you go for vacations. I will get whatever solution you feel is right.
On 01-12-15, 12:22, Viresh Kumar wrote:
Since these functions are *only* going to be called before any OPPs are added for the device, and hence ruling out any concurrent readers, maybe we can guarantee that with this:
@Rafael: Stephen has gone for vacations until end of year :(, can you please help with some reviews ?
On 04-12-15, 13:53, Viresh Kumar wrote:
On 01-12-15, 12:22, Viresh Kumar wrote:
Since these functions are *only* going to be called before any OPPs are added for the device, and hence ruling out any concurrent readers, maybe we can guarantee that with this:
@Rafael: Stephen has gone for vacations until end of year :(, can you please help with some reviews ?
Ping !!
On Tuesday, December 08, 2015 08:31:05 PM Viresh Kumar wrote:
On 04-12-15, 13:53, Viresh Kumar wrote:
On 01-12-15, 12:22, Viresh Kumar wrote:
Since these functions are *only* going to be called before any OPPs are added for the device, and hence ruling out any concurrent readers, maybe we can guarantee that with this:
@Rafael: Stephen has gone for vacations until end of year :(, can you please help with some reviews ?
Ping !!
I don't remember that code clearly and it would take some time for me to recall it and then figure out the details about the changes. Not impossible, but quite honestly I have other things to spend that time on.
Also, are the patches so urgent that they really can't wait for Stephen to get back? I'd really prefer the discussion between you guys to settle without my intervention.
Thanks, Rafael
On 08-12-15, 17:50, Rafael J. Wysocki wrote:
I don't remember that code clearly and it would take some time for me to recall it and then figure out the details about the changes. Not impossible, but quite honestly I have other things to spend that time on.
Also, are the patches so urgent that they really can't wait for Stephen to get back? I'd really prefer the discussion between you guys to settle without my intervention.
I am talking only about two patches, which are required to base other work that Lee would be doing. I will resend the patches with all the details in them. If Stephen finds something wrong about them, we can get that fixed later as well.
On Tuesday, December 08, 2015 10:01:26 PM Viresh Kumar wrote:
On 08-12-15, 17:50, Rafael J. Wysocki wrote:
I don't remember that code clearly and it would take some time for me to recall it and then figure out the details about the changes. Not impossible, but quite honestly I have other things to spend that time on.
Also, are the patches so urgent that they really can't wait for Stephen to get back? I'd really prefer the discussion between you guys to settle without my intervention.
I am talking only about two patches, which are required to base other work that Lee would be doing. I will resend the patches with all the details in them. If Stephen finds something wrong about them, we can get that fixed later as well.
If there are only the two patches, why don't you ask Lee to include them into the series he's working on? The dependency would be clear then too.
Thanks, Rafael
On 09-12-15, 02:15, Rafael J. Wysocki wrote:
If there are only the two patches, why don't you ask Lee to include them into the series he's working on? The dependency would be clear then too.
His series was already out, and it was really about STi's cpufreq driver and so I have resent my two patches again.
On Wednesday, December 09, 2015 08:04:20 AM Viresh Kumar wrote:
On 09-12-15, 02:15, Rafael J. Wysocki wrote:
If there are only the two patches, why don't you ask Lee to include them into the series he's working on? The dependency would be clear then too.
His series was already out, and it was really about STi's cpufreq driver and so I have resent my two patches again.
OK, I'll look at them shortly.
OPP bindings (for few properties) allow a platform to choose a value/range among a set of available options. The options are present as opp-<prop>-<name>, where the platform needs to supply the <name> string.
The OPP properties which allow such an option are: opp-microvolt and opp-microamp.
Add support to the OPP-core to parse these bindings, by introducing dev_pm_opp_{set|put}_prop_name() APIs.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- drivers/base/power/opp/core.c | 159 ++++++++++++++++++++++++++++++++++++++---- drivers/base/power/opp/opp.h | 2 + include/linux/pm_opp.h | 9 +++ 3 files changed, 155 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 5449bae74a44..18152e032b19 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -562,6 +562,9 @@ static void _remove_device_opp(struct device_opp *dev_opp) if (dev_opp->supported_hw) return;
+ if (dev_opp->prop_name) + return; + list_dev = list_first_entry(&dev_opp->dev_list, struct device_list_opp, node);
@@ -794,35 +797,48 @@ 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) +static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, + struct device_opp *dev_opp) { u32 microvolt[3] = {0}; u32 val; int count, ret; + struct property *prop = NULL; + char name[NAME_MAX]; + + /* Search for "opp-microvolt-<name>" */ + if (dev_opp->prop_name) { + sprintf(name, "opp-microvolt-%s", dev_opp->prop_name); + prop = of_find_property(opp->np, name, NULL); + } + + if (!prop) { + /* Search for "opp-microvolt" */ + name[13] = '\0'; + prop = of_find_property(opp->np, name, NULL);
- /* Missing property isn't a problem, but an invalid entry is */ - if (!of_find_property(opp->np, "opp-microvolt", NULL)) - return 0; + /* Missing property isn't a problem, but an invalid entry is */ + if (!prop) + return 0; + }
- count = of_property_count_u32_elems(opp->np, "opp-microvolt"); + count = of_property_count_u32_elems(opp->np, name); if (count < 0) { - dev_err(dev, "%s: Invalid opp-microvolt property (%d)\n", - __func__, count); + dev_err(dev, "%s: Invalid %s property (%d)\n", + __func__, name, count); return count; }
/* 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); + dev_err(dev, "%s: Invalid number of elements in %s property (%d)\n", + __func__, name, count); return -EINVAL; }
- ret = of_property_read_u32_array(opp->np, "opp-microvolt", microvolt, - count); + ret = of_property_read_u32_array(opp->np, name, microvolt, count); if (ret) { - dev_err(dev, "%s: error parsing opp-microvolt: %d\n", __func__, - ret); + dev_err(dev, "%s: error parsing %s: %d\n", __func__, name, ret); return -EINVAL; }
@@ -830,7 +846,20 @@ static int opp_parse_supplies(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)) + /* Search for "opp-microamp-<name>" */ + prop = NULL; + if (dev_opp->prop_name) { + sprintf(name, "opp-microamp-%s", dev_opp->prop_name); + prop = of_find_property(opp->np, name, NULL); + } + + if (!prop) { + /* Search for "opp-microamp" */ + name[12] = '\0'; + prop = of_find_property(opp->np, name, NULL); + } + + if (prop && !of_property_read_u32(opp->np, name, &val)) opp->u_amp = val;
return 0; @@ -942,6 +971,106 @@ void dev_pm_opp_put_supported_hw(struct device *dev) } EXPORT_SYMBOL_GPL(dev_pm_opp_put_supported_hw);
+/** + * dev_pm_opp_set_prop_name() - Set prop-extn name + * @dev: Device for which the regulator has to be set. + * @name: name to postfix to properties. + * + * This is required only for the V2 bindings, and it enables a platform to + * specify the extn to be used for certain property names. The properties to + * which the extension will apply are opp-microvolt and opp-microamp. OPP core + * should postfix the property name with -<name> while looking for them. + * + * Locking: The internal device_opp and opp structures are RCU protected. + * Hence this function internally uses RCU updater strategy with mutex locks + * to keep the integrity of the internal data structures. Callers should ensure + * that this function is *NOT* called under RCU protection or in contexts where + * mutex cannot be locked. + */ +int dev_pm_opp_set_prop_name(struct device *dev, const char *name) +{ + struct device_opp *dev_opp; + int ret = 0; + + /* Hold our list modification lock here */ + mutex_lock(&dev_opp_list_lock); + + dev_opp = _add_device_opp(dev); + if (!dev_opp) { + ret = -ENOMEM; + goto unlock; + } + + /* Do we already have a prop-name associated with dev_opp? */ + if (dev_opp->prop_name) { + dev_err(dev, "%s: Already have prop-name %s\n", __func__, + dev_opp->prop_name); + ret = -EBUSY; + goto err; + } + + dev_opp->prop_name = kstrdup(name, GFP_KERNEL); + if (!dev_opp->prop_name) { + ret = -ENOMEM; + goto err; + } + + mutex_unlock(&dev_opp_list_lock); + return 0; + +err: + _remove_device_opp(dev_opp); +unlock: + mutex_unlock(&dev_opp_list_lock); + + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_set_prop_name); + +/** + * dev_pm_opp_put_prop_name() - Releases resources blocked for prop-name + * @dev: Device for which the regulator has to be set. + * + * This is required only for the V2 bindings, and is called for a matching + * dev_pm_opp_set_prop_name(). Until this is called, the device_opp structure + * will not be freed. + * + * Locking: The internal device_opp and opp structures are RCU protected. + * Hence this function internally uses RCU updater strategy with mutex locks + * to keep the integrity of the internal data structures. Callers should ensure + * that this function is *NOT* called under RCU protection or in contexts where + * mutex cannot be locked. + */ +void dev_pm_opp_put_prop_name(struct device *dev) +{ + struct device_opp *dev_opp; + + /* Hold our list modification lock here */ + mutex_lock(&dev_opp_list_lock); + + /* Check for existing list for 'dev' first */ + dev_opp = _find_device_opp(dev); + if (IS_ERR(dev_opp)) { + dev_err(dev, "Failed to find dev_opp: %ld\n", PTR_ERR(dev_opp)); + goto unlock; + } + + if (!dev_opp->prop_name) { + dev_err(dev, "%s: Doesn't have a prop-name\n", __func__); + goto unlock; + } + + kfree(dev_opp->prop_name); + dev_opp->prop_name = NULL; + + /* Try freeing device_opp if this was the last blocking resource */ + _remove_device_opp(dev_opp); + +unlock: + mutex_unlock(&dev_opp_list_lock); +} +EXPORT_SYMBOL_GPL(dev_pm_opp_put_prop_name); + static bool _opp_is_supported(struct device *dev, struct device_opp *dev_opp, struct device_node *np) { @@ -1036,7 +1165,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, dev_opp); if (ret) goto free_opp;
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h index 70f4564a6ab9..690638ef36ee 100644 --- a/drivers/base/power/opp/opp.h +++ b/drivers/base/power/opp/opp.h @@ -131,6 +131,7 @@ struct device_list_opp { * @suspend_opp: Pointer to OPP to be used during device suspend. * @supported_hw: Array of version number to support. * @supported_hw_count: Number of elements in supported_hw array. + * @prop_name: A name to postfix to many DT properties, while parsing them. * @dentry: debugfs dentry pointer of the real device directory (not links). * @dentry_name: Name of the real dentry. * @@ -157,6 +158,7 @@ struct device_opp {
unsigned int *supported_hw; unsigned int supported_hw_count; + const char *prop_name;
#ifdef CONFIG_DEBUG_FS struct dentry *dentry; diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 3a85110242f0..95403d2ccaf5 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -58,6 +58,8 @@ struct srcu_notifier_head *dev_pm_opp_get_notifier(struct device *dev); int dev_pm_opp_set_supported_hw(struct device *dev, const u32 *versions, unsigned int count); void dev_pm_opp_put_supported_hw(struct device *dev); +int dev_pm_opp_set_prop_name(struct device *dev, const char *name); +void dev_pm_opp_put_prop_name(struct device *dev); #else static inline unsigned long dev_pm_opp_get_voltage(struct dev_pm_opp *opp) { @@ -142,6 +144,13 @@ static inline int dev_pm_opp_set_supported_hw(struct device *dev,
static inline void dev_pm_opp_put_supported_hw(struct device *dev) {}
+static inline int dev_pm_opp_set_prop_name(struct device *dev, const char *name) +{ + return -EINVAL; +} + +static inline void dev_pm_opp_put_prop_name(struct device *dev) {} + #endif /* CONFIG_PM_OPP */
#if defined(CONFIG_PM_OPP) && defined(CONFIG_OF)
linaro-kernel@lists.linaro.org
-
Rafael J. Wysocki -
Stephen Boyd -
Viresh Kumar