[PATCH v7 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices.

[PATCH v7 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices.

[MyungJoo Ham]

For a usage example, please look at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
and other related clocks simply follow the determined DDR RAM clock.

The DEVFREQ driver for Exynos4210 memory bus is at
/drivers/devfreq/exynos4210_memorybus.c in the git tree.

In the dd (writing and reading 360MiB) test with NURI board, the memory
throughput was not changed (the performance is not deteriorated) while
the SoC power consumption has been reduced by 1%. When the memory access
is not that intense while the CPU is heavily used, the SoC power consumption
has been reduced by 6%. The power consumption has been compared with the
case using the conventional Exynos4210 CPUFREQ driver, which sets memory
bus frequency according to the CPU core frequency. Besides, when the CPU core
running slow and the memory access is intense, the performance (memory
throughput) has been increased by 11% (with higher SoC power consumption of
5%). The tested governor is "simple-ondemand".

There are no major changes since the last revision (v6 / v6-resubmit)
The followings are the minor changes
- type changed for timing variables (signed->unsigned) (2/4)
- removed unnecessary code and variable (2/4)
- added sysfs store function for polling interval (4/4)

In the patchset, the patch 1/4 has no changes and 3/4 has only changes
in the line numbers due to changes in 2/4 patch.

MyungJoo Ham (4):
  PM / OPP: Add OPP availability change notifier.
  PM: Introduce DEVFREQ: generic DVFS framework with device-specific
    OPPs
  PM / DEVFREQ: add basic governors
  PM / DEVFREQ: add sysfs interface

 Documentation/ABI/testing/sysfs-devices-power |   47 +++
 drivers/Kconfig                               |    2 +
 drivers/Makefile                              |    2 +
 drivers/base/power/opp.c                      |   29 ++
 drivers/devfreq/Kconfig                       |   75 ++++
 drivers/devfreq/Makefile                      |    5 +
 drivers/devfreq/devfreq.c                     |  482 +++++++++++++++++++++++++
 drivers/devfreq/governor_performance.c        |   24 ++
 drivers/devfreq/governor_powersave.c          |   24 ++
 drivers/devfreq/governor_simpleondemand.c     |   88 +++++
 drivers/devfreq/governor_userspace.c          |   27 ++
 include/linux/devfreq.h                       |  148 ++++++++
 include/linux/opp.h                           |   12 +
 13 files changed, 965 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c
 create mode 100644 include/linux/devfreq.h

-- 
1.7.4.1

[PATCH v7 1/4] PM / OPP: Add OPP availability change notifier.

[MyungJoo Ham]

The patch enables to register notifier_block for an OPP-device in order
to get notified for any changes in the availability of OPPs of the
device. For example, if a new OPP is inserted or enable/disable status
of an OPP is changed, the notifier is executed.

This enables the usage of opp_add, opp_enable, and opp_disable to
directly take effect with any connected entities such as CPUFREQ or
DEVFREQ.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Mike Turquette <mturquette@ti.com>

---
No changes since v6
Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
---
 drivers/base/power/opp.c |   29 +++++++++++++++++++++++++++++
 include/linux/opp.h      |   12 ++++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index b23de18..e6b4c89 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -73,6 +73,7 @@ struct opp {
  *		RCU usage: nodes are not modified in the list of device_opp,
  *		however addition is possible and is secured by dev_opp_list_lock
  * @dev:	device pointer
+ * @head:	notifier head to notify the OPP availability changes.
  * @opp_list:	list of opps
  *
  * This is an internal data structure maintaining the link to opps attached to
@@ -83,6 +84,7 @@ struct device_opp {
 	struct list_head node;
 
 	struct device *dev;
+	struct srcu_notifier_head head;
 	struct list_head opp_list;
 };
 
@@ -404,6 +406,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 		}
 
 		dev_opp->dev = dev;
+		srcu_init_notifier_head(&dev_opp->head);
 		INIT_LIST_HEAD(&dev_opp->opp_list);
 
 		/* Secure the device list modification */
@@ -428,6 +431,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	list_add_rcu(&new_opp->node, head);
 	mutex_unlock(&dev_opp_list_lock);
 
+	/*
+	 * Notify the changes in the availability of the operable
+	 * frequency/voltage list.
+	 */
+	srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
 	return 0;
 }
 
@@ -504,6 +512,14 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
 	mutex_unlock(&dev_opp_list_lock);
 	synchronize_rcu();
 
+	/* Notify the change of the OPP availability */
+	if (availability_req)
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ENABLE,
+					 new_opp);
+	else
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_DISABLE,
+					 new_opp);
+
 	/* clean up old opp */
 	new_opp = opp;
 	goto out;
@@ -643,3 +659,16 @@ void opp_free_cpufreq_table(struct device *dev,
 	*table = NULL;
 }
 #endif		/* CONFIG_CPU_FREQ */
+
+/** opp_get_notifier() - find notifier_head of the device with opp
+ * @dev:	device pointer used to lookup device OPPs.
+ */
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	struct device_opp *dev_opp = find_device_opp(dev);
+
+	if (IS_ERR(dev_opp))
+		return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
+
+	return &dev_opp->head;
+}
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 7020e97..87a9208 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -16,9 +16,14 @@
 
 #include <linux/err.h>
 #include <linux/cpufreq.h>
+#include <linux/notifier.h>
 
 struct opp;
 
+enum opp_event {
+	OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
+};
+
 #if defined(CONFIG_PM_OPP)
 
 unsigned long opp_get_voltage(struct opp *opp);
@@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
 
 int opp_disable(struct device *dev, unsigned long freq);
 
+struct srcu_notifier_head *opp_get_notifier(struct device *dev);
+
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
@@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
 {
 	return 0;
 }
+
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif		/* CONFIG_PM */
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-- 
1.7.4.1

[PATCH v7 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs

[MyungJoo Ham]

With OPPs, a device may have multiple operable frequency and voltage
sets. However, there can be multiple possible operable sets and a system
will need to choose one from them. In order to reduce the power
consumption (by reducing frequency and voltage) without affecting the
performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
scheme may be used.

This patch introduces the DVFS capability to non-CPU devices with OPPs.
DVFS is a techique whereby the frequency and supplied voltage of a
device is adjusted on-the-fly. DVFS usually sets the frequency as low
as possible with given conditions (such as QoS assurance) and adjusts
voltage according to the chosen frequency in order to reduce power
consumption and heat dissipation.

The generic DVFS for devices, DEVFREQ, may appear quite similar with
/drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
devices registered and is not suitable to have multiple heterogenous
devices with different (but simple) governors.

Normally, DVFS mechanism controls frequency based on the demand for
the device, and then, chooses voltage based on the chosen frequency.
DEVFREQ also controls the frequency based on the governor's frequency
recommendation and let OPP pick up the pair of frequency and voltage
based on the recommended frequency. Then, the chosen OPP is passed to
device driver's "target" callback.

When PM QoS is going to be used with the DEVFREQ device, the device
driver should enable OPPs that are appropriate with the current PM QoS
requests. In order to do so, the device driver may call opp_enable and
opp_disable at the notifier callback of PM QoS so that PM QoS's
update_target() call enables the appropriate OPPs. Note that at least
one of OPPs should be enabled at any time; be careful when there is a
transition.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
The test code with board support for Exynos4-NURI is at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

---
Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
and Kevin.
Changes from v6
- Type revised for timing variables
- Removed unnecessary code and variable

Changes at v6-resubmit from v6
- Use jiffy directly instead of ktime
- Be prepared for profile->polling_ms changes (not supported fully at
  this stage)

Changes from v5
- Uses OPP availability change notifier
- Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
  polling interval based on the interval requirement of DEVFREQ
  devices.
- Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
  including governors and devfreq drivers.
- Coding style revised.
- Updated devfreq_add_device interface to get tunable values.

Changed from v4
- Removed tickle, which is a duplicated feature; PM QoS can do the same.
- Allow to extend polling interval if devices have longer polling intervals.
- Relocated private data of governors.
- Removed system-wide sysfs

Changed from v3
- In kerneldoc comments, DEVFREQ has ben replaced by devfreq
- Revised removing devfreq entries with error mechanism
- Added and revised comments
- Removed unnecessary codes
- Allow to give a name to a governor
- Bugfix: a tickle call may cancel an older tickle call that is still in
  effect.

Changed from v2
- Code style revised and cleaned up.
- Remove DEVFREQ entries that incur errors except for EAGAIN
- Bug fixed: tickle for devices without polling governors

Changes from v1(RFC)
- Rename: DVFS --> DEVFREQ
- Revised governor design
	. Governor receives the whole struct devfreq
	. Governor should gather usage information (thru get_dev_status) itself
- Periodic monitoring runs only when needed.
- DEVFREQ no more deals with voltage information directly
- Removed some printks.
- Some cosmetics update
- Use freezable_wq.
---
 drivers/Kconfig           |    2 +
 drivers/Makefile          |    2 +
 drivers/devfreq/Kconfig   |   39 ++++++
 drivers/devfreq/Makefile  |    1 +
 drivers/devfreq/devfreq.c |  297 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h   |  107 ++++++++++++++++
 6 files changed, 448 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 include/linux/devfreq.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9e7e..a1efd75 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
 
 source "drivers/virt/Kconfig"
 
+source "drivers/devfreq/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 7fa433a..97c957b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # Virtualization drivers
 obj-$(CONFIG_VIRT_DRIVERS)	+= virt/
+
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq/
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
new file mode 100644
index 0000000..1fb42de
--- /dev/null
+++ b/drivers/devfreq/Kconfig
@@ -0,0 +1,39 @@
+config ARCH_HAS_DEVFREQ
+	bool
+	depends on ARCH_HAS_OPP
+	help
+	  Denotes that the architecture supports DEVFREQ. If the architecture
+	  supports multiple OPP entries per device and the frequency of the
+	  devices with OPPs may be altered dynamically, the architecture
+	  supports DEVFREQ.
+
+menuconfig PM_DEVFREQ
+	bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
+	depends on PM_OPP && ARCH_HAS_DEVFREQ
+	help
+	  With OPP support, a device may have a list of frequencies and
+	  voltages available. DEVFREQ, a generic DVFS framework can be
+	  registered for a device with OPP support in order to let the
+	  governor provided to DEVFREQ choose an operating frequency
+	  based on the OPP's list and the policy given with DEVFREQ.
+
+	  Each device may have its own governor and policy. DEVFREQ can
+	  reevaluate the device state periodically and/or based on the
+	  OPP list changes (each frequency/voltage pair in OPP may be
+	  disabled or enabled).
+
+	  Like some CPUs with CPUFREQ, a device may have multiple clocks.
+	  However, because the clock frequencies of a single device are
+	  determined by the single device's state, an instance of DEVFREQ
+	  is attached to a single device and returns a "representative"
+	  clock frequency from the OPP of the device, which is also attached
+	  to a device by 1-to-1. The device registering DEVFREQ takes the
+	  responsiblity to "interpret" the frequency listed in OPP and
+	  to set its every clock accordingly with the "target" callback
+	  given to DEVFREQ.
+
+if PM_DEVFREQ
+
+comment "DEVFREQ Drivers"
+
+endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
new file mode 100644
index 0000000..168934a
--- /dev/null
+++ b/drivers/devfreq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
new file mode 100644
index 0000000..df63bdc
--- /dev/null
+++ b/drivers/devfreq/devfreq.c
@@ -0,0 +1,297 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/opp.h>
+#include <linux/devfreq.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/printk.h>
+#include <linux/hrtimer.h>
+
+/*
+ * devfreq_work periodically monitors every registered device.
+ * The minimum polling interval is one jiffy. The polling interval is
+ * determined by the minimum polling period among all polling devfreq
+ * devices. The resolution of polling interval is one jiffy.
+ */
+static bool polling;
+static struct workqueue_struct *devfreq_wq;
+static struct delayed_work devfreq_work;
+
+/* The list of all device-devfreq */
+static LIST_HEAD(devfreq_list);
+static DEFINE_MUTEX(devfreq_list_lock);
+
+/**
+ * find_device_devfreq() - find devfreq struct using device pointer
+ * @dev:	device pointer used to lookup device devfreq.
+ *
+ * Search the list of device devfreqs and return the matched device's
+ * devfreq info. devfreq_list_lock should be held by the caller.
+ */
+static struct devfreq *find_device_devfreq(struct device *dev)
+{
+	struct devfreq *tmp_devfreq;
+
+	if (unlikely(IS_ERR_OR_NULL(dev))) {
+		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+
+	list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
+		if (tmp_devfreq->dev == dev)
+			return tmp_devfreq;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * devfreq_do() - Check the usage profile of a given device and configure
+ *		frequency and voltage accordingly
+ * @devfreq:	devfreq info of the given device
+ */
+static int devfreq_do(struct devfreq *devfreq)
+{
+	struct opp *opp;
+	unsigned long freq;
+	int err;
+
+	err = devfreq->governor->get_target_freq(devfreq, &freq);
+	if (err)
+		return err;
+
+	opp = opp_find_freq_ceil(devfreq->dev, &freq);
+	if (opp == ERR_PTR(-ENODEV))
+		opp = opp_find_freq_floor(devfreq->dev, &freq);
+
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	if (devfreq->previous_freq == freq)
+		return 0;
+
+	err = devfreq->profile->target(devfreq->dev, opp);
+	if (err)
+		return err;
+
+	devfreq->previous_freq = freq;
+	return 0;
+}
+
+/**
+ * devfreq_update() - Notify that the device OPP has been changed.
+ * @dev:	the device whose OPP has been changed.
+ */
+static int devfreq_update(struct notifier_block *nb, unsigned long type,
+			  void *devp)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = container_of(nb, struct devfreq, nb);
+	/* Reevaluate the proper frequency */
+	err = devfreq_do(devfreq);
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+}
+
+/**
+ * devfreq_monitor() - Periodically run devfreq_do()
+ * @work: the work struct used to run devfreq_monitor periodically.
+ *
+ */
+static void devfreq_monitor(struct work_struct *work)
+{
+	static unsigned long last_polled_at;
+	struct devfreq *devfreq, *tmp;
+	int error;
+	unsigned long jiffies_passed;
+	unsigned long next_jiffies = ULONG_MAX, now = jiffies;
+
+	/* Initially last_polled_at = 0, polling every device at bootup */
+	jiffies_passed = now - last_polled_at;
+	last_polled_at = now;
+	if (jiffies_passed == 0)
+		jiffies_passed = 1;
+
+	mutex_lock(&devfreq_list_lock);
+
+	list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
+		if (devfreq->next_polling == 0)
+			continue;
+
+		/*
+		 * Reduce more next_polling if devfreq_wq took an extra
+		 * delay. (i.e., CPU has been idled.)
+		 */
+		if (devfreq->next_polling <= jiffies_passed) {
+			error = devfreq_do(devfreq);
+
+			/* Remove a devfreq with an error. */
+			if (error && error != -EAGAIN) {
+				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
+					error, devfreq->governor->name);
+
+				list_del(&devfreq->node);
+				kfree(devfreq);
+
+				continue;
+			}
+			devfreq->next_polling = devfreq->polling_jiffies;
+
+			/* No more polling required (polling_ms changed) */
+			if (devfreq->next_polling == 0)
+				continue;
+		} else {
+			devfreq->next_polling -= jiffies_passed;
+		}
+
+		next_jiffies = (next_jiffies > devfreq->next_polling) ?
+				devfreq->next_polling : next_jiffies;
+	}
+
+	if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
+	} else {
+		polling = false;
+	}
+
+	mutex_unlock(&devfreq_list_lock);
+}
+
+/**
+ * devfreq_add_device() - Add devfreq feature to the device
+ * @dev:	the device to add devfreq feature.
+ * @profile:	device-specific profile to run devfreq.
+ * @governor:	the policy to choose frequency.
+ * @data:	private data for the governor. The devfreq framework does not
+ *		touch this value.
+ */
+int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
+		       struct devfreq_governor *governor, void *data)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev || !profile || !governor) {
+		dev_err(dev, "%s: Invalid parameters.\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&devfreq_list_lock);
+
+	devfreq = find_device_devfreq(dev);
+	if (!IS_ERR(devfreq)) {
+		dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
+		err = -EINVAL;
+		goto out;
+	}
+
+	devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
+	if (!devfreq) {
+		dev_err(dev, "%s: Unable to create devfreq for the device\n",
+			__func__);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	devfreq->dev = dev;
+	devfreq->profile = profile;
+	devfreq->governor = governor;
+	devfreq->next_polling = devfreq->polling_jiffies
+			      = msecs_to_jiffies(devfreq->profile->polling_ms);
+	devfreq->previous_freq = profile->initial_freq;
+	devfreq->data = data;
+
+	devfreq->nb.notifier_call = devfreq_update;
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+	err = srcu_notifier_chain_register(nh, &devfreq->nb);
+	if (err)
+		goto out;
+
+	list_add(&devfreq->node, &devfreq_list);
+
+	if (devfreq_wq && devfreq->next_polling && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   devfreq->next_polling);
+	}
+out:
+	mutex_unlock(&devfreq_list_lock);
+
+	return err;
+}
+
+/**
+ * devfreq_remove_device() - Remove devfreq feature from a device.
+ * @device:	the device to remove devfreq feature.
+ */
+int devfreq_remove_device(struct device *dev)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+
+	list_del(&devfreq->node);
+	srcu_notifier_chain_unregister(nh, &devfreq->nb);
+	kfree(devfreq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return 0;
+}
+
+/**
+ * devfreq_init() - Initialize data structure for devfreq framework and
+ *		  start polling registered devfreq devices.
+ */
+static int __init devfreq_init(void)
+{
+	mutex_lock(&devfreq_list_lock);
+	polling = false;
+	devfreq_wq = create_freezable_workqueue("devfreq_wq");
+	INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
+	mutex_unlock(&devfreq_list_lock);
+
+	devfreq_monitor(&devfreq_work.work);
+	return 0;
+}
+late_initcall(devfreq_init);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
new file mode 100644
index 0000000..8252238
--- /dev/null
+++ b/include/linux/devfreq.h
@@ -0,0 +1,107 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_DEVFREQ_H__
+#define __LINUX_DEVFREQ_H__
+
+#include <linux/notifier.h>
+
+#define DEVFREQ_NAME_LEN 16
+
+struct devfreq;
+struct devfreq_dev_status {
+	/* both since the last measure */
+	unsigned long total_time;
+	unsigned long busy_time;
+	unsigned long current_frequency;
+};
+
+struct devfreq_dev_profile {
+	unsigned long max_freq; /* may be larger than the actual value */
+	unsigned long initial_freq;
+	unsigned int polling_ms;	/* 0 for at opp change only */
+
+	int (*target)(struct device *dev, struct opp *opp);
+	int (*get_dev_status)(struct device *dev,
+			      struct devfreq_dev_status *stat);
+};
+
+/**
+ * struct devfreq_governor - Devfreq policy governor
+ * @name		Governor's name
+ * @get_target_freq	Returns desired operating frequency for the device.
+ *			Basically, get_target_freq will run
+ *			devfreq_dev_profile.get_dev_status() to get the
+ *			status of the device (load = busy_time / total_time).
+ */
+struct devfreq_governor {
+	char name[DEVFREQ_NAME_LEN];
+	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
+};
+
+/**
+ * struct devfreq - Device devfreq structure
+ * @node	list node - contains the devices with devfreq that have been
+ *		registered.
+ * @dev		device pointer
+ * @profile	device-specific devfreq profile
+ * @governor	method how to choose frequency based on the usage.
+ * @nb		notifier block registered to the corresponding OPP to get
+ *		notified for frequency availability updates.
+ * @polling_jiffies	interval in jiffies.
+ * @previous_freq	previously configured frequency value.
+ * @next_polling	the number of remaining jiffies to poll with
+ *			"devfreq_monitor" executions to reevaluate
+ *			frequency/voltage of the device. Set by
+ *			profile's polling_ms interval.
+ * @data	Private data of the governor. The devfreq framework does not
+ *		touch this.
+ *
+ * This structure stores the devfreq information for a give device.
+ */
+struct devfreq {
+	struct list_head node;
+
+	struct device *dev;
+	struct devfreq_dev_profile *profile;
+	struct devfreq_governor *governor;
+	struct notifier_block nb;
+
+	unsigned long polling_jiffies;
+	unsigned long previous_freq;
+	unsigned int next_polling;
+
+	void *data; /* private data for governors */
+};
+
+#if defined(CONFIG_PM_DEVFREQ)
+extern int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data);
+extern int devfreq_remove_device(struct device *dev);
+#else /* !CONFIG_PM_DEVFREQ */
+static int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data)
+{
+	return 0;
+}
+
+static int devfreq_remove_device(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_DEVFREQ */
+
+#endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

Re: [PATCH v7 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs

[Turquette, Mike]

On Tue, Aug 23, 2011 at 12:54 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> With OPPs, a device may have multiple operable frequency and voltage
> sets. However, there can be multiple possible operable sets and a system
> will need to choose one from them. In order to reduce the power
> consumption (by reducing frequency and voltage) without affecting the
> performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
> scheme may be used.
>
> This patch introduces the DVFS capability to non-CPU devices with OPPs.
> DVFS is a techique whereby the frequency and supplied voltage of a
> device is adjusted on-the-fly. DVFS usually sets the frequency as low
> as possible with given conditions (such as QoS assurance) and adjusts
> voltage according to the chosen frequency in order to reduce power
> consumption and heat dissipation.
>
> The generic DVFS for devices, DEVFREQ, may appear quite similar with
> /drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
> devices registered and is not suitable to have multiple heterogenous
> devices with different (but simple) governors.
>
> Normally, DVFS mechanism controls frequency based on the demand for
> the device, and then, chooses voltage based on the chosen frequency.
> DEVFREQ also controls the frequency based on the governor's frequency
> recommendation and let OPP pick up the pair of frequency and voltage
> based on the recommended frequency. Then, the chosen OPP is passed to
> device driver's "target" callback.
>
> When PM QoS is going to be used with the DEVFREQ device, the device
> driver should enable OPPs that are appropriate with the current PM QoS
> requests. In order to do so, the device driver may call opp_enable and
> opp_disable at the notifier callback of PM QoS so that PM QoS's
> update_target() call enables the appropriate OPPs. Note that at least
> one of OPPs should be enabled at any time; be careful when there is a
> transition.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Looks OK to me.

Reviewed-by: Mike Turquette <mturquette@ti.com>

Regards,
Mike

>
> ---
> The test code with board support for Exynos4-NURI is at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>
> ---
> Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
> and Kevin.
> Changes from v6
> - Type revised for timing variables
> - Removed unnecessary code and variable
>
> Changes at v6-resubmit from v6
> - Use jiffy directly instead of ktime
> - Be prepared for profile->polling_ms changes (not supported fully at
>  this stage)
>
> Changes from v5
> - Uses OPP availability change notifier
> - Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
>  polling interval based on the interval requirement of DEVFREQ
>  devices.
> - Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
>  including governors and devfreq drivers.
> - Coding style revised.
> - Updated devfreq_add_device interface to get tunable values.
>
> Changed from v4
> - Removed tickle, which is a duplicated feature; PM QoS can do the same.
> - Allow to extend polling interval if devices have longer polling intervals.
> - Relocated private data of governors.
> - Removed system-wide sysfs
>
> Changed from v3
> - In kerneldoc comments, DEVFREQ has ben replaced by devfreq
> - Revised removing devfreq entries with error mechanism
> - Added and revised comments
> - Removed unnecessary codes
> - Allow to give a name to a governor
> - Bugfix: a tickle call may cancel an older tickle call that is still in
>  effect.
>
> Changed from v2
> - Code style revised and cleaned up.
> - Remove DEVFREQ entries that incur errors except for EAGAIN
> - Bug fixed: tickle for devices without polling governors
>
> Changes from v1(RFC)
> - Rename: DVFS --> DEVFREQ
> - Revised governor design
>        . Governor receives the whole struct devfreq
>        . Governor should gather usage information (thru get_dev_status) itself
> - Periodic monitoring runs only when needed.
> - DEVFREQ no more deals with voltage information directly
> - Removed some printks.
> - Some cosmetics update
> - Use freezable_wq.
> ---
>  drivers/Kconfig           |    2 +
>  drivers/Makefile          |    2 +
>  drivers/devfreq/Kconfig   |   39 ++++++
>  drivers/devfreq/Makefile  |    1 +
>  drivers/devfreq/devfreq.c |  297 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/devfreq.h   |  107 ++++++++++++++++
>  6 files changed, 448 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/Kconfig
>  create mode 100644 drivers/devfreq/Makefile
>  create mode 100644 drivers/devfreq/devfreq.c
>  create mode 100644 include/linux/devfreq.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 95b9e7e..a1efd75 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
>
>  source "drivers/virt/Kconfig"
>
> +source "drivers/devfreq/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 7fa433a..97c957b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT) += iommu/
>
>  # Virtualization drivers
>  obj-$(CONFIG_VIRT_DRIVERS)     += virt/
> +
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq/
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> new file mode 100644
> index 0000000..1fb42de
> --- /dev/null
> +++ b/drivers/devfreq/Kconfig
> @@ -0,0 +1,39 @@
> +config ARCH_HAS_DEVFREQ
> +       bool
> +       depends on ARCH_HAS_OPP
> +       help
> +         Denotes that the architecture supports DEVFREQ. If the architecture
> +         supports multiple OPP entries per device and the frequency of the
> +         devices with OPPs may be altered dynamically, the architecture
> +         supports DEVFREQ.
> +
> +menuconfig PM_DEVFREQ
> +       bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
> +       depends on PM_OPP && ARCH_HAS_DEVFREQ
> +       help
> +         With OPP support, a device may have a list of frequencies and
> +         voltages available. DEVFREQ, a generic DVFS framework can be
> +         registered for a device with OPP support in order to let the
> +         governor provided to DEVFREQ choose an operating frequency
> +         based on the OPP's list and the policy given with DEVFREQ.
> +
> +         Each device may have its own governor and policy. DEVFREQ can
> +         reevaluate the device state periodically and/or based on the
> +         OPP list changes (each frequency/voltage pair in OPP may be
> +         disabled or enabled).
> +
> +         Like some CPUs with CPUFREQ, a device may have multiple clocks.
> +         However, because the clock frequencies of a single device are
> +         determined by the single device's state, an instance of DEVFREQ
> +         is attached to a single device and returns a "representative"
> +         clock frequency from the OPP of the device, which is also attached
> +         to a device by 1-to-1. The device registering DEVFREQ takes the
> +         responsiblity to "interpret" the frequency listed in OPP and
> +         to set its every clock accordingly with the "target" callback
> +         given to DEVFREQ.
> +
> +if PM_DEVFREQ
> +
> +comment "DEVFREQ Drivers"
> +
> +endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> new file mode 100644
> index 0000000..168934a
> --- /dev/null
> +++ b/drivers/devfreq/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> new file mode 100644
> index 0000000..df63bdc
> --- /dev/null
> +++ b/drivers/devfreq/devfreq.c
> @@ -0,0 +1,297 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/opp.h>
> +#include <linux/devfreq.h>
> +#include <linux/workqueue.h>
> +#include <linux/platform_device.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/hrtimer.h>
> +
> +/*
> + * devfreq_work periodically monitors every registered device.
> + * The minimum polling interval is one jiffy. The polling interval is
> + * determined by the minimum polling period among all polling devfreq
> + * devices. The resolution of polling interval is one jiffy.
> + */
> +static bool polling;
> +static struct workqueue_struct *devfreq_wq;
> +static struct delayed_work devfreq_work;
> +
> +/* The list of all device-devfreq */
> +static LIST_HEAD(devfreq_list);
> +static DEFINE_MUTEX(devfreq_list_lock);
> +
> +/**
> + * find_device_devfreq() - find devfreq struct using device pointer
> + * @dev:       device pointer used to lookup device devfreq.
> + *
> + * Search the list of device devfreqs and return the matched device's
> + * devfreq info. devfreq_list_lock should be held by the caller.
> + */
> +static struct devfreq *find_device_devfreq(struct device *dev)
> +{
> +       struct devfreq *tmp_devfreq;
> +
> +       if (unlikely(IS_ERR_OR_NULL(dev))) {
> +               pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
> +               if (tmp_devfreq->dev == dev)
> +                       return tmp_devfreq;
> +       }
> +
> +       return ERR_PTR(-ENODEV);
> +}
> +
> +/**
> + * devfreq_do() - Check the usage profile of a given device and configure
> + *             frequency and voltage accordingly
> + * @devfreq:   devfreq info of the given device
> + */
> +static int devfreq_do(struct devfreq *devfreq)
> +{
> +       struct opp *opp;
> +       unsigned long freq;
> +       int err;
> +
> +       err = devfreq->governor->get_target_freq(devfreq, &freq);
> +       if (err)
> +               return err;
> +
> +       opp = opp_find_freq_ceil(devfreq->dev, &freq);
> +       if (opp == ERR_PTR(-ENODEV))
> +               opp = opp_find_freq_floor(devfreq->dev, &freq);
> +
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       if (devfreq->previous_freq == freq)
> +               return 0;
> +
> +       err = devfreq->profile->target(devfreq->dev, opp);
> +       if (err)
> +               return err;
> +
> +       devfreq->previous_freq = freq;
> +       return 0;
> +}
> +
> +/**
> + * devfreq_update() - Notify that the device OPP has been changed.
> + * @dev:       the device whose OPP has been changed.
> + */
> +static int devfreq_update(struct notifier_block *nb, unsigned long type,
> +                         void *devp)
> +{
> +       struct devfreq *devfreq;
> +       int err = 0;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = container_of(nb, struct devfreq, nb);
> +       /* Reevaluate the proper frequency */
> +       err = devfreq_do(devfreq);
> +       mutex_unlock(&devfreq_list_lock);
> +       return err;
> +}
> +
> +/**
> + * devfreq_monitor() - Periodically run devfreq_do()
> + * @work: the work struct used to run devfreq_monitor periodically.
> + *
> + */
> +static void devfreq_monitor(struct work_struct *work)
> +{
> +       static unsigned long last_polled_at;
> +       struct devfreq *devfreq, *tmp;
> +       int error;
> +       unsigned long jiffies_passed;
> +       unsigned long next_jiffies = ULONG_MAX, now = jiffies;
> +
> +       /* Initially last_polled_at = 0, polling every device at bootup */
> +       jiffies_passed = now - last_polled_at;
> +       last_polled_at = now;
> +       if (jiffies_passed == 0)
> +               jiffies_passed = 1;
> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
> +               if (devfreq->next_polling == 0)
> +                       continue;
> +
> +               /*
> +                * Reduce more next_polling if devfreq_wq took an extra
> +                * delay. (i.e., CPU has been idled.)
> +                */
> +               if (devfreq->next_polling <= jiffies_passed) {
> +                       error = devfreq_do(devfreq);
> +
> +                       /* Remove a devfreq with an error. */
> +                       if (error && error != -EAGAIN) {
> +                               dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
> +                                       error, devfreq->governor->name);
> +
> +                               list_del(&devfreq->node);
> +                               kfree(devfreq);
> +
> +                               continue;
> +                       }
> +                       devfreq->next_polling = devfreq->polling_jiffies;
> +
> +                       /* No more polling required (polling_ms changed) */
> +                       if (devfreq->next_polling == 0)
> +                               continue;
> +               } else {
> +                       devfreq->next_polling -= jiffies_passed;
> +               }
> +
> +               next_jiffies = (next_jiffies > devfreq->next_polling) ?
> +                               devfreq->next_polling : next_jiffies;
> +       }
> +
> +       if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
> +       } else {
> +               polling = false;
> +       }
> +
> +       mutex_unlock(&devfreq_list_lock);
> +}
> +
> +/**
> + * devfreq_add_device() - Add devfreq feature to the device
> + * @dev:       the device to add devfreq feature.
> + * @profile:   device-specific profile to run devfreq.
> + * @governor:  the policy to choose frequency.
> + * @data:      private data for the governor. The devfreq framework does not
> + *             touch this value.
> + */
> +int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
> +                      struct devfreq_governor *governor, void *data)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev || !profile || !governor) {
> +               dev_err(dev, "%s: Invalid parameters.\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       devfreq = find_device_devfreq(dev);
> +       if (!IS_ERR(devfreq)) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
> +       devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
> +       if (!devfreq) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device\n",
> +                       __func__);
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +
> +       devfreq->dev = dev;
> +       devfreq->profile = profile;
> +       devfreq->governor = governor;
> +       devfreq->next_polling = devfreq->polling_jiffies
> +                             = msecs_to_jiffies(devfreq->profile->polling_ms);
> +       devfreq->previous_freq = profile->initial_freq;
> +       devfreq->data = data;
> +
> +       devfreq->nb.notifier_call = devfreq_update;
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +       err = srcu_notifier_chain_register(nh, &devfreq->nb);
> +       if (err)
> +               goto out;
> +
> +       list_add(&devfreq->node, &devfreq_list);
> +
> +       if (devfreq_wq && devfreq->next_polling && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  devfreq->next_polling);
> +       }
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return err;
> +}
> +
> +/**
> + * devfreq_remove_device() - Remove devfreq feature from a device.
> + * @device:    the device to remove devfreq feature.
> + */
> +int devfreq_remove_device(struct device *dev)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev)
> +               return -EINVAL;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +
> +       list_del(&devfreq->node);
> +       srcu_notifier_chain_unregister(nh, &devfreq->nb);
> +       kfree(devfreq);
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       return 0;
> +}
> +
> +/**
> + * devfreq_init() - Initialize data structure for devfreq framework and
> + *               start polling registered devfreq devices.
> + */
> +static int __init devfreq_init(void)
> +{
> +       mutex_lock(&devfreq_list_lock);
> +       polling = false;
> +       devfreq_wq = create_freezable_workqueue("devfreq_wq");
> +       INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       devfreq_monitor(&devfreq_work.work);
> +       return 0;
> +}
> +late_initcall(devfreq_init);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> new file mode 100644
> index 0000000..8252238
> --- /dev/null
> +++ b/include/linux/devfreq.h
> @@ -0,0 +1,107 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __LINUX_DEVFREQ_H__
> +#define __LINUX_DEVFREQ_H__
> +
> +#include <linux/notifier.h>
> +
> +#define DEVFREQ_NAME_LEN 16
> +
> +struct devfreq;
> +struct devfreq_dev_status {
> +       /* both since the last measure */
> +       unsigned long total_time;
> +       unsigned long busy_time;
> +       unsigned long current_frequency;
> +};
> +
> +struct devfreq_dev_profile {
> +       unsigned long max_freq; /* may be larger than the actual value */
> +       unsigned long initial_freq;
> +       unsigned int polling_ms;        /* 0 for at opp change only */
> +
> +       int (*target)(struct device *dev, struct opp *opp);
> +       int (*get_dev_status)(struct device *dev,
> +                             struct devfreq_dev_status *stat);
> +};
> +
> +/**
> + * struct devfreq_governor - Devfreq policy governor
> + * @name               Governor's name
> + * @get_target_freq    Returns desired operating frequency for the device.
> + *                     Basically, get_target_freq will run
> + *                     devfreq_dev_profile.get_dev_status() to get the
> + *                     status of the device (load = busy_time / total_time).
> + */
> +struct devfreq_governor {
> +       char name[DEVFREQ_NAME_LEN];
> +       int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> +};
> +
> +/**
> + * struct devfreq - Device devfreq structure
> + * @node       list node - contains the devices with devfreq that have been
> + *             registered.
> + * @dev                device pointer
> + * @profile    device-specific devfreq profile
> + * @governor   method how to choose frequency based on the usage.
> + * @nb         notifier block registered to the corresponding OPP to get
> + *             notified for frequency availability updates.
> + * @polling_jiffies    interval in jiffies.
> + * @previous_freq      previously configured frequency value.
> + * @next_polling       the number of remaining jiffies to poll with
> + *                     "devfreq_monitor" executions to reevaluate
> + *                     frequency/voltage of the device. Set by
> + *                     profile's polling_ms interval.
> + * @data       Private data of the governor. The devfreq framework does not
> + *             touch this.
> + *
> + * This structure stores the devfreq information for a give device.
> + */
> +struct devfreq {
> +       struct list_head node;
> +
> +       struct device *dev;
> +       struct devfreq_dev_profile *profile;
> +       struct devfreq_governor *governor;
> +       struct notifier_block nb;
> +
> +       unsigned long polling_jiffies;
> +       unsigned long previous_freq;
> +       unsigned int next_polling;
> +
> +       void *data; /* private data for governors */
> +};
> +
> +#if defined(CONFIG_PM_DEVFREQ)
> +extern int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data);
> +extern int devfreq_remove_device(struct device *dev);
> +#else /* !CONFIG_PM_DEVFREQ */
> +static int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data)
> +{
> +       return 0;
> +}
> +
> +static int devfreq_remove_device(struct device *dev)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_PM_DEVFREQ */
> +
> +#endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

