Skip to contentSkip to conversation

PM / OPP: move cpufreq specific helpers out of OPP layer

PATCH In review Nishanth Menon 3 participants 8 messages May 5, 2014 – May 13, 2014

Subsystems (8)
lore
Share & tools
lore

Nishanth Menon

author maintainer
#0 lore raw

cover PM / OPP: move cpufreq specific helpers out of OPP layer

CPUFreq usage of OPP should be independent of the ordering of type of data storage inside OPP layer. The current operations can equally be performed by generic operations.

[RFC]: https://patchwork.kernel.org/patch/4100811/

Series based on: v3.15-rc1

Nishanth Menon (2):
PM / OPP: Remove cpufreq wrapper dependency on internal data
organization
PM / OPP: Move cpufreq specific OPP functions out of generic OPP
library

 Documentation/cpu-freq/core.txt |   29 +++++++++++
 Documentation/power/opp.txt     |   40 ++------------
 drivers/base/power/opp.c        |   91 --------------------------------
 drivers/cpufreq/Makefile        |    2 +
 drivers/cpufreq/cpufreq_opp.c   |  110 +++++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h         |   21 ++++++++
 include/linux/pm_opp.h          |   20 -------
 7 files changed, 167 insertions(+), 146 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq_opp.c
Signature · 1 line
1.7.9.5
author maintainer
#1 lore raw

PATCH 1/2 PM / OPP: Remove cpufreq wrapper dependency on internal data organization

CPUFREQ custom functions for OPP (Operating Performance Points) currently exist inside the OPP library. These custom functions currently depend on internal data structures to pick up OPP information to create the cpufreq table. For example, the cpufreq table is created precisely in the same order of how OPP entries are stored inside the list implementation.

This kind of tight interdependency is purely artificial since the same functionality can be achieved using the generic OPP functions meant to do the same. This interdependency also limits the independent modification of cpufreq and OPP library.

So use the generic dev_pm_opp_find_freq_ceil function that achieves the table organization as we currently use.

As a result of this, we dont need to use the internal device_opp structure anymore, and we hence we can switch over to rcu lock instead of the mutex holding the internal list lock.

This breaking of dependency on internal data structure imposes no change to usage of these.

NOTE: This change is a precursor to moving this cpufreq specific logic out of the generic library into cpufreq.

---
 drivers/base/power/opp.c |   55 +++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 27 deletions(-)
drivers/base/power/opp.c +28 -27
index 2553867..38b43bb 100644
@@ -617,53 +617,54 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
  * the table if any of the mentioned functions have been invoked in the interim.
  *
  * Locking: The internal device_opp and opp structures are RCU protected.
- * To simplify the logic, we pretend we are updater and hold relevant mutex here
- * Callers should ensure that this function is *NOT* called under RCU protection
- * or in contexts where mutex locking cannot be used.
+ * Since we just use the regular accessor functions to access the internal data
+ * structures, we use RCU read lock inside this function. As a result, users of
+ * this function DONOT need to use explicit locks for invoking.
  */
 int dev_pm_opp_init_cpufreq_table(struct device *dev,
 			    struct cpufreq_frequency_table **table)
 {
-	struct device_opp *dev_opp;
 	struct dev_pm_opp *opp;
-	struct cpufreq_frequency_table *freq_table;
-	int i = 0;
+	struct cpufreq_frequency_table *freq_table = NULL;
+	int i, max_opps, ret = 0;
+	unsigned long rate;
 
-	/* Pretend as if I am an updater */
-	mutex_lock(&dev_opp_list_lock);
+	rcu_read_lock();
 
-	dev_opp = find_device_opp(dev);
-	if (IS_ERR(dev_opp)) {
-		int r = PTR_ERR(dev_opp);
-		mutex_unlock(&dev_opp_list_lock);
-		dev_err(dev, "%s: Device OPP not found (%d)\n", __func__, r);
-		return r;
+	max_opps = dev_pm_opp_get_opp_count(dev);
+	if (max_opps <= 0) {
+		ret = max_opps ? max_opps : -ENODATA;
+		goto out;
 	}
 
-	freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) *
-			     (dev_pm_opp_get_opp_count(dev) + 1), GFP_KERNEL);
+	freq_table = kzalloc(sizeof(*freq_table) * (max_opps + 1), GFP_KERNEL);
 	if (!freq_table) {
-		mutex_unlock(&dev_opp_list_lock);
-		dev_warn(dev, "%s: Unable to allocate frequency table\n",
-			__func__);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto out;
 	}
 
