[PATCH v4 0/3] devfreq: Add support for devices which can idle

[PATCH v4 0/3] devfreq: Add support for devices which can idle

[Rajagopal Venkat]

This patchset updates devfreq core to add support for devices
which can idle. When device idleness is detected perhaps
through runtime-pm, need some mechanism to suspend devfreq
load monitoring and resume when device is back online.

patch 1 introduce core design changes - per device work, decouple
delayed work from core and event based interaction.
patch 2 add devfreq suspend and resume apis.
patch 3 add new sysfs attribute for governor predicted next target
frequency and callback for current device frequency.

The existing devfreq apis are kept intact. Two new apis
devfreq_suspend_device() and devfreq_resume_device() are
added to support suspend/resume of device devfreq.

Changes since v1:
- revised locking mechanism
- added kerneldoc comments for load monitoring helper functions
- fixed minor review comments

Changes since v2:
- added new helper function for polling interval update
- handled work suspend/resume contention between devfreq driver
and sysfs

Changes since v3:
- added additonal checks in suspend/resume to avoid invalid usage of apis
- added check in devfreq_monitor_start, not to start monitoring when
polling_ms is set to zero.

--
Rajagopal Venkat (3):
  devfreq: Core updates to support devices which can idle
  devfreq: Add suspend and resume apis
  devfreq: Add current freq callback in device profile

 Documentation/ABI/testing/sysfs-class-devfreq |  15 +-
 drivers/devfreq/devfreq.c                     | 481 ++++++++++++--------------
 drivers/devfreq/governor.h                    |  13 +
 drivers/devfreq/governor_performance.c        |  16 +-
 drivers/devfreq/governor_powersave.c          |  16 +-
 drivers/devfreq/governor_simpleondemand.c     |  33 ++
 drivers/devfreq/governor_userspace.c          |  23 +-
 include/linux/devfreq.h                       |  49 +--
 8 files changed, 353 insertions(+), 293 deletions(-)

-- 
1.7.11.3

[PATCH v4 1/3] devfreq: Core updates to support devices which can idle

[Rajagopal Venkat]

Prepare devfreq core framework to support devices which
can idle. When device idleness is detected perhaps through
runtime-pm, need some mechanism to suspend devfreq load
monitoring and resume back when device is online. Present
code continues monitoring unless device is removed from
devfreq core.

This patch introduces following design changes,

- use per device work instead of global work to monitor device
  load. This enables suspend/resume of device devfreq and
  reduces monitoring code complexity.
- decouple delayed work based load monitoring logic from core
  by introducing helpers functions to be used by governors. This
  provides flexibility for governors either to use delayed work
  based monitoring functions or to implement their own mechanism.
- devfreq core interacts with governors via events to perform
  specific actions. These events include start/stop devfreq.
  This sets ground for adding suspend/resume events.

The devfreq apis are not modified and are kept intact.

Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
---
 Documentation/ABI/testing/sysfs-class-devfreq |   8 -
 drivers/devfreq/devfreq.c                     | 443 +++++++++++---------------
 drivers/devfreq/governor.h                    |  11 +
 drivers/devfreq/governor_performance.c        |  16 +-
 drivers/devfreq/governor_powersave.c          |  16 +-
 drivers/devfreq/governor_simpleondemand.c     |  24 ++
 drivers/devfreq/governor_userspace.c          |  23 +-
 include/linux/devfreq.h                       |  34 +-
 8 files changed, 279 insertions(+), 296 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index 23d78b5..89283b1 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -21,14 +21,6 @@ Description:
 		The /sys/class/devfreq/.../cur_freq shows the current
 		frequency of the corresponding devfreq object.
 
-What:		/sys/class/devfreq/.../central_polling
-Date:		September 2011
-Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
-Description:
-		The /sys/class/devfreq/.../central_polling shows whether
-		the devfreq ojbect is using devfreq-provided central
-		polling mechanism or not.
-
 What:		/sys/class/devfreq/.../polling_interval
 Date:		September 2011
 Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b146d76..41a4099 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -30,17 +30,11 @@
 struct class *devfreq_class;
 
 /*
- * 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.
+ * devfreq core provides delayed work based load monitoring helper
+ * functions. Governors can use these or can implement their own
+ * monitoring mechanism.
  */
-static bool polling;
 static struct workqueue_struct *devfreq_wq;
-static struct delayed_work devfreq_work;
-
-/* wait removing if this is to be removed */
-static struct devfreq *wait_remove_device;
 
 /* The list of all device-devfreq */
 static LIST_HEAD(devfreq_list);
@@ -72,6 +66,8 @@ static struct devfreq *find_device_devfreq(struct device *dev)
 	return ERR_PTR(-ENODEV);
 }
 
