diff mbox

[v2,2/2] PM / devfreq: Add suspend frequency support

Message ID 1482926828-19746-3-git-send-email-cw00.choi@samsung.com (mailing list archive)
State Changes Requested, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Chanwoo Choi Dec. 28, 2016, 12:07 p.m. UTC 
From: Lin Huang <hl@rock-chips.com>

Add suspend frequency support and if needed set it to
the frequency obtained from the suspend opp (can be defined
using opp-v2 bindings and is optional).

Signed-off-by: Lin Huang <hl@rock-chips.com>
[cw00.choi: Support the passive governor and use separate devfreq_set_target()]
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 31 ++++++++++++++++++++++++++++++-
 include/linux/devfreq.h   |  2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

Comments

Viresh Kumar Jan. 30, 2017, 4:50 a.m. UTC | #1 
On Wed, Dec 28, 2016 at 5:37 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> +++ b/drivers/devfreq/devfreq.c
> @@ -620,6 +620,23 @@ struct devfreq *devfreq_add_device(struct device *dev,
>                 goto err_init;
>         }
>
> +       /*
> +        * Get the suspend frequency from OPP table. But the devfreq device
> +        * using passive governor don't need to get the suspend frequency
> +        * because the passive devfreq device depend on the parent devfreq
> +        * device.
> +        */
> +       devfreq->suspend_freq = 0L;
> +       if (strncmp(devfreq->governor_name, "passive", 7)) {
> +               struct dev_pm_opp *opp;
> +
> +               rcu_read_lock();
> +               opp = dev_pm_opp_get_suspend_opp(dev);
> +               if (opp)
> +                       devfreq->suspend_freq = dev_pm_opp_get_freq(opp);

The interface has changed a bit recently. Can you please use below
function instead ?

dev_pm_opp_get_suspend_opp_freq()
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chanwoo Choi Jan. 31, 2017, 12:33 a.m. UTC | #2 
Hi Viresh,

On 2017년 01월 30일 13:50, Viresh Kumar wrote:
> On Wed, Dec 28, 2016 at 5:37 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -620,6 +620,23 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>                 goto err_init;
>>         }
>>
>> +       /*
>> +        * Get the suspend frequency from OPP table. But the devfreq device
>> +        * using passive governor don't need to get the suspend frequency
>> +        * because the passive devfreq device depend on the parent devfreq
>> +        * device.
>> +        */
>> +       devfreq->suspend_freq = 0L;
>> +       if (strncmp(devfreq->governor_name, "passive", 7)) {
>> +               struct dev_pm_opp *opp;
>> +
>> +               rcu_read_lock();
>> +               opp = dev_pm_opp_get_suspend_opp(dev);
>> +               if (opp)
>> +                       devfreq->suspend_freq = dev_pm_opp_get_freq(opp);
> 
> The interface has changed a bit recently. Can you please use below
> function instead ?

I knew. This patch posted before applying your patch.

> 
> dev_pm_opp_get_suspend_opp_freq()

This patch has not yet reviewed by devfreq maintainer.
So, I'm just waiting the review.

But, this patch is wrong on latest opp patches.
I'll fix it and resend it.
diff mbox

Patch

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index e386f14d91c3..8109fb71177b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -620,6 +620,23 @@  struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_init;
 	}
 
+	/*
+	 * Get the suspend frequency from OPP table. But the devfreq device
+	 * using passive governor don't need to get the suspend frequency
+	 * because the passive devfreq device depend on the parent devfreq
+	 * device.
+	 */
+	devfreq->suspend_freq = 0L;
+	if (strncmp(devfreq->governor_name, "passive", 7)) {
+		struct dev_pm_opp *opp;
+
+		rcu_read_lock();
+		opp = dev_pm_opp_get_suspend_opp(dev);
+		if (opp)
+			devfreq->suspend_freq = dev_pm_opp_get_freq(opp);
+		rcu_read_unlock();
+	}
+
 	devfreq->governor = governor;
 	err = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_START,
 						NULL);
@@ -777,14 +794,26 @@  void devm_devfreq_remove_device(struct device *dev, struct devfreq *devfreq)
  */
 int devfreq_suspend_device(struct devfreq *devfreq)
 {
+	int ret;
+
 	if (!devfreq)
 		return -EINVAL;
 
 	if (!devfreq->governor)
 		return 0;
 
-	return devfreq->governor->event_handler(devfreq,
+	ret = devfreq->governor->event_handler(devfreq,
 				DEVFREQ_GOV_SUSPEND, NULL);
+	if (ret < 0)
+		return ret;
+
+	if (devfreq->suspend_freq) {
+		ret = devfreq_set_target(devfreq, devfreq->suspend_freq, 0);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(devfreq_suspend_device);
 
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 2de4e2eea180..926bef5a6332 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -141,6 +141,7 @@  struct devfreq_governor {
  *		devfreq.nb to the corresponding register notifier call chain.
  * @work:	delayed work for load monitoring.
  * @previous_freq:	previously configured frequency value.
+ * @suspend_freq:	frequency to set during suspend mode.
  * @data:	Private data of the governor. The devfreq framework does not
  *		touch this.
  * @min_freq:	Limit minimum frequency requested by user (0: none)
@@ -172,6 +173,7 @@  struct devfreq {
 	struct delayed_work work;
 
 	unsigned long previous_freq;
+	unsigned long suspend_freq;
 	struct devfreq_dev_status last_status;
 
 	void *data; /* private data for governors */