All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency
       [not found] <CGME20171023013213epcas1p165faa416f69659b60ec4c3f18ea251d8@epcas1p1.samsung.com>
@ 2017-10-23  1:32 ` Chanwoo Choi
       [not found]   ` <CGME20171023013214epcas1p4f0652eceade1e3812ed7781d60b18367@epcas1p4.samsung.com>
                     ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

These patches makes the devfreq to use the OPP interface and clean-up codes.

[Detaild Descripion]
The commit a76caf55e5b3 ("thermal: Add devfreq cooling") provides
the devfreq cooling device by using the OPP interface such as
dev_pm_opp_disable() and dev_pm_opp_enable(). It means that
the OPP interface is able to change the available status of the frequency.

Firstly, the existing devfreq doesn't reflect the result of OPP behavior
when showing the min/max frequency through the following sysfs nodes:
It shows the wrong frequency value because min_freq/max_freq don't
consider the frequency status by handling OPP interface (opp_dev_pm_opp_
{disable|add}()). So, these patches fix this issue.
- /sys/class/devfreq/devfreqX/min_freq
- /sys/class/devfreq/devfreqX/max_freq

Second, the 'available_frequencies' should show the all supported frequencies
even if the specific frequency is not available. It doesn't matter whether
frequneyc is available or not. Because the role of 'available_frequencies'
shows the all frequencies. Also, these patches fix this issue.
- /sys/class/devfreq/devfreqX/available_frequencies

Third, update_devfreq() get the available next frequency by using
new 'scaling_min/max_frewq' variables in order to consider the disabled OPP.

For example,
- devfreq's min_freq is 100Mhz and max_freq is 700Mhz.
- OPP disabled 500/600/700Mhz due to devfreq-cooling.c.
- simple_ondemand govenor decided the next target_freq (600Mhz)
|----------|-------------------------------------------------------------|
|Freq(MHz) |100     |200    |300    |400     |500     |600      |70 0    |
|Devfreq   |min_freq|       |       |        |        |         |max_freq|
|OPP avail |enabled |enabled|enabled|enabled |Disabled| Disabled|Disabled|
|Ondmenad  |        |       |       |        |        |next_freq|        |
|------------------------------------------------------------------------|

In result,
- Before this patch, target_freq is 600Mhz
  and TRANSITION_NOTIFIER sends the next_freq is 600Mhz to the notifiee.
- After this patch, target_freq is 400Mhz because 500/600 were disabled by OPP.
  and TRANSITION_NOTIFIER sends the next_freq is 400Mhz to the notifiee.

Lastly,
- patch6/7 fix the minor issue and cleanup codes.