+/* Load monitoring helper functions for governors use */
+
 /**
  * update_devfreq() - Reevaluate the device and configure frequency.
  * @devfreq:	the devfreq instance.
@@ -121,6 +117,152 @@ int update_devfreq(struct devfreq *devfreq)
 }
 
 /**
+ * devfreq_monitor() - Periodically poll devfreq objects.
+ * @work:	the work struct used to run devfreq_monitor periodically.
+ *
+ */
+static void devfreq_monitor(struct work_struct *work)
+{
+	int err;
+	struct devfreq *devfreq = container_of(work,
+					struct devfreq, work.work);
+
+	mutex_lock(&devfreq->lock);
+	err = update_devfreq(devfreq);
+	if (err)
+		dev_err(&devfreq->dev, "dvfs failed with (%d) error\n", err);
+
+	queue_delayed_work(devfreq_wq, &devfreq->work,
+				msecs_to_jiffies(devfreq->profile->polling_ms));
+	mutex_unlock(&devfreq->lock);
+}
+
+/**
+ * devfreq_monitor_start() - Start load monitoring of devfreq instance
+ * @devfreq:	the devfreq instance.
+ *
+ * Helper function for starting devfreq device load monitoing. By
+ * default delayed work based monitoring is supported. Function
+ * to be called from governor in response to DEVFREQ_GOV_START
+ * event when device is added to devfreq framework.
+ */
+void devfreq_monitor_start(struct devfreq *devfreq)
+{
+	INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);
+	if (devfreq->profile->polling_ms)
+		queue_delayed_work(devfreq_wq, &devfreq->work,
+			msecs_to_jiffies(devfreq->profile->polling_ms));
+}
+
+/**
+ * devfreq_monitor_stop() - Stop load monitoring of a devfreq instance
+ * @devfreq:	the devfreq instance.
+ *
+ * Helper function to stop devfreq device load monitoing. Function
+ * to be called from governor in response to DEVFREQ_GOV_STOP
+ * event when device is removed from devfreq framework.
+ */
+void devfreq_monitor_stop(struct devfreq *devfreq)
+{
+	cancel_delayed_work_sync(&devfreq->work);
+}
+
+/**
+ * devfreq_monitor_suspend() - Suspend load monitoring of a devfreq instance
+ * @devfreq:	the devfreq instance.
+ *
+ * Helper function to suspend devfreq device load monitoing. Function
+ * to be called from governor in response to DEVFREQ_GOV_SUSPEND
+ * event or when polling interval is set to zero.
+ *
+ * Note: Though this function is same as devfreq_monitor_stop(),
+ * intentionally kept separate to provide hooks for collecting
+ * transition statistics.
+ */
+void devfreq_monitor_suspend(struct devfreq *devfreq)
+{
+	mutex_lock(&devfreq->lock);
+	if (devfreq->stop_polling) {
+		mutex_unlock(&devfreq->lock);
+		return;
+	}
+
+	devfreq->stop_polling = true;
+	mutex_unlock(&devfreq->lock);
+	cancel_delayed_work_sync(&devfreq->work);
+}
+
+/**
+ * devfreq_monitor_resume() - Resume load monitoring of a devfreq instance
+ * @devfreq:    the devfreq instance.
+ *
+ * Helper function to resume devfreq device load monitoing. Function
+ * to be called from governor in response to DEVFREQ_GOV_RESUME
+ * event or when polling interval is set to non-zero.
+ */
+void devfreq_monitor_resume(struct devfreq *devfreq)
+{
+	mutex_lock(&devfreq->lock);
+	if (!devfreq->stop_polling)
+		goto out;
+
+	if (!delayed_work_pending(&devfreq->work) &&
+			devfreq->profile->polling_ms)
+		queue_delayed_work(devfreq_wq, &devfreq->work,
+			msecs_to_jiffies(devfreq->profile->polling_ms));
+	devfreq->stop_polling = false;
+
+out:
+	mutex_unlock(&devfreq->lock);
+}
+
+/**
+ * devfreq_interval_update() - Update device devfreq monitoring interval
+ * @devfreq:    the devfreq instance.
+ * @delay:      new polling interval to be set.
+ *
+ * Helper function to set new load monitoring polling interval. Function
+ * to be called from governor in response to DEVFREQ_GOV_INTERVAL event.
+ */
+void devfreq_interval_update(struct devfreq *devfreq, unsigned int *delay)
+{
+	unsigned int cur_delay = devfreq->profile->polling_ms;
+	unsigned int new_delay = *delay;
+
+	mutex_lock(&devfreq->lock);
+	devfreq->profile->polling_ms = new_delay;
+
+	if (devfreq->stop_polling)
+		goto out;
+
+	/* if new delay is zero, stop polling */
+	if (!new_delay) {
+		mutex_unlock(&devfreq->lock);
+		cancel_delayed_work_sync(&devfreq->work);
+		return;
+	}
+
+	/* if current delay is zero, start polling with new delay */
+	if (!cur_delay) {
+		queue_delayed_work(devfreq_wq, &devfreq->work,
+			msecs_to_jiffies(devfreq->profile->polling_ms));
+		goto out;
+	}
+
+	/* if current delay is greater than new delay, restart polling */
+	if (cur_delay > new_delay) {
+		mutex_unlock(&devfreq->lock);
+		cancel_delayed_work_sync(&devfreq->work);
+		mutex_lock(&devfreq->lock);
+		if (!devfreq->stop_polling)
+			queue_delayed_work(devfreq_wq, &devfreq->work,
+			      msecs_to_jiffies(devfreq->profile->polling_ms));
+	}
+out:
+	mutex_unlock(&devfreq->lock);
+}
+
+/**
  * devfreq_notifier_call() - Notify that the device frequency requirements
  *			   has been changed out of devfreq framework.
  * @nb		the notifier_block (supposed to be devfreq->nb)
@@ -143,59 +285,33 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 }
 
 /**
- * _remove_devfreq() - Remove devfreq from the device.
+ * _remove_devfreq() - Remove devfreq from the devfreq list and
+		release its resources.
  * @devfreq:	the devfreq struct
  * @skip:	skip calling device_unregister().
- *
- * Note that the caller should lock devfreq->lock before calling
- * this. _remove_devfreq() will unlock it and free devfreq
- * internally. devfreq_list_lock should be locked by the caller
- * as well (not relased at return)
- *
- * Lock usage:
- * devfreq->lock: locked before call.
- *		  unlocked at return (and freed)
- * devfreq_list_lock: locked before call.
- *		      kept locked at return.
- *		      if devfreq is centrally polled.
- *
- * Freed memory:
- * devfreq
  */
 static void _remove_devfreq(struct devfreq *devfreq, bool skip)
 {
-	if (!mutex_is_locked(&devfreq->lock)) {
-		WARN(true, "devfreq->lock must be locked by the caller.\n");
-		return;
-	}
-	if (!devfreq->governor->no_central_polling &&
-	    !mutex_is_locked(&devfreq_list_lock)) {
-		WARN(true, "devfreq_list_lock must be locked by the caller.\n");
+	mutex_lock(&devfreq_list_lock);
+	if (IS_ERR(find_device_devfreq(devfreq->dev.parent))) {
+		mutex_unlock(&devfreq_list_lock);
+		dev_warn(&devfreq->dev, "releasing devfreq which doesn't exist\n");
 		return;
 	}
+	list_del(&devfreq->node);
+	mutex_unlock(&devfreq_list_lock);
 
-	if (devfreq->being_removed)
-		return;
-
-	devfreq->being_removed = true;
+	devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_STOP, NULL);
 
 	if (devfreq->profile->exit)
 		devfreq->profile->exit(devfreq->dev.parent);
 
-	if (devfreq->governor->exit)
-		devfreq->governor->exit(devfreq);
-
 	if (!skip && get_device(&devfreq->dev)) {
 		device_unregister(&devfreq->dev);
 		put_device(&devfreq->dev);
 	}
 
-	if (!devfreq->governor->no_central_polling)
-		list_del(&devfreq->node);
-
-	mutex_unlock(&devfreq->lock);
 	mutex_destroy(&devfreq->lock);
-
 	kfree(devfreq);
 }
 
