* [PATCH v4 1/8] PM / devfreq: Set min/max_freq when adding the devfreq device
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 2/8] Revert "PM / devfreq: Add show_one macro to delete the duplicate code" Chanwoo Choi
` (6 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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.
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Chanwoo Choi <cw00.choi@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] 18+ messages in thread* [PATCH v4 2/8] Revert "PM / devfreq: Add show_one macro to delete the duplicate code"
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 1/8] PM / devfreq: Set min/max_freq when adding the devfreq device Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency Chanwoo Choi
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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] 18+ messages in thread* [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 1/8] PM / devfreq: Set min/max_freq when adding the devfreq device Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 2/8] Revert "PM / devfreq: Add show_one macro to delete the duplicate code" Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 14:43 ` MyungJoo Ham
2017-10-13 7:48 ` [PATCH v4 4/8] PM / devfreq: Change return type of devfreq_set_freq_table() Chanwoo Choi
` (4 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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 | 36 ++++++++++++++++++++++++++++--------
include/linux/devfreq.h | 4 ++++
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6ba24e5db0d..9de013ffeb67 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->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,7 @@ 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);
+ return sprintf(buf, "%lu\n", to_devfreq(dev)->scaling_min_freq);
}
static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
@@ -1161,7 +1181,7 @@ 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);
+ return sprintf(buf, "%lu\n", to_devfreq(dev)->scaling_max_freq);
}
static DEVICE_ATTR_RW(max_freq);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 597294e0cc40..fe7862060945 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 devfreq-cooling
+ * @scaling_max_freq: Limit maximum frequency requested by devfreq-cooling
* @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] 18+ messages in thread* Re: [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency
2017-10-13 7:48 ` [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency Chanwoo Choi
@ 2017-10-17 14:43 ` MyungJoo Ham
2017-10-18 1:31 ` Chanwoo Choi
0 siblings, 1 reply; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 14:43 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 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 | 36 ++++++++++++++++++++++++++++--------
> include/linux/devfreq.h | 4 ++++
> 2 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index b6ba24e5db0d..9de013ffeb67 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
[]
> @@ -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->max_freq) {
1. s/max_freq/scaling/max_freq/ ??
2. what if thermal is not active or has never triggered any event and
the user has never stated max/min? (making scaling_*_freq zero)
> + mutex_unlock(&devfreq->lock);
> + return -EINVAL;
> + }
> +
> ret = update_devfreq(devfreq);
> mutex_unlock(&devfreq->lock);
>
--
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency
2017-10-17 14:43 ` MyungJoo Ham
@ 2017-10-18 1:31 ` Chanwoo Choi
2017-10-18 2:12 ` Chanwoo Choi
0 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-18 1:31 UTC (permalink / raw)
To: myungjoo.ham
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
Hi,
On 2017년 10월 17일 23:43, MyungJoo Ham wrote:
> On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> 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 | 36 ++++++++++++++++++++++++++++--------
>> include/linux/devfreq.h | 4 ++++
>> 2 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index b6ba24e5db0d..9de013ffeb67 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
> []
>> @@ -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->max_freq) {
>
> 1. s/max_freq/scaling/max_freq/ ??
My mistake. The scaling_max_freq is right. I'll fix it.
>
> 2. what if thermal is not active or has never triggered any event and
> the user has never stated max/min? (making scaling_*_freq zero)
The devfreq-cooling.c of tmu uses the OPP interface
and then OPP interface affect the scaling_min/max_freq of devfreq
through dev_pm_opp_disable/enable(). So, even if 'thermal is not active
or has never triggered any event', devfreq will use the OPP interface
as a mandatory.
In result, I think that devfreq should maintain the correct frequency
of scaling_min/max_freq indicating the 'limit minimum/maximum frequency
requested by OPP interface' instead of zero.
So, I'll change the description of scaling_min/max_freq as following:
(by devfreq-cooling -> by OPP interface)
On v4:
+ * @scaling_min_freq: Limit minimum frequency requested by devfreq-cooling
+ * @scaling_max_freq: Limit maximum frequency requested by devfreq-cooling
On v5:
+ * @scaling_min_freq: Limit minimum frequency requested by OPP interface
+ * @scaling_max_freq: Limit maximum frequency requested by OPP interface
And, this patch showed the wrong value of min/max_freq_show() by my mistake.
I showed the 'min/max_freq' directly through min/max_freq_show()
without comparing with scaling_min/max_freq. So, I'll fix this issue as following:
---------------
On v5:
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));
---------------
>
>> + mutex_unlock(&devfreq->lock);
>> + return -EINVAL;
>> + }
>> +
>> ret = update_devfreq(devfreq);
>> mutex_unlock(&devfreq->lock);
>>
>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency
2017-10-18 1:31 ` Chanwoo Choi
@ 2017-10-18 2:12 ` Chanwoo Choi
0 siblings, 0 replies; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-18 2:12 UTC (permalink / raw)
To: myungjoo.ham
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
On 2017년 10월 18일 10:31, Chanwoo Choi wrote:
> Hi,
>
> On 2017년 10월 17일 23:43, MyungJoo Ham wrote:
>> On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>> 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 | 36 ++++++++++++++++++++++++++++--------
>>> include/linux/devfreq.h | 4 ++++
>>> 2 files changed, 32 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index b6ba24e5db0d..9de013ffeb67 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>> []
>>> @@ -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->max_freq) {
>>
>> 1. s/max_freq/scaling/max_freq/ ??
>
> My mistake. The scaling_max_freq is right. I'll fix it.
>
>>
>> 2. what if thermal is not active or has never triggered any event and
>> the user has never stated max/min? (making scaling_*_freq zero)
>
>
> The devfreq-cooling.c of tmu uses the OPP interface
> and then OPP interface affect the scaling_min/max_freq of devfreq
> through dev_pm_opp_disable/enable(). So, even if 'thermal is not active
> or has never triggered any event', devfreq will use the OPP interface
> as a mandatory.
>
> In result, I think that devfreq should maintain the correct frequency
> of scaling_min/max_freq indicating the 'limit minimum/maximum frequency
> requested by OPP interface' instead of zero.
>
> So, I'll change the description of scaling_min/max_freq as following:
> (by devfreq-cooling -> by OPP interface)
> On v4:
> + * @scaling_min_freq: Limit minimum frequency requested by devfreq-cooling
> + * @scaling_max_freq: Limit maximum frequency requested by devfreq-cooling
>
> On v5:
> + * @scaling_min_freq: Limit minimum frequency requested by OPP interface
> + * @scaling_max_freq: Limit maximum frequency requested by OPP interface
>
>
> And, this patch showed the wrong value of min/max_freq_show() by my mistake.
> I showed the 'min/max_freq' directly through min/max_freq_show()
> without comparing with scaling_min/max_freq. So, I'll fix this issue as following:
> ---------------
> On v5:
> 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));
> ---------------
If you agree my opinion, I'll send v5 patchset right now
because if patch3 gets the review, everything is done without patch8.
As I replied, I'll drop the patch8 from this patchset.
>
>
>>
>>> + mutex_unlock(&devfreq->lock);
>>> + return -EINVAL;
>>> + }
>>> +
>>> ret = update_devfreq(devfreq);
>>> mutex_unlock(&devfreq->lock);
>>>
>>
>>
>>
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 4/8] PM / devfreq: Change return type of devfreq_set_freq_table()
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
` (2 preceding siblings ...)
2017-10-13 7:48 ` [PATCH v4 3/8] PM / devfreq: Use the available min/max frequency Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 14:45 ` MyungJoo Ham
2017-10-13 7:48 ` [PATCH v4 5/8] PM / devfreq: Show the all available frequencies Chanwoo Choi
` (3 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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>
---
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 9de013ffeb67..909cedef7caa 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] 18+ messages in thread* Re: [PATCH v4 4/8] PM / devfreq: Change return type of devfreq_set_freq_table()
2017-10-13 7:48 ` [PATCH v4 4/8] PM / devfreq: Change return type of devfreq_set_freq_table() Chanwoo Choi
@ 2017-10-17 14:45 ` MyungJoo Ham
0 siblings, 0 replies; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 14:45 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 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(-)
>
[]
--
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 5/8] PM / devfreq: Show the all available frequencies
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
` (3 preceding siblings ...)
2017-10-13 7:48 ` [PATCH v4 4/8] PM / devfreq: Change return type of devfreq_set_freq_table() Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 14:50 ` MyungJoo Ham
2017-10-13 7:48 ` [PATCH v4 6/8] PM / devfreq: Remove unneeded conditional statement Chanwoo Choi
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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>
---
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 909cedef7caa..534ead60d1cc 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1190,22 +1190,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 fe7862060945..5dc6190e479c 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] 18+ messages in thread* Re: [PATCH v4 5/8] PM / devfreq: Show the all available frequencies
2017-10-13 7:48 ` [PATCH v4 5/8] PM / devfreq: Show the all available frequencies Chanwoo Choi
@ 2017-10-17 14:50 ` MyungJoo Ham
0 siblings, 0 replies; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 14:50 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 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 909cedef7caa..534ead60d1cc 100644
[]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 6/8] PM / devfreq: Remove unneeded conditional statement
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
` (4 preceding siblings ...)
2017-10-13 7:48 ` [PATCH v4 5/8] PM / devfreq: Show the all available frequencies Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 14:59 ` MyungJoo Ham
2017-10-13 7:48 ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
2017-10-13 7:48 ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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>
---
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 534ead60d1cc..191d4de7bd87 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] 18+ messages in thread* Re: [PATCH v4 6/8] PM / devfreq: Remove unneeded conditional statement
2017-10-13 7:48 ` [PATCH v4 6/8] PM / devfreq: Remove unneeded conditional statement Chanwoo Choi
@ 2017-10-17 14:59 ` MyungJoo Ham
0 siblings, 0 replies; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 14:59 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 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(-)
[]
--
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 7/8] PM / devfreq: Define the constant governor name
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
` (5 preceding siblings ...)
2017-10-13 7:48 ` [PATCH v4 6/8] PM / devfreq: Remove unneeded conditional statement Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 15:02 ` MyungJoo Ham
2017-10-13 7:48 ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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>
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 5dc6190e479c..682419a32aa3 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] 18+ messages in thread* Re: [PATCH v4 7/8] PM / devfreq: Define the constant governor name
2017-10-13 7:48 ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
@ 2017-10-17 15:02 ` MyungJoo Ham
0 siblings, 0 replies; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 15:02 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list, Kukjin Kim,
Krzysztof Kozlowski, Linux Samsung SoC, linux-arm-kernel
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> 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>
> 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
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
> 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(-)
>
[]
--
MyungJoo Ham, Ph.D.
S/W Center, Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
2017-10-13 7:48 ` [PATCH v4 0/8] PM / devfreq: Use OPP interface to handle the frequency Chanwoo Choi
` (6 preceding siblings ...)
2017-10-13 7:48 ` [PATCH v4 7/8] PM / devfreq: Define the constant governor name Chanwoo Choi
@ 2017-10-13 7:48 ` Chanwoo Choi
2017-10-17 15:11 ` MyungJoo Ham
7 siblings, 1 reply; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-13 7:48 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
This patch registers the Exynos Bus-Frequency scaling device
as a cooling device of thermal management.
Signed-off-by: Chanwoo Choi <cw00.choi@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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index c25658b26598..1c7521b65c2f 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -15,6 +15,7 @@
#include <linux/clk.h>
#include <linux/devfreq.h>
#include <linux/devfreq-event.h>
+#include <linux/devfreq_cooling.h>
#include <linux/device.h>
#include <linux/export.h>
#include <linux/module.h>
@@ -41,6 +42,8 @@ struct exynos_bus {
struct clk *clk;
unsigned int voltage_tolerance;
unsigned int ratio;
+
+ struct thermal_cooling_device *cdev;
};
/*
@@ -468,6 +471,19 @@ static int exynos_bus_probe(struct platform_device *pdev)
goto err;
}
+ /*
+ * Register devfreq cooling device if thermal DT code
+ * takes care of matching them.
+ */
+ if (of_find_property(np, "#cooling-cells", NULL)) {
+ bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
+ if (IS_ERR(bus->cdev)) {
+ dev_err(dev, "running exynos-bus without cooling device\n");
+ bus->cdev = NULL;
+ }
+ }
+ of_node_put(np);
+
goto out;
passive:
/* Initialize the struct profile and governor data for passive device */
@@ -514,6 +530,16 @@ static int exynos_bus_probe(struct platform_device *pdev)
return ret;
}
+static int exynos_bus_remove(struct platform_device *pdev)
+{
+ struct exynos_bus *bus = platform_get_drvdata(pdev);
+
+ if (bus->cdev)
+ devfreq_cooling_unregister(bus->cdev);
+
+ return 0;
+}
+
#ifdef CONFIG_PM_SLEEP
static int exynos_bus_resume(struct device *dev)
{
@@ -556,6 +582,7 @@ static int exynos_bus_suspend(struct device *dev)
static struct platform_driver exynos_bus_platdrv = {
.probe = exynos_bus_probe,
+ .remove = exynos_bus_remove,
.driver = {
.name = "exynos-bus",
.pm = &exynos_bus_pm,
--
1.9.1
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
2017-10-13 7:48 ` [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device Chanwoo Choi
@ 2017-10-17 15:11 ` MyungJoo Ham
2017-10-18 2:08 ` Chanwoo Choi
0 siblings, 1 reply; 18+ messages in thread
From: MyungJoo Ham @ 2017-10-17 15:11 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list, Kukjin Kim,
Krzysztof Kozlowski, Linux Samsung SoC, linux-arm-kernel
On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch registers the Exynos Bus-Frequency scaling device
> as a cooling device of thermal management.
>
> Signed-off-by: Chanwoo Choi <cw00.choi@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
I've got a question below.
> ---
> drivers/devfreq/exynos-bus.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index c25658b26598..1c7521b65c2f 100644
[]> @@ -468,6 +471,19 @@ static int exynos_bus_probe(struct
platform_device *pdev)
> goto err;
> }
>
> + /*
> + * Register devfreq cooling device if thermal DT code
> + * takes care of matching them.
> + */
> + if (of_find_property(np, "#cooling-cells", NULL)) {
> + bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
> + if (IS_ERR(bus->cdev)) {
> + dev_err(dev, "running exynos-bus without cooling device\n");
> + bus->cdev = NULL;
> + }
> + }
> + of_node_put(np);
Is this of_node_put() is a pair of of_find_property? or for something else?
(do you need to call put for of_find_property? or for something else?
I'm not seeing a function with "get")
Cheers,
MyungJoo
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v4 8/8] PM / devfreq: exynos-bus: Register cooling device
2017-10-17 15:11 ` MyungJoo Ham
@ 2017-10-18 2:08 ` Chanwoo Choi
0 siblings, 0 replies; 18+ messages in thread
From: Chanwoo Choi @ 2017-10-18 2:08 UTC (permalink / raw)
To: myungjoo.ham
Cc: Kyungmin Park, Rafael J . Wysocki, chanwoo,
대인기, LKML, Linux PM list, Kukjin Kim,
Krzysztof Kozlowski, Linux Samsung SoC, linux-arm-kernel
Hi,
On 2017년 10월 18일 00:11, MyungJoo Ham wrote:
> On Fri, Oct 13, 2017 at 4:48 PM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch registers the Exynos Bus-Frequency scaling device
>> as a cooling device of thermal management.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@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
>
> I've got a question below.
>
>> ---
>> drivers/devfreq/exynos-bus.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
>> index c25658b26598..1c7521b65c2f 100644
> []> @@ -468,6 +471,19 @@ static int exynos_bus_probe(struct
> platform_device *pdev)
>> goto err;
>> }
>>
>> + /*
>> + * Register devfreq cooling device if thermal DT code
>> + * takes care of matching them.
>> + */
>> + if (of_find_property(np, "#cooling-cells", NULL)) {
>> + bus->cdev = of_devfreq_cooling_register(np, bus->devfreq);
>> + if (IS_ERR(bus->cdev)) {
>> + dev_err(dev, "running exynos-bus without cooling device\n");
>> + bus->cdev = NULL;
>> + }
>> + }
>> + of_node_put(np);
>
> Is this of_node_put() is a pair of of_find_property? or for something else?
> (do you need to call put for of_find_property? or for something else?
> I'm not seeing a function with "get")
You're right. The of_node_put(np) call is unneeded.
Actually, the extcon-bus.c have to use the of_node_get instead of accessing
the 'dev->of_node' directly.
And, when I test this patch, I got a bug related to passive governor.
I need more time for debugging and redevelopment.
So, I'll drop this patch on next patchset (v5).
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread