[PATCH 00/12] PM / devfreq: Use OPP interface to handle the frequency

[PATCH 00/12] PM / devfreq: Use OPP interface to handle the frequency

[Chanwoo Choi]

These patches makes the devfreq to use the OPP interface as a mandatory
and fix the bug, clean-up codes.
- patch 1/2 fix the bug.
- patch 3 refactors the private code.
- patch 4~12 are related to the OPP interfaces 

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 use the OPP interface when showing
the minimum and maximum 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. So, these
patches fix this issue.
- /sys/class/devfreq/devfreq.X/min_freq
- /sys/class/devfreq/devfreq.X/max_freq

Second, the 'available_frequencies' should show the all supported frequencis
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/devfreq.X/available_frequencies

Lastly, the devfreq uses the OPP interface as a mandatory method
and so the devfreq has to support the OPP notifier in order to
catch the OPP events. So, patch12 adds the new opp_notifier_cb()
function pointer in the struct devfreq_dev_profile. The user can
add the their own callback function to receive the OPP events.

Also, the devfreq provides the default OPP notifeir callback
in order to remake the frequency table when OPP events happen.
- default_opp_notifier_cb()

Chanwoo Choi (12):
  PM / devfreq: Fix memory leak when fail to register device
  PM / devfreq: Fix locking range for making the frequency table
  PM / devfreq: Move private devfreq_update_stats() into devfreq
  PM / devfreq: Add dependency on PM_OPP
  PM / devfreq: Set min/max_freq when adding the devfreq device
  PM / devfreq: Check the entered min/max_freq is supported or not
  Revert "PM / devfreq: Add show_one macro to delete the duplicate code"
  PM / devfreq: Show the available min/max frequency through sysfs node
  PM / devfreq: Show whole available frequencies
  PM / devfreq: Remove 'devfreq' prefix from helper function
  PM / devfreq: Remove exported opp_notifier function
  PM / devfreq: Add opp_notifier_cb() function pointer to support OPP
    notifier

 drivers/devfreq/Kconfig      |   1 +
 drivers/devfreq/devfreq.c    | 286 +++++++++++++++++++++++++------------------
 drivers/devfreq/exynos-bus.c |   7 --
 drivers/devfreq/governor.h   |   4 +
 drivers/devfreq/rk3399_dmc.c |   1 -
 include/linux/devfreq.h      |  56 ++-------
 6 files changed, 181 insertions(+), 174 deletions(-)

-- 
1.9.1

[PATCH 01/12] PM / devfreq: Fix memory leak when fail to register device

[Chanwoo Choi]

When the devfreq_add_device fails to register deivce, the memory
leak of devfreq instance happen. So, this patch fix the memory
leak issue. Before freeing the devfreq instance checks whether
devfreq instance is NULL or not because the device_unregister()
frees the devfreq instance when jumping to the 'err_init'.
It is to prevent the duplicate the kfee(devfreq).

Cc: stable@vger.kernel.org
Fixes: ac4b281176a5 ("PM / devfreq: fix duplicated kfree on devfreq pointer")
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index dea04871b50d..a1c4ee818614 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -564,7 +564,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	err = device_register(&devfreq->dev);
 	if (err) {
 		mutex_unlock(&devfreq->lock);
-		goto err_out;
+		goto err_dev;
 	}
 
 	devfreq->trans_table =	devm_kzalloc(&devfreq->dev,
@@ -610,6 +610,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	mutex_unlock(&devfreq_list_lock);
 
 	device_unregister(&devfreq->dev);
+err_dev:
+	if (devfreq)
+		kfree(devfreq);
 err_out:
 	return ERR_PTR(err);
 }
-- 
1.9.1

[PATCH 02/12] PM / devfreq: Fix locking range for making the frequency table

[Chanwoo Choi]

Even if the freq_table is included in the struct devfreq,
the commit 0ec09ac2cebe ("PM / devfreq: Set the freq_table
of devfreq device") set the frequency table outside the mutex locking.

So, this patch initializes the frequency table within the mutex locking.

Cc: stable@vger.kernel.org
Fixes: 0ec09ac2cebe ("PM / devfreq: Set the freq_table of devfreq device")
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a1c4ee818614..3c5ccb96e165 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -553,11 +553,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->data = data;
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
-	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
-		mutex_unlock(&devfreq->lock);
+	if (!devfreq->profile->max_state && !devfreq->profile->freq_table)
 		devfreq_set_freq_table(devfreq);
-		mutex_lock(&devfreq->lock);
-	}
 
 	dev_set_name(&devfreq->dev, "devfreq%d",
 				atomic_inc_return(&devfreq_no));
-- 
1.9.1