@@ -210,130 +326,8 @@ static void _remove_devfreq(struct devfreq *devfreq, bool skip)
 static void devfreq_dev_release(struct device *dev)
 {
 	struct devfreq *devfreq = to_devfreq(dev);
-	bool central_polling = !devfreq->governor->no_central_polling;
-
-	/*
-	 * If devfreq_dev_release() was called by device_unregister() of
-	 * _remove_devfreq(), we cannot mutex_lock(&devfreq->lock) and
-	 * being_removed is already set. This also partially checks the case
-	 * where devfreq_dev_release() is called from a thread other than
-	 * the one called _remove_devfreq(); however, this case is
-	 * dealt completely with another following being_removed check.
-	 *
-	 * Because being_removed is never being
-	 * unset, we do not need to worry about race conditions on
-	 * being_removed.
-	 */
-	if (devfreq->being_removed)
-		return;
 
-	if (central_polling)
-		mutex_lock(&devfreq_list_lock);
-
-	mutex_lock(&devfreq->lock);
-
-	/*
-	 * Check being_removed flag again for the case where
-	 * devfreq_dev_release() was called in a thread other than the one
-	 * possibly called _remove_devfreq().
-	 */
-	if (devfreq->being_removed) {
-		mutex_unlock(&devfreq->lock);
-		goto out;
-	}
-
-	/* devfreq->lock is unlocked and removed in _removed_devfreq() */
 	_remove_devfreq(devfreq, true);
-
-out:
-	if (central_polling)
-		mutex_unlock(&devfreq_list_lock);
-}
-
-/**
- * devfreq_monitor() - Periodically poll devfreq objects.
- * @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;
-	struct device *dev;
-
-	/* 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) {
-		mutex_lock(&devfreq->lock);
-		dev = devfreq->dev.parent;
-
-		/* Do not remove tmp for a while */
-		wait_remove_device = tmp;
-
-		if (devfreq->governor->no_central_polling ||
-		    devfreq->next_polling == 0) {
-			mutex_unlock(&devfreq->lock);
-			continue;
-		}
-		mutex_unlock(&devfreq_list_lock);
-
-		/*
-		 * Reduce more next_polling if devfreq_wq took an extra
-		 * delay. (i.e., CPU has been idled.)
-		 */
-		if (devfreq->next_polling <= jiffies_passed) {
-			error = update_devfreq(devfreq);
-
-			/* Remove a devfreq with an error. */
-			if (error && error != -EAGAIN) {
-
-				dev_err(dev, "Due to update_devfreq error(%d), devfreq(%s) is removed from the device\n",
-					error, devfreq->governor->name);
-
-				/*
-				 * Unlock devfreq before locking the list
-				 * in order to avoid deadlock with
-				 * find_device_devfreq or others
-				 */
-				mutex_unlock(&devfreq->lock);
-				mutex_lock(&devfreq_list_lock);
-				/* Check if devfreq is already removed */
-				if (IS_ERR(find_device_devfreq(dev)))
-					continue;
-				mutex_lock(&devfreq->lock);
-				/* This unlocks devfreq->lock and free it */
-				_remove_devfreq(devfreq, false);
-				continue;
-			}
-			devfreq->next_polling = devfreq->polling_jiffies;
-		} else {
-			devfreq->next_polling -= jiffies_passed;
-		}
-
-		if (devfreq->next_polling)
-			next_jiffies = (next_jiffies > devfreq->next_polling) ?
-					devfreq->next_polling : next_jiffies;
-
-		mutex_unlock(&devfreq->lock);
-		mutex_lock(&devfreq_list_lock);
-	}
-	wait_remove_device = NULL;
-	mutex_unlock(&devfreq_list_lock);
-
-	if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
-		polling = true;
-		queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
-	} else {
-		polling = false;
-	}
 }
 
 /**
@@ -357,16 +351,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
-
-	if (!governor->no_central_polling) {
-		mutex_lock(&devfreq_list_lock);
-		devfreq = find_device_devfreq(dev);
-		mutex_unlock(&devfreq_list_lock);
-		if (!IS_ERR(devfreq)) {
-			dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
-			err = -EINVAL;
-			goto err_out;
-		}
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+	mutex_unlock(&devfreq_list_lock);
+	if (!IS_ERR(devfreq)) {
+		dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
+		err = -EINVAL;
+		goto err_out;
 	}
 
 	devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
@@ -386,48 +377,41 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->governor = governor;
 	devfreq->previous_freq = profile->initial_freq;
 	devfreq->data = data;
-	devfreq->next_polling = devfreq->polling_jiffies
-			      = msecs_to_jiffies(devfreq->profile->polling_ms);
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
 	dev_set_name(&devfreq->dev, dev_name(dev));
 	err = device_register(&devfreq->dev);
 	if (err) {
 		put_device(&devfreq->dev);
+		mutex_unlock(&devfreq->lock);
 		goto err_dev;
 	}
 
-	if (governor->init)
-		err = governor->init(devfreq);
-	if (err)
-		goto err_init;
-
 	mutex_unlock(&devfreq->lock);
 
-	if (governor->no_central_polling)
-		goto out;
-
 	mutex_lock(&devfreq_list_lock);
-
 	list_add(&devfreq->node, &devfreq_list);
+	mutex_unlock(&devfreq_list_lock);
 
-	if (devfreq_wq && devfreq->next_polling && !polling) {
-		polling = true;
-		queue_delayed_work(devfreq_wq, &devfreq_work,
-				   devfreq->next_polling);
+	err = devfreq->governor->event_handler(devfreq,
+				DEVFREQ_GOV_START, NULL);
+	if (err) {
+		dev_err(dev, "%s: Unable to start governor for the device\n",
+			__func__);
+		goto err_init;
 	}
-	mutex_unlock(&devfreq_list_lock);
-out:
+
 	return devfreq;
 
 err_init:
+	list_del(&devfreq->node);
 	device_unregister(&devfreq->dev);
 err_dev:
-	mutex_unlock(&devfreq->lock);
 	kfree(devfreq);
 err_out:
 	return ERR_PTR(err);
 }
+EXPORT_SYMBOL(devfreq_add_device);
 
 /**
  * devfreq_remove_device() - Remove devfreq feature from a device.
@@ -435,30 +419,14 @@ err_out:
  */
 int devfreq_remove_device(struct devfreq *devfreq)
 {
-	bool central_polling;
-
 	if (!devfreq)
 		return -EINVAL;
 
-	central_polling = !devfreq->governor->no_central_polling;
-
-	if (central_polling) {
-		mutex_lock(&devfreq_list_lock);
-		while (wait_remove_device == devfreq) {
-			mutex_unlock(&devfreq_list_lock);
-			schedule();
-			mutex_lock(&devfreq_list_lock);
-		}
-	}
-
-	mutex_lock(&devfreq->lock);
-	_remove_devfreq(devfreq, false); /* it unlocks devfreq->lock */
-
-	if (central_polling)
-		mutex_unlock(&devfreq_list_lock);
+	_remove_devfreq(devfreq, false);
 
 	return 0;
 }
+EXPORT_SYMBOL(devfreq_remove_device);
 
 static ssize_t show_governor(struct device *dev,
 			     struct device_attribute *attr, char *buf)
@@ -490,35 +458,13 @@ static ssize_t store_polling_interval(struct device *dev,
 	if (ret != 1)
 		goto out;
 
-	mutex_lock(&df->lock);
-	df->profile->polling_ms = value;
-	df->next_polling = df->polling_jiffies
-			 = msecs_to_jiffies(value);
-	mutex_unlock(&df->lock);
-
+	df->governor->event_handler(df, DEVFREQ_GOV_INTERVAL, &value);
 	ret = count;
 
-	if (df->governor->no_central_polling)
-		goto out;
-
-	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);
 out:
 	return ret;
 }
 
-static ssize_t show_central_polling(struct device *dev,
-				    struct device_attribute *attr, char *buf)
-{
-	return sprintf(buf, "%d\n",
-		       !to_devfreq(dev)->governor->no_central_polling);
-}
-
 static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -590,7 +536,6 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
 static struct device_attribute devfreq_attrs[] = {
 	__ATTR(governor, S_IRUGO, show_governor, NULL),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
-	__ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
 	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
 	       store_polling_interval),
 	__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
@@ -598,23 +543,6 @@ static struct device_attribute devfreq_attrs[] = {
 	{ },
 };
 
-/**
- * devfreq_start_polling() - Initialize data structure for devfreq framework and
- *			   start polling registered devfreq devices.
- */
-static int __init devfreq_start_polling(void)
-{
-	mutex_lock(&devfreq_list_lock);
-	polling = false;
-	devfreq_wq = create_freezable_workqueue("devfreq_wq");
-	INIT_DEFERRABLE_WORK(&devfreq_work, devfreq_monitor);
-	mutex_unlock(&devfreq_list_lock);
-
-	devfreq_monitor(&devfreq_work.work);
-	return 0;
-}
-late_initcall(devfreq_start_polling);
-
 static int __init devfreq_init(void)
 {
 	devfreq_class = class_create(THIS_MODULE, "devfreq");
@@ -622,7 +550,15 @@ static int __init devfreq_init(void)
 		pr_err("%s: couldn't create class\n", __FILE__);
 		return PTR_ERR(devfreq_class);
 	}