[PATCH v7 3/4] PM / DEVFREQ: add basic governors

[MyungJoo Ham]

Four CPUFREQ-like governors are provided as examples.

powersave: use the lowest frequency possible. The user (device) should
set the polling_ms as 0 because polling is useless for this governor.

performance: use the highest freqeuncy possible. The user (device)
should set the polling_ms as 0 because polling is useless for this
governor.

userspace: use the user specified frequency stored at
devfreq.user_set_freq. With sysfs support in the following patch, a user
may set the value with the sysfs interface.

simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.

When a user updates OPP entries (enable/disable/add), OPP framework
automatically notifies DEVFREQ to update operating frequency
accordingly. Thus, DEVFREQ users (device drivers) do not need to update
DEVFREQ manually with OPP entry updates or set polling_ms for powersave
, performance, userspace, or any other "static" governors.

Note that these are given only as basic examples for governors and any
devices with DEVFREQ may implement their own governors with the drivers
and use them.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Changed from v5
- Seperated governor files from devfreq.c
- Allow simple ondemand to be tuned for each device
---
 drivers/devfreq/Kconfig                   |   36 ++++++++++++
 drivers/devfreq/Makefile                  |    4 +
 drivers/devfreq/governor_performance.c    |   24 ++++++++
 drivers/devfreq/governor_powersave.c      |   24 ++++++++
 drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
 drivers/devfreq/governor_userspace.c      |   27 +++++++++
 include/linux/devfreq.h                   |   41 +++++++++++++
 7 files changed, 244 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 1fb42de..643b055 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
 
 if PM_DEVFREQ
 
+comment "DEVFREQ Governors"
+
+config DEVFREQ_GOV_SIMPLE_ONDEMAND
+	bool "Simple Ondemand"
+	help
+	  Chooses frequency based on the recent load on the device. Works
+	  similar as ONDEMAND governor of CPUFREQ does. A device with
+	  Simple-Ondemand should be able to provide busy/total counter
+	  values that imply the usage rate. A device may provide tuned
+	  values to the governor with data field at devfreq_add_device().
+
+config DEVFREQ_GOV_PERFORMANCE
+	bool "Performance"
+	help
+	  Sets the frequency at the maximum available frequency.
+	  This governor always returns UINT_MAX as frequency so that
+	  the DEVFREQ framework returns the highest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_POWERSAVE
+	bool "Powersave"
+	help
+	  Sets the frequency at the minimum available frequency.
+	  This governor always returns 0 as frequency so that
+	  the DEVFREQ framework returns the lowest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_USERSPACE
+	bool "Userspace"
+	help
+	  Sets the frequency at the user specified one.
+	  This governor returns the user configured frequency if there
+	  has been an input to /sys/devices/.../power/devfreq_set_freq.
+	  Otherwise, the governor does not change the frequnecy
+	  given at the initialization.
+
 comment "DEVFREQ Drivers"
 
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 168934a..4564a89 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -1 +1,5 @@
 obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
+obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
+obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
+obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
+obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
new file mode 100644
index 0000000..c47eff8
--- /dev/null
+++ b/drivers/devfreq/governor_performance.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_performance.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_performance_func(struct devfreq *df,
+				    unsigned long *freq)
+{
+	*freq = UINT_MAX; /* devfreq_do will run "floor" */
+	return 0;
+}
+
+struct devfreq_governor devfreq_performance = {
+	.name = "performance",
+	.get_target_freq = devfreq_performance_func,
+};
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
new file mode 100644
index 0000000..4f128d8
--- /dev/null
+++ b/drivers/devfreq/governor_powersave.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_powersave.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_powersave_func(struct devfreq *df,
+				  unsigned long *freq)
+{
+	*freq = 0; /* devfreq_do will run "ceiling" to 0 */
+	return 0;
+}
+
+struct devfreq_governor devfreq_powersave = {
+	.name = "powersave",
+	.get_target_freq = devfreq_powersave_func,
+};
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
new file mode 100644
index 0000000..18fe8be
--- /dev/null
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -0,0 +1,88 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/errno.h>
+#include <linux/devfreq.h>
+#include <linux/math64.h>
+
+/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
+#define DFSO_UPTHRESHOLD	(90)
+#define DFSO_DOWNDIFFERENCTIAL	(5)
+static int devfreq_simple_ondemand_func(struct devfreq *df,
+					unsigned long *freq)
+{
+	struct devfreq_dev_status stat;
+	int err = df->profile->get_dev_status(df->dev, &stat);
+	unsigned long long a, b;
+	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
+	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
+	struct devfreq_simple_ondemand_data *data = df->data;
+
+	if (err)
+		return err;
+
+	if (data) {
+		if (data->upthreshold)
+			dfso_upthreshold = data->upthreshold;
+		if (data->downdifferential)
+			dfso_downdifferential = data->downdifferential;
+	}
+	if (dfso_upthreshold > 100 ||
+	    dfso_upthreshold < dfso_downdifferential)
+		return -EINVAL;
+
+	/* Assume MAX if it is going to be divided by zero */
+	if (stat.total_time == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Prevent overflow */
+	if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
+		stat.busy_time >>= 7;
+		stat.total_time >>= 7;
+	}
+
+	/* Set MAX if it's busy enough */
+	if (stat.busy_time * 100 >
+	    stat.total_time * dfso_upthreshold) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Set MAX if we do not know the initial frequency */
+	if (stat.current_frequency == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Keep the current frequency */
+	if (stat.busy_time * 100 >
+	    stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
+		*freq = stat.current_frequency;
+		return 0;
+	}
+
+	/* Set the desired frequency based on the load */
+	a = stat.busy_time;
+	a *= stat.current_frequency;
+	b = div_u64(a, stat.total_time);
+	b *= 100;
+	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
+	*freq = (unsigned long) b;
+
+	return 0;
+}
+
+struct devfreq_governor devfreq_simple_ondemand = {
+	.name = "simple_ondemand",
+	.get_target_freq = devfreq_simple_ondemand_func,
+};
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
new file mode 100644
index 0000000..6af8b6d
--- /dev/null
+++ b/drivers/devfreq/governor_userspace.c
@@ -0,0 +1,27 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
+{
+	if (df->user_set_freq == 0)
+		*freq = df->previous_freq; /* No user freq specified yet */
+	else
+		*freq = df->user_set_freq;
+
+	return 0;
+}
+
+struct devfreq_governor devfreq_userspace = {
+	.name = "userspace",
+	.get_target_freq = devfreq_userspace_func,
+};
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 8252238..8f614fa 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -13,6 +13,7 @@
 #ifndef __LINUX_DEVFREQ_H__
 #define __LINUX_DEVFREQ_H__
 
+#include <linux/opp.h>
 #include <linux/notifier.h>
 
 #define DEVFREQ_NAME_LEN 16
@@ -63,6 +64,8 @@ struct devfreq_governor {
  *			"devfreq_monitor" executions to reevaluate
  *			frequency/voltage of the device. Set by
  *			profile's polling_ms interval.
+ * @user_set_freq	User specified adequete frequency value (thru sysfs
+ *		interface). Governors may and may not use this value.
  * @data	Private data of the governor. The devfreq framework does not
  *		touch this.
  *
@@ -80,6 +83,7 @@ struct devfreq {
 	unsigned long previous_freq;
 	unsigned int next_polling;
 
+	unsigned long user_set_freq; /* governors may ignore this. */
 	void *data; /* private data for governors */
 };
 
@@ -89,6 +93,37 @@ extern int devfreq_add_device(struct device *dev,
 			   struct devfreq_governor *governor,
 			   void *data);
 extern int devfreq_remove_device(struct device *dev);
+
+#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
+extern struct devfreq_governor devfreq_powersave;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
+extern struct devfreq_governor devfreq_performance;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
+extern struct devfreq_governor devfreq_userspace;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
+extern struct devfreq_governor devfreq_simple_ondemand;
+/**
+ * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
+ *	and devfreq_add_device
+ * @ upthreshold	If the load is over this value, the frequency jumps.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ * @ downdifferential	If the load is under upthreshold - downdifferential,
+ *			the governor may consider slowing the frequency down.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ *			downdifferential < upthreshold must hold.
+ *
+ * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
+ * the governor uses the default values.
+ */
+struct devfreq_simple_ondemand_data {
+	unsigned int upthreshold;
+	unsigned int downdifferential;
+};
+#endif
+
 #else /* !CONFIG_PM_DEVFREQ */
 static int devfreq_add_device(struct device *dev,
 			   struct devfreq_dev_profile *profile,
@@ -102,6 +137,12 @@ static int devfreq_remove_device(struct device *dev)
 {
 	return 0;
 }
+
+#define devfreq_powersave	NULL
+#define devfreq_performance	NULL
+#define devfreq_userspace	NULL
+#define devfreq_simple_ondemand	NULL
+
 #endif /* CONFIG_PM_DEVFREQ */
 
 #endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

Re: [PATCH v7 3/4] PM / DEVFREQ: add basic governors

[Turquette, Mike]

On Tue, Aug 23, 2011 at 12:54 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Four CPUFREQ-like governors are provided as examples.
>
> powersave: use the lowest frequency possible. The user (device) should
> set the polling_ms as 0 because polling is useless for this governor.
>
> performance: use the highest freqeuncy possible. The user (device)
> should set the polling_ms as 0 because polling is useless for this
> governor.
>
> userspace: use the user specified frequency stored at
> devfreq.user_set_freq. With sysfs support in the following patch, a user
> may set the value with the sysfs interface.
>
> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.
>
> When a user updates OPP entries (enable/disable/add), OPP framework
> automatically notifies DEVFREQ to update operating frequency
> accordingly. Thus, DEVFREQ users (device drivers) do not need to update
> DEVFREQ manually with OPP entry updates or set polling_ms for powersave
> , performance, userspace, or any other "static" governors.
>
> Note that these are given only as basic examples for governors and any
> devices with DEVFREQ may implement their own governors with the drivers
> and use them.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> Changed from v5
> - Seperated governor files from devfreq.c
> - Allow simple ondemand to be tuned for each device
> ---
>  drivers/devfreq/Kconfig                   |   36 ++++++++++++
>  drivers/devfreq/Makefile                  |    4 +
>  drivers/devfreq/governor_performance.c    |   24 ++++++++
>  drivers/devfreq/governor_powersave.c      |   24 ++++++++
>  drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
>  drivers/devfreq/governor_userspace.c      |   27 +++++++++
>  include/linux/devfreq.h                   |   41 +++++++++++++
>  7 files changed, 244 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/governor_performance.c
>  create mode 100644 drivers/devfreq/governor_powersave.c
>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>  create mode 100644 drivers/devfreq/governor_userspace.c
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 1fb42de..643b055 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>
>  if PM_DEVFREQ
>
> +comment "DEVFREQ Governors"
> +
> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
> +       bool "Simple Ondemand"
> +       help
> +         Chooses frequency based on the recent load on the device. Works
> +         similar as ONDEMAND governor of CPUFREQ does. A device with
> +         Simple-Ondemand should be able to provide busy/total counter
> +         values that imply the usage rate. A device may provide tuned
> +         values to the governor with data field at devfreq_add_device().
> +
> +config DEVFREQ_GOV_PERFORMANCE
> +       bool "Performance"
> +       help
> +         Sets the frequency at the maximum available frequency.
> +         This governor always returns UINT_MAX as frequency so that
> +         the DEVFREQ framework returns the highest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_POWERSAVE
> +       bool "Powersave"
> +       help
> +         Sets the frequency at the minimum available frequency.
> +         This governor always returns 0 as frequency so that
> +         the DEVFREQ framework returns the lowest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_USERSPACE
> +       bool "Userspace"
> +       help
> +         Sets the frequency at the user specified one.
> +         This governor returns the user configured frequency if there
> +         has been an input to /sys/devices/.../power/devfreq_set_freq.
> +         Otherwise, the governor does not change the frequnecy
> +         given at the initialization.
> +
>  comment "DEVFREQ Drivers"
>
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 168934a..4564a89 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -1 +1,5 @@
>  obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> +obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)      += governor_simpleondemand.o
> +obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)  += governor_performance.o
> +obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)    += governor_powersave.o
> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)    += governor_userspace.o
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> new file mode 100644
> index 0000000..c47eff8
> --- /dev/null
> +++ b/drivers/devfreq/governor_performance.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_performance.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_performance_func(struct devfreq *df,
> +                                   unsigned long *freq)
> +{
> +       *freq = UINT_MAX; /* devfreq_do will run "floor" */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_performance = {
> +       .name = "performance",
> +       .get_target_freq = devfreq_performance_func,
> +};
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> new file mode 100644
> index 0000000..4f128d8
> --- /dev/null
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_powersave.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_powersave_func(struct devfreq *df,
> +                                 unsigned long *freq)
> +{
> +       *freq = 0; /* devfreq_do will run "ceiling" to 0 */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_powersave = {
> +       .name = "powersave",
> +       .get_target_freq = devfreq_powersave_func,
> +};
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> new file mode 100644
> index 0000000..18fe8be
> --- /dev/null
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -0,0 +1,88 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/devfreq.h>
> +#include <linux/math64.h>
> +
> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
> +#define DFSO_UPTHRESHOLD       (90)
> +#define DFSO_DOWNDIFFERENCTIAL (5)
> +static int devfreq_simple_ondemand_func(struct devfreq *df,
> +                                       unsigned long *freq)
> +{
> +       struct devfreq_dev_status stat;
> +       int err = df->profile->get_dev_status(df->dev, &stat);
> +       unsigned long long a, b;
> +       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
> +       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
> +       struct devfreq_simple_ondemand_data *data = df->data;
> +
> +       if (err)
> +               return err;
> +
> +       if (data) {
> +               if (data->upthreshold)
> +                       dfso_upthreshold = data->upthreshold;
> +               if (data->downdifferential)
> +                       dfso_downdifferential = data->downdifferential;
> +       }
> +       if (dfso_upthreshold > 100 ||
> +           dfso_upthreshold < dfso_downdifferential)
> +               return -EINVAL;
> +
> +       /* Assume MAX if it is going to be divided by zero */
> +       if (stat.total_time == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Prevent overflow */
> +       if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
> +               stat.busy_time >>= 7;
> +               stat.total_time >>= 7;
> +       }
> +
> +       /* Set MAX if it's busy enough */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * dfso_upthreshold) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Set MAX if we do not know the initial frequency */
> +       if (stat.current_frequency == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Keep the current frequency */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
> +               *freq = stat.current_frequency;
> +               return 0;
> +       }
> +
> +       /* Set the desired frequency based on the load */
> +       a = stat.busy_time;
> +       a *= stat.current_frequency;
> +       b = div_u64(a, stat.total_time);
> +       b *= 100;
> +       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
> +       *freq = (unsigned long) b;
> +
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_simple_ondemand = {
> +       .name = "simple_ondemand",
> +       .get_target_freq = devfreq_simple_ondemand_func,
> +};
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> new file mode 100644
> index 0000000..6af8b6d
> --- /dev/null
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -0,0 +1,27 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
> +{
> +       if (df->user_set_freq == 0)
> +               *freq = df->previous_freq; /* No user freq specified yet */

I think that user_set_freq == 0 is a valid request from a user.  A
user might not know what the valid frequencies are for a device, but
knows that he/she wants the lowest one.  Writing 0 to the sysfs value
should give the floor of the available frequencies.

> +       else
> +               *freq = df->user_set_freq;
> +
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_userspace = {
> +       .name = "userspace",
> +       .get_target_freq = devfreq_userspace_func,
> +};

The set_user_frequency attribute should be moved out of devfreq.c
(patch 4) and live here.  There is no purpose to that entry except for
the userspace governor and it should not exist in sysfs unless this
governor is in use.  To be clear, there should still be a read-only
show_frequency or something in devfreq.c.

This again touches on my long-standing complaints with this series'
design.  set_user_frequency is an attribute that belongs to the
governor, not to the core.  Such misplacement of attribute/entity
ownership is common in this series, but I won't go down that rabbit
hole again.  In the mean time at least this one instance of the
problem for the userspace gov should be fixed up.

Regards,
Mike

> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 8252238..8f614fa 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -13,6 +13,7 @@
>  #ifndef __LINUX_DEVFREQ_H__
>  #define __LINUX_DEVFREQ_H__
>
> +#include <linux/opp.h>
>  #include <linux/notifier.h>
>
>  #define DEVFREQ_NAME_LEN 16
> @@ -63,6 +64,8 @@ struct devfreq_governor {
>  *                     "devfreq_monitor" executions to reevaluate
>  *                     frequency/voltage of the device. Set by
>  *                     profile's polling_ms interval.
> + * @user_set_freq      User specified adequete frequency value (thru sysfs
> + *             interface). Governors may and may not use this value.
>  * @data       Private data of the governor. The devfreq framework does not
>  *             touch this.
>  *
> @@ -80,6 +83,7 @@ struct devfreq {
>        unsigned long previous_freq;
>        unsigned int next_polling;
>
> +       unsigned long user_set_freq; /* governors may ignore this. */
>        void *data; /* private data for governors */
>  };
>
> @@ -89,6 +93,37 @@ extern int devfreq_add_device(struct device *dev,
>                           struct devfreq_governor *governor,
>                           void *data);
>  extern int devfreq_remove_device(struct device *dev);
> +
> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> +extern struct devfreq_governor devfreq_powersave;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
> +extern struct devfreq_governor devfreq_performance;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
> +extern struct devfreq_governor devfreq_userspace;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
> +extern struct devfreq_governor devfreq_simple_ondemand;
> +/**
> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
> + *     and devfreq_add_device
> + * @ upthreshold       If the load is over this value, the frequency jumps.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + * @ downdifferential  If the load is under upthreshold - downdifferential,
> + *                     the governor may consider slowing the frequency down.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + *                     downdifferential < upthreshold must hold.
> + *
> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
> + * the governor uses the default values.
> + */
> +struct devfreq_simple_ondemand_data {
> +       unsigned int upthreshold;
> +       unsigned int downdifferential;
> +};
> +#endif
> +
>  #else /* !CONFIG_PM_DEVFREQ */
>  static int devfreq_add_device(struct device *dev,
>                           struct devfreq_dev_profile *profile,
> @@ -102,6 +137,12 @@ static int devfreq_remove_device(struct device *dev)
>  {
>        return 0;
>  }
> +
> +#define devfreq_powersave      NULL
> +#define devfreq_performance    NULL
> +#define devfreq_userspace      NULL
> +#define devfreq_simple_ondemand        NULL
> +
>  #endif /* CONFIG_PM_DEVFREQ */
>
>  #endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

Re: [PATCH v7 3/4] PM / DEVFREQ: add basic governors

[MyungJoo Ham]

On Wed, Aug 24, 2011 at 2:29 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Tue, Aug 23, 2011 at 12:54 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>
> I think that user_set_freq == 0 is a valid request from a user.  A
> user might not know what the valid frequencies are for a device, but
> knows that he/she wants the lowest one.  Writing 0 to the sysfs value
> should give the floor of the available frequencies.
>

Users can know what is the minimum valid frequency with
devfreq_min_freq. However, writing 0 to the sysfs is a comfortable
method and I'll allow that input in the next revision along with the
patch of allowing governors to have their own sysfs entries with
example of userspace.

>> +       else
>> +               *freq = df->user_set_freq;
>> +
>> +       return 0;
>> +}
>> +
>> +struct devfreq_governor devfreq_userspace = {
>> +       .name = "userspace",
>> +       .get_target_freq = devfreq_userspace_func,
>> +};
>
> The set_user_frequency attribute should be moved out of devfreq.c
> (patch 4) and live here.  There is no purpose to that entry except for
> the userspace governor and it should not exist in sysfs unless this
> governor is in use.  To be clear, there should still be a read-only
> show_frequency or something in devfreq.c.
>
> This again touches on my long-standing complaints with this series'
> design.  set_user_frequency is an attribute that belongs to the
> governor, not to the core.  Such misplacement of attribute/entity
> ownership is common in this series, but I won't go down that rabbit
> hole again.  In the mean time at least this one instance of the
> problem for the userspace gov should be fixed up.
>
> Regards,
> Mike
>


I'll include a patch that shows an example of having a governor
specific sysfs entry at the next revision. It is enabled by
introducing a function to governors "struct devfreq
*get_devfreq(struct device *dev);" at /device/devfreq/governor.h
(local to governors) and adding .init and .exit callbacks at struct
devfreq_governor.


Cheers!

MyungJoo


-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH v7 4/4] PM / DEVFREQ: add sysfs interface

[MyungJoo Ham]

Device specific sysfs interface /sys/devices/.../power/devfreq_*
- governor	R: name of governor
- cur_freq	R: current frequency
- max_freq	R: maximum operable frequency
- min_freq	R: minimum operable frequency
- set_freq	R: read user specified frequency (0 if not specified yet)
		W: set user specified frequency
- polling_interval	R: polling interval in ms given with devfreq profile
			W: update polling interval.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Changed from v6
- poling_interval is writable.

Changed from v5
- updated devferq_update usage.

Changed from v4
- removed system-wide sysfs interface
- removed tickling sysfs interface
- added set_freq for userspace governor (and any other governors that
  require user input)

Changed from v3
- corrected sysfs API usage
- corrected error messages
- moved sysfs entry location
- added sysfs entries

Changed from v2
- add ABI entries for devfreq sysfs interface
---
 Documentation/ABI/testing/sysfs-devices-power |   47 +++++++
 drivers/devfreq/devfreq.c                     |  185 +++++++++++++++++++++++++
 2 files changed, 232 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
index 8ffbc25..ba8bc35 100644
--- a/Documentation/ABI/testing/sysfs-devices-power
+++ b/Documentation/ABI/testing/sysfs-devices-power
@@ -165,3 +165,50 @@ Description:
 
 		Not all drivers support this attribute.  If it isn't supported,
 		attempts to read or write it will yield I/O errors.
+
+What:		/sys/devices/.../power/devfreq_governor
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_governor shows the name
+		of the governor used by the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_cur_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the current
+		frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_max_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the
+		maximum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_min_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the
+		minimum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_set_freq
+Date:		August 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_set_freq sets and shows
+		the user specified desired frequency of the device. The
+		governor may and may not use the value. With the basic
+		governors given with devfreq.c, userspace governor is
+		using the value.
+
+What:		/sys/devices/.../power/devfreq_polling_interval
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_polling_interval sets and
+		shows the requested polling interval of the corresponding
+		device. The values are represented in ms. If the value is less
+		than 1 jiffy, it is considered to be 0, which means no polling.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index df63bdc..3070250 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
 static LIST_HEAD(devfreq_list);
 static DEFINE_MUTEX(devfreq_list_lock);
 
+static struct attribute_group dev_attr_group;
+
 /**
  * find_device_devfreq() - find devfreq struct using device pointer
  * @dev:	device pointer used to lookup device devfreq.
@@ -149,6 +151,8 @@ static void devfreq_monitor(struct work_struct *work)
 				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
 					error, devfreq->governor->name);
 
+				sysfs_unmerge_group(&devfreq->dev->kobj,
+						    &dev_attr_group);
 				list_del(&devfreq->node);
 				kfree(devfreq);
 
@@ -239,6 +243,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
 		queue_delayed_work(devfreq_wq, &devfreq_work,
 				   devfreq->next_polling);
 	}
+
+	sysfs_merge_group(&dev->kobj, &dev_attr_group);
 out:
 	mutex_unlock(&devfreq_list_lock);
 
@@ -271,6 +277,8 @@ int devfreq_remove_device(struct device *dev)
 		goto out;
 	}
 
+	sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
+
 	list_del(&devfreq->node);
 	srcu_notifier_chain_unregister(nh, &devfreq->nb);
 	kfree(devfreq);
@@ -279,6 +287,183 @@ out:
 	return 0;
 }
 
+static ssize_t show_governor(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->governor)
+		return -EINVAL;
+
+	return sprintf(buf, "%s\n", df->governor->name);
+}
+
+static ssize_t show_freq(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+
+	return sprintf(buf, "%lu\n", df->previous_freq);
+}
+
+static ssize_t show_max_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+	unsigned long freq = ULONG_MAX;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_floor(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_min_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+	unsigned long freq = 0;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_ceil(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_polling_interval(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->profile)
+		return -EINVAL;
+
+	return sprintf(buf, "%d\n", df->profile->polling_ms);
+}
+
+static ssize_t store_polling_interval(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+	unsigned int value;
+	int ret;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->profile)
+		return -EINVAL;
+
+	ret = sscanf(buf, "%u", &value);
+	if (ret != 1)
+		return -EINVAL;
+
+	df->profile->polling_ms = value;
+	df->next_polling = df->polling_jiffies
+			 = msecs_to_jiffies(value);
+
+	mutex_lock(&devfreq_list_lock);
+	if (df->next_polling > 0 && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   df->next_polling);
+	}
+	mutex_unlock(&devfreq_list_lock);
+
+	return count;
+}
+
+static ssize_t set_user_frequency(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct devfreq *devfreq;
+	unsigned long wanted;
+	int err = 0;
+
+	sscanf(buf, "%lu", &wanted);
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	devfreq->user_set_freq = wanted;
+	err = count;
+out:
+	mutex_unlock(&devfreq_list_lock);
+	if (err >= 0)
+		devfreq_update(&devfreq->nb, 0, NULL);
+	return err;
+}
+
+static ssize_t show_user_frequency(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+
+}
+
+static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
+static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
+static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
+static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
+static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
+		   store_polling_interval);
+static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
+		   set_user_frequency);
+static struct attribute *dev_entries[] = {
+	&dev_attr_devfreq_governor.attr,
+	&dev_attr_devfreq_cur_freq.attr,
+	&dev_attr_devfreq_max_freq.attr,
+	&dev_attr_devfreq_min_freq.attr,
+	&dev_attr_devfreq_polling_interval.attr,
+	&dev_attr_devfreq_set_freq.attr,
+	NULL,
+};
+static struct attribute_group dev_attr_group = {
+	.name	= power_group_name,
+	.attrs	= dev_entries,
+};
+
 /**
  * devfreq_init() - Initialize data structure for devfreq framework and
  *		  start polling registered devfreq devices.
-- 
1.7.4.1

Re: [PATCH v7 4/4] PM / DEVFREQ: add sysfs interface

[Turquette, Mike]

On Tue, Aug 23, 2011 at 12:54 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor      R: name of governor
> - cur_freq      R: current frequency
> - max_freq      R: maximum operable frequency
> - min_freq      R: minimum operable frequency
> - set_freq      R: read user specified frequency (0 if not specified yet)
>                W: set user specified frequency
> - polling_interval      R: polling interval in ms given with devfreq profile
>                        W: update polling interval.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> --
> Changed from v6
> - poling_interval is writable.
>
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
>  require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
>  Documentation/ABI/testing/sysfs-devices-power |   47 +++++++
>  drivers/devfreq/devfreq.c                     |  185 +++++++++++++++++++++++++
>  2 files changed, 232 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..ba8bc35 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,50 @@ Description:
>
>                Not all drivers support this attribute.  If it isn't supported,
>                attempts to read or write it will yield I/O errors.
> +
> +What:          /sys/devices/.../power/devfreq_governor
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_governor shows the name
> +               of the governor used by the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_cur_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
> +               frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_max_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the

Copy/paste error?  Description should reference devfreq_max_freq not
devfreq_cur_freq.

> +               maximum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_min_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the
> +               minimum operable frequency of the corresponding device.

Similar to the above.

> +
> +What:          /sys/devices/.../power/devfreq_set_freq
> +Date:          August 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_set_freq sets and shows
> +               the user specified desired frequency of the device. The
> +               governor may and may not use the value. With the basic
> +               governors given with devfreq.c, userspace governor is
> +               using the value.

As I stated in patch 3, this should conditionally exist only if the
userspace governor is being used for this device.

The existing devfreq_cur_freq covers the read-only case well.

Regards,
Mike

> +
> +What:          /sys/devices/.../power/devfreq_polling_interval
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_polling_interval sets and
> +               shows the requested polling interval of the corresponding
> +               device. The values are represented in ms. If the value is less
> +               than 1 jiffy, it is considered to be 0, which means no polling.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index df63bdc..3070250 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>  static LIST_HEAD(devfreq_list);
>  static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
>  /**
>  * find_device_devfreq() - find devfreq struct using device pointer
>  * @dev:       device pointer used to lookup device devfreq.
> @@ -149,6 +151,8 @@ static void devfreq_monitor(struct work_struct *work)
>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>                                        error, devfreq->governor->name);
>
> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
> +                                                   &dev_attr_group);
>                                list_del(&devfreq->node);
>                                kfree(devfreq);
>
> @@ -239,6 +243,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>                queue_delayed_work(devfreq_wq, &devfreq_work,
>                                   devfreq->next_polling);
>        }
> +
> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>  out:
>        mutex_unlock(&devfreq_list_lock);
>
> @@ -271,6 +277,8 @@ int devfreq_remove_device(struct device *dev)
>                goto out;
>        }
>
> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
>        list_del(&devfreq->node);
>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>        kfree(devfreq);
> @@ -279,6 +287,183 @@ out:
>        return 0;
>  }
>
> +static ssize_t show_governor(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->governor)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +
> +       return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +       unsigned long freq = ULONG_MAX;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_floor(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +       unsigned long freq = 0;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_ceil(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> +                                    struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t store_polling_interval(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     const char *buf, size_t count)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +       unsigned int value;
> +       int ret;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       ret = sscanf(buf, "%u", &value);
> +       if (ret != 1)
> +               return -EINVAL;
> +
> +       df->profile->polling_ms = value;
> +       df->next_polling = df->polling_jiffies
> +                        = msecs_to_jiffies(value);
> +
> +       mutex_lock(&devfreq_list_lock);
> +       if (df->next_polling > 0 && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  df->next_polling);
> +       }
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return count;
> +}
> +
> +static ssize_t set_user_frequency(struct device *dev,
> +                                 struct device_attribute *attr,
> +                                 const char *buf, size_t count)
> +{
> +       struct devfreq *devfreq;
> +       unsigned long wanted;
> +       int err = 0;
> +
> +       sscanf(buf, "%lu", &wanted);
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       devfreq->user_set_freq = wanted;
> +       err = count;
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       if (err >= 0)
> +               devfreq_update(&devfreq->nb, 0, NULL);
> +       return err;
> +}
> +
> +static ssize_t show_user_frequency(struct device *dev,
> +                                  struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *devfreq;
> +       int err = 0;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       return err;
> +
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
> +                  store_polling_interval);
> +static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
> +                  set_user_frequency);
> +static struct attribute *dev_entries[] = {
> +       &dev_attr_devfreq_governor.attr,
> +       &dev_attr_devfreq_cur_freq.attr,
> +       &dev_attr_devfreq_max_freq.attr,
> +       &dev_attr_devfreq_min_freq.attr,
> +       &dev_attr_devfreq_polling_interval.attr,
> +       &dev_attr_devfreq_set_freq.attr,
> +       NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> +       .name   = power_group_name,
> +       .attrs  = dev_entries,
> +};
> +
>  /**
>  * devfreq_init() - Initialize data structure for devfreq framework and
>  *               start polling registered devfreq devices.
> --
> 1.7.4.1
>
>

Re: [PATCH v7 4/4] PM / DEVFREQ: add sysfs interface

[MyungJoo Ham]

On Wed, Aug 24, 2011 at 2:34 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Tue, Aug 23, 2011 at 12:54 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> +               The /sys/devices/.../power/devfreq_cur_freq shows the
>
> Copy/paste error?  Description should reference devfreq_max_freq not
> devfreq_cur_freq.
>
>> +               minimum operable frequency of the corresponding device.
>
> Similar to the above.
>

Thank you. I'll resubmit soon with corrections (and w/ some further
changes to allow governors to have their own sysfs entries and access
more on the DEVFREQ core).

>> +
>> +What:          /sys/devices/.../power/devfreq_set_freq
>> +Date:          August 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_set_freq sets and shows
>> +               the user specified desired frequency of the device. The
>> +               governor may and may not use the value. With the basic
>> +               governors given with devfreq.c, userspace governor is
>> +               using the value.
>
> As I stated in patch 3, this should conditionally exist only if the
> userspace governor is being used for this device.

Ok, in the next revision, this entry will be removed and replaced with
an entry created by userspace governor.

>
> The existing devfreq_cur_freq covers the read-only case well.

It could be a bit different especially with another device driver
enabling and disabling OPPs and/or PM QoS.

For example, when user states "100MHz" on a device with
"50/100/200MHz", it will use 100. However, if someone has disabled
100MHz with opp_disable, devfreq_cur_freq will automatically changed
to 200 while the user value should remain at 100 in case where 100 is
later re-enabled.

>
> Regards,
> Mike
>

Thank you.


Cheers!

MyungJoo


-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[MyungJoo Ham]

The patchset revision v8 has minor updates since v7 and v6.
- Allow governors to have their own sysfs interface and init/exit callbacks.

The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
There has been reordering between "add common sysfs interfaces" patch
and "add basic governors" (3/5 and 5/5)
"add internal interfaces for governors (4/5)" patch has been newly
introduced at v8 patchset.

For a usage example, please look at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
and other related clocks simply follow the determined DDR RAM clock.

The DEVFREQ driver for Exynos4210 memory bus is at
/drivers/devfreq/exynos4210_memorybus.c in the git tree.

In the dd (writing and reading 360MiB) test with NURI board, the memory
throughput was not changed (the performance is not deteriorated) while
the SoC power consumption has been reduced by 1%. When the memory access
is not that intense while the CPU is heavily used, the SoC power consumption
has been reduced by 6%. The power consumption has been compared with the
case using the conventional Exynos4210 CPUFREQ driver, which sets memory
bus frequency according to the CPU core frequency. Besides, when the CPU core
running slow and the memory access is intense, the performance (memory
throughput) has been increased by 11% (with higher SoC power consumption of
5%). The tested governor is "simple-ondemand".

MyungJoo Ham (5):
  PM / OPP: Add OPP availability change notifier.
  PM: Introduce DEVFREQ: generic DVFS framework with device-specific
    OPPs
  PM / DEVFREQ: add common sysfs interfaces
  PM / DEVFREQ: add internal interfaces for governors
  PM / DEVFREQ: add basic governors

 Documentation/ABI/testing/sysfs-devices-power |   46 +++
 drivers/Kconfig                               |    2 +
 drivers/Makefile                              |    2 +
 drivers/base/power/opp.c                      |   29 ++
 drivers/devfreq/Kconfig                       |   75 ++++
 drivers/devfreq/Makefile                      |    5 +
 drivers/devfreq/devfreq.c                     |  463 +++++++++++++++++++++++++
 drivers/devfreq/governor.h                    |   20 +
 drivers/devfreq/governor_performance.c        |   24 ++
 drivers/devfreq/governor_powersave.c          |   24 ++
 drivers/devfreq/governor_simpleondemand.c     |   88 +++++
 drivers/devfreq/governor_userspace.c          |  119 +++++++
 include/linux/devfreq.h                       |  150 ++++++++
 include/linux/opp.h                           |   12 +
 14 files changed, 1059 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 drivers/devfreq/governor.h
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c
 create mode 100644 include/linux/devfreq.h

-- 
1.7.4.1

[PATCH v8 1/5] PM / OPP: Add OPP availability change notifier.

[MyungJoo Ham]

The patch enables to register notifier_block for an OPP-device in order
to get notified for any changes in the availability of OPPs of the
device. For example, if a new OPP is inserted or enable/disable status
of an OPP is changed, the notifier is executed.

This enables the usage of opp_add, opp_enable, and opp_disable to
directly take effect with any connected entities such as CPUFREQ or
DEVFREQ.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Mike Turquette <mturquette@ti.com>

---
Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
---
 drivers/base/power/opp.c |   29 +++++++++++++++++++++++++++++
 include/linux/opp.h      |   12 ++++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index b23de18..e6b4c89 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -73,6 +73,7 @@ struct opp {
  *		RCU usage: nodes are not modified in the list of device_opp,
  *		however addition is possible and is secured by dev_opp_list_lock
  * @dev:	device pointer
+ * @head:	notifier head to notify the OPP availability changes.
  * @opp_list:	list of opps
  *
  * This is an internal data structure maintaining the link to opps attached to
@@ -83,6 +84,7 @@ struct device_opp {
 	struct list_head node;
 
 	struct device *dev;
+	struct srcu_notifier_head head;
 	struct list_head opp_list;
 };
 
@@ -404,6 +406,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 		}
 
 		dev_opp->dev = dev;
+		srcu_init_notifier_head(&dev_opp->head);
 		INIT_LIST_HEAD(&dev_opp->opp_list);
 
 		/* Secure the device list modification */
@@ -428,6 +431,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	list_add_rcu(&new_opp->node, head);
 	mutex_unlock(&dev_opp_list_lock);
 
+	/*
+	 * Notify the changes in the availability of the operable
+	 * frequency/voltage list.
+	 */
+	srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
 	return 0;
 }
 
@@ -504,6 +512,14 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
 	mutex_unlock(&dev_opp_list_lock);
 	synchronize_rcu();
 
+	/* Notify the change of the OPP availability */
+	if (availability_req)
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ENABLE,
+					 new_opp);
+	else
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_DISABLE,
+					 new_opp);
+
 	/* clean up old opp */
 	new_opp = opp;
 	goto out;