[PATCH 03/12] PM / devfreq: Move private devfreq_update_stats() into devfreq

[Chanwoo Choi]

From: Chanwoo Choi <cwchoi00@gmail.com>

THe devfreq_update_stats() updates the 'struct devfreq_dev_status'
in order to get current status of devfreq device. It is only used
for the governors.

This patch moves the devfreq_update_stats() into devfreq directory.

Signed-off-by: Chanwoo Choi <cwchoi00@gmail.com>
---
 drivers/devfreq/governor.h |  4 ++++
 include/linux/devfreq.h    | 13 -------------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index a4f2fa1091e4..cfc50a61a90d 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -69,4 +69,8 @@ extern void devfreq_interval_update(struct devfreq *devfreq,
 
 extern int devfreq_update_status(struct devfreq *devfreq, unsigned long freq);
 
+static inline int devfreq_update_stats(struct devfreq *df)
+{
+	return df->profile->get_dev_status(df->dev.parent, &df->last_status);
+}
 #endif /* _GOVERNOR_H */
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 6c220e4ebb6b..597294e0cc40 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -214,19 +214,6 @@ extern void devm_devfreq_unregister_notifier(struct device *dev,
 extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev,
 						int index);
 
-/**
- * devfreq_update_stats() - update the last_status pointer in struct devfreq
- * @df:		the devfreq instance whose status needs updating
- *
- *  Governors are recommended to use this function along with last_status,
- * which allows other entities to reuse the last_status without affecting
- * the values fetched later by governors.
- */
-static inline int devfreq_update_stats(struct devfreq *df)
-{
-	return df->profile->get_dev_status(df->dev.parent, &df->last_status);
-}
-
 #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)
 /**
  * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
-- 
1.9.1

[PATCH 04/12] PM / devfreq: Add dependency on PM_OPP

[Chanwoo Choi]

The devfreq ues the OPP library to handle the voltage and frequency
for the device basically. This patch adds the dependency on CONFIG_PM_OPP
in order to prevent either the build break or the unknow behavior.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 41254e702f1e..6a172d338f6d 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -1,6 +1,7 @@
 menuconfig PM_DEVFREQ
 	bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
 	select SRCU
+	select PM_OPP
 	help
 	  A device may have a list of frequencies and voltages available.
 	  devfreq, a generic DVFS framework can be registered for a device
-- 
1.9.1

[PATCH 05/12] PM / devfreq: Set min/max_freq when adding the devfreq device

[Chanwoo Choi]

Previously, the min/max_freq of devfreq device are always zero
before the user changes the min/max_freq through sysfs entry.
It might make the confusion of min/max_freq.

This patch initializes the available min/max_freq by using the OPP API.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 3c5ccb96e165..56f8a0473834 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;
+
+	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;
+
+	dev_pm_opp_put(opp);
+
+	return max_freq;
+}
+
 /**
  * devfreq_get_freq_level() - Lookup freq_table for the frequency
  * @devfreq:	the devfreq instance
@@ -556,6 +584,21 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	if (!devfreq->profile->max_state && !devfreq->profile->freq_table)
 		devfreq_set_freq_table(devfreq);
 
+	/* Set the scaling available min_freq and max_freq */
+	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

[PATCH 06/12] PM / devfreq: Check the entered min/max_freq is supported or not

[Chanwoo Choi]

The existing {min|max}_freq_store() only check the range of
entered frequency. If the unsupported frequency is entered
and the entered frequency is within the ranges, {min|max}_freq_store()
will store it to devfreq->{min|max}_freq. And then when some user
try to read the {min|max}_freq, the devfreq show the unsupported
frequency value.

In order to fix this issue, this patch checks whether the entered
frequency is supported or not by OPP interface.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 56f8a0473834..f10017fe400f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -97,6 +97,25 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
 	return max_freq;
 }
 
+static int is_supported_freq(struct devfreq *devfreq, unsigned long freq)
+{
+	struct device *dev = devfreq->dev.parent;
+	struct dev_pm_opp *opp;
+	int ret = 0;
+
+	/* Check whether entered frequency is supported or not */
+	opp = dev_pm_opp_find_freq_exact(dev, freq, true);
+	if (PTR_ERR(opp) == -ERANGE)
+		opp = dev_pm_opp_find_freq_exact(dev, freq, false);
+
+	if (IS_ERR(opp))
+		ret = PTR_ERR(opp);
+
+	dev_pm_opp_put(opp);
+
+	return ret;
+}
+
 /**
  * devfreq_get_freq_level() - Lookup freq_table for the frequency
  * @devfreq:	the devfreq instance
@@ -1099,9 +1118,8 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
 	struct devfreq *df = to_devfreq(dev);
-	unsigned long value;
+	unsigned long value, max;
 	int ret;
-	unsigned long max;
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
@@ -1114,6 +1132,11 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 		goto unlock;
 	}
 
+	/* Check whether entered frequency is supported or not */
+	ret = is_supported_freq(df, value);
+	if (ret < 0)
+		goto unlock;
+
 	df->min_freq = value;
 	update_devfreq(df);
 	ret = count;