+
+	devfreq_wq = create_freezable_workqueue("devfreq_wq");
+	if (IS_ERR(devfreq_wq)) {
+		class_destroy(devfreq_class);
+		pr_err("%s: couldn't create workqueue\n", __FILE__);
+		return PTR_ERR(devfreq_wq);
+	}
 	devfreq_class->dev_attrs = devfreq_attrs;
+
 	return 0;
 }
 subsys_initcall(devfreq_init);
@@ -630,6 +566,7 @@ subsys_initcall(devfreq_init);
 static void __exit devfreq_exit(void)
 {
 	class_destroy(devfreq_class);
+	destroy_workqueue(devfreq_wq);
 }
 module_exit(devfreq_exit);
 
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index ea7f13c..bb3aff3 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -18,7 +18,18 @@
 
 #define to_devfreq(DEV)	container_of((DEV), struct devfreq, dev)
 
+/* Devfreq events */
+#define DEVFREQ_GOV_START			0x1
+#define DEVFREQ_GOV_STOP			0x2
+#define DEVFREQ_GOV_INTERVAL			0x3
+
 /* Caution: devfreq->lock must be locked before calling update_devfreq */
 extern int update_devfreq(struct devfreq *devfreq);
 
+extern void devfreq_monitor_start(struct devfreq *devfreq);
+extern void devfreq_monitor_stop(struct devfreq *devfreq);
+extern void devfreq_monitor_suspend(struct devfreq *devfreq);
+extern void devfreq_monitor_resume(struct devfreq *devfreq);
+extern void devfreq_interval_update(struct devfreq *devfreq,
+					unsigned int *delay);
 #endif /* _GOVERNOR_H */
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index af75ddd..eea3f9b 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -26,14 +26,22 @@ static int devfreq_performance_func(struct devfreq *df,
 	return 0;
 }
 
-static int performance_init(struct devfreq *devfreq)
+static int devfreq_performance_handler(struct devfreq *devfreq,
+				unsigned int event, void *data)
 {
-	return update_devfreq(devfreq);
+	int ret = 0;
+
+	if (event == DEVFREQ_GOV_START) {
+		mutex_lock(&devfreq->lock);
+		ret = update_devfreq(devfreq);
+		mutex_unlock(&devfreq->lock);
+	}
+
+	return ret;
 }
 
 const struct devfreq_governor devfreq_performance = {
 	.name = "performance",
-	.init = performance_init,
 	.get_target_freq = devfreq_performance_func,
-	.no_central_polling = true,
+	.event_handler = devfreq_performance_handler,
 };
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index fec0cdb..2868d98 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -23,14 +23,22 @@ static int devfreq_powersave_func(struct devfreq *df,
 	return 0;
 }
 
-static int powersave_init(struct devfreq *devfreq)
+static int devfreq_powersave_handler(struct devfreq *devfreq,
+				unsigned int event, void *data)
 {
-	return update_devfreq(devfreq);
+	int ret = 0;
+
+	if (event == DEVFREQ_GOV_START) {
+		mutex_lock(&devfreq->lock);
+		ret = update_devfreq(devfreq);
+		mutex_unlock(&devfreq->lock);
+	}
+
+	return ret;
 }
 
 const struct devfreq_governor devfreq_powersave = {
 	.name = "powersave",
-	.init = powersave_init,
 	.get_target_freq = devfreq_powersave_func,
-	.no_central_polling = true,
+	.event_handler = devfreq_powersave_handler,
 };
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index a2e3eae..cf94218 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -12,6 +12,7 @@
 #include <linux/errno.h>
 #include <linux/devfreq.h>
 #include <linux/math64.h>
+#include "governor.h"
 
 /* Default constants for DevFreq-Simple-Ondemand (DFSO) */
 #define DFSO_UPTHRESHOLD	(90)
@@ -88,7 +89,30 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	return 0;
 }
 