-	list_for_each_entry(opp, &dev_opp->opp_list, node) {
-		if (opp->available) {
-			freq_table[i].driver_data = i;
-			freq_table[i].frequency = opp->rate / 1000;
-			i++;
+	for (i = 0, rate = 0; i < max_opps; i++, rate++) {
+		/* find next rate */
+		opp = dev_pm_opp_find_freq_ceil(dev, &rate);
+		if (IS_ERR(opp)) {
+			ret = PTR_ERR(opp);
+			goto out;
 		}
+		freq_table[i].driver_data = i;
+		freq_table[i].frequency = rate / 1000;
 	}
-	mutex_unlock(&dev_opp_list_lock);
 
 	freq_table[i].driver_data = i;
 	freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	*table = &freq_table[0];
 
-	return 0;
+out:
+	rcu_read_unlock();
+	if (ret)
+		kfree(freq_table);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);
 
Signature · 1 line
1.7.9.5
maintainer
#2 lore raw

Re: [PATCH 1/2] PM / OPP: Remove cpufreq wrapper dependency on internal data organization

Quoted 4 lines · Nishanth Menon
drivers/base/power/opp.c +0 -0
 int dev_pm_opp_init_cpufreq_table(struct device *dev,
                            struct cpufreq_frequency_table **table)
 {

drivers/base/power/opp.c EXPORT_SYMBOL_GPL(dev_pm_opp_disable); · line 633

Quoted 10 lines
drivers/base/power/opp.c +3 -5
-       struct device_opp *dev_opp;
        struct dev_pm_opp *opp;
-       struct cpufreq_frequency_table *freq_table;
-       int i = 0;
+       struct cpufreq_frequency_table *freq_table = NULL;
+       int i, max_opps, ret = 0;
+       unsigned long rate;
 
-       /* Pretend as if I am an updater */
-       mutex_lock(&dev_opp_list_lock);

What if opp is being added for some reason at the same time? I hope we can surely see some awkward results, maybe some NULL pointers invocations as well..

author maintainer
#3 lore raw

Re: [PATCH 1/2] PM / OPP: Remove cpufreq wrapper dependency on internal data organization

Quoted 5 lines · Viresh Kumar

What if opp is being added for some reason at the same time? I hope we can surely see some awkward results, maybe some NULL pointers invocations as well..

we wont - rcu operations ensure that.

maintainer Acked-by
#4 lore raw

Re: [PATCH 1/2] PM / OPP: Remove cpufreq wrapper dependency on internal data organization

Quoted 8 lines · Nishanth Menon
Quoted 5 lines · Viresh Kumar

What if opp is being added for some reason at the same time? I hope we can surely see some awkward results, maybe some NULL pointers invocations as well..

we wont - rcu operations ensure that.

Stupid !!

author maintainer
#5 lore raw

PATCH 2/2 PM / OPP: Move cpufreq specific OPP functions out of generic OPP library

In reply to Nishanth Menon

CPUFreq specific helper functions for OPP (Operating Performance Points) now use generic OPP functions that allow CPUFreq to be be moved back into CPUFreq framework. This allows for independent modifications or future enhancements as needed isolated to just CPUFreq framework alone.

Here, we just move relevant code and documentation to make this part of CPUFreq infrastructure.

---
List of files impacted by this change detected by:

  # check if pm_opp.h header is needed anymore:
  git grep dev_pm_opp_init_cpufreq_table .|cut -d ':' -f1|sort|uniq|xargs
  grep dev_pm_opp |grep -v cpufreq
  # check if any file is missing cpufreq.h header is needed anymore:
 git grep dev_pm_opp_init_cpufreq_table .|cut -d ':' -f1|sort|uniq|xargs
 grep "cpufreq.h"
 Documentation/cpu-freq/core.txt |   29 +++++++++++
 Documentation/power/opp.txt     |   40 ++------------
 drivers/base/power/opp.c        |   92 --------------------------------
 drivers/cpufreq/Makefile        |    2 +
 drivers/cpufreq/cpufreq_opp.c   |  110 +++++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h         |   21 ++++++++
 include/linux/pm_opp.h          |   20 -------
 7 files changed, 167 insertions(+), 147 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq_opp.c
Documentation/cpu-freq/core.txt +29 -0
index 0060d76..70933ea 100644
@@ -20,6 +20,7 @@ Contents:
 ---------
 1.  CPUFreq core and interfaces
 2.  CPUFreq notifiers
+3.  CPUFreq Table Generation with Operating Performance Point (OPP)
 
 1. General Information
 =======================
@@ -92,3 +93,31 @@ values:
 cpu	- number of the affected CPU
 old	- old frequency
 new	- new frequency
+
+3. CPUFreq Table Generation with Operating Performance Point (OPP)
+==================================================================
+For details about OPP, see Documentation/power/opp.txt
+
+dev_pm_opp_init_cpufreq_table - cpufreq framework typically is initialized with
+	cpufreq_frequency_table_cpuinfo which is provided with the list of
+	frequencies that are available for operation. This function provides
+	a ready to use conversion routine to translate the OPP layer's internal
+	information about the available frequencies into a format readily
+	providable to cpufreq.
+
+	WARNING: Do not use this function in interrupt context.
+
+	Example:
+	 soc_pm_init()
+	 {
+		/* Do things */
+		r = dev_pm_opp_init_cpufreq_table(dev, &freq_table);
+		if (!r)
+			cpufreq_frequency_table_cpuinfo(policy, freq_table);
+		/* Do other things */
+	 }
+
+	NOTE: This function is available only if CONFIG_CPU_FREQ is enabled in
+	addition to CONFIG_PM_OPP.
+
+dev_pm_opp_free_cpufreq_table - Free up the table allocated by dev_pm_opp_init_cpufreq_table
Documentation/power/opp.txt +5 -35
index b8a907d..a9adad8 100644
@@ -10,8 +10,7 @@ Contents
 3. OPP Search Functions
 4. OPP Availability Control Functions
 5. OPP Data Retrieval Functions
-6. Cpufreq Table Generation
-7. Data Structures
+6. Data Structures
 
 1. Introduction
 ===============
@@ -72,7 +71,6 @@ operations until that OPP could be re-enabled if possible.
 OPP library facilitates this concept in it's implementation. The following
 operational functions operate only on available opps:
 opp_find_freq_{ceil, floor}, dev_pm_opp_get_voltage, dev_pm_opp_get_freq, dev_pm_opp_get_opp_count
-and dev_pm_opp_init_cpufreq_table
 
 dev_pm_opp_find_freq_exact is meant to be used to find the opp pointer which can then
 be used for dev_pm_opp_enable/disable functions to make an opp available as required.
@@ -96,10 +94,9 @@ using RCU read locks. The opp_find_freq_{exact,ceil,floor},
 opp_get_{voltage, freq, opp_count} fall into this category.
 
 opp_{add,enable,disable} are updaters which use mutex and implement it's own
-RCU locking mechanisms. dev_pm_opp_init_cpufreq_table acts as an updater and uses
-mutex to implment RCU updater strategy. These functions should *NOT* be called
-under RCU locks and other contexts that prevent blocking functions in RCU or
-mutex operations from working.
+RCU locking mechanisms. These functions should *NOT* be called under RCU locks
+and other contexts that prevent blocking functions in RCU or mutex operations
+from working.
 
 2. Initial OPP List Registration
 ================================
@@ -311,34 +308,7 @@ dev_pm_opp_get_opp_count - Retrieve the number of available opps for a device
 		/* Do other things */
 	 }
 
-6. Cpufreq Table Generation
-===========================
-dev_pm_opp_init_cpufreq_table - cpufreq framework typically is initialized with
-	cpufreq_frequency_table_cpuinfo which is provided with the list of
-	frequencies that are available for operation. This function provides
-	a ready to use conversion routine to translate the OPP layer's internal
-	information about the available frequencies into a format readily
-	providable to cpufreq.
-
-	WARNING: Do not use this function in interrupt context.
-
-	Example:
-	 soc_pm_init()
-	 {
-		/* Do things */
-		r = dev_pm_opp_init_cpufreq_table(dev, &freq_table);
-		if (!r)
-			cpufreq_frequency_table_cpuinfo(policy, freq_table);
-		/* Do other things */
-	 }
-
-	NOTE: This function is available only if CONFIG_CPU_FREQ is enabled in
-	addition to CONFIG_PM as power management feature is required to
-	dynamically scale voltage and frequency in a system.
-
-dev_pm_opp_free_cpufreq_table - Free up the table allocated by dev_pm_opp_init_cpufreq_table
-
-7. Data Structures
+6. Data Structures
 ==================
 Typically an SoC contains multiple voltage domains which are variable. Each
 domain is represented by a device pointer. The relationship to OPP can be
drivers/base/power/opp.c +0 -92

104 lines not on this page · raw message

drivers/cpufreq/Makefile +2 -0
index 0dbb963..738c8b7 100644
@@ -1,5 +1,7 @@
 # CPUfreq core
 obj-$(CONFIG_CPU_FREQ)			+= cpufreq.o freq_table.o
+obj-$(CONFIG_PM_OPP)			+= cpufreq_opp.o
+
 # CPUfreq stats
 obj-$(CONFIG_CPU_FREQ_STAT)             += cpufreq_stats.o
 
drivers/cpufreq/cpufreq_opp.c new file +110 -0

110 lines not on this page · raw message

include/linux/cpufreq.h +21 -0
index 5ae5100..044ec4b 100644
@@ -468,6 +468,27 @@ struct cpufreq_frequency_table {
 				    * order */
 };
 
+#if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
+int dev_pm_opp_init_cpufreq_table(struct device *dev,
+				  struct cpufreq_frequency_table **table);
+void dev_pm_opp_free_cpufreq_table(struct device *dev,
+				   struct cpufreq_frequency_table **table);
+#else
+static inline int dev_pm_opp_init_cpufreq_table(struct device *dev,
+						struct cpufreq_frequency_table
+						**table)
+{
+	return -EINVAL;
+}
+
+static inline void dev_pm_opp_free_cpufreq_table(struct device *dev,
+						 struct cpufreq_frequency_table
+						 **table)
+{
+}
+#endif
+
+
 int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy,
 				    struct cpufreq_frequency_table *table);
 