@@ -1126,9 +1149,8 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
 	struct devfreq *df = to_devfreq(dev);
-	unsigned long value;
+	unsigned long value, min;
 	int ret;
-	unsigned long min;
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
@@ -1141,6 +1163,11 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 		goto unlock;
 	}
 
+	/* Check whether entered frequency is supported or not */
+	ret = is_supported_freq(df, value);
+	if (ret < 0)
+		goto unlock;
+
 	df->max_freq = value;
 	update_devfreq(df);
 	ret = count;
-- 
1.9.1

[PATCH 07/12] Revert "PM / devfreq: Add show_one macro to delete the duplicate code"

[Chanwoo Choi]

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 f10017fe400f..be3d42f140ff 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1145,6 +1145,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)
 {
@@ -1175,17 +1181,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

[PATCH 08/12] PM / devfreq: Show the available min/max frequency through sysfs node

[Chanwoo Choi]

The existing {min|max}_freq sysfs nodes don't consider whether min/max_freq
are available or not. Those sysfs nodes show just the stored value
in the struct devfreq.

The devfreq uses the OPP interface and then dev_pm_opp_{disable|add}()
might change the state of the device's supported frequency. This patch
shows the available minimum and maximum frequency through sysfs node.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index be3d42f140ff..8b0bbfa0a50b 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1148,7 +1148,14 @@ 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);
+	unsigned long min_freq = to_devfreq(dev)->min_freq;
+	unsigned long available_min_freq = find_available_min_freq(df);
+
+	if (available_min_freq != 0 && min_freq < available_min_freq)
+		min_freq = available_min_freq;
+
+	return sprintf(buf, "%lu\n", min_freq);
 }
 
 static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
@@ -1186,7 +1193,14 @@ 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);
+	unsigned long max_freq = to_devfreq(dev)->max_freq;
+	unsigned long available_max_freq = find_available_max_freq(df);
+
+	if (available_max_freq != 0 && max_freq > available_max_freq)
+		max_freq = available_max_freq;
+
+	return sprintf(buf, "%lu\n", max_freq);
 }
 static DEVICE_ATTR_RW(max_freq);
 
-- 
1.9.1

[PATCH 09/12] PM / devfreq: Show whole available frequencies

[Chanwoo Choi]

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 whole 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 whole 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'.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 8b0bbfa0a50b..d8ff16419452 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1209,21 +1209,12 @@ 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;
-
-	do {
-		opp = dev_pm_opp_find_freq_ceil(dev, &freq);
-		if (IS_ERR(opp))
-			break;
+	int i;
 
-		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]);
 
 	/* Truncate the trailing space */
 	if (count)
-- 
1.9.1

[PATCH 10/12] PM / devfreq: Remove 'devfreq' prefix from helper function

[Chanwoo Choi]

This patch just removes the 'devfreq' prefix from internal helper
function in order to clarify the role of the following functions.
- devfreq_get_freq_level() - get_freq_level()
- devfreq_set_freq_table() - set_freq_table()

Also, this patch changes the return value of set_freq_table()
from 'void' to 'int' and then removes the function description
of internal helper function. Because the internal helper function
is used by the devfreq core.

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

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index d8ff16419452..77eb3edf6bf3 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -116,12 +116,7 @@ static int is_supported_freq(struct devfreq *devfreq, unsigned long freq)
 	return ret;
 }
 
-/**
- * devfreq_get_freq_level() - Lookup freq_table for the frequency
- * @devfreq:	the devfreq instance
- * @freq:	the target frequency
- */
-static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
+static int get_freq_level(struct devfreq *devfreq, unsigned long freq)
 {
 	int lev;
 
@@ -132,11 +127,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;
@@ -146,7 +137,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 count;
 
 	profile->max_state = count;
 	profile->freq_table = devm_kcalloc(devfreq->dev.parent,
@@ -155,7 +146,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++) {
@@ -163,11 +154,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 -EINVAL;
 		}
 		dev_pm_opp_put(opp);
 		profile->freq_table[i] = freq;
 	}