+int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
+				unsigned int event, void *data)
+{
+	switch (event) {
+	case DEVFREQ_GOV_START:
+		devfreq_monitor_start(devfreq);
+		break;
+
+	case DEVFREQ_GOV_STOP:
+		devfreq_monitor_stop(devfreq);
+		break;
+
+	case DEVFREQ_GOV_INTERVAL:
+		devfreq_interval_update(devfreq, (unsigned int *)data);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 const struct devfreq_governor devfreq_simple_ondemand = {
 	.name = "simple_ondemand",
 	.get_target_freq = devfreq_simple_ondemand_func,
+	.event_handler = devfreq_simple_ondemand_handler,
 };
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 0681246..1fddee5 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -116,10 +116,27 @@ static void userspace_exit(struct devfreq *devfreq)
 	devfreq->data = NULL;
 }
 
+int devfreq_userspace_handler(struct devfreq *devfreq,
+			unsigned int event, void *data)
+{
+	int ret = 0;
+
+	switch (event) {
+	case DEVFREQ_GOV_START:
+		ret = userspace_init(devfreq);
+		break;
+	case DEVFREQ_GOV_STOP:
+		userspace_exit(devfreq);
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
 const struct devfreq_governor devfreq_userspace = {
 	.name = "userspace",
 	.get_target_freq = devfreq_userspace_func,
-	.init = userspace_init,
-	.exit = userspace_exit,
-	.no_central_polling = true,
+	.event_handler = devfreq_userspace_handler,
 };
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 281c72a..9cdffde 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -91,25 +91,18 @@ struct devfreq_dev_profile {
  *			status of the device (load = busy_time / total_time).
  *			If no_central_polling is set, this callback is called
  *			only with update_devfreq() notified by OPP.
- * @init		Called when the devfreq is being attached to a device
- * @exit		Called when the devfreq is being removed from a
- *			device. Governor should stop any internal routines
- *			before return because related data may be
- *			freed after exit().
- * @no_central_polling	Do not use devfreq's central polling mechanism.
- *			When this is set, devfreq will not call
- *			get_target_freq with devfreq_monitor(). However,
- *			devfreq will call get_target_freq with
- *			devfreq_update() notified by OPP framework.
+ * @event_handler       Callback for devfreq core framework to notify events
+ *                      to governors. Events include per device governor
+ *                      init and exit, opp changes out of devfreq, suspend
+ *                      and resume of per device devfreq during device idle.
  *
  * Note that the callbacks are called with devfreq->lock locked by devfreq.
  */
 struct devfreq_governor {
 	const 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);
-	const bool no_central_polling;
+	int (*event_handler)(struct devfreq *devfreq,
+				unsigned int event, void *data);
 };
 
 /**
@@ -124,18 +117,13 @@ struct devfreq_governor {
  * @nb		notifier block used to notify devfreq object that it should
  *		reevaluate operable frequencies. Devfreq users may use
  *		devfreq.nb to the corresponding register notifier call chain.
- * @polling_jiffies	interval in jiffies.
+ * @work	delayed work for load monitoring.
  * @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.
- * @being_removed	a flag to mark that this object is being removed in
- *			order to prevent trying to remove the object multiple times.
  * @min_freq	Limit minimum frequency requested by user (0: none)
  * @max_freq	Limit maximum frequency requested by user (0: none)
+ * @stop_polling	 devfreq polling status of a device.
  *
  * This structure stores the devfreq information for a give device.
  *
@@ -153,17 +141,15 @@ struct devfreq {
 	struct devfreq_dev_profile *profile;
 	const struct devfreq_governor *governor;
 	struct notifier_block nb;
+	struct delayed_work work;
 
-	unsigned long polling_jiffies;
 	unsigned long previous_freq;
-	unsigned int next_polling;
 
 	void *data; /* private data for governors */
 
-	bool being_removed;
-
 	unsigned long min_freq;
 	unsigned long max_freq;
+	bool stop_polling;
 };
 
 #if defined(CONFIG_PM_DEVFREQ)
-- 
1.7.11.3

[PATCH v4 2/3] devfreq: Add suspend and resume apis

[Rajagopal Venkat]

Add devfreq suspend/resume apis for devfreq users. This patch
supports suspend and resume of devfreq load monitoring, required
for devices which can idle.

Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/devfreq/devfreq.c                 | 28 ++++++++++++++++++++++++++++
 drivers/devfreq/governor.h                |  2 ++
 drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
 include/linux/devfreq.h                   | 12 ++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 41a4099..63e075e 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -428,6 +428,34 @@ int devfreq_remove_device(struct devfreq *devfreq)
 }
 EXPORT_SYMBOL(devfreq_remove_device);
 
+/**
+ * devfreq_suspend_device() - Suspend devfreq of a device.
+ * @devfreq	the devfreq instance to be suspended
+ */
+int devfreq_suspend_device(struct devfreq *devfreq)
+{
+	if (!devfreq)
+		return -EINVAL;
+
+	return devfreq->governor->event_handler(devfreq,
+				DEVFREQ_GOV_SUSPEND, NULL);
+}
+EXPORT_SYMBOL(devfreq_suspend_device);
+
+/**
+ * devfreq_resume_device() - Resume devfreq of a device.
+ * @devfreq	the devfreq instance to be resumed
+ */
+int devfreq_resume_device(struct devfreq *devfreq)
+{
+	if (!devfreq)
+		return -EINVAL;
+
+	return devfreq->governor->event_handler(devfreq,
+				DEVFREQ_GOV_RESUME, NULL);
+}
+EXPORT_SYMBOL(devfreq_resume_device);
+
 static ssize_t show_governor(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index bb3aff3..26432ac 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -22,6 +22,8 @@
 #define DEVFREQ_GOV_START			0x1
 #define DEVFREQ_GOV_STOP			0x2
 #define DEVFREQ_GOV_INTERVAL			0x3
+#define DEVFREQ_GOV_SUSPEND			0x4
+#define DEVFREQ_GOV_RESUME			0x5
 
 /* Caution: devfreq->lock must be locked before calling update_devfreq */
 extern int update_devfreq(struct devfreq *devfreq);
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index cf94218..a8ba78c 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -104,6 +104,15 @@ int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
 	case DEVFREQ_GOV_INTERVAL:
 		devfreq_interval_update(devfreq, (unsigned int *)data);
 		break;
+
+	case DEVFREQ_GOV_SUSPEND:
+		devfreq_monitor_suspend(devfreq);
+		break;
+
+	case DEVFREQ_GOV_RESUME:
+		devfreq_monitor_resume(devfreq);
+		break;
+
 	default:
 		break;
 	}
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 9cdffde..ee243a3 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -158,6 +158,8 @@ extern struct devfreq *devfreq_add_device(struct device *dev,
 				  const struct devfreq_governor *governor,
 				  void *data);
 extern int devfreq_remove_device(struct devfreq *devfreq);
+extern int devfreq_suspend_device(struct devfreq *devfreq);
+extern int devfreq_resume_device(struct devfreq *devfreq);
 
 /* Helper functions for devfreq user device driver with OPP. */
 extern struct opp *devfreq_recommended_opp(struct device *dev,
@@ -211,6 +213,16 @@ static int devfreq_remove_device(struct devfreq *devfreq)
 	return 0;
 }
 
+static int devfreq_suspend_device(struct devfreq *devfreq)
+{
+	return 0;
+}
+
+static int devfreq_resume_device(struct devfreq *devfreq)
+{
+	return 0;
+}
+
 static struct opp *devfreq_recommended_opp(struct device *dev,
 					   unsigned long *freq, u32 flags)
 {
-- 
1.7.11.3

Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[Rafael J. Wysocki]

On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
> Add devfreq suspend/resume apis for devfreq users. This patch
> supports suspend and resume of devfreq load monitoring, required
> for devices which can idle.
> 
> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>

Well, I wonder if this may be tied in to the runtime PM framework, so that,
for example, pm_runtime_suspend() will automatically suspend devfreq on
success (and the runtime resume of the device will resume devfreq)?

Rafael


> ---
>  drivers/devfreq/devfreq.c                 | 28 ++++++++++++++++++++++++++++
>  drivers/devfreq/governor.h                |  2 ++
>  drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
>  include/linux/devfreq.h                   | 12 ++++++++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 41a4099..63e075e 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -428,6 +428,34 @@ int devfreq_remove_device(struct devfreq *devfreq)
>  }
>  EXPORT_SYMBOL(devfreq_remove_device);
>  
> +/**
> + * devfreq_suspend_device() - Suspend devfreq of a device.
> + * @devfreq	the devfreq instance to be suspended
> + */
> +int devfreq_suspend_device(struct devfreq *devfreq)
> +{
> +	if (!devfreq)
> +		return -EINVAL;
> +
> +	return devfreq->governor->event_handler(devfreq,
> +				DEVFREQ_GOV_SUSPEND, NULL);
> +}
> +EXPORT_SYMBOL(devfreq_suspend_device);
> +
> +/**
> + * devfreq_resume_device() - Resume devfreq of a device.
> + * @devfreq	the devfreq instance to be resumed
> + */
> +int devfreq_resume_device(struct devfreq *devfreq)
> +{
> +	if (!devfreq)
> +		return -EINVAL;
> +
> +	return devfreq->governor->event_handler(devfreq,
> +				DEVFREQ_GOV_RESUME, NULL);
> +}
> +EXPORT_SYMBOL(devfreq_resume_device);
> +
>  static ssize_t show_governor(struct device *dev,
>  			     struct device_attribute *attr, char *buf)
>  {
> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> index bb3aff3..26432ac 100644
> --- a/drivers/devfreq/governor.h
> +++ b/drivers/devfreq/governor.h
> @@ -22,6 +22,8 @@
>  #define DEVFREQ_GOV_START			0x1
>  #define DEVFREQ_GOV_STOP			0x2
>  #define DEVFREQ_GOV_INTERVAL			0x3
> +#define DEVFREQ_GOV_SUSPEND			0x4
> +#define DEVFREQ_GOV_RESUME			0x5
>  
>  /* Caution: devfreq->lock must be locked before calling update_devfreq */
>  extern int update_devfreq(struct devfreq *devfreq);
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index cf94218..a8ba78c 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -104,6 +104,15 @@ int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
>  	case DEVFREQ_GOV_INTERVAL:
>  		devfreq_interval_update(devfreq, (unsigned int *)data);
>  		break;
> +
> +	case DEVFREQ_GOV_SUSPEND:
> +		devfreq_monitor_suspend(devfreq);
> +		break;
> +
> +	case DEVFREQ_GOV_RESUME:
> +		devfreq_monitor_resume(devfreq);
> +		break;
> +
>  	default:
>  		break;
>  	}
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 9cdffde..ee243a3 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -158,6 +158,8 @@ extern struct devfreq *devfreq_add_device(struct device *dev,
>  				  const struct devfreq_governor *governor,
>  				  void *data);
>  extern int devfreq_remove_device(struct devfreq *devfreq);
> +extern int devfreq_suspend_device(struct devfreq *devfreq);
> +extern int devfreq_resume_device(struct devfreq *devfreq);
>  
>  /* Helper functions for devfreq user device driver with OPP. */
>  extern struct opp *devfreq_recommended_opp(struct device *dev,
> @@ -211,6 +213,16 @@ static int devfreq_remove_device(struct devfreq *devfreq)
>  	return 0;
>  }
>  
> +static int devfreq_suspend_device(struct devfreq *devfreq)
> +{
> +	return 0;
> +}
> +
> +static int devfreq_resume_device(struct devfreq *devfreq)
> +{
> +	return 0;
> +}
> +
>  static struct opp *devfreq_recommended_opp(struct device *dev,
>  					   unsigned long *freq, u32 flags)
>  {
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[Rajagopal Venkat]

On 8 October 2012 03:31, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
>> Add devfreq suspend/resume apis for devfreq users. This patch
>> supports suspend and resume of devfreq load monitoring, required
>> for devices which can idle.
>>
>> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
>> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>
> Well, I wonder if this may be tied in to the runtime PM framework, so that,
> for example, pm_runtime_suspend() will automatically suspend devfreq on
> success (and the runtime resume of the device will resume devfreq)?

That's a good idea. If you agree, we can handle this as separate patch on
top this patchset.

>
> Rafael
>
>
>> ---
>>  drivers/devfreq/devfreq.c                 | 28 ++++++++++++++++++++++++++++
>>  drivers/devfreq/governor.h                |  2 ++
>>  drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
>>  include/linux/devfreq.h                   | 12 ++++++++++++
>>  4 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 41a4099..63e075e 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -428,6 +428,34 @@ int devfreq_remove_device(struct devfreq *devfreq)
>>  }
>>  EXPORT_SYMBOL(devfreq_remove_device);
>>
>> +/**
>> + * devfreq_suspend_device() - Suspend devfreq of a device.
>> + * @devfreq  the devfreq instance to be suspended
>> + */
>> +int devfreq_suspend_device(struct devfreq *devfreq)
>> +{
>> +     if (!devfreq)
>> +             return -EINVAL;
>> +
>> +     return devfreq->governor->event_handler(devfreq,
>> +                             DEVFREQ_GOV_SUSPEND, NULL);
>> +}
>> +EXPORT_SYMBOL(devfreq_suspend_device);
>> +
>> +/**
>> + * devfreq_resume_device() - Resume devfreq of a device.
>> + * @devfreq  the devfreq instance to be resumed
>> + */
>> +int devfreq_resume_device(struct devfreq *devfreq)
>> +{
>> +     if (!devfreq)
>> +             return -EINVAL;
>> +
>> +     return devfreq->governor->event_handler(devfreq,
>> +                             DEVFREQ_GOV_RESUME, NULL);
>> +}
>> +EXPORT_SYMBOL(devfreq_resume_device);
>> +
>>  static ssize_t show_governor(struct device *dev,
>>                            struct device_attribute *attr, char *buf)
>>  {
>> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
>> index bb3aff3..26432ac 100644
>> --- a/drivers/devfreq/governor.h
>> +++ b/drivers/devfreq/governor.h
>> @@ -22,6 +22,8 @@
>>  #define DEVFREQ_GOV_START                    0x1
>>  #define DEVFREQ_GOV_STOP                     0x2
>>  #define DEVFREQ_GOV_INTERVAL                 0x3
>> +#define DEVFREQ_GOV_SUSPEND                  0x4
>> +#define DEVFREQ_GOV_RESUME                   0x5
>>
>>  /* Caution: devfreq->lock must be locked before calling update_devfreq */
>>  extern int update_devfreq(struct devfreq *devfreq);
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> index cf94218..a8ba78c 100644
>> --- a/drivers/devfreq/governor_simpleondemand.c
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -104,6 +104,15 @@ int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
>>       case DEVFREQ_GOV_INTERVAL:
>>               devfreq_interval_update(devfreq, (unsigned int *)data);
>>               break;
>> +
>> +     case DEVFREQ_GOV_SUSPEND:
>> +             devfreq_monitor_suspend(devfreq);
>> +             break;
>> +
>> +     case DEVFREQ_GOV_RESUME:
>> +             devfreq_monitor_resume(devfreq);
>> +             break;
>> +
>>       default:
>>               break;
>>       }
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 9cdffde..ee243a3 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -158,6 +158,8 @@ extern struct devfreq *devfreq_add_device(struct device *dev,
>>                                 const struct devfreq_governor *governor,
>>                                 void *data);
>>  extern int devfreq_remove_device(struct devfreq *devfreq);
>> +extern int devfreq_suspend_device(struct devfreq *devfreq);
>> +extern int devfreq_resume_device(struct devfreq *devfreq);
>>
>>  /* Helper functions for devfreq user device driver with OPP. */
>>  extern struct opp *devfreq_recommended_opp(struct device *dev,
>> @@ -211,6 +213,16 @@ static int devfreq_remove_device(struct devfreq *devfreq)
>>       return 0;
>>  }
>>
>> +static int devfreq_suspend_device(struct devfreq *devfreq)
>> +{
>> +     return 0;
>> +}
>> +
>> +static int devfreq_resume_device(struct devfreq *devfreq)
>> +{
>> +     return 0;
>> +}
>> +
>>  static struct opp *devfreq_recommended_opp(struct device *dev,
>>                                          unsigned long *freq, u32 flags)
>>  {
>>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.



-- 
Regards,
Rajagopal

[PATCH v4 3/3] devfreq: Add current freq callback in device profile

[Rajagopal Venkat]

Devfreq returns governor predicted frequency as current frequency
via sysfs interface. But device may not support all frequencies
that governor predicts. So add a callback in device profile to get
current freq from driver. Also add a new sysfs node to expose
governor predicted next target frequency.

Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 Documentation/ABI/testing/sysfs-class-devfreq | 11 ++++++++++-
 drivers/devfreq/devfreq.c                     | 14 ++++++++++++++
 include/linux/devfreq.h                       |  3 +++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index 89283b1..e6cf08e 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -19,7 +19,16 @@ Date:		September 2011
 Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
 Description:
 		The /sys/class/devfreq/.../cur_freq shows the current
-		frequency of the corresponding devfreq object.
+		frequency of the corresponding devfreq object. Same as
+		target_freq when get_cur_freq() is not implemented by
+		devfreq driver.
+
+What:		/sys/class/devfreq/.../target_freq
+Date:		September 2012
+Contact:	Rajagopal Venkat <rajagopal.venkat@linaro.org>
+Description:
+		The /sys/class/devfreq/.../target_freq shows the next governor
+		predicted target frequency of the corresponding devfreq object.
 
 What:		/sys/class/devfreq/.../polling_interval
 Date:		September 2011
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 63e075e..9fdcb5d 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -465,6 +465,19 @@ static ssize_t show_governor(struct device *dev,
 static ssize_t show_freq(struct device *dev,
 			 struct device_attribute *attr, char *buf)
 {
+	unsigned long freq;
+	struct devfreq *devfreq = to_devfreq(dev);
+
+	if (devfreq->profile->get_cur_freq &&
+		!devfreq->profile->get_cur_freq(devfreq->dev.parent, &freq))
+			return sprintf(buf, "%lu\n", freq);
+
+	return sprintf(buf, "%lu\n", devfreq->previous_freq);
+}
+
+static ssize_t show_target_freq(struct device *dev,
+			struct device_attribute *attr, char *buf)
+{
 	return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
 }
 
@@ -564,6 +577,7 @@ static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
 static struct device_attribute devfreq_attrs[] = {
 	__ATTR(governor, S_IRUGO, show_governor, NULL),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
+	__ATTR(target_freq, S_IRUGO, show_target_freq, NULL),
 	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
 	       store_polling_interval),
 	__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index ee243a3..7e2e2ea 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -66,6 +66,8 @@ struct devfreq_dev_status {
  *			explained above with "DEVFREQ_FLAG_*" macros.
  * @get_dev_status	The device should provide the current performance
  *			status to devfreq, which is used by governors.
+ * @get_cur_freq	The device should provide the current frequency
+ *			at which it is operating.
  * @exit		An optional callback that is called when devfreq
  *			is removing the devfreq object due to error or
  *			from devfreq_remove_device() call. If the user
@@ -79,6 +81,7 @@ struct devfreq_dev_profile {
 	int (*target)(struct device *dev, unsigned long *freq, u32 flags);
 	int (*get_dev_status)(struct device *dev,
 			      struct devfreq_dev_status *stat);
+	int (*get_cur_freq)(struct device *dev, unsigned long *freq);
 	void (*exit)(struct device *dev);
 };
 
-- 
1.7.11.3

Re: Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 6064 bytes --]

> On 8 October 2012 03:31, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
> >> Add devfreq suspend/resume apis for devfreq users. This patch
> >> supports suspend and resume of devfreq load monitoring, required
> >> for devices which can idle.
> >>
> >> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >
> > Well, I wonder if this may be tied in to the runtime PM framework, so that,
> > for example, pm_runtime_suspend() will automatically suspend devfreq on
> > success (and the runtime resume of the device will resume devfreq)?
> 
> That's a good idea. If you agree, we can handle this as separate patch on
> top this patchset.

I guess implementing the idea may be not so obvious; thus, seperating
the patchset would be appropriate.

When we implement the idea, we may need to implement at the
pm_runtime core. Because devfreq->dev is a child of the target dev, not
a parent, runtime-pm event of the target dev does not automatically
invoke a cascaded action on the devfreq->dev.

Maybe either
1) capability to allow a child to monitor the power state of a parent
(I remember Inki Dae once suggested to add notifiers at runtime-pm, but
it seems to be not merged)
or
2) letting runtime-pm be aware of devfreq
(doesn't feel alright with this??)
is required?



> 
> >
> > Rafael
> >
> >
> >> ---
> >>  drivers/devfreq/devfreq.c                 | 28 ++++++++++++++++++++++++++++
> >>  drivers/devfreq/governor.h                |  2 ++
> >>  drivers/devfreq/governor_simpleondemand.c |  9 +++++++++
> >>  include/linux/devfreq.h                   | 12 ++++++++++++
> >>  4 files changed, 51 insertions(+)
> >>
> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> >> index 41a4099..63e075e 100644
> >> --- a/drivers/devfreq/devfreq.c
> >> +++ b/drivers/devfreq/devfreq.c
> >> @@ -428,6 +428,34 @@ int devfreq_remove_device(struct devfreq *devfreq)
> >>  }
> >>  EXPORT_SYMBOL(devfreq_remove_device);
> >>
> >> +/**
> >> + * devfreq_suspend_device() - Suspend devfreq of a device.
> >> + * @devfreq  the devfreq instance to be suspended
> >> + */
> >> +int devfreq_suspend_device(struct devfreq *devfreq)
> >> +{
> >> +     if (!devfreq)
> >> +             return -EINVAL;
> >> +
> >> +     return devfreq->governor->event_handler(devfreq,
> >> +                             DEVFREQ_GOV_SUSPEND, NULL);
> >> +}
> >> +EXPORT_SYMBOL(devfreq_suspend_device);
> >> +
> >> +/**
> >> + * devfreq_resume_device() - Resume devfreq of a device.
> >> + * @devfreq  the devfreq instance to be resumed
> >> + */
> >> +int devfreq_resume_device(struct devfreq *devfreq)
> >> +{
> >> +     if (!devfreq)
> >> +             return -EINVAL;
> >> +
> >> +     return devfreq->governor->event_handler(devfreq,
> >> +                             DEVFREQ_GOV_RESUME, NULL);
> >> +}
> >> +EXPORT_SYMBOL(devfreq_resume_device);
> >> +
> >>  static ssize_t show_governor(struct device *dev,
> >>                            struct device_attribute *attr, char *buf)
> >>  {
> >> diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
> >> index bb3aff3..26432ac 100644
> >> --- a/drivers/devfreq/governor.h
> >> +++ b/drivers/devfreq/governor.h
> >> @@ -22,6 +22,8 @@
> >>  #define DEVFREQ_GOV_START                    0x1
> >>  #define DEVFREQ_GOV_STOP                     0x2
> >>  #define DEVFREQ_GOV_INTERVAL                 0x3
> >> +#define DEVFREQ_GOV_SUSPEND                  0x4
> >> +#define DEVFREQ_GOV_RESUME                   0x5
> >>
> >>  /* Caution: devfreq->lock must be locked before calling update_devfreq */
> >>  extern int update_devfreq(struct devfreq *devfreq);
> >> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> >> index cf94218..a8ba78c 100644
> >> --- a/drivers/devfreq/governor_simpleondemand.c
> >> +++ b/drivers/devfreq/governor_simpleondemand.c
> >> @@ -104,6 +104,15 @@ int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
> >>       case DEVFREQ_GOV_INTERVAL:
> >>               devfreq_interval_update(devfreq, (unsigned int *)data);
> >>               break;
> >> +
> >> +     case DEVFREQ_GOV_SUSPEND:
> >> +             devfreq_monitor_suspend(devfreq);
> >> +             break;
> >> +
> >> +     case DEVFREQ_GOV_RESUME:
> >> +             devfreq_monitor_resume(devfreq);
> >> +             break;
> >> +
> >>       default:
> >>               break;
> >>       }
> >> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> >> index 9cdffde..ee243a3 100644
> >> --- a/include/linux/devfreq.h
> >> +++ b/include/linux/devfreq.h
> >> @@ -158,6 +158,8 @@ extern struct devfreq *devfreq_add_device(struct device *dev,
> >>                                 const struct devfreq_governor *governor,
> >>                                 void *data);
> >>  extern int devfreq_remove_device(struct devfreq *devfreq);
> >> +extern int devfreq_suspend_device(struct devfreq *devfreq);
> >> +extern int devfreq_resume_device(struct devfreq *devfreq);
> >>
> >>  /* Helper functions for devfreq user device driver with OPP. */
> >>  extern struct opp *devfreq_recommended_opp(struct device *dev,
> >> @@ -211,6 +213,16 @@ static int devfreq_remove_device(struct devfreq *devfreq)
> >>       return 0;
> >>  }
> >>
> >> +static int devfreq_suspend_device(struct devfreq *devfreq)
> >> +{
> >> +     return 0;
> >> +}
> >> +
> >> +static int devfreq_resume_device(struct devfreq *devfreq)
> >> +{
> >> +     return 0;
> >> +}
> >> +
> >>  static struct opp *devfreq_recommended_opp(struct device *dev,
> >>                                          unsigned long *freq, u32 flags)
> >>  {
> >>
> > --
> > I speak only for myself.
> > Rafael J. Wysocki, Intel Open Source Technology Center.
> 
> 
> 
> -- 
> Regards,
> Rajagopal
> 
> 
> 
>        
>   
>          
> 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[Rafael J. Wysocki]

On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
> > On 8 October 2012 03:31, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
> > >> Add devfreq suspend/resume apis for devfreq users. This patch
> > >> supports suspend and resume of devfreq load monitoring, required
> > >> for devices which can idle.
> > >>
> > >> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> > >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> > >
> > > Well, I wonder if this may be tied in to the runtime PM framework, so that,
> > > for example, pm_runtime_suspend() will automatically suspend devfreq on
> > > success (and the runtime resume of the device will resume devfreq)?
> > 
> > That's a good idea. If you agree, we can handle this as separate patch on
> > top this patchset.

Sure.

> I guess implementing the idea may be not so obvious; thus, seperating
> the patchset would be appropriate.
> 
> When we implement the idea, we may need to implement at the
> pm_runtime core. Because devfreq->dev is a child of the target dev, not
> a parent, runtime-pm event of the target dev does not automatically
> invoke a cascaded action on the devfreq->dev.

I'm not exactly sure what you mean, care to explain?

> Maybe either
> 1) capability to allow a child to monitor the power state of a parent
> (I remember Inki Dae once suggested to add notifiers at runtime-pm, but
> it seems to be not merged)
> or
> 2) letting runtime-pm be aware of devfreq
> (doesn't feel alright with this??)
> is required?

I'm not a big fan of notifiers, so I'd prefer to avoid using them, if
possible.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 2915 bytes --]