include/linux/pm_opp.h +0 -20
index 5151b00..0330217 100644
@@ -15,7 +15,6 @@
 #define __LINUX_OPP_H__
 
 #include <linux/err.h>
-#include <linux/cpufreq.h>
 #include <linux/notifier.h>
 
 struct dev_pm_opp;
@@ -117,23 +116,4 @@ static inline int of_init_opp_table(struct device *dev)
 }
 #endif
 
-#if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-int dev_pm_opp_init_cpufreq_table(struct device *dev,
-			    struct cpufreq_frequency_table **table);
-void dev_pm_opp_free_cpufreq_table(struct device *dev,
-				struct cpufreq_frequency_table **table);
-#else
-static inline int dev_pm_opp_init_cpufreq_table(struct device *dev,
-			    struct cpufreq_frequency_table **table)
-{
-	return -EINVAL;
-}
-
-static inline
-void dev_pm_opp_free_cpufreq_table(struct device *dev,
-				struct cpufreq_frequency_table **table)
-{
-}
-#endif		/* CONFIG_CPU_FREQ */
-
 #endif		/* __LINUX_OPP_H__ */
Signature · 1 line
1.7.9.5
maintainer Acked-by on v1 2/2
#6 lore raw

Re: [PATCH 2/2] PM / OPP: Move cpufreq specific OPP functions out of generic OPP library