+
+	return 0;
 }
 
 /**
@@ -186,7 +179,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
 	if (!devfreq->previous_freq)
 		goto out;
 
-	prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq);
+	prev_lev = get_freq_level(devfreq, devfreq->previous_freq);
 	if (prev_lev < 0) {
 		ret = prev_lev;
 		goto out;
@@ -195,7 +188,7 @@ int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
 	devfreq->time_in_state[prev_lev] +=
 			 cur_time - devfreq->last_stat_updated;
 
-	lev = devfreq_get_freq_level(devfreq, freq);
+	lev = get_freq_level(devfreq, freq);
 	if (lev < 0) {
 		ret = lev;
 		goto out;
@@ -600,8 +593,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->data = data;
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
-	if (!devfreq->profile->max_state && !devfreq->profile->freq_table)
-		devfreq_set_freq_table(devfreq);
+	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
+		err = set_freq_table(devfreq);
+		if (err < 0) {
+			mutex_unlock(&devfreq->lock);
+			goto err_dev;
+		}
+	}
 
 	/* Set the scaling available min_freq and max_freq */
 	devfreq->min_freq = find_available_min_freq(devfreq);
-- 
1.9.1

[PATCH 11/12] PM / devfreq: Remove exported opp_notifier function

[Chanwoo Choi]

The devfreq provides the helper function to register the OPP notifier
which is used to catch the change of OPP entry such as OPP_EVENT_ADD,
OPP_EVENT_REMOVE, OPP_EVENT_DISABLE and OPP_EVENT_ENABLE. The OPP
notifier is optional.

But, the devfreq uses the OPP interface as a mandatory method
to handle the frequency and voltage. If so, the OPP notifier is
mandatory. So, the OPP notifier have to be handled in the devfreq core
instead of each device driver.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c    | 77 --------------------------------------------
 drivers/devfreq/exynos-bus.c |  7 ----
 drivers/devfreq/rk3399_dmc.c |  1 -
 include/linux/devfreq.h      | 31 ------------------
 4 files changed, 116 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 77eb3edf6bf3..7efa867e4aea 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -1344,83 +1344,6 @@ struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 EXPORT_SYMBOL(devfreq_recommended_opp);
 
 /**
- * devfreq_register_opp_notifier() - Helper function to get devfreq notified
- *				   for any changes in the OPP availability
- *				   changes
- * @dev:	The devfreq user device. (parent of devfreq)
- * @devfreq:	The devfreq object.
- */
-int devfreq_register_opp_notifier(struct device *dev, struct devfreq *devfreq)
-{
-	return dev_pm_opp_register_notifier(dev, &devfreq->nb);
-}
-EXPORT_SYMBOL(devfreq_register_opp_notifier);
-
-/**
- * devfreq_unregister_opp_notifier() - Helper function to stop getting devfreq
- *				     notified for any changes in the OPP
- *				     availability changes anymore.
- * @dev:	The devfreq user device. (parent of devfreq)
- * @devfreq:	The devfreq object.
- *
- * At exit() callback of devfreq_dev_profile, this must be included if
- * devfreq_recommended_opp is used.
- */
-int devfreq_unregister_opp_notifier(struct device *dev, struct devfreq *devfreq)
-{
-	return dev_pm_opp_unregister_notifier(dev, &devfreq->nb);
-}
-EXPORT_SYMBOL(devfreq_unregister_opp_notifier);
-
-static void devm_devfreq_opp_release(struct device *dev, void *res)
-{
-	devfreq_unregister_opp_notifier(dev, *(struct devfreq **)res);
-}
-
-/**
- * devm_ devfreq_register_opp_notifier()
- *		- Resource-managed devfreq_register_opp_notifier()
- * @dev:	The devfreq user device. (parent of devfreq)
- * @devfreq:	The devfreq object.
- */
-int devm_devfreq_register_opp_notifier(struct device *dev,
-				       struct devfreq *devfreq)
-{
-	struct devfreq **ptr;
-	int ret;
-
-	ptr = devres_alloc(devm_devfreq_opp_release, sizeof(*ptr), GFP_KERNEL);
-	if (!ptr)
-		return -ENOMEM;
-
-	ret = devfreq_register_opp_notifier(dev, devfreq);
-	if (ret) {
-		devres_free(ptr);
-		return ret;
-	}
-
-	*ptr = devfreq;
-	devres_add(dev, ptr);
-
-	return 0;
-}
-EXPORT_SYMBOL(devm_devfreq_register_opp_notifier);
-
-/**
- * devm_devfreq_unregister_opp_notifier()
- *		- Resource-managed devfreq_unregister_opp_notifier()
- * @dev:	The devfreq user device. (parent of devfreq)
- * @devfreq:	The devfreq object.
- */
-void devm_devfreq_unregister_opp_notifier(struct device *dev,
-					 struct devfreq *devfreq)
-{
-	WARN_ON(devres_release(dev, devm_devfreq_opp_release,
-			       devm_devfreq_dev_match, devfreq));
-}
-EXPORT_SYMBOL(devm_devfreq_unregister_opp_notifier);
-
-/**
  * devfreq_register_notifier() - Register a driver with devfreq
  * @devfreq:	The devfreq object.
  * @nb:		The notifier block to register.
diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
index 49f68929e024..a144d7ae066e 100644
--- a/drivers/devfreq/exynos-bus.c
+++ b/drivers/devfreq/exynos-bus.c
@@ -444,13 +444,6 @@ static int exynos_bus_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	/* Register opp_notifier to catch the change of OPP  */
-	ret = devm_devfreq_register_opp_notifier(dev, bus->devfreq);
-	if (ret < 0) {
-		dev_err(dev, "failed to register opp notifier\n");
-		goto err;
-	}
-
 	/*
 	 * Enable devfreq-event to get raw data which is used to determine
 	 * current bus load.
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 1b89ebbad02c..4fb1041b628e 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -435,7 +435,6 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
 					   &data->ondemand_data);
 	if (IS_ERR(data->devfreq))
 		return PTR_ERR(data->devfreq);
-	devm_devfreq_register_opp_notifier(dev, data->devfreq);
 
 	data->dev = dev;
 	platform_set_drvdata(pdev, data);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 597294e0cc40..d6f054545799 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -189,14 +189,6 @@ extern void devm_devfreq_remove_device(struct device *dev,
 /* Helper functions for devfreq user device driver with OPP. */
 extern struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 					   unsigned long *freq, u32 flags);