> On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
> > > On 8 October 2012 03:31, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
> > > >> Add devfreq suspend/resume apis for devfreq users. This patch
> > > >> supports suspend and resume of devfreq load monitoring, required
> > > >> for devices which can idle.
> > > >>
> > > >> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> > > >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > >
> > > > Well, I wonder if this may be tied in to the runtime PM framework, so that,
> > > > for example, pm_runtime_suspend() will automatically suspend devfreq on
> > > > success (and the runtime resume of the device will resume devfreq)?
> > > 
> > > That's a good idea. If you agree, we can handle this as separate patch on
> > > top this patchset.
> 
> Sure.
> 
> > I guess implementing the idea may be not so obvious; thus, seperating
> > the patchset would be appropriate.
> > 
> > When we implement the idea, we may need to implement at the
> > pm_runtime core. Because devfreq->dev is a child of the target dev, not
> > a parent, runtime-pm event of the target dev does not automatically
> > invoke a cascaded action on the devfreq->dev.
> 
> I'm not exactly sure what you mean, care to explain?

When a device "p" creates devfreq, devfreq->dev->parent = p.
And, devfreq's functions need to react to the "p"'s runtime-pm events.

However, when "p"'s runtime-pm-suspend is being invoked,
devfreq->dev's runtime-pm-suspend won't be automatically invoked.