@@ -643,3 +659,16 @@ void opp_free_cpufreq_table(struct device *dev,
 	*table = NULL;
 }
 #endif		/* CONFIG_CPU_FREQ */
+
+/** opp_get_notifier() - find notifier_head of the device with opp
+ * @dev:	device pointer used to lookup device OPPs.
+ */
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	struct device_opp *dev_opp = find_device_opp(dev);
+
+	if (IS_ERR(dev_opp))
+		return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
+
+	return &dev_opp->head;
+}
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 7020e97..87a9208 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -16,9 +16,14 @@
 
 #include <linux/err.h>
 #include <linux/cpufreq.h>
+#include <linux/notifier.h>
 
 struct opp;
 
+enum opp_event {
+	OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
+};
+
 #if defined(CONFIG_PM_OPP)
 
 unsigned long opp_get_voltage(struct opp *opp);
@@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
 
 int opp_disable(struct device *dev, unsigned long freq);
 
+struct srcu_notifier_head *opp_get_notifier(struct device *dev);
+
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
@@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
 {
 	return 0;
 }
+
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif		/* CONFIG_PM */
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-- 
1.7.4.1

[PATCH v8 2/5] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs

[MyungJoo Ham]

With OPPs, a device may have multiple operable frequency and voltage
sets. However, there can be multiple possible operable sets and a system
will need to choose one from them. In order to reduce the power
consumption (by reducing frequency and voltage) without affecting the
performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
scheme may be used.

This patch introduces the DVFS capability to non-CPU devices with OPPs.
DVFS is a techique whereby the frequency and supplied voltage of a
device is adjusted on-the-fly. DVFS usually sets the frequency as low
as possible with given conditions (such as QoS assurance) and adjusts
voltage according to the chosen frequency in order to reduce power
consumption and heat dissipation.

The generic DVFS for devices, DEVFREQ, may appear quite similar with
/drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
devices registered and is not suitable to have multiple heterogenous
devices with different (but simple) governors.

Normally, DVFS mechanism controls frequency based on the demand for
the device, and then, chooses voltage based on the chosen frequency.
DEVFREQ also controls the frequency based on the governor's frequency
recommendation and let OPP pick up the pair of frequency and voltage
based on the recommended frequency. Then, the chosen OPP is passed to
device driver's "target" callback.

When PM QoS is going to be used with the DEVFREQ device, the device
driver should enable OPPs that are appropriate with the current PM QoS
requests. In order to do so, the device driver may call opp_enable and
opp_disable at the notifier callback of PM QoS so that PM QoS's
update_target() call enables the appropriate OPPs. Note that at least
one of OPPs should be enabled at any time; be careful when there is a
transition.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Mike Turquette <mturquette@ti.com>

---
The test code with board support for Exynos4-NURI is at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

---
Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
and Kevin.

At v8, there is no changes since v7

Changes from v6
- Type revised for timing variables
- Removed unnecessary code and variable

Changes at v6-resubmit from v6
- Use jiffy directly instead of ktime
- Be prepared for profile->polling_ms changes (not supported fully at
  this stage)

Changes from v5
- Uses OPP availability change notifier
- Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
  polling interval based on the interval requirement of DEVFREQ
  devices.
- Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
  including governors and devfreq drivers.
- Coding style revised.
- Updated devfreq_add_device interface to get tunable values.

Changed from v4
- Removed tickle, which is a duplicated feature; PM QoS can do the same.
- Allow to extend polling interval if devices have longer polling intervals.
- Relocated private data of governors.
- Removed system-wide sysfs

Changed from v3
- In kerneldoc comments, DEVFREQ has ben replaced by devfreq
- Revised removing devfreq entries with error mechanism
- Added and revised comments
- Removed unnecessary codes
- Allow to give a name to a governor
- Bugfix: a tickle call may cancel an older tickle call that is still in
  effect.

Changed from v2
- Code style revised and cleaned up.
- Remove DEVFREQ entries that incur errors except for EAGAIN
- Bug fixed: tickle for devices without polling governors

Changes from v1(RFC)
- Rename: DVFS --> DEVFREQ
- Revised governor design
	. Governor receives the whole struct devfreq
	. Governor should gather usage information (thru get_dev_status) itself
- Periodic monitoring runs only when needed.
- DEVFREQ no more deals with voltage information directly
- Removed some printks.
- Some cosmetics update
- Use freezable_wq.
---
 drivers/Kconfig           |    2 +
 drivers/Makefile          |    2 +
 drivers/devfreq/Kconfig   |   39 ++++++
 drivers/devfreq/Makefile  |    1 +
 drivers/devfreq/devfreq.c |  297 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h   |  107 ++++++++++++++++
 6 files changed, 448 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 include/linux/devfreq.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9e7e..a1efd75 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
 
 source "drivers/virt/Kconfig"
 
+source "drivers/devfreq/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 7fa433a..97c957b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # Virtualization drivers
 obj-$(CONFIG_VIRT_DRIVERS)	+= virt/