Changes from v4:
(https://lkml.org/lkml/2017/10/13/78)
- Add acked-by tag from Myungjoo Ham on patch4/5/6/7.
- Fix the typo and bug of min/max_freq_show() on patch3.
- Drop the exynos-bus.c patch for using it as a cooling device.

Changes from v3:
- Add the new 'scaling_min/max_freq' to 'struct devfreq'
  in order to reflect the OPP behavior such as dev_pm_opp_disable/enable().
- Drop the call of devfreq_recommended_opp() in the update_devfreq().
- Show the available frequencies in an ascending order.
- Check '#cooling-cells' property before registering cooling device on
  exynos-bus.c.
- Change the return type of devfreq_set_freq_table() and remvoe 'devfreq'
  prefix from function name.
- Add acked-by tag from Myungjoo Ham on patch1.

Changes from v2:
(https://lkml.org/lkml/2017/9/20/886)
- Fix the exynos-bus.c for the cooling device on patch8

Changes from v1:
(https://lkml.org/lkml/2017/8/23/785)
- Show the available frequencies as an ascending order
- Change the author info from cwchoi00@gmail.com to cw00.choi@samsung.com
- Drop the patches related to opp_notifier
- Add new patch5/6/7/8

Chanwoo Choi (7):
  PM / devfreq: Set min/max_freq when adding the devfreq device
  Revert "PM / devfreq: Add show_one macro to delete the duplicate code"
  PM / devfreq: Use the available min/max frequency
  PM / devfreq: Change return type of devfreq_set_freq_table()
  PM / devfreq: Show the all available frequencies
  PM / devfreq: Remove unneeded conditional statement
  PM / devfreq: Define the constant governor name

 drivers/devfreq/devfreq.c                 | 139 +++++++++++++++++++++---------
 drivers/devfreq/exynos-bus.c              |   5 +-
 drivers/devfreq/governor_passive.c        |   2 +-
 drivers/devfreq/governor_performance.c    |   2 +-
 drivers/devfreq/governor_powersave.c      |   2 +-
 drivers/devfreq/governor_simpleondemand.c |   2 +-
 drivers/devfreq/governor_userspace.c      |   2 +-
 drivers/devfreq/rk3399_dmc.c              |   2 +-
 include/linux/devfreq.h                   |  16 +++-
 9 files changed, 123 insertions(+), 49 deletions(-)

--
1.9.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v5 1/7] PM / devfreq: Set min/max_freq when adding the devfreq device
       [not found]   ` <CGME20171023013214epcas1p4f0652eceade1e3812ed7781d60b18367@epcas1p4.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

Prior to that, the min/max_freq of the devfreq device are always zero
before the user changes the min/max_freq through sysfs entries.
It might make the confusion for the min/max_freq.

This patch initializes the available min/max_freq by using the OPP
during adding the devfreq device.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/devfreq/devfreq.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a1c4ee818614..6a6f88bccdee 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -69,6 +69,34 @@ static struct devfreq *find_device_devfreq(struct device *dev)
 	return ERR_PTR(-ENODEV);
 }
 
+static unsigned long find_available_min_freq(struct devfreq *devfreq)
+{
+	struct dev_pm_opp *opp;
+	unsigned long min_freq = 0;
+
+	opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &min_freq);
+	if (IS_ERR(opp))
+		min_freq = 0;
+	else
+		dev_pm_opp_put(opp);
+
+	return min_freq;
+}
+
+static unsigned long find_available_max_freq(struct devfreq *devfreq)
+{
+	struct dev_pm_opp *opp;
+	unsigned long max_freq = ULONG_MAX;
+
+	opp = dev_pm_opp_find_freq_floor(devfreq->dev.parent, &max_freq);
+	if (IS_ERR(opp))
+		max_freq = 0;
+	else
+		dev_pm_opp_put(opp);
+
+	return max_freq;
+}
+
 /**
  * devfreq_get_freq_level() - Lookup freq_table for the frequency
  * @devfreq:	the devfreq instance
@@ -559,6 +587,20 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		mutex_lock(&devfreq->lock);
 	}
 
+	devfreq->min_freq = find_available_min_freq(devfreq);
+	if (!devfreq->min_freq) {
+		mutex_unlock(&devfreq->lock);
+		err = -EINVAL;
+		goto err_dev;
+	}
+
+	devfreq->max_freq = find_available_max_freq(devfreq);
+	if (!devfreq->max_freq) {
+		mutex_unlock(&devfreq->lock);
+		err = -EINVAL;
+		goto err_dev;
+	}
+
 	dev_set_name(&devfreq->dev, "devfreq%d",
 				atomic_inc_return(&devfreq_no));
 	err = device_register(&devfreq->dev);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/7] Revert "PM / devfreq: Add show_one macro to delete the duplicate code"
       [not found]   ` <CGME20171023013214epcas1p341c10076a9dfdea26510c12fac0d723a@epcas1p3.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

This reverts commit 3104fa3081126c9bda35793af5f335d0ee0d5818.

The {min|max}_freq_show() show the stored value of the struct devfreq.
But, if the drivers/thermal/devfreq_cooling.c disables the specific
frequency value, {min|max}_freq_show() have to check this situation
before showing the stored value. So, this patch revert the macro
in order to add the additional codes.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 6a6f88bccdee..b6ba24e5db0d 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1124,6 +1124,12 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
+}
+
 static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -1150,17 +1156,13 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 	mutex_unlock(&df->lock);
 	return ret;
 }
+static DEVICE_ATTR_RW(min_freq);
 
-#define show_one(name)						\
-static ssize_t name##_show					\
-(struct device *dev, struct device_attribute *attr, char *buf)	\
-{								\
-	return sprintf(buf, "%lu\n", to_devfreq(dev)->name);	\
+static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
 }
-show_one(min_freq);
-show_one(max_freq);
-
-static DEVICE_ATTR_RW(min_freq);
 static DEVICE_ATTR_RW(max_freq);
 
 static ssize_t available_frequencies_show(struct device *d,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 3/7] PM / devfreq: Use the available min/max frequency
       [not found]   ` <CGME20171023013214epcas1p353a430282734319c904ea2edaa737dce@epcas1p3.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

The commit a76caf55e5b35 ("thermal: Add devfreq cooling") is able
to disable OPP as a cooling device. In result, both update_devfreq()
and {min|max}_freq_show() have to consider the 'opp->available'
status of each OPP.

So, this patch adds the 'scaling_{min|max}_freq' to struct devfreq
in order to indicate the available mininum and maximum frequency
by adjusting OPP interface such as dev_pm_opp_{disable|enable}().
The 'scaling_{min|max}_freq' are used for on both update_devfreq()
and {min|max}_freq_show().

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 40 ++++++++++++++++++++++++++++++++--------
 include/linux/devfreq.h   |  4 ++++
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6ba24e5db0d..ee3e7cee30b6 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -28,6 +28,9 @@
 #include <linux/of.h>
 #include "governor.h"
 
+#define MAX(a,b)	((a > b) ? a : b)
+#define MIN(a,b)	((a < b) ? a : b)
+
 static struct class *devfreq_class;
 
 /*
@@ -255,7 +258,7 @@ static int devfreq_notify_transition(struct devfreq *devfreq,
 int update_devfreq(struct devfreq *devfreq)
 {
 	struct devfreq_freqs freqs;
-	unsigned long freq, cur_freq;
+	unsigned long freq, cur_freq, min_freq, max_freq;
 	int err = 0;
 	u32 flags = 0;
 
@@ -273,19 +276,21 @@ int update_devfreq(struct devfreq *devfreq)
 		return err;
 
 	/*
-	 * Adjust the frequency with user freq and QoS.
+	 * Adjust the frequency with user freq, QoS and available freq.
 	 *
 	 * List from the highest priority
 	 * max_freq
 	 * min_freq
 	 */
+	max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
+	min_freq = MAX(devfreq->scaling_min_freq, devfreq->min_freq);
 
-	if (devfreq->min_freq && freq < devfreq->min_freq) {
-		freq = devfreq->min_freq;
+	if (min_freq && freq < min_freq) {
+		freq = min_freq;
 		flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
 	}
-	if (devfreq->max_freq && freq > devfreq->max_freq) {
-		freq = devfreq->max_freq;
+	if (max_freq && freq > max_freq) {
+		freq = max_freq;
 		flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
 	}
 
@@ -494,6 +499,19 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 	int ret;
 
 	mutex_lock(&devfreq->lock);
+
+	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
+	if (!devfreq->scaling_min_freq) {
+		mutex_unlock(&devfreq->lock);
+		return -EINVAL;
+	}
+
+	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
+	if (!devfreq->scaling_max_freq) {
+		mutex_unlock(&devfreq->lock);
+		return -EINVAL;
+	}
+
 	ret = update_devfreq(devfreq);
 	mutex_unlock(&devfreq->lock);
 
@@ -593,6 +611,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		err = -EINVAL;
 		goto err_dev;
 	}
+	devfreq->scaling_min_freq = devfreq->min_freq;
 
 	devfreq->max_freq = find_available_max_freq(devfreq);
 	if (!devfreq->max_freq) {
@@ -600,6 +619,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		err = -EINVAL;
 		goto err_dev;
 	}
+	devfreq->scaling_max_freq = devfreq->max_freq;
 
 	dev_set_name(&devfreq->dev, "devfreq%d",
 				atomic_inc_return(&devfreq_no));
@@ -1127,7 +1147,9 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
-	return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
+	struct devfreq *df = to_devfreq(dev);
+
+	return sprintf(buf, "%lu\n", MAX(df->scaling_min_freq, df->min_freq));
 }
 
 static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
@@ -1161,7 +1183,9 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
-	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
+	struct devfreq *df = to_devfreq(dev);
+
+	return sprintf(buf, "%lu\n", MIN(df->scaling_max_freq, df->max_freq));
 }
 static DEVICE_ATTR_RW(max_freq);
 
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 597294e0cc40..997a9eb34191 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -120,6 +120,8 @@ struct devfreq_dev_profile {
  *		touch this.
  * @min_freq:	Limit minimum frequency requested by user (0: none)
  * @max_freq:	Limit maximum frequency requested by user (0: none)
+ * @scaling_min_freq:	Limit minimum frequency requested by OPP interface
+ * @scaling_max_freq:	Limit maximum frequency requested by OPP interface
  * @stop_polling:	 devfreq polling status of a device.
  * @total_trans:	Number of devfreq transitions
  * @trans_table:	Statistics of devfreq transitions
@@ -153,6 +155,8 @@ struct devfreq {
 
 	unsigned long min_freq;
 	unsigned long max_freq;
+	unsigned long scaling_min_freq;
+	unsigned long scaling_max_freq;
 	bool stop_polling;
 
 	/* information for device frequency transition */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 4/7] PM / devfreq: Change return type of devfreq_set_freq_table()
       [not found]   ` <CGME20171023013215epcas1p3e43fa1b26d20fcb7d7d5d89fd28f9166@epcas1p3.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

This patch changes the return type of devfreq_set_freq_table()
from 'void' to 'int' in order to check whether it fails or not.

And This patch just removes the 'devfreq' prefix and the description
of function. Because the helper functions are only used by the devfreq.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/devfreq/devfreq.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ee3e7cee30b6..b2920cd2b78e 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -116,11 +116,7 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
 	return -EINVAL;
 }
 
-/**
- * devfreq_set_freq_table() - Initialize freq_table for the frequency
- * @devfreq:	the devfreq instance
- */
-static void devfreq_set_freq_table(struct devfreq *devfreq)
+static int set_freq_table(struct devfreq *devfreq)
 {
 	struct devfreq_dev_profile *profile = devfreq->profile;
 	struct dev_pm_opp *opp;
@@ -130,7 +126,7 @@ static void devfreq_set_freq_table(struct devfreq *devfreq)
 	/* Initialize the freq_table from OPP table */
 	count = dev_pm_opp_get_opp_count(devfreq->dev.parent);
 	if (count <= 0)
-		return;
+		return -EINVAL;
 
 	profile->max_state = count;
 	profile->freq_table = devm_kcalloc(devfreq->dev.parent,
@@ -139,7 +135,7 @@ static void devfreq_set_freq_table(struct devfreq *devfreq)
 					GFP_KERNEL);
 	if (!profile->freq_table) {
 		profile->max_state = 0;
-		return;
+		return -ENOMEM;
 	}
 
 	for (i = 0, freq = 0; i < profile->max_state; i++, freq++) {
@@ -147,11 +143,13 @@ static void devfreq_set_freq_table(struct devfreq *devfreq)
 		if (IS_ERR(opp)) {
 			devm_kfree(devfreq->dev.parent, profile->freq_table);
 			profile->max_state = 0;
-			return;
+			return PTR_ERR(opp);
 		}
 		dev_pm_opp_put(opp);
 		profile->freq_table[i] = freq;
 	}
+
+	return 0;
 }
 
 /**
@@ -601,7 +599,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
 		mutex_unlock(&devfreq->lock);
-		devfreq_set_freq_table(devfreq);
+		err = set_freq_table(devfreq);
+		if (err < 0)
+			goto err_out;
 		mutex_lock(&devfreq->lock);
 	}
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 5/7] PM / devfreq: Show the all available frequencies
       [not found]   ` <CGME20171023013215epcas2p1abdf1f948cc704f113d741e08d42a15b@epcas2p1.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

The commit a76caf55e5b35 ("thermal: Add devfreq cooling") allows
the devfreq device to use the cooling device. When the cooling down
are required, the devfreq_cooling.c disables the OPP entry with
the dev_pm_opp_disable(). In result, 'available_frequencies'[1]
sysfs node never came to show the all available frequencies.
[1] /sys/class/devfreq/.../available_frequencies

So, this patch uses the 'freq_table' in the 'struct devfreq_dev_profile'
in order to show the all available frequencies.
- If 'freq_table' is NULL, devfreq core initializes them by using OPP values.
- If 'freq_table' is initialized, devfreq core just uses the 'freq_table'.

And this patch adds some comment about the sort way of 'freq_table'.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/devfreq/devfreq.c | 16 +++++-----------
 include/linux/devfreq.h   |  5 +++--
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b2920cd2b78e..381f92e5e794 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1194,22 +1194,16 @@ static ssize_t available_frequencies_show(struct device *d,
 					  char *buf)
 {
 	struct devfreq *df = to_devfreq(d);
-	struct device *dev = df->dev.parent;
-	struct dev_pm_opp *opp;
 	ssize_t count = 0;
-	unsigned long freq = 0;
+	int i;
 
-	do {
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp))
-			break;
+	mutex_lock(&df->lock);
 
-		dev_pm_opp_put(opp);
+	for (i = 0; i < df->profile->max_state; i++)
 		count += scnprintf(&buf[count], (PAGE_SIZE - count - 2),
-				   "%lu ", freq);
-		freq++;
-	} while (1);
+				"%lu ", df->profile->freq_table[i]);
 
+	mutex_unlock(&df->lock);
 	/* Truncate the trailing space */
 	if (count)
 		count--;
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 997a9eb34191..19520625ea94 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -84,8 +84,9 @@ struct devfreq_dev_status {
  *			from devfreq_remove_device() call. If the user
  *			has registered devfreq->nb at a notifier-head,
  *			this is the time to unregister it.
- * @freq_table:	Optional list of frequencies to support statistics.
- * @max_state:	The size of freq_table.
+ * @freq_table:		Optional list of frequencies to support statistics
+ *			and freq_table must be generated in ascending order.
+ * @max_state:		The size of freq_table.
  */
 struct devfreq_dev_profile {
 	unsigned long initial_freq;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 6/7] PM / devfreq: Remove unneeded conditional statement
       [not found]   ` <CGME20171023013215epcas1p1d25606a511fb6dc2c037cc978ad5dd5a@epcas1p1.samsung.com>
@ 2017-10-23  1:32     ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm

The freq_table array of each devfreq device is always not NULL.
In result, it is unneeded to check whether profile->freq_table
is NULL or not.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 drivers/devfreq/devfreq.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 381f92e5e794..78fb496ecb4e 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -311,10 +311,9 @@ int update_devfreq(struct devfreq *devfreq)
 	freqs.new = freq;
 	devfreq_notify_transition(devfreq, &freqs, DEVFREQ_POSTCHANGE);
 
-	if (devfreq->profile->freq_table)
-		if (devfreq_update_status(devfreq, freq))
-			dev_err(&devfreq->dev,
-				"Couldn't update frequency transition information.\n");
+	if (devfreq_update_status(devfreq, freq))
+		dev_err(&devfreq->dev,
+			"Couldn't update frequency transition information.\n");
 
 	devfreq->previous_freq = freq;
 	return err;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 7/7] PM / devfreq: Define the constant governor name
       [not found]   ` <CGME20171023013216epcas2p4be3855bdb37dbaf4b5b9b9fdd6f65289@epcas2p4.samsung.com>
  2017-10-23  1:32       ` Chanwoo Choi
@ 2017-10-23  1:32       ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: rafael.j.wysocki, chanwoo, inki.dae, linux-kernel, linux-pm,
	Kukjin Kim, Krzysztof Kozlowski, linux-samsung-soc,
	linux-arm-kernel

Prior to that, the devfreq device uses the governor name when adding
the itself. In order to prevent the mistake used the wrong governor name,
this patch defines the governor name as a constant and then uses them
instead of using the string directly.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/devfreq/exynos-bus.c              | 5 +++--
 drivers/devfreq/governor_passive.c        | 2 +-
 drivers/devfreq/governor_performance.c    | 2 +-
 drivers/devfreq/governor_powersave.c      | 2 +-
 drivers/devfreq/governor_simpleondemand.c | 2 +-
 drivers/devfreq/governor_userspace.c      | 2 +-
 drivers/devfreq/rk3399_dmc.c              | 2 +-
 include/linux/devfreq.h                   | 7 +++++++
 8 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 49f68929e024..c25658b26598 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -436,7 +436,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	ondemand_data->downdifferential = 5;
 
 	/* Add devfreq device to monitor and handle the exynos bus */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "simple_ondemand",
+	bus->devfreq = devm_devfreq_add_device(dev, profile,
+						DEVFREQ_GOV_SIMPLE_ONDEMAND,
 						ondemand_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev, "failed to add devfreq device\n");
@@ -488,7 +489,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	passive_data->parent = parent_devfreq;
 
 	/* Add devfreq device for exynos bus with passive governor */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "passive",
+	bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
 						passive_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev,
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 673ad8cc9a1d..3bc29acbd54e 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -183,7 +183,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_passive = {
-	.name = "passive",
+	.name = DEVFREQ_GOV_PASSIVE,
 	.immutable = 1,
 	.get_target_freq = devfreq_passive_get_target_freq,
 	.event_handler = devfreq_passive_event_handler,
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index c72f942f30a8..4d23ecfbd948 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -42,7 +42,7 @@ static int devfreq_performance_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_performance = {
-	.name = "performance",
+	.name = DEVFREQ_GOV_PERFORMANCE,
 	.get_target_freq = devfreq_performance_func,
 	.event_handler = devfreq_performance_handler,
 };
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c6bed567e6d..0c42f23249ef 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -39,7 +39,7 @@ static int devfreq_powersave_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_powersave = {
-	.name = "powersave",
+	.name = DEVFREQ_GOV_POWERSAVE,
 	.get_target_freq = devfreq_powersave_func,
 	.event_handler = devfreq_powersave_handler,
 };
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5e78df..28e0f2de7100 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -125,7 +125,7 @@ static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_simple_ondemand = {
-	.name = "simple_ondemand",
+	.name = DEVFREQ_GOV_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 77028c27593c..080607c3f34d 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -87,7 +87,7 @@ static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
 	NULL,
 };
 static const struct attribute_group dev_attr_group = {
-	.name	= "userspace",
+	.name	= DEVFREQ_GOV_USERSPACE,
 	.attrs	= dev_entries,
 };
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 1b89ebbad02c..5dfbfa3cc878 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -431,7 +431,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 
 	data->devfreq = devm_devfreq_add_device(dev,
 					   &rk3399_devfreq_dmc_profile,
-					   "simple_ondemand",
+					   DEVFREQ_GOV_SIMPLE_ONDEMAND,
 					   &data->ondemand_data);
 	if (IS_ERR(data->devfreq))
 		return PTR_ERR(data->devfreq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 19520625ea94..3aae5b3af87c 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -19,6 +19,13 @@
 
 #define DEVFREQ_NAME_LEN 16
 
+/* DEVFREQ governor name */
+#define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
+#define DEVFREQ_GOV_PERFORMANCE		"performance"
+#define DEVFREQ_GOV_POWERSAVE		"powersave"
+#define DEVFREQ_GOV_USERSPACE		"userspace"
+#define DEVFREQ_GOV_PASSIVE		"passive"
+
 /* DEVFREQ notifier interface */
 #define DEVFREQ_TRANSITION_NOTIFIER	(0)
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 7/7] PM / devfreq: Define the constant governor name
@ 2017-10-23  1:32       ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: myungjoo.ham, kyungmin.park, cw00.choi
  Cc: linux-samsung-soc, linux-pm, rafael.j.wysocki, linux-kernel,
	Krzysztof Kozlowski, inki.dae, Kukjin Kim, chanwoo,
	linux-arm-kernel

Prior to that, the devfreq device uses the governor name when adding
the itself. In order to prevent the mistake used the wrong governor name,
this patch defines the governor name as a constant and then uses them
instead of using the string directly.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/devfreq/exynos-bus.c              | 5 +++--
 drivers/devfreq/governor_passive.c        | 2 +-
 drivers/devfreq/governor_performance.c    | 2 +-
 drivers/devfreq/governor_powersave.c      | 2 +-
 drivers/devfreq/governor_simpleondemand.c | 2 +-
 drivers/devfreq/governor_userspace.c      | 2 +-
 drivers/devfreq/rk3399_dmc.c              | 2 +-
 include/linux/devfreq.h                   | 7 +++++++
 8 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 49f68929e024..c25658b26598 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -436,7 +436,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	ondemand_data->downdifferential = 5;
 
 	/* Add devfreq device to monitor and handle the exynos bus */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "simple_ondemand",
+	bus->devfreq = devm_devfreq_add_device(dev, profile,
+						DEVFREQ_GOV_SIMPLE_ONDEMAND,
 						ondemand_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev, "failed to add devfreq device\n");
@@ -488,7 +489,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	passive_data->parent = parent_devfreq;
 
 	/* Add devfreq device for exynos bus with passive governor */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "passive",
+	bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
 						passive_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev,
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 673ad8cc9a1d..3bc29acbd54e 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -183,7 +183,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_passive = {
-	.name = "passive",
+	.name = DEVFREQ_GOV_PASSIVE,
 	.immutable = 1,
 	.get_target_freq = devfreq_passive_get_target_freq,
 	.event_handler = devfreq_passive_event_handler,
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index c72f942f30a8..4d23ecfbd948 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -42,7 +42,7 @@ static int devfreq_performance_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_performance = {
-	.name = "performance",
+	.name = DEVFREQ_GOV_PERFORMANCE,
 	.get_target_freq = devfreq_performance_func,
 	.event_handler = devfreq_performance_handler,
 };
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c6bed567e6d..0c42f23249ef 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -39,7 +39,7 @@ static int devfreq_powersave_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_powersave = {
-	.name = "powersave",
+	.name = DEVFREQ_GOV_POWERSAVE,
 	.get_target_freq = devfreq_powersave_func,
 	.event_handler = devfreq_powersave_handler,
 };
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5e78df..28e0f2de7100 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -125,7 +125,7 @@ static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_simple_ondemand = {
-	.name = "simple_ondemand",
+	.name = DEVFREQ_GOV_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 77028c27593c..080607c3f34d 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -87,7 +87,7 @@ static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
 	NULL,
 };
 static const struct attribute_group dev_attr_group = {
-	.name	= "userspace",
+	.name	= DEVFREQ_GOV_USERSPACE,
 	.attrs	= dev_entries,
 };
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 1b89ebbad02c..5dfbfa3cc878 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -431,7 +431,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 
 	data->devfreq = devm_devfreq_add_device(dev,
 					   &rk3399_devfreq_dmc_profile,
-					   "simple_ondemand",
+					   DEVFREQ_GOV_SIMPLE_ONDEMAND,
 					   &data->ondemand_data);
 	if (IS_ERR(data->devfreq))
 		return PTR_ERR(data->devfreq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 19520625ea94..3aae5b3af87c 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -19,6 +19,13 @@
 
 #define DEVFREQ_NAME_LEN 16
 
+/* DEVFREQ governor name */
+#define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
+#define DEVFREQ_GOV_PERFORMANCE		"performance"
+#define DEVFREQ_GOV_POWERSAVE		"powersave"
+#define DEVFREQ_GOV_USERSPACE		"userspace"
+#define DEVFREQ_GOV_PASSIVE		"passive"
+
 /* DEVFREQ notifier interface */
 #define DEVFREQ_TRANSITION_NOTIFIER	(0)
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 7/7] PM / devfreq: Define the constant governor name
@ 2017-10-23  1:32       ` Chanwoo Choi
  0 siblings, 0 replies; 15+ messages in thread
From: Chanwoo Choi @ 2017-10-23  1:32 UTC (permalink / raw)
  To: linux-arm-kernel

Prior to that, the devfreq device uses the governor name when adding
the itself. In order to prevent the mistake used the wrong governor name,
this patch defines the governor name as a constant and then uses them
instead of using the string directly.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-samsung-soc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/devfreq/exynos-bus.c              | 5 +++--
 drivers/devfreq/governor_passive.c        | 2 +-
 drivers/devfreq/governor_performance.c    | 2 +-
 drivers/devfreq/governor_powersave.c      | 2 +-
 drivers/devfreq/governor_simpleondemand.c | 2 +-
 drivers/devfreq/governor_userspace.c      | 2 +-
 drivers/devfreq/rk3399_dmc.c              | 2 +-
 include/linux/devfreq.h                   | 7 +++++++
 8 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 49f68929e024..c25658b26598 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -436,7 +436,8 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	ondemand_data->downdifferential = 5;
 
 	/* Add devfreq device to monitor and handle the exynos bus */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "simple_ondemand",
+	bus->devfreq = devm_devfreq_add_device(dev, profile,
+						DEVFREQ_GOV_SIMPLE_ONDEMAND,
 						ondemand_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev, "failed to add devfreq device\n");
@@ -488,7 +489,7 @@ static int exynos_bus_probe(struct platform_device *pdev)
 	passive_data->parent = parent_devfreq;
 
 	/* Add devfreq device for exynos bus with passive governor */
-	bus->devfreq = devm_devfreq_add_device(dev, profile, "passive",
+	bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
 						passive_data);
 	if (IS_ERR(bus->devfreq)) {
 		dev_err(dev,
diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c
index 673ad8cc9a1d..3bc29acbd54e 100644
--- a/drivers/devfreq/governor_passive.c
+++ b/drivers/devfreq/governor_passive.c
@@ -183,7 +183,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_passive = {
-	.name = "passive",
+	.name = DEVFREQ_GOV_PASSIVE,
 	.immutable = 1,
 	.get_target_freq = devfreq_passive_get_target_freq,
 	.event_handler = devfreq_passive_event_handler,
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index c72f942f30a8..4d23ecfbd948 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -42,7 +42,7 @@ static int devfreq_performance_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_performance = {
-	.name = "performance",
+	.name = DEVFREQ_GOV_PERFORMANCE,
 	.get_target_freq = devfreq_performance_func,
 	.event_handler = devfreq_performance_handler,
 };
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 0c6bed567e6d..0c42f23249ef 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -39,7 +39,7 @@ static int devfreq_powersave_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_powersave = {
-	.name = "powersave",
+	.name = DEVFREQ_GOV_POWERSAVE,
 	.get_target_freq = devfreq_powersave_func,
 	.event_handler = devfreq_powersave_handler,
 };
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index ae72ba5e78df..28e0f2de7100 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -125,7 +125,7 @@ static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
 }
 
 static struct devfreq_governor devfreq_simple_ondemand = {
-	.name = "simple_ondemand",
+	.name = DEVFREQ_GOV_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 77028c27593c..080607c3f34d 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -87,7 +87,7 @@ static ssize_t show_freq(struct device *dev, struct device_attribute *attr,
 	NULL,
 };
 static const struct attribute_group dev_attr_group = {
-	.name	= "userspace",
+	.name	= DEVFREQ_GOV_USERSPACE,
 	.attrs	= dev_entries,
 };
 
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 1b89ebbad02c..5dfbfa3cc878 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -431,7 +431,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 
 	data->devfreq = devm_devfreq_add_device(dev,
 					   &rk3399_devfreq_dmc_profile,
-					   "simple_ondemand",
+					   DEVFREQ_GOV_SIMPLE_ONDEMAND,
 					   &data->ondemand_data);
 	if (IS_ERR(data->devfreq))
 		return PTR_ERR(data->devfreq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 19520625ea94..3aae5b3af87c 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -19,6 +19,13 @@
 
 #define DEVFREQ_NAME_LEN 16
 
+/* DEVFREQ governor name */
+#define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
+#define DEVFREQ_GOV_PERFORMANCE		"performance"
+#define DEVFREQ_GOV_POWERSAVE		"powersave"
+#define DEVFREQ_GOV_USERSPACE		"userspace"
+#define DEVFREQ_GOV_PASSIVE		"passive"
+
 /* DEVFREQ notifier interface */
 #define DEVFREQ_TRANSITION_NOTIFIER	(0)
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [PATCH v5 3/7] PM / devfreq: Use the available min/max frequency
       [not found]   ` <CGME20171023013214epcas1p353a430282734319c904ea2edaa737dce@epcms1p6>
@ 2017-10-23  5:10     ` MyungJoo Ham
  0 siblings, 0 replies; 15+ messages in thread
From: MyungJoo Ham @ 2017-10-23  5:10 UTC (permalink / raw)
  To: Chanwoo Choi, Kyungmin Park
  Cc: rafael.j.wysocki, chanwoo, Inki Dae, linux-kernel, linux-pm

> The commit a76caf55e5b35 ("thermal: Add devfreq cooling") is able
> to disable OPP as a cooling device. In result, both update_devfreq()
> and {min|max}_freq_show() have to consider the 'opp->available'
> status of each OPP.
> 
> So, this patch adds the 'scaling_{min|max}_freq' to struct devfreq
> in order to indicate the available mininum and maximum frequency
> by adjusting OPP interface such as dev_pm_opp_{disable|enable}().
> The 'scaling_{min|max}_freq' are used for on both update_devfreq()
> and {min|max}_freq_show().
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
(along with the 2/7, revert commit)

> ---
>  drivers/devfreq/devfreq.c | 40 ++++++++++++++++++++++++++++++++--------
>  include/linux/devfreq.h   |  4 ++++
>  2 files changed, 36 insertions(+), 8 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency
  2017-10-23  1:32 ` [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
                     ` (7 preceding siblings ...)
       [not found]   ` <CGME20171023013214epcas1p353a430282734319c904ea2edaa737dce@epcms1p6>
@ 2017-11-06 18:11   ` Bjorn Andersson
       [not found]   ` <CGME20171023013213epcas1p165faa416f69659b60ec4c3f18ea251d8@epcms1p2>
  9 siblings, 0 replies; 15+ messages in thread
From: Bjorn Andersson @ 2017-11-06 18:11 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: MyungJoo Ham, Kyungmin Park, rafael.j.wysocki, chanwoo, inki.dae,
	lkml, Linux PM list, Vinayak Holikatti, Vivek Gautam

On Sun, Oct 22, 2017 at 6:32 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> These patches makes the devfreq to use the OPP interface and clean-up codes.
>

Hi Chanwoo,

This patch series breaks UFS support on modern Qualcomm platforms
(e.g. MSM8996).

Prior to this series the call to devm_devfreq_add_device() would
return ok even though the ufshcd device does not come with a opp
table. Afaict the code then relies on the devfreq code to target 0 and
MAX_INT (see ufshcd_devfreq_target()), to switch between a "high" and
"low" frequency.

I'm quite skeptical of the ufshcd code, so can you please confirm if
this usage of the devfreq interface is valid or not?

PS. If I do register some frequencies (and hack up
ufshcd_devfreq_target() accordingly) this works again, but the "high"
and "low" currently affects two clocks with different frequencies, so
I'm not sure how to properly describe the two clocks frequencies.
Should I just map "high"/"low" of the primary clock to "high"/"low"
frequencies of the secondary clock?

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Re: [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency
       [not found]   ` <CGME20171023013213epcas1p165faa416f69659b60ec4c3f18ea251d8@epcms1p2>
@ 2017-11-09 23:56       ` MyungJoo Ham
  0 siblings, 0 replies; 15+ messages in thread
From: MyungJoo Ham @ 2017-11-09 23:56 UTC (permalink / raw)
  To: Bjorn Andersson, Chanwoo Choi
  Cc: Kyungmin Park, rafael.j.wysocki, chanwoo, Inki Dae, lkml,
	Linux PM list, Vinayak Holikatti, Vivek Gautam

> On Sun, Oct 22, 2017 at 6:32 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> > These patches makes the devfreq to use the OPP interface and clean-up codes.
> >
>
> Hi Chanwoo,
>
> This patch series breaks UFS support on modern Qualcomm platforms
> (e.g. MSM8996).

Could you please show us the code location of it? 

Cheers,
MyungJoo 
 
--
MyungJoo Ham (함명주), Ph.D.
AI-3-Lab, AI Team, Samsung Research.
Samsung Electronics
Cell: +82-10-6714-2858

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Re: [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency
@ 2017-11-09 23:56       ` MyungJoo Ham
  0 siblings, 0 replies; 15+ messages in thread
From: MyungJoo Ham @ 2017-11-09 23:56 UTC (permalink / raw)
  To: Bjorn Andersson, Chanwoo Choi
  Cc: Kyungmin Park, rafael.j.wysocki, chanwoo, Inki Dae, lkml,
	Linux PM list, Vinayak Holikatti, Vivek Gautam

> On Sun, Oct 22, 2017 at 6:32 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> > These patches makes the devfreq to use the OPP interface and clean-up codes.
> >
>
> Hi Chanwoo,
>
> This patch series breaks UFS support on modern Qualcomm platforms
> (e.g. MSM8996).

Could you please show us the code location of it? 

Cheers,
MyungJoo 
 
--
MyungJoo Ham (함명주), Ph.D.
AI-3-Lab, AI Team, Samsung Research.
Samsung Electronics
Cell: +82-10-6714-2858

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency
  2017-11-09 23:56       ` MyungJoo Ham
  (?)
@ 2017-11-10  0:12       ` Bjorn Andersson
  -1 siblings, 0 replies; 15+ messages in thread
From: Bjorn Andersson @ 2017-11-10  0:12 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Chanwoo Choi, Kyungmin Park, rafael.j.wysocki, chanwoo, Inki Dae,
	lkml, Linux PM list, Vinayak Holikatti, Vivek Gautam

On Thu 09 Nov 15:56 PST 2017, MyungJoo Ham wrote:

> > On Sun, Oct 22, 2017 at 6:32 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> > > These patches makes the devfreq to use the OPP interface and clean-up codes.
> > >
> >
> > Hi Chanwoo,
> >
> > This patch series breaks UFS support on modern Qualcomm platforms
> > (e.g. MSM8996).
> 
> Could you please show us the code location of it? 
> 

The driver is drivers/scsi/ufs/ufshcd.c function ufshcd_probe_hba()
calls devm_devfreq_add_device() without there being an opp table
associated with hba->dev. This used to return fine and
ufshcd_devfreq_target() was called with *freq = UINT_MAX.

Unfortunately the driver seems to be designed in such a way that without
a call to ufshcd_devfreq_target() with *freq 0 or UINT_MAX some clock is
left off, causing my board to reboot; so there's unfortunately a few
levels of issues to fix here.

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-11-10  0:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20171023013213epcas1p165faa416f69659b60ec4c3f18ea251d8@epcas1p1.samsung.com>
2017-10-23  1:32 ` [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
     [not found]   ` <CGME20171023013214epcas1p4f0652eceade1e3812ed7781d60b18367@epcas1p4.samsung.com>
2017-10-23  1:32     ` [PATCH v5 1/7] PM / devfreq: Set min/max_freq when adding the devfreq device Chanwoo Choi
     [not found]   ` <CGME20171023013214epcas1p341c10076a9dfdea26510c12fac0d723a@epcas1p3.samsung.com>
2017-10-23  1:32     ` [PATCH v5 2/7] Revert "PM / devfreq: Add show_one macro to delete the duplicate code" Chanwoo Choi
     [not found]   ` <CGME20171023013214epcas1p353a430282734319c904ea2edaa737dce@epcas1p3.samsung.com>
2017-10-23  1:32     ` [PATCH v5 3/7] PM / devfreq: Use the available min/max frequency Chanwoo Choi
     [not found]   ` <CGME20171023013215epcas1p3e43fa1b26d20fcb7d7d5d89fd28f9166@epcas1p3.samsung.com>
2017-10-23  1:32     ` [PATCH v5 4/7] PM / devfreq: Change return type of devfreq_set_freq_table() Chanwoo Choi
     [not found]   ` <CGME20171023013215epcas2p1abdf1f948cc704f113d741e08d42a15b@epcas2p1.samsung.com>
2017-10-23  1:32     ` [PATCH v5 5/7] PM / devfreq: Show the all available frequencies Chanwoo Choi
     [not found]   ` <CGME20171023013215epcas1p1d25606a511fb6dc2c037cc978ad5dd5a@epcas1p1.samsung.com>
2017-10-23  1:32     ` [PATCH v5 6/7] PM / devfreq: Remove unneeded conditional statement Chanwoo Choi
     [not found]   ` <CGME20171023013216epcas2p4be3855bdb37dbaf4b5b9b9fdd6f65289@epcas2p4.samsung.com>
2017-10-23  1:32     ` [PATCH v5 7/7] PM / devfreq: Define the constant governor name Chanwoo Choi
2017-10-23  1:32       ` Chanwoo Choi
2017-10-23  1:32       ` Chanwoo Choi
     [not found]   ` <CGME20171023013214epcas1p353a430282734319c904ea2edaa737dce@epcms1p6>
2017-10-23  5:10     ` [PATCH v5 3/7] PM / devfreq: Use the available min/max frequency MyungJoo Ham
2017-11-06 18:11   ` [PATCH v5 0/7] PM / devfreq: Use OPP interface to handle the frequency Bjorn Andersson
     [not found]   ` <CGME20171023013213epcas1p165faa416f69659b60ec4c3f18ea251d8@epcms1p2>
2017-11-09 23:56     ` MyungJoo Ham
2017-11-09 23:56       ` MyungJoo Ham
2017-11-10  0:12       ` Bjorn Andersson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.