Thus, we will need a mechanism that invokes devfreq->dev's runtime-pm
callbacks when p's runtime-pm is invoked. (at the runtime-pm core)
Or
A mechanism that one can notify others (including its children) when
the one's runtime-pm is invoked. (probably at the runtime-pm core, too)

Without such support, it appears that the current implementation
(calling runtime-pm suspend/resume equivalent devfreq functions
manually at device drivers) seems to be inevitable.



Anyway, if devfreq->dev is a parent of "p", runtime-pm core will
do the required task automatically; however, it isn't and I don't
think it'd be appropriate.

> 
> > Maybe either
> > 1) capability to allow a child to monitor the power state of a parent
> > (I remember Inki Dae once suggested to add notifiers at runtime-pm, but
> > it seems to be not merged)
> > or
> > 2) letting runtime-pm be aware of devfreq
> > (doesn't feel alright with this??)
> > is required?
> 
> I'm not a big fan of notifiers, so I'd prefer to avoid using them, if
> possible.
> 
> Thanks,
> Rafael
> 
> 
> -- 
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> 

Cheers,
MyungJoo
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4 2/3] devfreq: Add suspend and resume apis

[Rafael J. Wysocki]

On Tuesday 16 of October 2012 06:40:19 MyungJoo Ham wrote:
> > On Monday 08 of October 2012 10:48:24 MyungJoo Ham wrote:
> > > > On 8 October 2012 03:31, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > > > On Thursday 04 of October 2012 14:58:33 Rajagopal Venkat wrote:
> > > > >> Add devfreq suspend/resume apis for devfreq users. This patch
> > > > >> supports suspend and resume of devfreq load monitoring, required
> > > > >> for devices which can idle.
> > > > >>
> > > > >> Signed-off-by: Rajagopal Venkat <rajagopal.venkat@linaro.org>
> > > > >> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> > > > >
> > > > > Well, I wonder if this may be tied in to the runtime PM framework, so that,
> > > > > for example, pm_runtime_suspend() will automatically suspend devfreq on
> > > > > success (and the runtime resume of the device will resume devfreq)?
> > > > 
> > > > That's a good idea. If you agree, we can handle this as separate patch on
> > > > top this patchset.
> > 
> > Sure.
> > 
> > > I guess implementing the idea may be not so obvious; thus, seperating
> > > the patchset would be appropriate.
> > > 
> > > When we implement the idea, we may need to implement at the
> > > pm_runtime core. Because devfreq->dev is a child of the target dev, not
> > > a parent, runtime-pm event of the target dev does not automatically
> > > invoke a cascaded action on the devfreq->dev.
> > 
> > I'm not exactly sure what you mean, care to explain?
> 
> When a device "p" creates devfreq, devfreq->dev->parent = p.