+
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq/
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
new file mode 100644
index 0000000..1fb42de
--- /dev/null
+++ b/drivers/devfreq/Kconfig
@@ -0,0 +1,39 @@
+config ARCH_HAS_DEVFREQ
+	bool
+	depends on ARCH_HAS_OPP
+	help
+	  Denotes that the architecture supports DEVFREQ. If the architecture
+	  supports multiple OPP entries per device and the frequency of the
+	  devices with OPPs may be altered dynamically, the architecture
+	  supports DEVFREQ.
+
+menuconfig PM_DEVFREQ
+	bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
+	depends on PM_OPP && ARCH_HAS_DEVFREQ
+	help
+	  With OPP support, a device may have a list of frequencies and
+	  voltages available. DEVFREQ, a generic DVFS framework can be
+	  registered for a device with OPP support in order to let the
+	  governor provided to DEVFREQ choose an operating frequency
+	  based on the OPP's list and the policy given with DEVFREQ.
+
+	  Each device may have its own governor and policy. DEVFREQ can
+	  reevaluate the device state periodically and/or based on the
+	  OPP list changes (each frequency/voltage pair in OPP may be
+	  disabled or enabled).
+
+	  Like some CPUs with CPUFREQ, a device may have multiple clocks.
+	  However, because the clock frequencies of a single device are
+	  determined by the single device's state, an instance of DEVFREQ
+	  is attached to a single device and returns a "representative"
+	  clock frequency from the OPP of the device, which is also attached
+	  to a device by 1-to-1. The device registering DEVFREQ takes the
+	  responsiblity to "interpret" the frequency listed in OPP and
+	  to set its every clock accordingly with the "target" callback
+	  given to DEVFREQ.
+
+if PM_DEVFREQ
+
+comment "DEVFREQ Drivers"
+
+endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
new file mode 100644
index 0000000..168934a
--- /dev/null
+++ b/drivers/devfreq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
new file mode 100644
index 0000000..df63bdc
--- /dev/null
+++ b/drivers/devfreq/devfreq.c
@@ -0,0 +1,297 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/opp.h>
+#include <linux/devfreq.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/printk.h>
+#include <linux/hrtimer.h>
+
+/*
+ * devfreq_work periodically monitors every registered device.
+ * The minimum polling interval is one jiffy. The polling interval is
+ * determined by the minimum polling period among all polling devfreq
+ * devices. The resolution of polling interval is one jiffy.
+ */
+static bool polling;
+static struct workqueue_struct *devfreq_wq;
+static struct delayed_work devfreq_work;
+
+/* The list of all device-devfreq */
+static LIST_HEAD(devfreq_list);
+static DEFINE_MUTEX(devfreq_list_lock);
+
+/**
+ * find_device_devfreq() - find devfreq struct using device pointer
+ * @dev:	device pointer used to lookup device devfreq.
+ *
+ * Search the list of device devfreqs and return the matched device's
+ * devfreq info. devfreq_list_lock should be held by the caller.
+ */
+static struct devfreq *find_device_devfreq(struct device *dev)
+{
+	struct devfreq *tmp_devfreq;
+
+	if (unlikely(IS_ERR_OR_NULL(dev))) {
+		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+
+	list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
+		if (tmp_devfreq->dev == dev)
+			return tmp_devfreq;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * devfreq_do() - Check the usage profile of a given device and configure
+ *		frequency and voltage accordingly
+ * @devfreq:	devfreq info of the given device
+ */
+static int devfreq_do(struct devfreq *devfreq)
+{
+	struct opp *opp;
+	unsigned long freq;
+	int err;
+
+	err = devfreq->governor->get_target_freq(devfreq, &freq);
+	if (err)
+		return err;
+
+	opp = opp_find_freq_ceil(devfreq->dev, &freq);
+	if (opp == ERR_PTR(-ENODEV))
+		opp = opp_find_freq_floor(devfreq->dev, &freq);
+
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	if (devfreq->previous_freq == freq)
+		return 0;
+
+	err = devfreq->profile->target(devfreq->dev, opp);
+	if (err)
+		return err;
+
+	devfreq->previous_freq = freq;
+	return 0;
+}
+
+/**
+ * devfreq_update() - Notify that the device OPP has been changed.
+ * @dev:	the device whose OPP has been changed.
+ */
+static int devfreq_update(struct notifier_block *nb, unsigned long type,
+			  void *devp)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = container_of(nb, struct devfreq, nb);
+	/* Reevaluate the proper frequency */
+	err = devfreq_do(devfreq);
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+}
+
+/**
+ * devfreq_monitor() - Periodically run devfreq_do()
+ * @work: the work struct used to run devfreq_monitor periodically.
+ *
+ */
+static void devfreq_monitor(struct work_struct *work)
+{
+	static unsigned long last_polled_at;
+	struct devfreq *devfreq, *tmp;
+	int error;
+	unsigned long jiffies_passed;
+	unsigned long next_jiffies = ULONG_MAX, now = jiffies;
+
+	/* Initially last_polled_at = 0, polling every device at bootup */
+	jiffies_passed = now - last_polled_at;
+	last_polled_at = now;
+	if (jiffies_passed == 0)
+		jiffies_passed = 1;
+
+	mutex_lock(&devfreq_list_lock);
+
+	list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
+		if (devfreq->next_polling == 0)
+			continue;
+
+		/*
+		 * Reduce more next_polling if devfreq_wq took an extra
+		 * delay. (i.e., CPU has been idled.)
+		 */
+		if (devfreq->next_polling <= jiffies_passed) {
+			error = devfreq_do(devfreq);
+
+			/* Remove a devfreq with an error. */
+			if (error && error != -EAGAIN) {
+				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
+					error, devfreq->governor->name);
+
+				list_del(&devfreq->node);
+				kfree(devfreq);
+
+				continue;
+			}
+			devfreq->next_polling = devfreq->polling_jiffies;
+
+			/* No more polling required (polling_ms changed) */
+			if (devfreq->next_polling == 0)
+				continue;
+		} else {
+			devfreq->next_polling -= jiffies_passed;
+		}
+
+		next_jiffies = (next_jiffies > devfreq->next_polling) ?
+				devfreq->next_polling : next_jiffies;
+	}
+
+	if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
+	} else {
+		polling = false;
+	}
+
+	mutex_unlock(&devfreq_list_lock);
+}
+
+/**
+ * devfreq_add_device() - Add devfreq feature to the device
+ * @dev:	the device to add devfreq feature.
+ * @profile:	device-specific profile to run devfreq.
+ * @governor:	the policy to choose frequency.
+ * @data:	private data for the governor. The devfreq framework does not
+ *		touch this value.
+ */
+int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
+		       struct devfreq_governor *governor, void *data)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev || !profile || !governor) {
+		dev_err(dev, "%s: Invalid parameters.\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&devfreq_list_lock);
+
+	devfreq = find_device_devfreq(dev);
+	if (!IS_ERR(devfreq)) {
+		dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
+		err = -EINVAL;
+		goto out;
+	}
+
+	devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
+	if (!devfreq) {
+		dev_err(dev, "%s: Unable to create devfreq for the device\n",
+			__func__);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	devfreq->dev = dev;
+	devfreq->profile = profile;
+	devfreq->governor = governor;
+	devfreq->next_polling = devfreq->polling_jiffies
+			      = msecs_to_jiffies(devfreq->profile->polling_ms);
+	devfreq->previous_freq = profile->initial_freq;
+	devfreq->data = data;
+
+	devfreq->nb.notifier_call = devfreq_update;
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+	err = srcu_notifier_chain_register(nh, &devfreq->nb);
+	if (err)
+		goto out;
+
+	list_add(&devfreq->node, &devfreq_list);
+
+	if (devfreq_wq && devfreq->next_polling && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   devfreq->next_polling);
+	}
+out:
+	mutex_unlock(&devfreq_list_lock);
+
+	return err;
+}
+
+/**
+ * devfreq_remove_device() - Remove devfreq feature from a device.
+ * @device:	the device to remove devfreq feature.
+ */
+int devfreq_remove_device(struct device *dev)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+
+	list_del(&devfreq->node);
+	srcu_notifier_chain_unregister(nh, &devfreq->nb);
+	kfree(devfreq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return 0;
+}
+
+/**
+ * devfreq_init() - Initialize data structure for devfreq framework and
+ *		  start polling registered devfreq devices.
+ */
+static int __init devfreq_init(void)
+{
+	mutex_lock(&devfreq_list_lock);
+	polling = false;
+	devfreq_wq = create_freezable_workqueue("devfreq_wq");
+	INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
+	mutex_unlock(&devfreq_list_lock);
+
+	devfreq_monitor(&devfreq_work.work);
+	return 0;
+}
+late_initcall(devfreq_init);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
new file mode 100644
index 0000000..8252238
--- /dev/null
+++ b/include/linux/devfreq.h
@@ -0,0 +1,107 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX_DEVFREQ_H__
+#define __LINUX_DEVFREQ_H__
+
+#include <linux/notifier.h>
+
+#define DEVFREQ_NAME_LEN 16
+
+struct devfreq;
+struct devfreq_dev_status {
+	/* both since the last measure */
+	unsigned long total_time;
+	unsigned long busy_time;
+	unsigned long current_frequency;
+};
+
+struct devfreq_dev_profile {
+	unsigned long max_freq; /* may be larger than the actual value */
+	unsigned long initial_freq;
+	unsigned int polling_ms;	/* 0 for at opp change only */
+
+	int (*target)(struct device *dev, struct opp *opp);
+	int (*get_dev_status)(struct device *dev,
+			      struct devfreq_dev_status *stat);
+};
+
+/**
+ * struct devfreq_governor - Devfreq policy governor
+ * @name		Governor's name
+ * @get_target_freq	Returns desired operating frequency for the device.
+ *			Basically, get_target_freq will run
+ *			devfreq_dev_profile.get_dev_status() to get the
+ *			status of the device (load = busy_time / total_time).
+ */
+struct devfreq_governor {
+	char name[DEVFREQ_NAME_LEN];
+	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
+};
+
+/**
+ * struct devfreq - Device devfreq structure
+ * @node	list node - contains the devices with devfreq that have been
+ *		registered.
+ * @dev		device pointer
+ * @profile	device-specific devfreq profile
+ * @governor	method how to choose frequency based on the usage.
+ * @nb		notifier block registered to the corresponding OPP to get
+ *		notified for frequency availability updates.
+ * @polling_jiffies	interval in jiffies.
+ * @previous_freq	previously configured frequency value.
+ * @next_polling	the number of remaining jiffies to poll with
+ *			"devfreq_monitor" executions to reevaluate
+ *			frequency/voltage of the device. Set by
+ *			profile's polling_ms interval.
+ * @data	Private data of the governor. The devfreq framework does not
+ *		touch this.
+ *
+ * This structure stores the devfreq information for a give device.
+ */
+struct devfreq {
+	struct list_head node;
+
+	struct device *dev;
+	struct devfreq_dev_profile *profile;
+	struct devfreq_governor *governor;
+	struct notifier_block nb;
+
+	unsigned long polling_jiffies;
+	unsigned long previous_freq;
+	unsigned int next_polling;
+
+	void *data; /* private data for governors */
+};
+
+#if defined(CONFIG_PM_DEVFREQ)
+extern int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data);
+extern int devfreq_remove_device(struct device *dev);
+#else /* !CONFIG_PM_DEVFREQ */
+static int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data)
+{
+	return 0;
+}
+
+static int devfreq_remove_device(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_DEVFREQ */
+
+#endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

[PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces

[MyungJoo Ham]

Device specific sysfs interface /sys/devices/.../power/devfreq_*
- governor	R: name of governor
- cur_freq	R: current frequency
- max_freq	R: maximum operable frequency
- min_freq	R: minimum operable frequency
- polling_interval	R: polling interval in ms given with devfreq profile
			W: update polling interval.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Changed from v7
- removed set_freq from the common devfreq interface
- added get_devfreq, a mutex-protected wrapper for find_device_devfreq
(for sysfs interfaces and later with governor-support)
- corrected ABI documentation.

Changed from v6
- poling_interval is writable.

Changed from v5
- updated devferq_update usage.

Changed from v4
- removed system-wide sysfs interface
- removed tickling sysfs interface
- added set_freq for userspace governor (and any other governors that
  require user input)

Changed from v3
- corrected sysfs API usage
- corrected error messages
- moved sysfs entry location
- added sysfs entries

Changed from v2
- add ABI entries for devfreq sysfs interface
---
 Documentation/ABI/testing/sysfs-devices-power |   37 ++++++
 drivers/devfreq/devfreq.c                     |  150 +++++++++++++++++++++++++
 2 files changed, 187 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
index 8ffbc25..57f4591 100644
--- a/Documentation/ABI/testing/sysfs-devices-power
+++ b/Documentation/ABI/testing/sysfs-devices-power
@@ -165,3 +165,40 @@ Description:
 
 		Not all drivers support this attribute.  If it isn't supported,
 		attempts to read or write it will yield I/O errors.
+
+What:		/sys/devices/.../power/devfreq_governor
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_governor shows the name
+		of the governor used by the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_cur_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the current
+		frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_max_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_max_freq shows the
+		maximum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_min_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_min_freq shows the
+		minimum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_polling_interval
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_polling_interval sets and
+		shows the requested polling interval of the corresponding
+		device. The values are represented in ms. If the value is less
+		than 1 jiffy, it is considered to be 0, which means no polling.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index df63bdc..5bbece6 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
 static LIST_HEAD(devfreq_list);
 static DEFINE_MUTEX(devfreq_list_lock);
 
+static struct attribute_group dev_attr_group;
+
 /**
  * find_device_devfreq() - find devfreq struct using device pointer
  * @dev:	device pointer used to lookup device devfreq.
@@ -62,6 +64,22 @@ static struct devfreq *find_device_devfreq(struct device *dev)
 }
 
 /**
+ * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
+ *		with mutex protection.
+ * @dev:	device pointer used to lookup device devfreq.
+ */
+static struct devfreq *get_devfreq(struct device *dev)
+{
+	struct devfreq *ret;
+
+	mutex_lock(&devfreq_list_lock);
+	ret = find_device_devfreq(dev);
+	mutex_unlock(&devfreq_list_lock);
+
+	return ret;
+}
+
+/**
  * devfreq_do() - Check the usage profile of a given device and configure
  *		frequency and voltage accordingly
  * @devfreq:	devfreq info of the given device
@@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
 				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
 					error, devfreq->governor->name);
 
+				sysfs_unmerge_group(&devfreq->dev->kobj,
+						    &dev_attr_group);
 				list_del(&devfreq->node);
 				kfree(devfreq);
 
@@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
 		queue_delayed_work(devfreq_wq, &devfreq_work,
 				   devfreq->next_polling);
 	}
+
+	sysfs_merge_group(&dev->kobj, &dev_attr_group);
 out:
 	mutex_unlock(&devfreq_list_lock);
 
@@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
 		goto out;
 	}
 
+	sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
+
 	list_del(&devfreq->node);
 	srcu_notifier_chain_unregister(nh, &devfreq->nb);
 	kfree(devfreq);
@@ -279,6 +303,132 @@ out:
 	return 0;
 }
 
+static ssize_t show_governor(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = get_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->governor)
+		return -EINVAL;
+
+	return sprintf(buf, "%s\n", df->governor->name);
+}
+
+static ssize_t show_freq(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = get_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+
+	return sprintf(buf, "%lu\n", df->previous_freq);
+}
+
+static ssize_t show_max_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = get_devfreq(dev);
+	unsigned long freq = ULONG_MAX;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_floor(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_min_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = get_devfreq(dev);
+	unsigned long freq = 0;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_ceil(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_polling_interval(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = get_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->profile)
+		return -EINVAL;
+
+	return sprintf(buf, "%d\n", df->profile->polling_ms);
+}
+
+static ssize_t store_polling_interval(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct devfreq *df = get_devfreq(dev);
+	unsigned int value;
+	int ret;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->profile)
+		return -EINVAL;
+
+	ret = sscanf(buf, "%u", &value);
+	if (ret != 1)
+		return -EINVAL;
+
+	df->profile->polling_ms = value;
+	df->next_polling = df->polling_jiffies
+			 = msecs_to_jiffies(value);
+
+	mutex_lock(&devfreq_list_lock);
+	if (df->next_polling > 0 && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   df->next_polling);
+	}
+	mutex_unlock(&devfreq_list_lock);
+
+	return count;
+}
+
+static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
+static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
+static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
+static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
+static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
+		   store_polling_interval);
+static struct attribute *dev_entries[] = {
+	&dev_attr_devfreq_governor.attr,
+	&dev_attr_devfreq_cur_freq.attr,
+	&dev_attr_devfreq_max_freq.attr,
+	&dev_attr_devfreq_min_freq.attr,
+	&dev_attr_devfreq_polling_interval.attr,
+	NULL,
+};
+static struct attribute_group dev_attr_group = {
+	.name	= power_group_name,
+	.attrs	= dev_entries,
+};
+
 /**
  * devfreq_init() - Initialize data structure for devfreq framework and
  *		  start polling registered devfreq devices.
-- 
1.7.4.1

Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces

[Turquette, Mike]

On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor      R: name of governor
> - cur_freq      R: current frequency
> - max_freq      R: maximum operable frequency
> - min_freq      R: minimum operable frequency
> - polling_interval      R: polling interval in ms given with devfreq profile
>                        W: update polling interval.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Looks good to me.

Reviewed-by: Mike Turquette <mturquette@ti.com>

Regards,
Mike

>
> --
> Changed from v7
> - removed set_freq from the common devfreq interface
> - added get_devfreq, a mutex-protected wrapper for find_device_devfreq
> (for sysfs interfaces and later with governor-support)
> - corrected ABI documentation.
>
> Changed from v6
> - poling_interval is writable.
>
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
>  require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
>  Documentation/ABI/testing/sysfs-devices-power |   37 ++++++
>  drivers/devfreq/devfreq.c                     |  150 +++++++++++++++++++++++++
>  2 files changed, 187 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..57f4591 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,40 @@ Description:
>
>                Not all drivers support this attribute.  If it isn't supported,
>                attempts to read or write it will yield I/O errors.
> +
> +What:          /sys/devices/.../power/devfreq_governor
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_governor shows the name
> +               of the governor used by the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_cur_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
> +               frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_max_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_max_freq shows the
> +               maximum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_min_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_min_freq shows the
> +               minimum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_polling_interval
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_polling_interval sets and
> +               shows the requested polling interval of the corresponding
> +               device. The values are represented in ms. If the value is less
> +               than 1 jiffy, it is considered to be 0, which means no polling.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index df63bdc..5bbece6 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>  static LIST_HEAD(devfreq_list);
>  static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
>  /**
>  * find_device_devfreq() - find devfreq struct using device pointer
>  * @dev:       device pointer used to lookup device devfreq.
> @@ -62,6 +64,22 @@ static struct devfreq *find_device_devfreq(struct device *dev)
>  }
>
>  /**
> + * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> + *             with mutex protection.
> + * @dev:       device pointer used to lookup device devfreq.
> + */
> +static struct devfreq *get_devfreq(struct device *dev)
> +{
> +       struct devfreq *ret;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       ret = find_device_devfreq(dev);
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return ret;
> +}
> +
> +/**
>  * devfreq_do() - Check the usage profile of a given device and configure
>  *             frequency and voltage accordingly
>  * @devfreq:   devfreq info of the given device
> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>                                        error, devfreq->governor->name);
>
> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
> +                                                   &dev_attr_group);
>                                list_del(&devfreq->node);
>                                kfree(devfreq);
>
> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>                queue_delayed_work(devfreq_wq, &devfreq_work,
>                                   devfreq->next_polling);
>        }
> +
> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>  out:
>        mutex_unlock(&devfreq_list_lock);
>
> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
>                goto out;
>        }
>
> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
>        list_del(&devfreq->node);
>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>        kfree(devfreq);
> @@ -279,6 +303,132 @@ out:
>        return 0;
>  }
>
> +static ssize_t show_governor(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->governor)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +
> +       return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned long freq = ULONG_MAX;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_floor(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned long freq = 0;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_ceil(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> +                                    struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t store_polling_interval(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     const char *buf, size_t count)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned int value;
> +       int ret;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       ret = sscanf(buf, "%u", &value);
> +       if (ret != 1)
> +               return -EINVAL;
> +
> +       df->profile->polling_ms = value;
> +       df->next_polling = df->polling_jiffies
> +                        = msecs_to_jiffies(value);
> +
> +       mutex_lock(&devfreq_list_lock);
> +       if (df->next_polling > 0 && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  df->next_polling);
> +       }
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
> +                  store_polling_interval);
> +static struct attribute *dev_entries[] = {
> +       &dev_attr_devfreq_governor.attr,
> +       &dev_attr_devfreq_cur_freq.attr,
> +       &dev_attr_devfreq_max_freq.attr,
> +       &dev_attr_devfreq_min_freq.attr,
> +       &dev_attr_devfreq_polling_interval.attr,
> +       NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> +       .name   = power_group_name,
> +       .attrs  = dev_entries,
> +};
> +
>  /**
>  * devfreq_init() - Initialize data structure for devfreq framework and
>  *               start polling registered devfreq devices.
> --
> 1.7.4.1
>
>

Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces

[Turquette, Mike]

On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor      R: name of governor
> - cur_freq      R: current frequency
> - max_freq      R: maximum operable frequency
> - min_freq      R: minimum operable frequency
> - polling_interval      R: polling interval in ms given with devfreq profile
>                        W: update polling interval.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> --
> Changed from v7
> - removed set_freq from the common devfreq interface
> - added get_devfreq, a mutex-protected wrapper for find_device_devfreq
> (for sysfs interfaces and later with governor-support)
> - corrected ABI documentation.
>
> Changed from v6
> - poling_interval is writable.
>
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
>  require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
>  Documentation/ABI/testing/sysfs-devices-power |   37 ++++++
>  drivers/devfreq/devfreq.c                     |  150 +++++++++++++++++++++++++
>  2 files changed, 187 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..57f4591 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,40 @@ Description:
>
>                Not all drivers support this attribute.  If it isn't supported,
>                attempts to read or write it will yield I/O errors.
> +
> +What:          /sys/devices/.../power/devfreq_governor
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_governor shows the name
> +               of the governor used by the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_cur_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
> +               frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_max_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_max_freq shows the
> +               maximum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_min_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_min_freq shows the
> +               minimum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_polling_interval
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_polling_interval sets and
> +               shows the requested polling interval of the corresponding
> +               device. The values are represented in ms. If the value is less
> +               than 1 jiffy, it is considered to be 0, which means no polling.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index df63bdc..5bbece6 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>  static LIST_HEAD(devfreq_list);
>  static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
>  /**
>  * find_device_devfreq() - find devfreq struct using device pointer
>  * @dev:       device pointer used to lookup device devfreq.
> @@ -62,6 +64,22 @@ static struct devfreq *find_device_devfreq(struct device *dev)
>  }
>
>  /**
> + * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> + *             with mutex protection.
> + * @dev:       device pointer used to lookup device devfreq.
> + */
> +static struct devfreq *get_devfreq(struct device *dev)
> +{
> +       struct devfreq *ret;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       ret = find_device_devfreq(dev);
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return ret;
> +}

Sorry I'll have to rescind my ACK.  I just reviewed patch 5/5 and the
show_freq and restore_freq functions for the userspace governor do not
protect the private data accesses with a mutex.  Looking a bit further
I see that they do call get_devfreq; I think the semantics for this
are wrong.

get_devfreq only protects the list as long as it takes to do a lookup.
 However neither struct devfreq or struct userspace_data have a mutex
associated with them, so concurrent operations are not protected.

One heavy-handed way of solving this is to not have get_devfreq
release the mutex, and then have those functions call a new
put_devfreq which does release the mutex.  However that is really bad
locking.

What do you think about putting a mutex in struct devfreq?  The rule
for governors can be that access to the governor-specific private data
requires taking that devfreq's mutex.

Regards,
Mike

> +
> +/**
>  * devfreq_do() - Check the usage profile of a given device and configure
>  *             frequency and voltage accordingly
>  * @devfreq:   devfreq info of the given device
> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>                                        error, devfreq->governor->name);
>
> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
> +                                                   &dev_attr_group);
>                                list_del(&devfreq->node);
>                                kfree(devfreq);
>
> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>                queue_delayed_work(devfreq_wq, &devfreq_work,
>                                   devfreq->next_polling);
>        }
> +
> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>  out:
>        mutex_unlock(&devfreq_list_lock);
>
> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
>                goto out;
>        }
>
> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
>        list_del(&devfreq->node);
>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>        kfree(devfreq);
> @@ -279,6 +303,132 @@ out:
>        return 0;
>  }
>
> +static ssize_t show_governor(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->governor)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +
> +       return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned long freq = ULONG_MAX;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_floor(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned long freq = 0;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_ceil(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> +                                    struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t store_polling_interval(struct device *dev,
> +                                     struct device_attribute *attr,
> +                                     const char *buf, size_t count)
> +{
> +       struct devfreq *df = get_devfreq(dev);
> +       unsigned int value;
> +       int ret;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       ret = sscanf(buf, "%u", &value);
> +       if (ret != 1)
> +               return -EINVAL;
> +
> +       df->profile->polling_ms = value;
> +       df->next_polling = df->polling_jiffies
> +                        = msecs_to_jiffies(value);
> +
> +       mutex_lock(&devfreq_list_lock);
> +       if (df->next_polling > 0 && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  df->next_polling);
> +       }
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return count;
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
> +                  store_polling_interval);
> +static struct attribute *dev_entries[] = {
> +       &dev_attr_devfreq_governor.attr,
> +       &dev_attr_devfreq_cur_freq.attr,
> +       &dev_attr_devfreq_max_freq.attr,
> +       &dev_attr_devfreq_min_freq.attr,
> +       &dev_attr_devfreq_polling_interval.attr,
> +       NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> +       .name   = power_group_name,
> +       .attrs  = dev_entries,
> +};
> +
>  /**
>  * devfreq_init() - Initialize data structure for devfreq framework and
>  *               start polling registered devfreq devices.
> --
> 1.7.4.1
>
>

Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces

[MyungJoo Ham]

On Tue, Aug 30, 2011 at 4:17 AM, Turquette, Mike <mturquette@ti.com> wrote:
>
> Sorry I'll have to rescind my ACK.  I just reviewed patch 5/5 and the
> show_freq and restore_freq functions for the userspace governor do not
> protect the private data accesses with a mutex.  Looking a bit further
> I see that they do call get_devfreq; I think the semantics for this
> are wrong.
>
> get_devfreq only protects the list as long as it takes to do a lookup.
>  However neither struct devfreq or struct userspace_data have a mutex
> associated with them, so concurrent operations are not protected.
>
> One heavy-handed way of solving this is to not have get_devfreq
> release the mutex, and then have those functions call a new
> put_devfreq which does release the mutex.  However that is really bad
> locking.
>
> What do you think about putting a mutex in struct devfreq?  The rule
> for governors can be that access to the governor-specific private data
> requires taking that devfreq's mutex.


A lock in private data at DEVFREQ framework won't be needed as it is a
governor-specific issue; however, it appears that we need a locking
mechanism for accessing struct devfreq in general for governors.
Although we do not have any governor that writes something or reads
multiple times on struct devfreq (except for the private data) out of
get_target_freq ops, which does not need an additional lock, we may
have one soon.

Then, I will need to update the DEVFREQ core as well.
There will be two locks in devfreq.c: the global devfreq_list_lock and
per-devfreq devfreq_lock in struct devfreq.
And the role of devfreq_lock will be protecting the access to struct
devfreq (some functions in devfreq.c will use this lock and some usage
of devfreq_list_lock will be removed).

This will result in more general sysfs interface (or functions other
than the standard ops) support in governors.

Thank you so much!


Cheers,
MyungJoo.

>
> Regards,
> Mike
>
>> +
>> +/**
>>  * devfreq_do() - Check the usage profile of a given device and configure
>>  *             frequency and voltage accordingly
>>  * @devfreq:   devfreq info of the given device
>> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
>>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>>                                        error, devfreq->governor->name);
>>
>> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
>> +                                                   &dev_attr_group);
>>                                list_del(&devfreq->node);
>>                                kfree(devfreq);
>>
>> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>>                queue_delayed_work(devfreq_wq, &devfreq_work,
>>                                   devfreq->next_polling);
>>        }
>> +
>> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>>  out:
>>        mutex_unlock(&devfreq_list_lock);
>>
>> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
>>                goto out;
>>        }
>>
>> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>> +
>>        list_del(&devfreq->node);
>>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>>        kfree(devfreq);
>> @@ -279,6 +303,132 @@ out:
>>        return 0;
>>  }
>>
>> +static ssize_t show_governor(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->governor)
>> +               return -EINVAL;
>> +
>> +       return sprintf(buf, "%s\n", df->governor->name);
>> +}
>> +
>> +static ssize_t show_freq(struct device *dev,
>> +                        struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +
>> +       return sprintf(buf, "%lu\n", df->previous_freq);
>> +}
>> +
>> +static ssize_t show_max_freq(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +       unsigned long freq = ULONG_MAX;
>> +       struct opp *opp;
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->dev)
>> +               return -EINVAL;
>> +
>> +       opp = opp_find_freq_floor(df->dev, &freq);
>> +       if (IS_ERR(opp))
>> +               return PTR_ERR(opp);
>> +
>> +       return sprintf(buf, "%lu\n", freq);
>> +}
>> +
>> +static ssize_t show_min_freq(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +       unsigned long freq = 0;
>> +       struct opp *opp;
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->dev)
>> +               return -EINVAL;
>> +
>> +       opp = opp_find_freq_ceil(df->dev, &freq);
>> +       if (IS_ERR(opp))
>> +               return PTR_ERR(opp);
>> +
>> +       return sprintf(buf, "%lu\n", freq);
>> +}
>> +
>> +static ssize_t show_polling_interval(struct device *dev,
>> +                                    struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->profile)
>> +               return -EINVAL;
>> +
>> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
>> +}
>> +
>> +static ssize_t store_polling_interval(struct device *dev,
>> +                                     struct device_attribute *attr,
>> +                                     const char *buf, size_t count)
>> +{
>> +       struct devfreq *df = get_devfreq(dev);
>> +       unsigned int value;
>> +       int ret;
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->profile)
>> +               return -EINVAL;
>> +
>> +       ret = sscanf(buf, "%u", &value);
>> +       if (ret != 1)
>> +               return -EINVAL;
>> +
>> +       df->profile->polling_ms = value;
>> +       df->next_polling = df->polling_jiffies
>> +                        = msecs_to_jiffies(value);
>> +
>> +       mutex_lock(&devfreq_list_lock);
>> +       if (df->next_polling > 0 && !polling) {
>> +               polling = true;
>> +               queue_delayed_work(devfreq_wq, &devfreq_work,
>> +                                  df->next_polling);
>> +       }
>> +       mutex_unlock(&devfreq_list_lock);
>> +
>> +       return count;
>> +}
>> +
>> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
>> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
>> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
>> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
>> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
>> +                  store_polling_interval);
>> +static struct attribute *dev_entries[] = {
>> +       &dev_attr_devfreq_governor.attr,
>> +       &dev_attr_devfreq_cur_freq.attr,
>> +       &dev_attr_devfreq_max_freq.attr,
>> +       &dev_attr_devfreq_min_freq.attr,
>> +       &dev_attr_devfreq_polling_interval.attr,
>> +       NULL,
>> +};
>> +static struct attribute_group dev_attr_group = {
>> +       .name   = power_group_name,
>> +       .attrs  = dev_entries,
>> +};
>> +
>>  /**
>>  * devfreq_init() - Initialize data structure for devfreq framework and
>>  *               start polling registered devfreq devices.
>> --
>> 1.7.4.1
>>
>>
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH v8 3/5] PM / DEVFREQ: add common sysfs interfaces

[Turquette, Mike]

On Mon, Aug 29, 2011 at 9:28 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Tue, Aug 30, 2011 at 4:17 AM, Turquette, Mike <mturquette@ti.com> wrote:
>>
>> Sorry I'll have to rescind my ACK.  I just reviewed patch 5/5 and the
>> show_freq and restore_freq functions for the userspace governor do not
>> protect the private data accesses with a mutex.  Looking a bit further
>> I see that they do call get_devfreq; I think the semantics for this
>> are wrong.
>>
>> get_devfreq only protects the list as long as it takes to do a lookup.
>>  However neither struct devfreq or struct userspace_data have a mutex
>> associated with them, so concurrent operations are not protected.
>>
>> One heavy-handed way of solving this is to not have get_devfreq
>> release the mutex, and then have those functions call a new
>> put_devfreq which does release the mutex.  However that is really bad
>> locking.
>>
>> What do you think about putting a mutex in struct devfreq?  The rule
>> for governors can be that access to the governor-specific private data
>> requires taking that devfreq's mutex.
>
>
> A lock in private data at DEVFREQ framework won't be needed as it is a
> governor-specific issue; however, it appears that we need a locking
> mechanism for accessing struct devfreq in general for governors.
> Although we do not have any governor that writes something or reads
> multiple times on struct devfreq (except for the private data) out of
> get_target_freq ops, which does not need an additional lock, we may
> have one soon.
>
> Then, I will need to update the DEVFREQ core as well.
> There will be two locks in devfreq.c: the global devfreq_list_lock and
> per-devfreq devfreq_lock in struct devfreq.
> And the role of devfreq_lock will be protecting the access to struct
> devfreq (some functions in devfreq.c will use this lock and some usage
> of devfreq_list_lock will be removed).
>
> This will result in more general sysfs interface (or functions other
> than the standard ops) support in governors.

Sounds good.  Also, please add a comment somewhere in devfreq.h
(probably above definition for struct devfreq) that the lock must also
be held by governors when accessing devfreq->data.

Regards,
Mike

> Thank you so much!
>
>
> Cheers,
> MyungJoo.
>
>>
>> Regards,
>> Mike
>>
>>> +
>>> +/**
>>>  * devfreq_do() - Check the usage profile of a given device and configure
>>>  *             frequency and voltage accordingly
>>>  * @devfreq:   devfreq info of the given device
>>> @@ -149,6 +167,8 @@ static void devfreq_monitor(struct work_struct *work)
>>>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>>>                                        error, devfreq->governor->name);
>>>
>>> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
>>> +                                                   &dev_attr_group);
>>>                                list_del(&devfreq->node);
>>>                                kfree(devfreq);
>>>
>>> @@ -239,6 +259,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>>>                queue_delayed_work(devfreq_wq, &devfreq_work,
>>>                                   devfreq->next_polling);
>>>        }
>>> +
>>> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>>>  out:
>>>        mutex_unlock(&devfreq_list_lock);
>>>
>>> @@ -271,6 +293,8 @@ int devfreq_remove_device(struct device *dev)
>>>                goto out;
>>>        }
>>>
>>> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>>> +
>>>        list_del(&devfreq->node);
>>>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>>>        kfree(devfreq);
>>> @@ -279,6 +303,132 @@ out:
>>>        return 0;
>>>  }
>>>
>>> +static ssize_t show_governor(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->governor)
>>> +               return -EINVAL;
>>> +
>>> +       return sprintf(buf, "%s\n", df->governor->name);
>>> +}
>>> +
>>> +static ssize_t show_freq(struct device *dev,
>>> +                        struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +
>>> +       return sprintf(buf, "%lu\n", df->previous_freq);
>>> +}
>>> +
>>> +static ssize_t show_max_freq(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +       unsigned long freq = ULONG_MAX;
>>> +       struct opp *opp;
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->dev)
>>> +               return -EINVAL;
>>> +
>>> +       opp = opp_find_freq_floor(df->dev, &freq);
>>> +       if (IS_ERR(opp))
>>> +               return PTR_ERR(opp);
>>> +
>>> +       return sprintf(buf, "%lu\n", freq);
>>> +}
>>> +
>>> +static ssize_t show_min_freq(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +       unsigned long freq = 0;
>>> +       struct opp *opp;
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->dev)
>>> +               return -EINVAL;
>>> +
>>> +       opp = opp_find_freq_ceil(df->dev, &freq);
>>> +       if (IS_ERR(opp))
>>> +               return PTR_ERR(opp);
>>> +
>>> +       return sprintf(buf, "%lu\n", freq);
>>> +}
>>> +
>>> +static ssize_t show_polling_interval(struct device *dev,
>>> +                                    struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->profile)
>>> +               return -EINVAL;
>>> +
>>> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
>>> +}
>>> +
>>> +static ssize_t store_polling_interval(struct device *dev,
>>> +                                     struct device_attribute *attr,
>>> +                                     const char *buf, size_t count)
>>> +{
>>> +       struct devfreq *df = get_devfreq(dev);
>>> +       unsigned int value;
>>> +       int ret;
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->profile)
>>> +               return -EINVAL;
>>> +
>>> +       ret = sscanf(buf, "%u", &value);
>>> +       if (ret != 1)
>>> +               return -EINVAL;
>>> +
>>> +       df->profile->polling_ms = value;
>>> +       df->next_polling = df->polling_jiffies
>>> +                        = msecs_to_jiffies(value);
>>> +
>>> +       mutex_lock(&devfreq_list_lock);
>>> +       if (df->next_polling > 0 && !polling) {
>>> +               polling = true;
>>> +               queue_delayed_work(devfreq_wq, &devfreq_work,
>>> +                                  df->next_polling);
>>> +       }
>>> +       mutex_unlock(&devfreq_list_lock);
>>> +
>>> +       return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
>>> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_polling_interval, 0644, show_polling_interval,
>>> +                  store_polling_interval);
>>> +static struct attribute *dev_entries[] = {
>>> +       &dev_attr_devfreq_governor.attr,
>>> +       &dev_attr_devfreq_cur_freq.attr,
>>> +       &dev_attr_devfreq_max_freq.attr,
>>> +       &dev_attr_devfreq_min_freq.attr,
>>> +       &dev_attr_devfreq_polling_interval.attr,
>>> +       NULL,
>>> +};
>>> +static struct attribute_group dev_attr_group = {
>>> +       .name   = power_group_name,
>>> +       .attrs  = dev_entries,
>>> +};
>>> +
>>>  /**
>>>  * devfreq_init() - Initialize data structure for devfreq framework and
>>>  *               start polling registered devfreq devices.
>>> --
>>> 1.7.4.1
>>>
>>>
>>
>
>
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH v8 4/5] PM / DEVFREQ: add internal interfaces for governors

[MyungJoo Ham]

- get_devfreq(): governors may convert struct dev -> struct devfreq
- update_devfreq(): governors may notify DEVFREQ core to reevaluate
frequencies.
- Governors may have .init and .exit callbacks

In order to use the internal interface, get_devfreq() and
update_devfreq(), governor should include "governor.h"

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/devfreq/devfreq.c  |   32 ++++++++++++++++++++++++--------
 drivers/devfreq/governor.h |   20 ++++++++++++++++++++
 include/linux/devfreq.h    |    2 ++
 3 files changed, 46 insertions(+), 8 deletions(-)
 create mode 100644 drivers/devfreq/governor.h

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 5bbece6..8de3b21 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -65,10 +65,10 @@ static struct devfreq *find_device_devfreq(struct device *dev)
 
 /**
  * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
- *		with mutex protection.
+ *		with mutex protection. exported for governors
  * @dev:	device pointer used to lookup device devfreq.
  */
-static struct devfreq *get_devfreq(struct device *dev)
+struct devfreq *get_devfreq(struct device *dev)
 {
 	struct devfreq *ret;
 
@@ -113,17 +113,15 @@ static int devfreq_do(struct devfreq *devfreq)
 }
 
 /**
- * devfreq_update() - Notify that the device OPP has been changed.
- * @dev:	the device whose OPP has been changed.
+ * update_devfreq() - Notify that the device OPP or frequency requirement
+ *		has been changed. This function is exported for governors.
+ * @devfreq:	the devfreq instance.
  */
-static int devfreq_update(struct notifier_block *nb, unsigned long type,
-			  void *devp)
+int update_devfreq(struct devfreq *devfreq)
 {
-	struct devfreq *devfreq;
 	int err = 0;
 
 	mutex_lock(&devfreq_list_lock);
-	devfreq = container_of(nb, struct devfreq, nb);
 	/* Reevaluate the proper frequency */
 	err = devfreq_do(devfreq);
 	mutex_unlock(&devfreq_list_lock);
@@ -131,6 +129,18 @@ static int devfreq_update(struct notifier_block *nb, unsigned long type,
 }
 
 /**
+ * devfreq_update() - Notify that the device OPP has been changed.
+ * @dev:	the device whose OPP has been changed.
+ */
+static int devfreq_update(struct notifier_block *nb, unsigned long type,
+			  void *devp)
+{
+	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
+
+	return update_devfreq(devfreq);
+}
+
+/**
  * devfreq_monitor() - Periodically run devfreq_do()
  * @work: the work struct used to run devfreq_monitor periodically.
  *
@@ -254,6 +264,9 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
 
 	list_add(&devfreq->node, &devfreq_list);
 
+	if (governor->init)
+		governor->init(devfreq);
+
 	if (devfreq_wq && devfreq->next_polling && !polling) {
 		polling = true;
 		queue_delayed_work(devfreq_wq, &devfreq_work,
@@ -295,6 +308,9 @@ int devfreq_remove_device(struct device *dev)
 
 	sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
 
+	if (devfreq->governor->exit)
+		devfreq->governor->exit(devfreq);
+
 	list_del(&devfreq->node);
 	srcu_notifier_chain_unregister(nh, &devfreq->nb);
 	kfree(devfreq);
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
new file mode 100644
index 0000000..bb5d964
--- /dev/null
+++ b/drivers/devfreq/governor.h
@@ -0,0 +1,20 @@
+/*
+ * governor.h - internal header for governors.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This header is for devfreq governors in drivers/devfreq/
+ */
+
+#ifndef _GOVERNOR_H
+#define _GOVERNOR_H
+
+extern struct devfreq *get_devfreq(struct device *dev);
+extern int update_devfreq(struct devfreq *devfreq);
+
+#endif /* _GOVERNOR_H */
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 8252238..fdc6916 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -46,6 +46,8 @@ struct devfreq_dev_profile {
 struct devfreq_governor {
 	char name[DEVFREQ_NAME_LEN];
 	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
+	int (*init)(struct devfreq *this);
+	void (*exit)(struct devfreq *this);
 };
 
 /**
-- 
1.7.4.1

Re: [PATCH v8 4/5] PM / DEVFREQ: add internal interfaces for governors

[Turquette, Mike]

On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> - get_devfreq(): governors may convert struct dev -> struct devfreq
> - update_devfreq(): governors may notify DEVFREQ core to reevaluate
> frequencies.
> - Governors may have .init and .exit callbacks
>
> In order to use the internal interface, get_devfreq() and
> update_devfreq(), governor should include "governor.h"
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Please see patch 3 and 5 for my thoughts on locking in struct devfreq.
 Also, this patch doesn't need to be separate, but can be rolled into
patch 3.

Regards,
Mike

> ---
>  drivers/devfreq/devfreq.c  |   32 ++++++++++++++++++++++++--------
>  drivers/devfreq/governor.h |   20 ++++++++++++++++++++
>  include/linux/devfreq.h    |    2 ++
>  3 files changed, 46 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/devfreq/governor.h
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 5bbece6..8de3b21 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -65,10 +65,10 @@ static struct devfreq *find_device_devfreq(struct device *dev)
>
>  /**
>  * get_devfreq() - find devfreq struct. a wrapped find_device_devfreq()
> - *             with mutex protection.
> + *             with mutex protection. exported for governors
>  * @dev:       device pointer used to lookup device devfreq.
>  */
> -static struct devfreq *get_devfreq(struct device *dev)
> +struct devfreq *get_devfreq(struct device *dev)
>  {
>        struct devfreq *ret;
>
> @@ -113,17 +113,15 @@ static int devfreq_do(struct devfreq *devfreq)
>  }
>
>  /**
> - * devfreq_update() - Notify that the device OPP has been changed.
> - * @dev:       the device whose OPP has been changed.
> + * update_devfreq() - Notify that the device OPP or frequency requirement
> + *             has been changed. This function is exported for governors.
> + * @devfreq:   the devfreq instance.
>  */
> -static int devfreq_update(struct notifier_block *nb, unsigned long type,
> -                         void *devp)
> +int update_devfreq(struct devfreq *devfreq)
>  {
> -       struct devfreq *devfreq;
>        int err = 0;
>
>        mutex_lock(&devfreq_list_lock);
> -       devfreq = container_of(nb, struct devfreq, nb);
>        /* Reevaluate the proper frequency */
>        err = devfreq_do(devfreq);
>        mutex_unlock(&devfreq_list_lock);
> @@ -131,6 +129,18 @@ static int devfreq_update(struct notifier_block *nb, unsigned long type,
>  }
>
>  /**
> + * devfreq_update() - Notify that the device OPP has been changed.
> + * @dev:       the device whose OPP has been changed.
> + */
> +static int devfreq_update(struct notifier_block *nb, unsigned long type,
> +                         void *devp)
> +{
> +       struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> +
> +       return update_devfreq(devfreq);
> +}
> +
> +/**
>  * devfreq_monitor() - Periodically run devfreq_do()
>  * @work: the work struct used to run devfreq_monitor periodically.
>  *
> @@ -254,6 +264,9 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>
>        list_add(&devfreq->node, &devfreq_list);
>
> +       if (governor->init)
> +               governor->init(devfreq);
> +
>        if (devfreq_wq && devfreq->next_polling && !polling) {
>                polling = true;
>                queue_delayed_work(devfreq_wq, &devfreq_work,
> @@ -295,6 +308,9 @@ int devfreq_remove_device(struct device *dev)
>
>        sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>
> +       if (devfreq->governor->exit)
> +               devfreq->governor->exit(devfreq);
> +
>        list_del(&devfreq->node);
>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>        kfree(devfreq);
> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> new file mode 100644
> index 0000000..bb5d964
> --- /dev/null
> +++ b/drivers/devfreq/governor.h
> @@ -0,0 +1,20 @@
> +/*
> + * governor.h - internal header for governors.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This header is for devfreq governors in drivers/devfreq/
> + */
> +
> +#ifndef _GOVERNOR_H
> +#define _GOVERNOR_H
> +
> +extern struct devfreq *get_devfreq(struct device *dev);
> +extern int update_devfreq(struct devfreq *devfreq);
> +
> +#endif /* _GOVERNOR_H */
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 8252238..fdc6916 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -46,6 +46,8 @@ struct devfreq_dev_profile {
>  struct devfreq_governor {
>        char name[DEVFREQ_NAME_LEN];
>        int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> +       int (*init)(struct devfreq *this);
> +       void (*exit)(struct devfreq *this);
>  };
>
>  /**
> --
> 1.7.4.1
>
>

[PATCH v8 5/5] PM / DEVFREQ: add basic governors

[MyungJoo Ham]

Four CPUFREQ-like governors are provided as examples.

powersave: use the lowest frequency possible. The user (device) should
set the polling_ms as 0 because polling is useless for this governor.

performance: use the highest freqeuncy possible. The user (device)
should set the polling_ms as 0 because polling is useless for this
governor.

userspace: use the user specified frequency stored at
devfreq.user_set_freq. With sysfs support in the following patch, a user
may set the value with the sysfs interface.

simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.

When a user updates OPP entries (enable/disable/add), OPP framework
automatically notifies DEVFREQ to update operating frequency
accordingly. Thus, DEVFREQ users (device drivers) do not need to update
DEVFREQ manually with OPP entry updates or set polling_ms for powersave
, performance, userspace, or any other "static" governors.

Note that these are given only as basic examples for governors and any
devices with DEVFREQ may implement their own governors with the drivers
and use them.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Changed from v7
- Userspace uses its own sysfs interface.

Changed from v5
- Seperated governor files from devfreq.c
- Allow simple ondemand to be tuned for each device
---
 Documentation/ABI/testing/sysfs-devices-power |    9 ++
 drivers/devfreq/Kconfig                       |   36 ++++++++
 drivers/devfreq/Makefile                      |    4 +
 drivers/devfreq/governor_performance.c        |   24 +++++
 drivers/devfreq/governor_powersave.c          |   24 +++++
 drivers/devfreq/governor_simpleondemand.c     |   88 ++++++++++++++++++
 drivers/devfreq/governor_userspace.c          |  119 +++++++++++++++++++++++++
 include/linux/devfreq.h                       |   41 +++++++++
 8 files changed, 345 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c

diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
index 57f4591..c7f6977 100644
--- a/Documentation/ABI/testing/sysfs-devices-power
+++ b/Documentation/ABI/testing/sysfs-devices-power
@@ -202,3 +202,12 @@ Description:
 		shows the requested polling interval of the corresponding
 		device. The values are represented in ms. If the value is less
 		than 1 jiffy, it is considered to be 0, which means no polling.
+
+What:		/sys/devices/.../power/devfreq_userspace_set_freq
+Date:		August 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_userspace_set_freq sets
+		and shows the user specified frequency in kHz. This sysfs
+		entry is created and managed by userspace DEVFREQ governor.
+		If other governors are used, it won't be supported.
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 1fb42de..643b055 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
 
 if PM_DEVFREQ
 
+comment "DEVFREQ Governors"
+
+config DEVFREQ_GOV_SIMPLE_ONDEMAND
+	bool "Simple Ondemand"
+	help
+	  Chooses frequency based on the recent load on the device. Works
+	  similar as ONDEMAND governor of CPUFREQ does. A device with
+	  Simple-Ondemand should be able to provide busy/total counter
+	  values that imply the usage rate. A device may provide tuned
+	  values to the governor with data field at devfreq_add_device().
+
+config DEVFREQ_GOV_PERFORMANCE
+	bool "Performance"
+	help
+	  Sets the frequency at the maximum available frequency.
+	  This governor always returns UINT_MAX as frequency so that
+	  the DEVFREQ framework returns the highest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_POWERSAVE
+	bool "Powersave"
+	help
+	  Sets the frequency at the minimum available frequency.
+	  This governor always returns 0 as frequency so that
+	  the DEVFREQ framework returns the lowest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_USERSPACE
+	bool "Userspace"
+	help
+	  Sets the frequency at the user specified one.
+	  This governor returns the user configured frequency if there
+	  has been an input to /sys/devices/.../power/devfreq_set_freq.
+	  Otherwise, the governor does not change the frequnecy
+	  given at the initialization.
+
 comment "DEVFREQ Drivers"
 
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 168934a..4564a89 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -1 +1,5 @@
 obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
+obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
+obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
+obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
+obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
new file mode 100644
index 0000000..c47eff8
--- /dev/null
+++ b/drivers/devfreq/governor_performance.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_performance.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_performance_func(struct devfreq *df,
+				    unsigned long *freq)
+{
+	*freq = UINT_MAX; /* devfreq_do will run "floor" */
+	return 0;
+}
+
+struct devfreq_governor devfreq_performance = {
+	.name = "performance",
+	.get_target_freq = devfreq_performance_func,
+};
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
new file mode 100644
index 0000000..4f128d8
--- /dev/null
+++ b/drivers/devfreq/governor_powersave.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_powersave.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_powersave_func(struct devfreq *df,
+				  unsigned long *freq)
+{
+	*freq = 0; /* devfreq_do will run "ceiling" to 0 */
+	return 0;
+}
+
+struct devfreq_governor devfreq_powersave = {
+	.name = "powersave",
+	.get_target_freq = devfreq_powersave_func,
+};
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
new file mode 100644
index 0000000..18fe8be
--- /dev/null
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -0,0 +1,88 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/errno.h>
+#include <linux/devfreq.h>
+#include <linux/math64.h>
+
+/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
+#define DFSO_UPTHRESHOLD	(90)
+#define DFSO_DOWNDIFFERENCTIAL	(5)
+static int devfreq_simple_ondemand_func(struct devfreq *df,
+					unsigned long *freq)
+{
+	struct devfreq_dev_status stat;
+	int err = df->profile->get_dev_status(df->dev, &stat);
+	unsigned long long a, b;
+	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
+	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
+	struct devfreq_simple_ondemand_data *data = df->data;
+
+	if (err)
+		return err;
+
+	if (data) {
+		if (data->upthreshold)
+			dfso_upthreshold = data->upthreshold;
+		if (data->downdifferential)
+			dfso_downdifferential = data->downdifferential;
+	}
+	if (dfso_upthreshold > 100 ||
+	    dfso_upthreshold < dfso_downdifferential)
+		return -EINVAL;
+
+	/* Assume MAX if it is going to be divided by zero */
+	if (stat.total_time == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Prevent overflow */
+	if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
+		stat.busy_time >>= 7;
+		stat.total_time >>= 7;
+	}
+
+	/* Set MAX if it's busy enough */
+	if (stat.busy_time * 100 >
+	    stat.total_time * dfso_upthreshold) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Set MAX if we do not know the initial frequency */
+	if (stat.current_frequency == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Keep the current frequency */
+	if (stat.busy_time * 100 >
+	    stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
+		*freq = stat.current_frequency;
+		return 0;
+	}
+
+	/* Set the desired frequency based on the load */
+	a = stat.busy_time;
+	a *= stat.current_frequency;
+	b = div_u64(a, stat.total_time);
+	b *= 100;
+	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
+	*freq = (unsigned long) b;
+
+	return 0;
+}
+
+struct devfreq_governor devfreq_simple_ondemand = {
+	.name = "simple_ondemand",
+	.get_target_freq = devfreq_simple_ondemand_func,
+};
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
new file mode 100644
index 0000000..53a4574
--- /dev/null
+++ b/drivers/devfreq/governor_userspace.c
@@ -0,0 +1,119 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/devfreq.h>
+#include <linux/pm.h>
+#include "governor.h"
+
+struct userspace_data {
+	unsigned long user_frequency;
+	bool valid;
+};
+
+static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
+{
+	struct userspace_data *data = df->data;
+
+	if (!data->valid)
+		*freq = df->previous_freq; /* No user freq specified yet */
+	else
+		*freq = data->user_frequency;
+	return 0;
+}
+
+static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	struct devfreq *devfreq = get_devfreq(dev);
+	struct userspace_data *data;
+	unsigned long wanted;
+	int err = 0;
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+	data = devfreq->data;
+
+	sscanf(buf, "%lu", &wanted);
+	data->user_frequency = wanted;
+	data->valid = true;
+	err = update_devfreq(devfreq);
+	if (err == 0)
+		err = count;
+out:
+	return err;
+}
+
+static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct devfreq *devfreq = get_devfreq(dev);
+	struct userspace_data *data;
+	int err = 0;
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+	data = devfreq->data;
+
+	if (data->valid)
+		err = sprintf(buf, "%lu\n", data->user_frequency);
+	else
+		err = sprintf(buf, "undefined\n");
+out:
+	return err;
+}
+
+static DEVICE_ATTR(devfreq_userspace_set_freq, 0644, show_freq, store_freq);
+static struct attribute *dev_entries[] = {
+	&dev_attr_devfreq_userspace_set_freq.attr,
+	NULL,
+};
+static struct attribute_group dev_attr_group = {
+	.name	= power_group_name,
+	.attrs	= dev_entries,
+};
+
+static int userspace_init(struct devfreq *devfreq)
+{
+	int err = 0;
+	struct userspace_data *data = kzalloc(sizeof(struct userspace_data),
+					      GFP_KERNEL);
+
+	if (!data) {
+		err = -ENOMEM;
+		goto out;
+	}
+	data->valid = false;
+	devfreq->data = data;
+
+	sysfs_merge_group(&devfreq->dev->kobj, &dev_attr_group);
+out:
+	return err;
+}
+
+static void userspace_exit(struct devfreq *devfreq)
+{
+	sysfs_unmerge_group(&devfreq->dev->kobj, &dev_attr_group);
+	kfree(devfreq->data);
+	devfreq->data = NULL;
+}
+
+struct devfreq_governor devfreq_userspace = {
+	.name = "userspace",
+	.get_target_freq = devfreq_userspace_func,
+	.init = userspace_init,
+	.exit = userspace_exit,
+};
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index fdc6916..cbafcdf 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -13,6 +13,7 @@
 #ifndef __LINUX_DEVFREQ_H__
 #define __LINUX_DEVFREQ_H__
 
+#include <linux/opp.h>
 #include <linux/notifier.h>
 
 #define DEVFREQ_NAME_LEN 16
@@ -65,6 +66,8 @@ struct devfreq_governor {
  *			"devfreq_monitor" executions to reevaluate
  *			frequency/voltage of the device. Set by
  *			profile's polling_ms interval.
+ * @user_set_freq	User specified adequete frequency value (thru sysfs
+ *		interface). Governors may and may not use this value.
  * @data	Private data of the governor. The devfreq framework does not
  *		touch this.
  *
@@ -82,6 +85,7 @@ struct devfreq {
 	unsigned long previous_freq;
 	unsigned int next_polling;
 
+	unsigned long user_set_freq; /* governors may ignore this. */
 	void *data; /* private data for governors */
 };
 
@@ -91,6 +95,37 @@ extern int devfreq_add_device(struct device *dev,
 			   struct devfreq_governor *governor,
 			   void *data);
 extern int devfreq_remove_device(struct device *dev);
+
+#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
+extern struct devfreq_governor devfreq_powersave;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
+extern struct devfreq_governor devfreq_performance;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
+extern struct devfreq_governor devfreq_userspace;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
+extern struct devfreq_governor devfreq_simple_ondemand;
+/**
+ * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
+ *	and devfreq_add_device
+ * @ upthreshold	If the load is over this value, the frequency jumps.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ * @ downdifferential	If the load is under upthreshold - downdifferential,
+ *			the governor may consider slowing the frequency down.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ *			downdifferential < upthreshold must hold.
+ *
+ * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
+ * the governor uses the default values.
+ */
+struct devfreq_simple_ondemand_data {
+	unsigned int upthreshold;
+	unsigned int downdifferential;
+};
+#endif
+
 #else /* !CONFIG_PM_DEVFREQ */
 static int devfreq_add_device(struct device *dev,
 			   struct devfreq_dev_profile *profile,
@@ -104,6 +139,12 @@ static int devfreq_remove_device(struct device *dev)
 {
 	return 0;
 }
+
+#define devfreq_powersave	NULL
+#define devfreq_performance	NULL
+#define devfreq_userspace	NULL
+#define devfreq_simple_ondemand	NULL
+
 #endif /* CONFIG_PM_DEVFREQ */
 
 #endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

Re: [PATCH v8 5/5] PM / DEVFREQ: add basic governors

[Turquette, Mike]

On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Four CPUFREQ-like governors are provided as examples.
>
> powersave: use the lowest frequency possible. The user (device) should
> set the polling_ms as 0 because polling is useless for this governor.
>
> performance: use the highest freqeuncy possible. The user (device)
> should set the polling_ms as 0 because polling is useless for this
> governor.
>
> userspace: use the user specified frequency stored at
> devfreq.user_set_freq. With sysfs support in the following patch, a user
> may set the value with the sysfs interface.
>
> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.
>
> When a user updates OPP entries (enable/disable/add), OPP framework
> automatically notifies DEVFREQ to update operating frequency
> accordingly. Thus, DEVFREQ users (device drivers) do not need to update
> DEVFREQ manually with OPP entry updates or set polling_ms for powersave
> , performance, userspace, or any other "static" governors.
>
> Note that these are given only as basic examples for governors and any
> devices with DEVFREQ may implement their own governors with the drivers
> and use them.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> Changed from v7
> - Userspace uses its own sysfs interface.
>
> Changed from v5
> - Seperated governor files from devfreq.c
> - Allow simple ondemand to be tuned for each device
> ---
>  Documentation/ABI/testing/sysfs-devices-power |    9 ++
>  drivers/devfreq/Kconfig                       |   36 ++++++++
>  drivers/devfreq/Makefile                      |    4 +
>  drivers/devfreq/governor_performance.c        |   24 +++++
>  drivers/devfreq/governor_powersave.c          |   24 +++++
>  drivers/devfreq/governor_simpleondemand.c     |   88 ++++++++++++++++++
>  drivers/devfreq/governor_userspace.c          |  119 +++++++++++++++++++++++++
>  include/linux/devfreq.h                       |   41 +++++++++
>  8 files changed, 345 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/governor_performance.c
>  create mode 100644 drivers/devfreq/governor_powersave.c
>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>  create mode 100644 drivers/devfreq/governor_userspace.c
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 57f4591..c7f6977 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -202,3 +202,12 @@ Description:
>                shows the requested polling interval of the corresponding
>                device. The values are represented in ms. If the value is less
>                than 1 jiffy, it is considered to be 0, which means no polling.
> +
> +What:          /sys/devices/.../power/devfreq_userspace_set_freq

How about just .../devfreq_set_freq?  I think the userspace bit is
implied and the name is very long.

> +Date:          August 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_userspace_set_freq sets
> +               and shows the user specified frequency in kHz. This sysfs
> +               entry is created and managed by userspace DEVFREQ governor.
> +               If other governors are used, it won't be supported.
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 1fb42de..643b055 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>
>  if PM_DEVFREQ
>
> +comment "DEVFREQ Governors"
> +
> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
> +       bool "Simple Ondemand"
> +       help
> +         Chooses frequency based on the recent load on the device. Works
> +         similar as ONDEMAND governor of CPUFREQ does. A device with
> +         Simple-Ondemand should be able to provide busy/total counter
> +         values that imply the usage rate. A device may provide tuned
> +         values to the governor with data field at devfreq_add_device().
> +
> +config DEVFREQ_GOV_PERFORMANCE
> +       bool "Performance"
> +       help
> +         Sets the frequency at the maximum available frequency.
> +         This governor always returns UINT_MAX as frequency so that
> +         the DEVFREQ framework returns the highest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_POWERSAVE
> +       bool "Powersave"
> +       help
> +         Sets the frequency at the minimum available frequency.
> +         This governor always returns 0 as frequency so that
> +         the DEVFREQ framework returns the lowest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_USERSPACE
> +       bool "Userspace"
> +       help
> +         Sets the frequency at the user specified one.
> +         This governor returns the user configured frequency if there
> +         has been an input to /sys/devices/.../power/devfreq_set_freq.
> +         Otherwise, the governor does not change the frequnecy
> +         given at the initialization.
> +
>  comment "DEVFREQ Drivers"
>
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 168934a..4564a89 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -1 +1,5 @@
>  obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> +obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)      += governor_simpleondemand.o
> +obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)  += governor_performance.o
> +obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)    += governor_powersave.o
> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)    += governor_userspace.o
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> new file mode 100644
> index 0000000..c47eff8
> --- /dev/null
> +++ b/drivers/devfreq/governor_performance.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_performance.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_performance_func(struct devfreq *df,
> +                                   unsigned long *freq)
> +{
> +       *freq = UINT_MAX; /* devfreq_do will run "floor" */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_performance = {
> +       .name = "performance",
> +       .get_target_freq = devfreq_performance_func,
> +};
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> new file mode 100644
> index 0000000..4f128d8
> --- /dev/null
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_powersave.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_powersave_func(struct devfreq *df,
> +                                 unsigned long *freq)
> +{
> +       *freq = 0; /* devfreq_do will run "ceiling" to 0 */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_powersave = {
> +       .name = "powersave",
> +       .get_target_freq = devfreq_powersave_func,
> +};
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> new file mode 100644
> index 0000000..18fe8be
> --- /dev/null
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -0,0 +1,88 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/devfreq.h>
> +#include <linux/math64.h>
> +
> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
> +#define DFSO_UPTHRESHOLD       (90)
> +#define DFSO_DOWNDIFFERENCTIAL (5)
> +static int devfreq_simple_ondemand_func(struct devfreq *df,
> +                                       unsigned long *freq)
> +{
> +       struct devfreq_dev_status stat;
> +       int err = df->profile->get_dev_status(df->dev, &stat);
> +       unsigned long long a, b;
> +       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
> +       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
> +       struct devfreq_simple_ondemand_data *data = df->data;
> +
> +       if (err)
> +               return err;
> +
> +       if (data) {
> +               if (data->upthreshold)
> +                       dfso_upthreshold = data->upthreshold;
> +               if (data->downdifferential)
> +                       dfso_downdifferential = data->downdifferential;
> +       }
> +       if (dfso_upthreshold > 100 ||
> +           dfso_upthreshold < dfso_downdifferential)
> +               return -EINVAL;
> +
> +       /* Assume MAX if it is going to be divided by zero */
> +       if (stat.total_time == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Prevent overflow */
> +       if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
> +               stat.busy_time >>= 7;
> +               stat.total_time >>= 7;
> +       }
> +
> +       /* Set MAX if it's busy enough */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * dfso_upthreshold) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Set MAX if we do not know the initial frequency */
> +       if (stat.current_frequency == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Keep the current frequency */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
> +               *freq = stat.current_frequency;
> +               return 0;
> +       }
> +
> +       /* Set the desired frequency based on the load */
> +       a = stat.busy_time;
> +       a *= stat.current_frequency;
> +       b = div_u64(a, stat.total_time);
> +       b *= 100;
> +       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
> +       *freq = (unsigned long) b;
> +
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_simple_ondemand = {
> +       .name = "simple_ondemand",
> +       .get_target_freq = devfreq_simple_ondemand_func,
> +};
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> new file mode 100644
> index 0000000..53a4574
> --- /dev/null
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -0,0 +1,119 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/device.h>
> +#include <linux/devfreq.h>
> +#include <linux/pm.h>
> +#include "governor.h"
> +
> +struct userspace_data {
> +       unsigned long user_frequency;
> +       bool valid;
> +};
> +
> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
> +{
> +       struct userspace_data *data = df->data;
> +
> +       if (!data->valid)
> +               *freq = df->previous_freq; /* No user freq specified yet */
> +       else
> +               *freq = data->user_frequency;
> +       return 0;
> +}
> +
> +static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
> +                         const char *buf, size_t count)
> +{
> +       struct devfreq *devfreq = get_devfreq(dev);
> +       struct userspace_data *data;
> +       unsigned long wanted;
> +       int err = 0;
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +       data = devfreq->data;
> +
> +       sscanf(buf, "%lu", &wanted);
> +       data->user_frequency = wanted;
> +       data->valid = true;
> +       err = update_devfreq(devfreq);
> +       if (err == 0)
> +               err = count;
> +out:
> +       return err;
> +}
> +
> +static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
> +                        char *buf)
> +{
> +       struct devfreq *devfreq = get_devfreq(dev);
> +       struct userspace_data *data;
> +       int err = 0;
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +       data = devfreq->data;
> +
> +       if (data->valid)
> +               err = sprintf(buf, "%lu\n", data->user_frequency);
> +       else
> +               err = sprintf(buf, "undefined\n");
> +out:
> +       return err;
> +}

Shouldn't accesses to devfreq->data be protected by a mutex?

Regards,
Mike

> +
> +static DEVICE_ATTR(devfreq_userspace_set_freq, 0644, show_freq, store_freq);
> +static struct attribute *dev_entries[] = {
> +       &dev_attr_devfreq_userspace_set_freq.attr,
> +       NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> +       .name   = power_group_name,
> +       .attrs  = dev_entries,
> +};
> +
> +static int userspace_init(struct devfreq *devfreq)
> +{
> +       int err = 0;
> +       struct userspace_data *data = kzalloc(sizeof(struct userspace_data),
> +                                             GFP_KERNEL);
> +
> +       if (!data) {
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +       data->valid = false;
> +       devfreq->data = data;
> +
> +       sysfs_merge_group(&devfreq->dev->kobj, &dev_attr_group);
> +out:
> +       return err;
> +}
> +
> +static void userspace_exit(struct devfreq *devfreq)
> +{
> +       sysfs_unmerge_group(&devfreq->dev->kobj, &dev_attr_group);
> +       kfree(devfreq->data);
> +       devfreq->data = NULL;
> +}
> +
> +struct devfreq_governor devfreq_userspace = {
> +       .name = "userspace",
> +       .get_target_freq = devfreq_userspace_func,
> +       .init = userspace_init,
> +       .exit = userspace_exit,
> +};
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index fdc6916..cbafcdf 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -13,6 +13,7 @@
>  #ifndef __LINUX_DEVFREQ_H__
>  #define __LINUX_DEVFREQ_H__
>
> +#include <linux/opp.h>
>  #include <linux/notifier.h>
>
>  #define DEVFREQ_NAME_LEN 16
> @@ -65,6 +66,8 @@ struct devfreq_governor {
>  *                     "devfreq_monitor" executions to reevaluate
>  *                     frequency/voltage of the device. Set by
>  *                     profile's polling_ms interval.
> + * @user_set_freq      User specified adequete frequency value (thru sysfs
> + *             interface). Governors may and may not use this value.
>  * @data       Private data of the governor. The devfreq framework does not
>  *             touch this.
>  *
> @@ -82,6 +85,7 @@ struct devfreq {
>        unsigned long previous_freq;
>        unsigned int next_polling;
>
> +       unsigned long user_set_freq; /* governors may ignore this. */
>        void *data; /* private data for governors */
>  };
>
> @@ -91,6 +95,37 @@ extern int devfreq_add_device(struct device *dev,
>                           struct devfreq_governor *governor,
>                           void *data);
>  extern int devfreq_remove_device(struct device *dev);
> +
> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> +extern struct devfreq_governor devfreq_powersave;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
> +extern struct devfreq_governor devfreq_performance;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
> +extern struct devfreq_governor devfreq_userspace;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
> +extern struct devfreq_governor devfreq_simple_ondemand;
> +/**
> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
> + *     and devfreq_add_device
> + * @ upthreshold       If the load is over this value, the frequency jumps.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + * @ downdifferential  If the load is under upthreshold - downdifferential,
> + *                     the governor may consider slowing the frequency down.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + *                     downdifferential < upthreshold must hold.
> + *
> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
> + * the governor uses the default values.
> + */
> +struct devfreq_simple_ondemand_data {
> +       unsigned int upthreshold;
> +       unsigned int downdifferential;
> +};
> +#endif
> +
>  #else /* !CONFIG_PM_DEVFREQ */
>  static int devfreq_add_device(struct device *dev,
>                           struct devfreq_dev_profile *profile,
> @@ -104,6 +139,12 @@ static int devfreq_remove_device(struct device *dev)
>  {
>        return 0;
>  }
> +
> +#define devfreq_powersave      NULL
> +#define devfreq_performance    NULL
> +#define devfreq_userspace      NULL
> +#define devfreq_simple_ondemand        NULL
> +
>  #endif /* CONFIG_PM_DEVFREQ */
>
>  #endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

Re: [PATCH v8 5/5] PM / DEVFREQ: add basic governors

[MyungJoo Ham]

On Tue, Aug 30, 2011 at 3:58 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> +What:          /sys/devices/.../power/devfreq_userspace_set_freq
>
> How about just .../devfreq_set_freq?  I think the userspace bit is
> implied and the name is very long.
>

Umm.. as this entry became userspace specific, I though I need it be
to distinguished from entries created by devfreq framework. However,
this one is created only while userspace is being used (device may
replace governors by calling remove and add); thus, there wouldn't be
any duplicated name issues. So, I think removing userspace from the
name should be fine. I will do so in the next revision.

>> +
>> +static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
>> +                        char *buf)
>> +{
>> +       struct devfreq *devfreq = get_devfreq(dev);
>> +       struct userspace_data *data;
>> +       int err = 0;
>> +
>> +       if (IS_ERR(devfreq)) {
>> +               err = PTR_ERR(devfreq);
>> +               goto out;
>> +       }
>> +       data = devfreq->data;
>> +
>> +       if (data->valid)
>> +               err = sprintf(buf, "%lu\n", data->user_frequency);
>> +       else
>> +               err = sprintf(buf, "undefined\n");
>> +out:
>> +       return err;
>> +}
>
> Shouldn't accesses to devfreq->data be protected by a mutex?
>
> Regards,
> Mike


No, it doesn't need mutex here. Although generally, a mutex will be
needed for simliar codes, in this specific case, devfreq->data does
not need a mutex protection.

There are two variables in data: "user_frequency" and "valid"
- valid is initially false and becomes true at store_freq() and never changes.
- store_freq() writes user_frequency and then writes valid as true.
(no one writes false on valid)
- user_frequency is read only when valid is true.

Thus, the case where mutex protection serializes with some dictinction is:
1. Init. (valid = false)
2. store_freq() writes user_frequency = X
3. show_freq()/devfreq_userspace_func() reads valid (false)
4. store_freq() writes valid = true
5. show_freq()/devfreq_userspace_func() returns without reading
devfreq_userspace_func()
6. store_freq() returns.

into
A.
 1. Init (valid = false)
 2. store_freq() starts and finishes
 3. show_freq()/devfreq_userspace_func() starts and finishes
B.
 1. Init (valid = false)
 2. show_freq()/devfreq_userspace_func() starts and finishes
 3. store_freq() starts and finishes

where B results in the same thing as non-mutex version does.


If there is an operation that makes valid false, we will need a mutex
(local to the governor) anyway.



Anyway, I agree with you on the point that governors might need a
locking mechanism in general; not just on the private data, but on the
devfreq access. I'll put more on this issue on the other thread.



Thank you.


Cheers,
MyungJoo


>
>> +
>> +static DEVICE_ATTR(devfreq_userspace_set_freq, 0644, show_freq, store_freq);
>> +static struct attribute *dev_entries[] = {
>> +       &dev_attr_devfreq_userspace_set_freq.attr,
>> +       NULL,
>> +};
>> +static struct attribute_group dev_attr_group = {
>> +       .name   = power_group_name,
>> +       .attrs  = dev_entries,
>> +};
>> +
>> +static int userspace_init(struct devfreq *devfreq)
>> +{
>> +       int err = 0;
>> +       struct userspace_data *data = kzalloc(sizeof(struct userspace_data),
>> +                                             GFP_KERNEL);
>> +
>> +       if (!data) {
>> +               err = -ENOMEM;
>> +               goto out;
>> +       }
>> +       data->valid = false;
>> +       devfreq->data = data;
>> +
>> +       sysfs_merge_group(&devfreq->dev->kobj, &dev_attr_group);
>> +out:
>> +       return err;
>> +}
>> +
>> +static void userspace_exit(struct devfreq *devfreq)
>> +{
>> +       sysfs_unmerge_group(&devfreq->dev->kobj, &dev_attr_group);
>> +       kfree(devfreq->data);
>> +       devfreq->data = NULL;
>> +}
>> +
>> +struct devfreq_governor devfreq_userspace = {
>> +       .name = "userspace",
>> +       .get_target_freq = devfreq_userspace_func,
>> +       .init = userspace_init,
>> +       .exit = userspace_exit,
>> +};
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index fdc6916..cbafcdf 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -13,6 +13,7 @@
>>  #ifndef __LINUX_DEVFREQ_H__
>>  #define __LINUX_DEVFREQ_H__
>>
>> +#include <linux/opp.h>
>>  #include <linux/notifier.h>
>>
>>  #define DEVFREQ_NAME_LEN 16
>> @@ -65,6 +66,8 @@ struct devfreq_governor {
>>  *                     "devfreq_monitor" executions to reevaluate
>>  *                     frequency/voltage of the device. Set by
>>  *                     profile's polling_ms interval.
>> + * @user_set_freq      User specified adequete frequency value (thru sysfs
>> + *             interface). Governors may and may not use this value.
>>  * @data       Private data of the governor. The devfreq framework does not
>>  *             touch this.
>>  *
>> @@ -82,6 +85,7 @@ struct devfreq {
>>        unsigned long previous_freq;
>>        unsigned int next_polling;
>>
>> +       unsigned long user_set_freq; /* governors may ignore this. */
>>        void *data; /* private data for governors */
>>  };
>>
>> @@ -91,6 +95,37 @@ extern int devfreq_add_device(struct device *dev,
>>                           struct devfreq_governor *governor,
>>                           void *data);
>>  extern int devfreq_remove_device(struct device *dev);
>> +
>> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
>> +extern struct devfreq_governor devfreq_powersave;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
>> +extern struct devfreq_governor devfreq_performance;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
>> +extern struct devfreq_governor devfreq_userspace;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +extern struct devfreq_governor devfreq_simple_ondemand;
>> +/**
>> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
>> + *     and devfreq_add_device
>> + * @ upthreshold       If the load is over this value, the frequency jumps.
>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>> + * @ downdifferential  If the load is under upthreshold - downdifferential,
>> + *                     the governor may consider slowing the frequency down.
>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>> + *                     downdifferential < upthreshold must hold.
>> + *
>> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
>> + * the governor uses the default values.
>> + */
>> +struct devfreq_simple_ondemand_data {
>> +       unsigned int upthreshold;
>> +       unsigned int downdifferential;
>> +};
>> +#endif
>> +
>>  #else /* !CONFIG_PM_DEVFREQ */
>>  static int devfreq_add_device(struct device *dev,
>>                           struct devfreq_dev_profile *profile,
>> @@ -104,6 +139,12 @@ static int devfreq_remove_device(struct device *dev)
>>  {
>>        return 0;
>>  }
>> +
>> +#define devfreq_powersave      NULL
>> +#define devfreq_performance    NULL
>> +#define devfreq_userspace      NULL
>> +#define devfreq_simple_ondemand        NULL
>> +
>>  #endif /* CONFIG_PM_DEVFREQ */
>>
>>  #endif /* __LINUX_DEVFREQ_H__ */
>> --
>> 1.7.4.1
>>
>>
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH v8 5/5] PM / DEVFREQ: add basic governors

[Turquette, Mike]

On Mon, Aug 29, 2011 at 9:19 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Tue, Aug 30, 2011 at 3:58 AM, Turquette, Mike <mturquette@ti.com> wrote:
>> On Wed, Aug 24, 2011 at 1:22 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>>> +What:          /sys/devices/.../power/devfreq_userspace_set_freq
>>
>> How about just .../devfreq_set_freq?  I think the userspace bit is
>> implied and the name is very long.
>>
>
> Umm.. as this entry became userspace specific, I though I need it be
> to distinguished from entries created by devfreq framework. However,
> this one is created only while userspace is being used (device may
> replace governors by calling remove and add); thus, there wouldn't be
> any duplicated name issues. So, I think removing userspace from the
> name should be fine. I will do so in the next revision.

It is your choice.  You have a good point that it is specific to the
"userspace" governor.  I hadn't thought of that at the time, so I
don't care either way.

Regards,
Mike

>>> +
>>> +static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
>>> +                        char *buf)
>>> +{
>>> +       struct devfreq *devfreq = get_devfreq(dev);
>>> +       struct userspace_data *data;
>>> +       int err = 0;
>>> +
>>> +       if (IS_ERR(devfreq)) {
>>> +               err = PTR_ERR(devfreq);
>>> +               goto out;
>>> +       }
>>> +       data = devfreq->data;
>>> +
>>> +       if (data->valid)
>>> +               err = sprintf(buf, "%lu\n", data->user_frequency);
>>> +       else
>>> +               err = sprintf(buf, "undefined\n");
>>> +out:
>>> +       return err;
>>> +}
>>
>> Shouldn't accesses to devfreq->data be protected by a mutex?
>>
>> Regards,
>> Mike
>
>
> No, it doesn't need mutex here. Although generally, a mutex will be
> needed for simliar codes, in this specific case, devfreq->data does
> not need a mutex protection.
>
> There are two variables in data: "user_frequency" and "valid"
> - valid is initially false and becomes true at store_freq() and never changes.
> - store_freq() writes user_frequency and then writes valid as true.
> (no one writes false on valid)
> - user_frequency is read only when valid is true.
>
> Thus, the case where mutex protection serializes with some dictinction is:
> 1. Init. (valid = false)
> 2. store_freq() writes user_frequency = X
> 3. show_freq()/devfreq_userspace_func() reads valid (false)
> 4. store_freq() writes valid = true
> 5. show_freq()/devfreq_userspace_func() returns without reading
> devfreq_userspace_func()
> 6. store_freq() returns.
>
> into
> A.
>  1. Init (valid = false)
>  2. store_freq() starts and finishes
>  3. show_freq()/devfreq_userspace_func() starts and finishes
> B.
>  1. Init (valid = false)
>  2. show_freq()/devfreq_userspace_func() starts and finishes
>  3. store_freq() starts and finishes
>
> where B results in the same thing as non-mutex version does.
>
>
> If there is an operation that makes valid false, we will need a mutex
> (local to the governor) anyway.
>
>
>
> Anyway, I agree with you on the point that governors might need a
> locking mechanism in general; not just on the private data, but on the
> devfreq access. I'll put more on this issue on the other thread.
>
>
>
> Thank you.
>
>
> Cheers,
> MyungJoo
>
>
>>
>>> +
>>> +static DEVICE_ATTR(devfreq_userspace_set_freq, 0644, show_freq, store_freq);
>>> +static struct attribute *dev_entries[] = {
>>> +       &dev_attr_devfreq_userspace_set_freq.attr,
>>> +       NULL,
>>> +};
>>> +static struct attribute_group dev_attr_group = {
>>> +       .name   = power_group_name,
>>> +       .attrs  = dev_entries,
>>> +};
>>> +
>>> +static int userspace_init(struct devfreq *devfreq)
>>> +{
>>> +       int err = 0;
>>> +       struct userspace_data *data = kzalloc(sizeof(struct userspace_data),
>>> +                                             GFP_KERNEL);
>>> +
>>> +       if (!data) {
>>> +               err = -ENOMEM;
>>> +               goto out;
>>> +       }
>>> +       data->valid = false;
>>> +       devfreq->data = data;
>>> +
>>> +       sysfs_merge_group(&devfreq->dev->kobj, &dev_attr_group);
>>> +out:
>>> +       return err;
>>> +}
>>> +
>>> +static void userspace_exit(struct devfreq *devfreq)
>>> +{
>>> +       sysfs_unmerge_group(&devfreq->dev->kobj, &dev_attr_group);
>>> +       kfree(devfreq->data);
>>> +       devfreq->data = NULL;
>>> +}
>>> +
>>> +struct devfreq_governor devfreq_userspace = {
>>> +       .name = "userspace",
>>> +       .get_target_freq = devfreq_userspace_func,
>>> +       .init = userspace_init,
>>> +       .exit = userspace_exit,
>>> +};
>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>> index fdc6916..cbafcdf 100644
>>> --- a/include/linux/devfreq.h
>>> +++ b/include/linux/devfreq.h
>>> @@ -13,6 +13,7 @@
>>>  #ifndef __LINUX_DEVFREQ_H__
>>>  #define __LINUX_DEVFREQ_H__
>>>
>>> +#include <linux/opp.h>
>>>  #include <linux/notifier.h>
>>>
>>>  #define DEVFREQ_NAME_LEN 16
>>> @@ -65,6 +66,8 @@ struct devfreq_governor {
>>>  *                     "devfreq_monitor" executions to reevaluate
>>>  *                     frequency/voltage of the device. Set by
>>>  *                     profile's polling_ms interval.
>>> + * @user_set_freq      User specified adequete frequency value (thru sysfs
>>> + *             interface). Governors may and may not use this value.
>>>  * @data       Private data of the governor. The devfreq framework does not
>>>  *             touch this.
>>>  *
>>> @@ -82,6 +85,7 @@ struct devfreq {
>>>        unsigned long previous_freq;
>>>        unsigned int next_polling;
>>>
>>> +       unsigned long user_set_freq; /* governors may ignore this. */
>>>        void *data; /* private data for governors */
>>>  };
>>>
>>> @@ -91,6 +95,37 @@ extern int devfreq_add_device(struct device *dev,
>>>                           struct devfreq_governor *governor,
>>>                           void *data);
>>>  extern int devfreq_remove_device(struct device *dev);
>>> +
>>> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
>>> +extern struct devfreq_governor devfreq_powersave;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
>>> +extern struct devfreq_governor devfreq_performance;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
>>> +extern struct devfreq_governor devfreq_userspace;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
>>> +extern struct devfreq_governor devfreq_simple_ondemand;
>>> +/**
>>> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
>>> + *     and devfreq_add_device
>>> + * @ upthreshold       If the load is over this value, the frequency jumps.
>>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>>> + * @ downdifferential  If the load is under upthreshold - downdifferential,
>>> + *                     the governor may consider slowing the frequency down.
>>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>>> + *                     downdifferential < upthreshold must hold.
>>> + *
>>> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
>>> + * the governor uses the default values.
>>> + */
>>> +struct devfreq_simple_ondemand_data {
>>> +       unsigned int upthreshold;
>>> +       unsigned int downdifferential;
>>> +};
>>> +#endif
>>> +
>>>  #else /* !CONFIG_PM_DEVFREQ */
>>>  static int devfreq_add_device(struct device *dev,
>>>                           struct devfreq_dev_profile *profile,
>>> @@ -104,6 +139,12 @@ static int devfreq_remove_device(struct device *dev)
>>>  {
>>>        return 0;
>>>  }
>>> +
>>> +#define devfreq_powersave      NULL
>>> +#define devfreq_performance    NULL
>>> +#define devfreq_userspace      NULL
>>> +#define devfreq_simple_ondemand        NULL
>>> +
>>>  #endif /* CONFIG_PM_DEVFREQ */
>>>
>>>  #endif /* __LINUX_DEVFREQ_H__ */
>>> --
>>> 1.7.4.1
>>>
>>>
>>
>
>
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[Rafael J. Wysocki]

Mike, are patches [3-5/5] in this revision fine by you?

Rafael


On Wednesday, August 24, 2011, MyungJoo Ham wrote:
> The patchset revision v8 has minor updates since v7 and v6.
> - Allow governors to have their own sysfs interface and init/exit callbacks.
> 
> The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
> There has been reordering between "add common sysfs interfaces" patch
> and "add basic governors" (3/5 and 5/5)
> "add internal interfaces for governors (4/5)" patch has been newly
> introduced at v8 patchset.
> 
> For a usage example, please look at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
> 
> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
> and other related clocks simply follow the determined DDR RAM clock.
> 
> The DEVFREQ driver for Exynos4210 memory bus is at
> /drivers/devfreq/exynos4210_memorybus.c in the git tree.
> 
> In the dd (writing and reading 360MiB) test with NURI board, the memory
> throughput was not changed (the performance is not deteriorated) while
> the SoC power consumption has been reduced by 1%. When the memory access
> is not that intense while the CPU is heavily used, the SoC power consumption
> has been reduced by 6%. The power consumption has been compared with the
> case using the conventional Exynos4210 CPUFREQ driver, which sets memory
> bus frequency according to the CPU core frequency. Besides, when the CPU core
> running slow and the memory access is intense, the performance (memory
> throughput) has been increased by 11% (with higher SoC power consumption of
> 5%). The tested governor is "simple-ondemand".
> 
> MyungJoo Ham (5):
>   PM / OPP: Add OPP availability change notifier.
>   PM: Introduce DEVFREQ: generic DVFS framework with device-specific
>     OPPs
>   PM / DEVFREQ: add common sysfs interfaces
>   PM / DEVFREQ: add internal interfaces for governors
>   PM / DEVFREQ: add basic governors
> 
>  Documentation/ABI/testing/sysfs-devices-power |   46 +++
>  drivers/Kconfig                               |    2 +
>  drivers/Makefile                              |    2 +
>  drivers/base/power/opp.c                      |   29 ++
>  drivers/devfreq/Kconfig                       |   75 ++++
>  drivers/devfreq/Makefile                      |    5 +
>  drivers/devfreq/devfreq.c                     |  463 +++++++++++++++++++++++++
>  drivers/devfreq/governor.h                    |   20 +
>  drivers/devfreq/governor_performance.c        |   24 ++
>  drivers/devfreq/governor_powersave.c          |   24 ++
>  drivers/devfreq/governor_simpleondemand.c     |   88 +++++
>  drivers/devfreq/governor_userspace.c          |  119 +++++++
>  include/linux/devfreq.h                       |  150 ++++++++
>  include/linux/opp.h                           |   12 +
>  14 files changed, 1059 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/Kconfig
>  create mode 100644 drivers/devfreq/Makefile
>  create mode 100644 drivers/devfreq/devfreq.c
>  create mode 100644 drivers/devfreq/governor.h
>  create mode 100644 drivers/devfreq/governor_performance.c
>  create mode 100644 drivers/devfreq/governor_powersave.c
>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>  create mode 100644 drivers/devfreq/governor_userspace.c
>  create mode 100644 include/linux/devfreq.h
> 
> 

Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[Turquette, Mike]

Rafael,

Locking doesn't seem safe in v8; I replied with my comments.

I should be free this week to review the next round a little more
quickly.  Last week was hectic.

Regards,
Mike

2011/8/27 Rafael J. Wysocki <rjw@sisk.pl>:
> Mike, are patches [3-5/5] in this revision fine by you?
>
> Rafael
>
>
> On Wednesday, August 24, 2011, MyungJoo Ham wrote:
>> The patchset revision v8 has minor updates since v7 and v6.
>> - Allow governors to have their own sysfs interface and init/exit callbacks.
>>
>> The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
>> There has been reordering between "add common sysfs interfaces" patch
>> and "add basic governors" (3/5 and 5/5)
>> "add internal interfaces for governors (4/5)" patch has been newly
>> introduced at v8 patchset.
>>
>> For a usage example, please look at
>> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>>
>> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
>> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
>> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
>> and other related clocks simply follow the determined DDR RAM clock.
>>
>> The DEVFREQ driver for Exynos4210 memory bus is at
>> /drivers/devfreq/exynos4210_memorybus.c in the git tree.
>>
>> In the dd (writing and reading 360MiB) test with NURI board, the memory
>> throughput was not changed (the performance is not deteriorated) while
>> the SoC power consumption has been reduced by 1%. When the memory access
>> is not that intense while the CPU is heavily used, the SoC power consumption
>> has been reduced by 6%. The power consumption has been compared with the
>> case using the conventional Exynos4210 CPUFREQ driver, which sets memory
>> bus frequency according to the CPU core frequency. Besides, when the CPU core
>> running slow and the memory access is intense, the performance (memory
>> throughput) has been increased by 11% (with higher SoC power consumption of
>> 5%). The tested governor is "simple-ondemand".
>>
>> MyungJoo Ham (5):
>>   PM / OPP: Add OPP availability change notifier.
>>   PM: Introduce DEVFREQ: generic DVFS framework with device-specific
>>     OPPs
>>   PM / DEVFREQ: add common sysfs interfaces
>>   PM / DEVFREQ: add internal interfaces for governors
>>   PM / DEVFREQ: add basic governors
>>
>>  Documentation/ABI/testing/sysfs-devices-power |   46 +++
>>  drivers/Kconfig                               |    2 +
>>  drivers/Makefile                              |    2 +
>>  drivers/base/power/opp.c                      |   29 ++
>>  drivers/devfreq/Kconfig                       |   75 ++++
>>  drivers/devfreq/Makefile                      |    5 +
>>  drivers/devfreq/devfreq.c                     |  463 +++++++++++++++++++++++++
>>  drivers/devfreq/governor.h                    |   20 +
>>  drivers/devfreq/governor_performance.c        |   24 ++
>>  drivers/devfreq/governor_powersave.c          |   24 ++
>>  drivers/devfreq/governor_simpleondemand.c     |   88 +++++
>>  drivers/devfreq/governor_userspace.c          |  119 +++++++
>>  include/linux/devfreq.h                       |  150 ++++++++
>>  include/linux/opp.h                           |   12 +
>>  14 files changed, 1059 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/devfreq/Kconfig
>>  create mode 100644 drivers/devfreq/Makefile
>>  create mode 100644 drivers/devfreq/devfreq.c
>>  create mode 100644 drivers/devfreq/governor.h
>>  create mode 100644 drivers/devfreq/governor_performance.c
>>  create mode 100644 drivers/devfreq/governor_powersave.c
>>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>>  create mode 100644 drivers/devfreq/governor_userspace.c
>>  create mode 100644 include/linux/devfreq.h
>>
>>
>
>

Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[Rafael J. Wysocki]

Hi,

On Monday, August 29, 2011, Turquette, Mike wrote:
> Rafael,
> 
> Locking doesn't seem safe in v8; I replied with my comments.
> 
> I should be free this week to review the next round a little more
> quickly.

Cool, thanks a lot for your hard work as a reviewer.

> Last week was hectic.

Sorry about that.

Thanks,
Rafael

Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[Kevin Hilman]

MyungJoo Ham <myungjoo.ham@samsung.com> writes:

> The patchset revision v8 has minor updates since v7 and v6.
> - Allow governors to have their own sysfs interface and init/exit callbacks.
>
> The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
> There has been reordering between "add common sysfs interfaces" patch
> and "add basic governors" (3/5 and 5/5)
> "add internal interfaces for governors (4/5)" patch has been newly
> introduced at v8 patchset.
>
> For a usage example, please look at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>
> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
> and other related clocks simply follow the determined DDR RAM clock.
>
> The DEVFREQ driver for Exynos4210 memory bus is at
> /drivers/devfreq/exynos4210_memorybus.c in the git tree.

Minor nit: you continue to use DEVFREQ (all caps) throughout subjects
and changelogs etc. when it should be called devfreq since it's not an
acryonym.

Kevin

Re: [PATCH v8 0/5] DEVFREQ, DVFS Framework for Non-CPU Devices.

[MyungJoo Ham]

On Wed, Aug 31, 2011 at 8:32 AM, Kevin Hilman <khilman@ti.com> wrote:
> MyungJoo Ham <myungjoo.ham@samsung.com> writes:
>
>> The patchset revision v8 has minor updates since v7 and v6.
>> - Allow governors to have their own sysfs interface and init/exit callbacks.
>>
>> The patches 1/5 (OPP notifier) and 2/5 (DEVFREQ core) have no changes since v7.
>> There has been reordering between "add common sysfs interfaces" patch
>> and "add basic governors" (3/5 and 5/5)
>> "add internal interfaces for governors (4/5)" patch has been newly
>> introduced at v8 patchset.
>>
>> For a usage example, please look at
>> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>>
>> In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
>> is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
>> In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
>> and other related clocks simply follow the determined DDR RAM clock.
>>
>> The DEVFREQ driver for Exynos4210 memory bus is at
>> /drivers/devfreq/exynos4210_memorybus.c in the git tree.
>
> Minor nit: you continue to use DEVFREQ (all caps) throughout subjects
> and changelogs etc. when it should be called devfreq since it's not an
> acryonym.
>
> Kevin
>

Fine, I'll regard devfreq as a common noun.


Cheers,
MyungJoo

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm