Hi Rafael,
These patches parse the newly added opp-supported-hw/opp-<prop>-<name> bindings.
Rebased over: Latest pm/linux-next Tested-on: Exynos 5250, dual core A15.
V2->V3: - Stephen had an interesting point to make for V2, as the patches created the dev_opp first and then updated it within the OPP-list lock. But a simultaneous reader of the dev_opp list need a rcu way of handling this. - The big difference, why its not a problem here, is that the platform needs to call this routine before the OPPs are parsed from DT and the opp-list will be empty. And so no-readers. - It wasn't guaranteed earlier, but now we have installed few WARN_ON() to make sure about that there are no OPPs while these routines are called. - It should be pretty safe to get these merged now. - Lee's work depends on this to be merged, http://marc.info/?l=linux-kernel&m=144958553611215&w=2
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 (2): PM / OPP: Parse 'opp-supported-hw' binding PM / OPP: Parse 'opp-<prop>-<name>' bindings
drivers/base/power/opp/core.c | 313 ++++++++++++++++++++++++++++++++++++++++-- drivers/base/power/opp/opp.h | 7 + include/linux/pm_opp.h | 22 +++ 3 files changed, 327 insertions(+), 15 deletions(-)
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 | 148 ++++++++++++++++++++++++++++++++++++++++++ drivers/base/power/opp/opp.h | 5 ++ include/linux/pm_opp.h | 13 ++++ 3 files changed, 166 insertions(+)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6aa172be6e8e..55cf1a99b532 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,145 @@ 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; + } + + /* Make sure there are no concurrent readers while updating dev_opp */ + WARN_ON(!list_empty(&dev_opp->opp_list)); + + /* 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; + } + + /* 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__); + 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 +1021,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 9 December 2015 at 02:31, Viresh Kumar viresh.kumar@linaro.org 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 | 148 ++++++++++++++++++++++++++++++++++++++++++ drivers/base/power/opp/opp.h | 5 ++ include/linux/pm_opp.h | 13 ++++ 3 files changed, 166 insertions(+)
Tested-by: Lee Jones lee.jones@linaro.org
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 6aa172be6e8e..55cf1a99b532 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,145 @@ 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;
}
/* Make sure there are no concurrent readers while updating dev_opp */
WARN_ON(!list_empty(&dev_opp->opp_list));
/* 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;
}
/* 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__);
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 +1021,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)
2.6.2.198.g614a2ac
linaro-kernel mailing list linaro-kernel@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-kernel
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 | 165 ++++++++++++++++++++++++++++++++++++++---- drivers/base/power/opp/opp.h | 2 + include/linux/pm_opp.h | 9 +++ 3 files changed, 161 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 55cf1a99b532..5c01fec1ed14 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; @@ -948,6 +977,112 @@ 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; + } + + /* Make sure there are no concurrent readers while updating dev_opp */ + WARN_ON(!list_empty(&dev_opp->opp_list)); + + /* 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; + } + + /* Make sure there are no concurrent readers while updating dev_opp */ + WARN_ON(!list_empty(&dev_opp->opp_list)); + + 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) { @@ -1042,7 +1177,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)
On 9 December 2015 at 02:31, Viresh Kumar viresh.kumar@linaro.org wrote:
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 | 165 ++++++++++++++++++++++++++++++++++++++---- drivers/base/power/opp/opp.h | 2 + include/linux/pm_opp.h | 9 +++ 3 files changed, 161 insertions(+), 15 deletions(-)
Tested-by: Lee Jones lee.jones@linaro.org
diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index 55cf1a99b532..5c01fec1ed14 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;
@@ -948,6 +977,112 @@ 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;
}
/* Make sure there are no concurrent readers while updating dev_opp */
WARN_ON(!list_empty(&dev_opp->opp_list));
/* 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;
}
/* Make sure there are no concurrent readers while updating dev_opp */
WARN_ON(!list_empty(&dev_opp->opp_list));
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) { @@ -1042,7 +1177,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)
2.6.2.198.g614a2ac
linaro-kernel mailing list linaro-kernel@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-kernel
On Thursday, December 10, 2015 09:54:45 AM Lee Jones wrote:
On 9 December 2015 at 02:31, Viresh Kumar viresh.kumar@linaro.org wrote:
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 | 165 ++++++++++++++++++++++++++++++++++++++---- drivers/base/power/opp/opp.h | 2 + include/linux/pm_opp.h | 9 +++ 3 files changed, 161 insertions(+), 15 deletions(-)
Tested-by: Lee Jones lee.jones@linaro.org
OK, so tentatively I can take these two.
What exectly does depend on them and how that thing is intended to be handled?
Thanks, Rafael
On 10-12-15, 22:45, Rafael J. Wysocki wrote:
On Thursday, December 10, 2015 09:54:45 AM Lee Jones wrote:
On 9 December 2015 at 02:31, Viresh Kumar viresh.kumar@linaro.org wrote:
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 | 165 ++++++++++++++++++++++++++++++++++++++---- drivers/base/power/opp/opp.h | 2 + include/linux/pm_opp.h | 9 +++ 3 files changed, 161 insertions(+), 15 deletions(-)
Tested-by: Lee Jones lee.jones@linaro.org
OK, so tentatively I can take these two.
What exectly does depend on them and how that thing is intended to be handled?
The other two patches from Lee, Perhaps he missed mentioning that in his cover-letter.
[PICKME 0/2] cpufreq: New ST driver
They must be rebased on my patches as they use the new APIs defined here.
Hi Viresh,
On Wed, Dec 9, 2015 at 3:31 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
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
@@ -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);
Any chance an attacker can overflow name[] by providing a very long dev_opp->prop_name?
Better safe than sorry:
snprintf(name, sizeof(name), ...);
prop = of_find_property(opp->np, name, NULL);
}
@@ -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);
Likewise
prop = of_find_property(opp->np, name, NULL);
}
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 05-01-16, 11:31, Geert Uytterhoeven wrote:
Any chance an attacker can overflow name[] by providing a very long dev_opp->prop_name?
Better safe than sorry:
snprintf(name, sizeof(name), ...);
Sent a patch to you..
linaro-kernel@lists.linaro.org