Quoted 9 lines · Nishanth Menon

CPUFreq specific helper functions for OPP (Operating Performance Points) now use generic OPP functions that allow CPUFreq to be be moved back into CPUFreq framework. This allows for independent modifications or future enhancements as needed isolated to just CPUFreq framework alone.

Here, we just move relevant code and documentation to make this part of CPUFreq infrastructure.

Quoted 9 lines
Tested-by on the v1 series
#7 lore raw
In reply to Nishanth Menon
Quoted 14 lines · Nishanth Menon

CPUFreq usage of OPP should be independent of the ordering of type of data storage inside OPP layer. The current operations can equally be performed by generic operations.

[RFC]: https://patchwork.kernel.org/patch/4100811/

Series based on: v3.15-rc1

Nishanth Menon (2):
PM / OPP: Remove cpufreq wrapper dependency on internal data
organization
PM / OPP: Move cpufreq specific OPP functions out of generic OPP
library

Quoted 9 lines
 Documentation/cpu-freq/core.txt |   29 +++++++++++
 Documentation/power/opp.txt     |   40 ++------------
 drivers/base/power/opp.c        |   91 --------------------------------
 drivers/cpufreq/Makefile        |    2 +
 drivers/cpufreq/cpufreq_opp.c   |  110 +++++++++++++++++++++++++++++++++++++++
 include/linux/cpufreq.h         |   21 ++++++++
 include/linux/pm_opp.h          |   20 -------
 7 files changed, 167 insertions(+), 146 deletions(-)
 create mode 100644 drivers/cpufreq/cpufreq_opp.c

Works fine on Exynos.

Quoted 8 lines

--
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html