This is wrong.  It shouldn't have been designed this way in the first place
and it was my mistake to merge it, apparently.

> And, devfreq's functions need to react to the "p"'s runtime-pm events.
>
> However, when "p"'s runtime-pm-suspend is being invoked,
> devfreq->dev's runtime-pm-suspend won't be automatically invoked.

Well, the parent should not be able to suspend at all before the children
have suspended and your design doesn't seem to match that principle.

> Thus, we will need a mechanism that invokes devfreq->dev's runtime-pm
> callbacks when p's runtime-pm is invoked. (at the runtime-pm core)

No, this would be going backwards.

> Or
> A mechanism that one can notify others (including its children) when
> the one's runtime-pm is invoked. (probably at the runtime-pm core, too)

You don't appear to understand how the runtime PM framework works.

There's no need to notify a child about the parent's runtime suspend,
because normally the parent won't be suspended before all of its children
have been suspended.

> Without such support, it appears that the current implementation
> (calling runtime-pm suspend/resume equivalent devfreq functions
> manually at device drivers) seems to be inevitable.

Which only means that the devfreq design in incorrect, so please fix it.

> Anyway, if devfreq->dev is a parent of "p", runtime-pm core will
> do the required task automatically; however, it isn't and I don't
> think it'd be appropriate.

No, it wouldn't.  Actually, I'm not sure what you need the dev in struct
devfreq for at all.

As for finding the given device's devfreq structure, for now we can just
walk the devfreq_list.  To avoid doing that for devices that don't have a
struct devfreq attached to them, we can add a devfreq_in_use flag in
struct dev_pm_info.  That may even help some functions in devfreq.c as far
as I can say.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.