-extern int devfreq_register_opp_notifier(struct device *dev,
-					 struct devfreq *devfreq);
-extern int devfreq_unregister_opp_notifier(struct device *dev,
-					   struct devfreq *devfreq);
-extern int devm_devfreq_register_opp_notifier(struct device *dev,
-					      struct devfreq *devfreq);
-extern void devm_devfreq_unregister_opp_notifier(struct device *dev,
-						struct devfreq *devfreq);
 extern int devfreq_register_notifier(struct devfreq *devfreq,
 					struct notifier_block *nb,
 					unsigned int list);
@@ -310,29 +302,6 @@ static inline struct dev_pm_opp *devfreq_recommended_opp(struct device *dev,
 	return ERR_PTR(-EINVAL);
 }
 
-static inline int devfreq_register_opp_notifier(struct device *dev,
-					 struct devfreq *devfreq)
-{
-	return -EINVAL;
-}
-
-static inline int devfreq_unregister_opp_notifier(struct device *dev,
-					   struct devfreq *devfreq)
-{
-	return -EINVAL;
-}
-
-static inline int devm_devfreq_register_opp_notifier(struct device *dev,
-						     struct devfreq *devfreq)
-{
-	return -EINVAL;
-}
-
-static inline void devm_devfreq_unregister_opp_notifier(struct device *dev,
-							struct devfreq *devfreq)
-{
-}
-
 static inline int devfreq_register_notifier(struct devfreq *devfreq,
 					struct notifier_block *nb,
 					unsigned int list)
-- 
1.9.1

[PATCH 12/12] PM / devfreq: Add opp_notifier_cb() function pointer to support OPP notifier

[Chanwoo Choi]

The devfreq uses the OPP interface as a mandatory method
and so the devfreq has to support the OPP notifier in order to
catch the OPP events. So, this patch adds the new opp_notifier_cb()
function pointer in the struct devfreq_dev_profile. The user can
add the their own callback function to receive the OPP events.

Also, the devfreq provides the default OPP notifeir callback
in order to remake the frequency table when OPP events happen.
- default_opp_notifier_cb()

After merged the commit 0ec09ac2cebe ("PM / devfreq: Set the
freq_table of devfreq device"), if the freq_table is NULL,
the devfreq_add_device() makes the freq_table by using OPP interface.
In this case, the devfreq should handle the freq_table
when OPP events happen such as OPP_EVENT_REMOVE, OPP_EVENT_ADD.

When the dev_pm_opp_add/remove() are called, the devfreq core
has to remake the frequency table with the changed OPP information
in the default_opp_notifier_cb().

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/devfreq.h   | 12 ++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 7efa867e4aea..d313bed95871 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -163,6 +163,39 @@ static int set_freq_table(struct devfreq *devfreq)
 	return 0;
 }
 
+static int default_opp_notifier_cb(struct notifier_block *nb,
+				unsigned long opp_event, void *opp)
+{
+	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
+	struct devfreq_dev_profile *profile = devfreq->profile;
+	struct device *dev = devfreq->dev.parent;
+	int ret = 0;
+
+	mutex_lock(&devfreq->lock);
+
+	switch (opp_event) {
+	case OPP_EVENT_ADD:
+	case OPP_EVENT_REMOVE:
+		/* Free the frequency table */
+		profile->max_state = 0;
+		kfree(dev, profile->freq_table);
+
+		/* Remake the frequency table with the changed OPP  */
+		ret = set_freq_table(devfreq);
+		if (ret < 0)
+			goto out;
+		break;
+	case OPP_EVENT_DISABLE:
+	case OPP_EVENT_ENABLE:
+	default:
+		break;
+	}
+
+out:
+	mutex_unlock(&devfreq->lock);
+	return ret;
+}
+
 /**
  * devfreq_update_status() - Update statistics of devfreq behavior
  * @devfreq:	the devfreq instance
@@ -637,6 +670,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);
 
+	/* Register OPP notifier to catch the change of OPP entries */
+	if (!devfreq->nb.notifier_call)
+		devfreq->nb.notifier_call = default_opp_notifier_cb;
+	err = dev_pm_opp_register_notifier(dev, &devfreq->nb);
+	if (err < 0) {
+		mutex_unlock(&devfreq->lock);
+		dev_err(dev, "Unable to register opp notifier (%d)\n", err);
+		goto err_reg;
+	}
 	mutex_unlock(&devfreq->lock);
 
 	mutex_lock(&devfreq_list_lock);
@@ -663,9 +705,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	return devfreq;
 
 err_init:
+	dev_pm_opp_unregister_notifier(dev, &devfreq->nb);
 	list_del(&devfreq->node);
 	mutex_unlock(&devfreq_list_lock);
-
+err_reg:
 	device_unregister(&devfreq->dev);
 err_dev:
 	if (devfreq)
@@ -686,6 +729,7 @@ int devfreq_remove_device(struct devfreq *devfreq)
 	if (!devfreq)
 		return -EINVAL;
 
+	dev_pm_opp_unregister_notifier(devfreq->dev.parent, &devfreq->nb);
 	device_unregister(&devfreq->dev);
 
 	return 0;
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index d6f054545799..9596fd195986 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -84,6 +84,16 @@ 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.
+ * @opp_notifier_cb:	And optional callback that is called when following
+ *			OPP events happen from OPP interface. If the user
+ *			doesn't add this callback, the devfreq core add
+ *			the default callback funtion to handle the OPP events.
+ *			- Second parameter : the OPP event
+ *				: OPP_EVENT_ADD
+ *				: OPP_EVENT_REMOVE
+ *				: OPP_EVENT_ENABLE
+ *				: OPP_EVENT_DISABLE,
+ *			- Third parameter : the instance of struct dev_pm_opp
  * @freq_table:	Optional list of frequencies to support statistics.
  * @max_state:	The size of freq_table.
  */
@@ -96,6 +106,8 @@ struct devfreq_dev_profile {
 			      struct devfreq_dev_status *stat);
 	int (*get_cur_freq)(struct device *dev, unsigned long *freq);
 	void (*exit)(struct device *dev);
+	int (*opp_notifier_cb)(struct notifier_block *nb,
+				unsigned long opp_event, void *opp);
 
 	unsigned long *freq_table;
 	unsigned int max_state;
-- 
1.9.1

Re: [PATCH 12/12] PM / devfreq: Add opp_notifier_cb() function pointer to support OPP notifier

[Chanwoo Choi]

Dear all,

On 2017년 08월 24일 10:42, Chanwoo Choi wrote:
> The devfreq uses the OPP interface as a mandatory method
> and so the devfreq has to support the OPP notifier in order to
> catch the OPP events. So, this patch adds the new opp_notifier_cb()
> function pointer in the struct devfreq_dev_profile. The user can
> add the their own callback function to receive the OPP events.
> 
> Also, the devfreq provides the default OPP notifeir callback
> in order to remake the frequency table when OPP events happen.
> - default_opp_notifier_cb()
> 
> After merged the commit 0ec09ac2cebe ("PM / devfreq: Set the
> freq_table of devfreq device"), if the freq_table is NULL,
> the devfreq_add_device() makes the freq_table by using OPP interface.
> In this case, the devfreq should handle the freq_table
> when OPP events happen such as OPP_EVENT_REMOVE, OPP_EVENT_ADD.
> 
> When the dev_pm_opp_add/remove() are called, the devfreq core
> has to remake the frequency table with the changed OPP information
> in the default_opp_notifier_cb().
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/devfreq.h   | 12 ++++++++++++
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 7efa867e4aea..d313bed95871 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -163,6 +163,39 @@ static int set_freq_table(struct devfreq *devfreq)
>  	return 0;
>  }
>  
> +static int default_opp_notifier_cb(struct notifier_block *nb,
> +				unsigned long opp_event, void *opp)
> +{
> +	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> +	struct devfreq_dev_profile *profile = devfreq->profile;
> +	struct device *dev = devfreq->dev.parent;
> +	int ret = 0;
> +
> +	mutex_lock(&devfreq->lock);
> +
> +	switch (opp_event) {
> +	case OPP_EVENT_ADD:
> +	case OPP_EVENT_REMOVE:
> +		/* Free the frequency table */
> +		profile->max_state = 0;
> +		kfree(dev, profile->freq_table);

It is my mistake when changing from devm_kfree to kfree.
I'll fix on next patchset.


[ditto]

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

RE: [PATCH 01/12] PM / devfreq: Fix memory leak when fail to register device

[MyungJoo Ham]

> When the devfreq_add_device fails to register deivce, the memory
> leak of devfreq instance happen. So, this patch fix the memory
> leak issue. Before freeing the devfreq instance checks whether
> devfreq instance is NULL or not because the device_unregister()
> frees the devfreq instance when jumping to the 'err_init'.
> It is to prevent the duplicate the kfee(devfreq).
> 
> Cc: stable@vger.kernel.org
> Fixes: ac4b281176a5 ("PM / devfreq: fix duplicated kfree on devfreq pointer")
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

This looks like to be duplicated. Please check your repo basis.


Cheers,
MyungJoo

> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index dea04871b50d..a1c4ee818614 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -564,7 +564,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	err = device_register(&devfreq->dev);
>  	if (err) {
>  		mutex_unlock(&devfreq->lock);
> -		goto err_out;
> +		goto err_dev;
>  	}
>  
>  	devfreq->trans_table =	devm_kzalloc(&devfreq->dev,
> @@ -610,6 +610,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	mutex_unlock(&devfreq_list_lock);
>  
>  	device_unregister(&devfreq->dev);
> +err_dev:
> +	if (devfreq)
> +		kfree(devfreq);
>  err_out:
>  	return ERR_PTR(err);
>  }
> -- 
> 1.9.1

Re: [PATCH 01/12] PM / devfreq: Fix memory leak when fail to register device

[Chanwoo Choi]

On 2017년 08월 28일 09:17, MyungJoo Ham wrote:
>> When the devfreq_add_device fails to register deivce, the memory
>> leak of devfreq instance happen. So, this patch fix the memory
>> leak issue. Before freeing the devfreq instance checks whether
>> devfreq instance is NULL or not because the device_unregister()
>> frees the devfreq instance when jumping to the 'err_init'.
>> It is to prevent the duplicate the kfee(devfreq).
>>
>> Cc: stable@vger.kernel.org
>> Fixes: ac4b281176a5 ("PM / devfreq: fix duplicated kfree on devfreq pointer")
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/devfreq/devfreq.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> This looks like to be duplicated. Please check your repo basis.

These patches is based on the v4.13-rc6.

After merged ("PM / devfreq: fix duplicated kfree on devfreq pointer"),
this patch doesn't consider the free of memory when error case.

After applying this patch, this patch consider the error case
when jumping the err_dev statement with goto.
		device_unregister(&devfreq->dev);
	err_dev:
		if (devfreq)		
			kfree(devfreq);


> 
> 
> Cheers,
> MyungJoo
> 
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index dea04871b50d..a1c4ee818614 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -564,7 +564,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>  	err = device_register(&devfreq->dev);
>>  	if (err) {
>>  		mutex_unlock(&devfreq->lock);
>> -		goto err_out;
>> +		goto err_dev;
>>  	}
>>  
>>  	devfreq->trans_table =	devm_kzalloc(&devfreq->dev,
>> @@ -610,6 +610,9 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>  	mutex_unlock(&devfreq_list_lock);
>>  
>>  	device_unregister(&devfreq->dev);
>> +err_dev:
>> +	if (devfreq)
>> +		kfree(devfreq);
>>  err_out:
>>  	return ERR_PTR(err);
>>  }
>> -- 
>> 1.9.1
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

RE: Re: [PATCH 01/12] PM / devfreq: Fix memory leak when fail to register device

[MyungJoo Ham]

> On 2017년 08월 28일 09:17, MyungJoo Ham wrote:

> >> When the devfreq_add_device fails to register deivce, the memory

> >> leak of devfreq instance happen. So, this patch fix the memory

> >> leak issue. Before freeing the devfreq instance checks whether

> >> devfreq instance is NULL or not because the device_unregister()

> >> frees the devfreq instance when jumping to the 'err_init'.

> >> It is to prevent the duplicate the kfee(devfreq).

> >>

> >> Cc: stable@vger.kernel.org

> >> Fixes: ac4b281176a5 ("PM / devfreq: fix duplicated kfree on devfreq pointer")

> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

> >> ---

> >>  drivers/devfreq/devfreq.c | 5 ++++-

> >>  1 file changed, 4 insertions(+), 1 deletion(-)

> > 

> > This looks like to be duplicated. Please check your repo basis.

> 

> These patches is based on the v4.13-rc6.

> 

> After merged ("PM / devfreq: fix duplicated kfree on devfreq pointer"),

> this patch doesn't consider the free of memory when error case.

> 

> After applying this patch, this patch consider the error case

> when jumping the err_dev statement with goto.

> 		device_unregister(&devfreq->dev);

> 	err_dev:

> 		if (devfreq)		

> 			kfree(devfreq);



Ah sorry. It looks like I was confused of - and + somehow.



Acked-By: MyungJoo Ham.



(I'll merge trivial fixes first)



> 

> 

RE: [PATCH 02/12] PM / devfreq: Fix locking range for making the frequency table

[MyungJoo Ham]

> Even if the freq_table is included in the struct devfreq,
> the commit 0ec09ac2cebe ("PM / devfreq: Set the freq_table
> of devfreq device") set the frequency table outside the mutex locking.
> 
> So, this patch initializes the frequency table within the mutex locking.
> 
> Cc: stable@vger.kernel.org
> Fixes: 0ec09ac2cebe ("PM / devfreq: Set the freq_table of devfreq device")
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>

Is this because you do not have locks inside
devfreq_set_freq_table() anymore?

> ---
>  drivers/devfreq/devfreq.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index a1c4ee818614..3c5ccb96e165 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -553,11 +553,8 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	devfreq->data = data;
>  	devfreq->nb.notifier_call = devfreq_notifier_call;
>  
> -	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
> -		mutex_unlock(&devfreq->lock);
> +	if (!devfreq->profile->max_state && !devfreq->profile->freq_table)
>  		devfreq_set_freq_table(devfreq);
> -		mutex_lock(&devfreq->lock);
> -	}
>  
>  	dev_set_name(&devfreq->dev, "devfreq%d",
>  				atomic_inc_return(&devfreq_no));

RE: [PATCH 03/12] PM / devfreq: Move private devfreq_update_stats() into devfreq

[MyungJoo Ham]

> From: Chanwoo Choi <cwchoi00@gmail.com>
> 
> THe devfreq_update_stats() updates the 'struct devfreq_dev_status'
> in order to get current status of devfreq device. It is only used
> for the governors.
> 
> This patch moves the devfreq_update_stats() into devfreq directory.
> 
> Signed-off-by: Chanwoo Choi <cwchoi00@gmail.com>
> ---
>  drivers/devfreq/governor.h |  4 ++++
>  include/linux/devfreq.h    | 13 -------------
>  2 files changed, 4 insertions(+), 13 deletions(-)

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>

RE: [PATCH 04/12] PM / devfreq: Add dependency on PM_OPP

[MyungJoo Ham]

> The devfreq ues the OPP library to handle the voltage and frequency
> for the device basically. This patch adds the dependency on CONFIG_PM_OPP
> in order to prevent either the build break or the unknow behavior.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>

RE: [PATCH 10/12] PM / devfreq: Remove 'devfreq' prefix from helper function

[MyungJoo Ham]

> This patch just removes the 'devfreq' prefix from internal helper
> function in order to clarify the role of the following functions.
> - devfreq_get_freq_level() - get_freq_level()
> - devfreq_set_freq_table() - set_freq_table()
> 
> Also, this patch changes the return value of set_freq_table()
> from 'void' to 'int' and then removes the function description
> of internal helper function. Because the internal helper function
> is used by the devfreq core.
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 34 ++++++++++++++++------------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
> 

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>


(note that you are alternating betgween two email addresses, gmail.com/samsung.com in a single series of commits)

Re: [PATCH 10/12] PM / devfreq: Remove 'devfreq' prefix from helper function

[Chanwoo Choi]

On 2017년 08월 28일 10:37, MyungJoo Ham wrote:
>> This patch just removes the 'devfreq' prefix from internal helper
>> function in order to clarify the role of the following functions.
>> - devfreq_get_freq_level() - get_freq_level()
>> - devfreq_set_freq_table() - set_freq_table()
>>
>> Also, this patch changes the return value of set_freq_table()
>> from 'void' to 'int' and then removes the function description
>> of internal helper function. Because the internal helper function
>> is used by the devfreq core.
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/devfreq/devfreq.c | 34 ++++++++++++++++------------------
>>  1 file changed, 16 insertions(+), 18 deletions(-)
>>
> 
> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> 
> 
> (note that you are alternating betgween two email addresses, gmail.com/samsung.com in a single series of commits)

It is my mistake. On v2 patchset, I'll use the samsung.com account. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics