[PATCH v10] PM / devfreq: Add dev_pm_qos support

[PATCH v10] PM / devfreq: Add dev_pm_qos support

[Leonard Crestez]

Add dev_pm_qos notifiers to devfreq core in order to support frequency
limits via dev_pm_qos_add_request.

Unlike the rest of devfreq the dev_pm_qos frequency is measured in Khz,
this is consistent with current dev_pm_qos usage for cpufreq and
allows frequencies above 2Ghz (pm_qos expresses limits as s32).

Like with cpufreq the handling of min_freq/max_freq is moved to PM QoS
mechanisms. Constraints from userspace are no longer clamped on store,
instead all values can be written and we only check against OPPs in a
new get_freq_range function. This is consistent with the design of
dev_pm_qos.

Notifiers from pm_qos are executed under a single global dev_pm_qos_mtx and
need to take devfreq->lock. Notifier registration takes the same dev_pm_qos_mtx
so in order to prevent lockdep warnings it must be done outside devfreq->lock.
Current devfreq_add_device does all initialization under devfreq->lock and that
needs to be relaxed.

Some of first patches in the series are bugfixes and cleanups, they could be
applied separately. The simplifications are used for PM QoS
implementations but they're also nice on their own.

The middle section restores DEV_PM_QOS_MIN/MAX_FREQUENCY after it was
removed by cpufreq folks; this device-centered mechanism is still a very
good match for devfreq. Plan is to use this for interconnect on i.MX but
Samsung also expressed interest:

---
Series is against next-20191031. Version 9 no longer compiles.

Let me know if prefer multiple smaller series. For example patches 1-5
could be split waiting for further discussion on PM QoS specifics.

Changes since v9:
* Restore DEV_PM_QOS_MIN/MAX_FREQUENCY based on new frequency qos
* Preserve devm usage in devfreq_add_device by splitting device_register
into device_initialize and device_add. This also simplifies cleanup.
* Adjust printk messages
* Drop check for zero scaling_max_freq in get_freq_range.
* Collect reviews.
Link to v9: https://patchwork.kernel.org/cover/11171807/

Changes since v8:
* Fix incorrectly reading qos max twice in get_freq_range.
* Make devfreq_notifier_call set scaling_max_freq to ULONG_MAX instead of 0 on
error. This avoids special-casing this in get_freq_range.
* Add a fix for devfreq_notifier_call returning -errno.
* Replace S32_MAX with PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE. This seems cleaner
and avoids overflow when multiplying S32_MAX with 1000 on 32bit platforms. It
overflowed to 0xfffffc18 hz so it was mostly safe anyway.
* Warn when encountering errors on cleanup (but continue).
* Incorporate other smaller comment and printk adjustments from review.
Link to v8: https://patchwork.kernel.org/cover/11158383/

Changes since v7:
* Only #define HZ_PER_KHZ in patch where it's used.
* Drop devfreq_ prefix for some internal functions.
* Improve qos update error message.
* Remove some unnecessary comments.
* Collect reviews
Link to v7: https://patchwork.kernel.org/cover/11157649/

Changes since v6:
* Don't return errno from devfreq_qos_notifier_call, return NOTIFY_DONE
and print the error.
* More spelling and punctuation nits
Link to v6: https://patchwork.kernel.org/cover/11157201/

Changes since v5:

Leonard Crestez (11):
  PM / devfreq: Fix devfreq_notifier_call returning errno
  PM / devfreq: Set scaling_max_freq to max on OPP notifier error
  PM / devfreq: Split device_register usage
  PM / devfreq: Move more initialization before registration
  PM / devfreq: Don't take lock in devfreq_add_device
  PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs
  PM / QoS: Export _freq_qos_apply
  PM / QoS: Restore DEV_PM_QOS_MIN/MAX_FREQUENCY
  PM / devfreq: Introduce get_freq_range helper
  PM / devfreq: Add PM QoS support
  PM / devfreq: Use PM QoS for sysfs min/max_freq

 drivers/base/power/qos.c  |  69 +++++++++-
 drivers/devfreq/devfreq.c | 280 ++++++++++++++++++++++++++------------
 include/linux/devfreq.h   |  14 +-
 include/linux/pm_qos.h    |  86 +++++++-----
 kernel/power/qos.c        |  11 +-
 5 files changed, 322 insertions(+), 138 deletions(-)

-- 
2.17.1

[PATCH v10 01/11] PM / devfreq: Fix devfreq_notifier_call returning errno

[Leonard Crestez]

Notifier callbacks shouldn't return negative errno but one of the
NOTIFY_OK/DONE/BAD values.

The OPP core will ignore return values from notifiers but returning a
value that matches NOTIFY_STOP_MASK will stop the notification chain.

Fix by always returning NOTIFY_OK.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 323d43315d1e..b65faa1a2baa 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -548,30 +548,32 @@ EXPORT_SYMBOL(devfreq_interval_update);
  */
 static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 				 void *devp)
 {
 	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
-	int ret;
+	int err = -EINVAL;
 
 	mutex_lock(&devfreq->lock);
 
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
-	if (!devfreq->scaling_min_freq) {
-		mutex_unlock(&devfreq->lock);
-		return -EINVAL;
-	}
+	if (!devfreq->scaling_min_freq)
+		goto out;
 
 	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
-	if (!devfreq->scaling_max_freq) {
-		mutex_unlock(&devfreq->lock);
-		return -EINVAL;
-	}
+	if (!devfreq->scaling_max_freq)
+		goto out;
+
+	err = update_devfreq(devfreq);
 
-	ret = update_devfreq(devfreq);
+out:
 	mutex_unlock(&devfreq->lock);
+	if (err)
+		dev_err(devfreq->dev.parent,
+			"failed to update frequency from OPP notifier (%d)\n",
+			err);
 
-	return ret;
+	return NOTIFY_OK;
 }
 
 /**
  * devfreq_dev_release() - Callback for struct device to release the device.
  * @dev:	the devfreq device
-- 
2.17.1

[PATCH v10 02/11] PM / devfreq: Set scaling_max_freq to max on OPP notifier error

[Leonard Crestez]

The devfreq_notifier_call functions will update scaling_min_freq and
scaling_max_freq when the OPP table is updated.

If fetching the maximum frequency fails then scaling_max_freq remains
set to zero which is confusing. Set to ULONG_MAX instead so we don't
need special handling for this case in other places.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b65faa1a2baa..715127f1cda5 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -557,12 +557,14 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
 	if (!devfreq->scaling_min_freq)
 		goto out;
 
 	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
-	if (!devfreq->scaling_max_freq)
+	if (!devfreq->scaling_max_freq) {
+		devfreq->scaling_max_freq = ULONG_MAX;
 		goto out;
+	}
 
 	err = update_devfreq(devfreq);
 
 out:
 	mutex_unlock(&devfreq->lock);
-- 
2.17.1

[PATCH v10 03/11] PM / devfreq: Split device_register usage

[Leonard Crestez]

Splitting device_register into device_initialize and device_add allows
devm-based allocations to be performed before device_add.

It also simplifies error paths in devfreq_add_device: just call
put_device instead of duplicating parts of devfreq_dev_release.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/devfreq/devfreq.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 715127f1cda5..603681f8cd73 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -639,10 +639,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	mutex_init(&devfreq->lock);
 	mutex_lock(&devfreq->lock);
 	devfreq->dev.parent = dev;
 	devfreq->dev.class = devfreq_class;
 	devfreq->dev.release = devfreq_dev_release;
+	device_initialize(&devfreq->dev);
 	INIT_LIST_HEAD(&devfreq->node);
 	devfreq->profile = profile;
 	strncpy(devfreq->governor_name, governor_name, DEVFREQ_NAME_LEN);
 	devfreq->previous_freq = profile->initial_freq;
 	devfreq->last_status.current_frequency = profile->initial_freq;
@@ -676,15 +677,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	atomic_set(&devfreq->suspend_count, 0);
 
 	dev_set_name(&devfreq->dev, "devfreq%d",
 				atomic_inc_return(&devfreq_no));
-	err = device_register(&devfreq->dev);
+	err = device_add(&devfreq->dev);
 	if (err) {
 		mutex_unlock(&devfreq->lock);
-		put_device(&devfreq->dev);
-		goto err_out;
+		goto err_dev;
 	}
 
 	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
 			array3_size(sizeof(unsigned int),
 				    devfreq->profile->max_state,
@@ -739,13 +739,13 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 err_init:
 	mutex_unlock(&devfreq_list_lock);
 err_devfreq:
 	devfreq_remove_device(devfreq);
-	devfreq = NULL;
+	return ERR_PTR(err);
 err_dev:
-	kfree(devfreq);
+	put_device(&devfreq->dev);
 err_out:
 	return ERR_PTR(err);
 }
 EXPORT_SYMBOL(devfreq_add_device);
 
-- 
2.17.1

[PATCH v10 04/11] PM / devfreq: Move more initialization before registration

[Leonard Crestez]

In general it is a better to initialize an object before making it
accessible externally (through device_add). This simplifies the code and
makes it possible to avoid locking the partially initialized object.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/devfreq.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 603681f8cd73..d89de2c37dfa 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -675,43 +675,43 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->max_freq = devfreq->scaling_max_freq;
 
 	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	atomic_set(&devfreq->suspend_count, 0);
 
-	dev_set_name(&devfreq->dev, "devfreq%d",
-				atomic_inc_return(&devfreq_no));
-	err = device_add(&devfreq->dev);
-	if (err) {
-		mutex_unlock(&devfreq->lock);
-		goto err_dev;
-	}
-
 	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
 			array3_size(sizeof(unsigned int),
 				    devfreq->profile->max_state,
 				    devfreq->profile->max_state),
 			GFP_KERNEL);
 	if (!devfreq->trans_table) {
 		mutex_unlock(&devfreq->lock);
 		err = -ENOMEM;
-		goto err_devfreq;
+		goto err_dev;
 	}
 
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
 			devfreq->profile->max_state,
 			sizeof(unsigned long),
 			GFP_KERNEL);
 	if (!devfreq->time_in_state) {
 		mutex_unlock(&devfreq->lock);
 		err = -ENOMEM;
-		goto err_devfreq;
+		goto err_dev;
 	}
 
 	devfreq->last_stat_updated = jiffies;
 
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);
 
+	dev_set_name(&devfreq->dev, "devfreq%d",
+				atomic_inc_return(&devfreq_no));
+	err = device_add(&devfreq->dev);
+	if (err) {
+		mutex_unlock(&devfreq->lock);
+		goto err_dev;
+	}
+
 	mutex_unlock(&devfreq->lock);
 
 	mutex_lock(&devfreq_list_lock);
 
 	governor = try_then_request_governor(devfreq->governor_name);
@@ -737,11 +737,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	return devfreq;
 
 err_init:
 	mutex_unlock(&devfreq_list_lock);
-err_devfreq:
 	devfreq_remove_device(devfreq);
 	return ERR_PTR(err);
 err_dev:
 	put_device(&devfreq->dev);
 err_out:
-- 
2.17.1

[PATCH v10 05/11] PM / devfreq: Don't take lock in devfreq_add_device

[Leonard Crestez]

A device usually doesn't need to lock itself during initialization
because it is not yet reachable from other threads.

This simplifies the code and helps avoid recursive lock warnings.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index d89de2c37dfa..ab12fd22aa08 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -635,11 +635,10 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		err = -ENOMEM;
 		goto err_out;
 	}
 
 	mutex_init(&devfreq->lock);
-	mutex_lock(&devfreq->lock);
 	devfreq->dev.parent = dev;
 	devfreq->dev.class = devfreq_class;
 	devfreq->dev.release = devfreq_dev_release;
 	device_initialize(&devfreq->dev);
 	INIT_LIST_HEAD(&devfreq->node);
@@ -649,28 +648,24 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->last_status.current_frequency = profile->initial_freq;
 	devfreq->data = data;
 	devfreq->nb.notifier_call = devfreq_notifier_call;
 
 	if (!devfreq->profile->max_state && !devfreq->profile->freq_table) {
-		mutex_unlock(&devfreq->lock);
 		err = set_freq_table(devfreq);
 		if (err < 0)
 			goto err_dev;
-		mutex_lock(&devfreq->lock);
 	}
 
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
 	if (!devfreq->scaling_min_freq) {
-		mutex_unlock(&devfreq->lock);
 		err = -EINVAL;
 		goto err_dev;
 	}
 	devfreq->min_freq = devfreq->scaling_min_freq;
 
 	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
 	if (!devfreq->scaling_max_freq) {
-		mutex_unlock(&devfreq->lock);
 		err = -EINVAL;
 		goto err_dev;
 	}
 	devfreq->max_freq = devfreq->scaling_max_freq;
 
@@ -681,21 +676,19 @@ struct devfreq *devfreq_add_device(struct device *dev,
 			array3_size(sizeof(unsigned int),
 				    devfreq->profile->max_state,
 				    devfreq->profile->max_state),
 			GFP_KERNEL);
 	if (!devfreq->trans_table) {
-		mutex_unlock(&devfreq->lock);
 		err = -ENOMEM;
 		goto err_dev;
 	}
 
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
 			devfreq->profile->max_state,
 			sizeof(unsigned long),
 			GFP_KERNEL);
 	if (!devfreq->time_in_state) {
-		mutex_unlock(&devfreq->lock);
 		err = -ENOMEM;
 		goto err_dev;
 	}
 
 	devfreq->last_stat_updated = jiffies;
@@ -703,16 +696,12 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);
 
 	dev_set_name(&devfreq->dev, "devfreq%d",
 				atomic_inc_return(&devfreq_no));
 	err = device_add(&devfreq->dev);
-	if (err) {
-		mutex_unlock(&devfreq->lock);
+	if (err)
 		goto err_dev;
-	}
-
-	mutex_unlock(&devfreq->lock);
 
 	mutex_lock(&devfreq_list_lock);
 
 	governor = try_then_request_governor(devfreq->governor_name);
 	if (IS_ERR(governor)) {
-- 
2.17.1

[PATCH v10 06/11] PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs

[Leonard Crestez]

This allows dev_pm_qos to embed freq_qos structs, which is done in the
next patch. Separate commit to make it easier to review.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 36 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index c35ff21e8a40..a8e1486e3200 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -47,25 +47,10 @@ struct pm_qos_request {
 struct pm_qos_flags_request {
 	struct list_head node;
 	s32 flags;	/* Do not change to 64 bit */
 };
 
-enum dev_pm_qos_req_type {
-	DEV_PM_QOS_RESUME_LATENCY = 1,
-	DEV_PM_QOS_LATENCY_TOLERANCE,
-	DEV_PM_QOS_FLAGS,
-};
-
-struct dev_pm_qos_request {
-	enum dev_pm_qos_req_type type;
-	union {
-		struct plist_node pnode;
-		struct pm_qos_flags_request flr;
-	} data;
-	struct device *dev;
-};
-
 enum pm_qos_type {
 	PM_QOS_UNITIALIZED,
 	PM_QOS_MAX,		/* return the largest value */
 	PM_QOS_MIN,		/* return the smallest value */
 	PM_QOS_SUM		/* return the sum */
@@ -88,10 +73,48 @@ struct pm_qos_constraints {
 struct pm_qos_flags {
 	struct list_head list;
 	s32 effective_flags;	/* Do not change to 64 bit */
 };
 
+
+#define FREQ_QOS_MIN_DEFAULT_VALUE	0
+#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
+
+enum freq_qos_req_type {
+	FREQ_QOS_MIN = 1,
+	FREQ_QOS_MAX,
+};
+
+struct freq_constraints {
+	struct pm_qos_constraints min_freq;
+	struct blocking_notifier_head min_freq_notifiers;
+	struct pm_qos_constraints max_freq;
+	struct blocking_notifier_head max_freq_notifiers;
+};
+
+struct freq_qos_request {
+	enum freq_qos_req_type type;
+	struct plist_node pnode;
+	struct freq_constraints *qos;
+};
+
+
+enum dev_pm_qos_req_type {
+	DEV_PM_QOS_RESUME_LATENCY = 1,
+	DEV_PM_QOS_LATENCY_TOLERANCE,
+	DEV_PM_QOS_FLAGS,
+};
+
+struct dev_pm_qos_request {
+	enum dev_pm_qos_req_type type;
+	union {
+		struct plist_node pnode;
+		struct pm_qos_flags_request flr;
+	} data;
+	struct device *dev;
+};
+
 struct dev_pm_qos {
 	struct pm_qos_constraints resume_latency;
 	struct pm_qos_constraints latency_tolerance;
 	struct pm_qos_flags flags;
 	struct dev_pm_qos_request *resume_latency_req;
@@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 {
 	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
 }
 #endif
 
-#define FREQ_QOS_MIN_DEFAULT_VALUE	0
-#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
-
-enum freq_qos_req_type {
-	FREQ_QOS_MIN = 1,
-	FREQ_QOS_MAX,
-};
-
-struct freq_constraints {
-	struct pm_qos_constraints min_freq;
-	struct blocking_notifier_head min_freq_notifiers;
-	struct pm_qos_constraints max_freq;
-	struct blocking_notifier_head max_freq_notifiers;
-};
-
-struct freq_qos_request {
-	enum freq_qos_req_type type;
-	struct plist_node pnode;
-	struct freq_constraints *qos;
-};
-
 static inline int freq_qos_request_active(struct freq_qos_request *req)
 {
 	return !IS_ERR_OR_NULL(req->qos);
 }
 
-- 
2.17.1

[PATCH v10 07/11] PM / QoS: Export _freq_qos_apply

[Leonard Crestez]

This is exported only for dev_pm_qos to use in order to implement
per-device freq constraints.

Export with a leading underscore because this is an implementation
detail, it's not meant to be used by drivers making QoS requests.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 include/linux/pm_qos.h |  2 ++
 kernel/power/qos.c     | 11 ++++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index a8e1486e3200..89a8e7a4710f 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -291,10 +291,12 @@ s32 freq_qos_read_value(struct freq_constraints *qos,
 int freq_qos_add_request(struct freq_constraints *qos,
 			 struct freq_qos_request *req,
 			 enum freq_qos_req_type type, s32 value);
 int freq_qos_update_request(struct freq_qos_request *req, s32 new_value);
 int freq_qos_remove_request(struct freq_qos_request *req);
+int _freq_qos_apply(struct freq_qos_request *req,
+		    enum pm_qos_req_action action, s32 value);
 
 int freq_qos_add_notifier(struct freq_constraints *qos,
 			  enum freq_qos_req_type type,
 			  struct notifier_block *notifier);
 int freq_qos_remove_notifier(struct freq_constraints *qos,
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 04e83fdfbe80..ea38ae86bd66 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -708,16 +708,16 @@ s32 freq_qos_read_value(struct freq_constraints *qos,
 
 	return ret;
 }
 
 /**
- * freq_qos_apply - Add/modify/remove frequency QoS request.
+ * _freq_qos_apply - Add/modify/remove frequency QoS request.
  * @req: Constraint request to apply.
  * @action: Action to perform (add/update/remove).
  * @value: Value to assign to the QoS request.
  */
-static int freq_qos_apply(struct freq_qos_request *req,
+int _freq_qos_apply(struct freq_qos_request *req,
 			  enum pm_qos_req_action action, s32 value)
 {
 	int ret;
 
 	switch(req->type) {
@@ -733,10 +733,11 @@ static int freq_qos_apply(struct freq_qos_request *req,
 		ret = -EINVAL;
 	}
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(_freq_qos_apply);
 
 /**
  * freq_qos_add_request - Insert new frequency QoS request into a given list.
  * @qos: Constraints to update.
  * @req: Preallocated request object.
@@ -763,11 +764,11 @@ int freq_qos_add_request(struct freq_constraints *qos,
 		 "%s() called for active request\n", __func__))
 		return -EINVAL;
 
 	req->qos = qos;
 	req->type = type;
-	ret = freq_qos_apply(req, PM_QOS_ADD_REQ, value);
+	ret = _freq_qos_apply(req, PM_QOS_ADD_REQ, value);
 	if (ret < 0) {
 		req->qos = NULL;
 		req->type = 0;
 	}
 
@@ -796,11 +797,11 @@ int freq_qos_update_request(struct freq_qos_request *req, s32 new_value)
 		return -EINVAL;
 
 	if (req->pnode.prio == new_value)
 		return 0;
 
-	return freq_qos_apply(req, PM_QOS_UPDATE_REQ, new_value);
+	return _freq_qos_apply(req, PM_QOS_UPDATE_REQ, new_value);
 }
 EXPORT_SYMBOL_GPL(freq_qos_update_request);
 
 /**
  * freq_qos_remove_request - Remove frequency QoS request from its list.
@@ -819,11 +820,11 @@ int freq_qos_remove_request(struct freq_qos_request *req)
 
 	if (WARN(!freq_qos_request_active(req),
 		 "%s() called for unknown object\n", __func__))
 		return -EINVAL;
 
-	return freq_qos_apply(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
+	return _freq_qos_apply(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
 }
 EXPORT_SYMBOL_GPL(freq_qos_remove_request);
 
 /**
  * freq_qos_add_notifier - Add frequency QoS change notifier.
-- 
2.17.1

[PATCH v10 08/11] PM / QoS: Restore DEV_PM_QOS_MIN/MAX_FREQUENCY

[Leonard Crestez]

Support for adding per-device frequency limits was removed in commit
2aac8bdf7a0f ("PM: QoS: Drop frequency QoS types from device PM QoS")
after cpufreq switched to use a new "freq_constraints" construct.

Restore support for per-device freq limits but base this upon
freq_constraints. This is primarily meant to be used by the devfreq
subsystem.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/base/power/qos.c | 69 ++++++++++++++++++++++++++++++++++++----
 include/linux/pm_qos.h   | 10 ++++++
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 350dcafd751f..52f74edee548 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -113,14 +113,24 @@ s32 dev_pm_qos_read_value(struct device *dev, enum dev_pm_qos_req_type type)
 	unsigned long flags;
 	s32 ret;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
 
-	if (type == DEV_PM_QOS_RESUME_LATENCY) {
+	switch (type) {
+	case DEV_PM_QOS_RESUME_LATENCY:
 		ret = IS_ERR_OR_NULL(qos) ? PM_QOS_RESUME_LATENCY_NO_CONSTRAINT
 			: pm_qos_read_value(&qos->resume_latency);
-	} else {
+		break;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+		ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE
+			: freq_qos_read_value(&qos->freq, FREQ_QOS_MIN);
+		break;
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		ret = IS_ERR_OR_NULL(qos) ? PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE
+			: freq_qos_read_value(&qos->freq, FREQ_QOS_MAX);
+		break;
+	default:
 		WARN_ON(1);
 		ret = 0;
 	}
 
 	spin_unlock_irqrestore(&dev->power.lock, flags);
@@ -157,10 +167,14 @@ static int apply_constraint(struct dev_pm_qos_request *req,
 		if (ret) {
 			value = pm_qos_read_value(&qos->latency_tolerance);
 			req->dev->power.set_latency_tolerance(req->dev, value);
 		}
 		break;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		ret = _freq_qos_apply(&req->data.freq, action, value);
+		break;
 	case DEV_PM_QOS_FLAGS:
 		ret = pm_qos_update_flags(&qos->flags, &req->data.flr,
 					  action, value);
 		break;
 	default:
@@ -207,10 +221,12 @@ static int dev_pm_qos_constraints_allocate(struct device *dev)
 	c->target_value = PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE;
 	c->default_value = PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE;
 	c->no_constraint_value = PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT;
 	c->type = PM_QOS_MIN;
 
+	freq_constraints_init(&qos->freq);
+
 	INIT_LIST_HEAD(&qos->flags.list);
 
 	spin_lock_irq(&dev->power.lock);
 	dev->power.qos = qos;
 	spin_unlock_irq(&dev->power.lock);
@@ -267,10 +283,22 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
 	plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
 		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
 		memset(req, 0, sizeof(*req));
 	}
 
+	c = &qos->freq.min_freq;
+	plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
+		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE);
+		memset(req, 0, sizeof(*req));
+	}
+
+	c = &qos->freq.max_freq;
+	plist_for_each_entry_safe(req, tmp, &c->list, data.freq.pnode) {
+		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
+		memset(req, 0, sizeof(*req));
+	}
+
 	f = &qos->flags;
 	list_for_each_entry_safe(req, tmp, &f->list, data.flr.node) {
 		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
 		memset(req, 0, sizeof(*req));
 	}
@@ -312,15 +340,26 @@ static int __dev_pm_qos_add_request(struct device *dev,
 		ret = -ENODEV;
 	else if (!dev->power.qos)
 		ret = dev_pm_qos_constraints_allocate(dev);
 
 	trace_dev_pm_qos_add_request(dev_name(dev), type, value);
-	if (!ret) {
-		req->dev = dev;
-		req->type = type;
+	if (ret)
+		return ret;
+
+	req->dev = dev;
+	req->type = type;
+	if (req->type == DEV_PM_QOS_MIN_FREQUENCY)
+		ret = freq_qos_add_request(&dev->power.qos->freq,
+					   &req->data.freq,
+					   FREQ_QOS_MIN, value);
+	else if (req->type == DEV_PM_QOS_MAX_FREQUENCY)
+		ret = freq_qos_add_request(&dev->power.qos->freq,
+					   &req->data.freq,
+					   FREQ_QOS_MAX, value);
+	else
 		ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
-	}
+
 	return ret;
 }
 
 /**
  * dev_pm_qos_add_request - inserts new qos request into the list
@@ -380,10 +419,14 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
 	switch(req->type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 	case DEV_PM_QOS_LATENCY_TOLERANCE:
 		curr_value = req->data.pnode.prio;
 		break;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		curr_value = req->data.freq.pnode.prio;
+		break;
 	case DEV_PM_QOS_FLAGS:
 		curr_value = req->data.flr.flags;
 		break;
 	default:
 		return -EINVAL;
@@ -505,10 +548,16 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier,
 	switch (type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 		ret = blocking_notifier_chain_register(dev->power.qos->resume_latency.notifiers,
 						       notifier);
 		break;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+		ret = freq_qos_add_notifier(&dev->power.qos->freq, FREQ_QOS_MIN, notifier);
+		break;
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		ret = freq_qos_add_notifier(&dev->power.qos->freq, FREQ_QOS_MAX, notifier);
+		break;
 	default:
 		WARN_ON(1);
 		ret = -EINVAL;
 	}
 
@@ -544,10 +593,18 @@ int dev_pm_qos_remove_notifier(struct device *dev,
 	switch (type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 		ret = blocking_notifier_chain_unregister(dev->power.qos->resume_latency.notifiers,
 							 notifier);
 		break;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+		ret = freq_qos_remove_notifier(&dev->power.qos->freq,
+					       FREQ_QOS_MIN, notifier);
+		break;
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		ret = freq_qos_remove_notifier(&dev->power.qos->freq,
+					       FREQ_QOS_MAX, notifier);
+		break;
 	default:
 		WARN_ON(1);
 		ret = -EINVAL;
 	}
 
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 89a8e7a4710f..90b147b7d7a3 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -32,10 +32,12 @@ enum pm_qos_flags_status {
 #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE	(2000 * USEC_PER_SEC)
 #define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE	PM_QOS_LATENCY_ANY
 #define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT	PM_QOS_LATENCY_ANY
 #define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS	PM_QOS_LATENCY_ANY_NS
 #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE	0
+#define PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE	0
+#define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE	(-1)
 #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT	(-1)
 
 #define PM_QOS_FLAG_NO_POWER_OFF	(1 << 0)
 
 struct pm_qos_request {
@@ -99,25 +101,29 @@ struct freq_qos_request {
 
 
 enum dev_pm_qos_req_type {
 	DEV_PM_QOS_RESUME_LATENCY = 1,
 	DEV_PM_QOS_LATENCY_TOLERANCE,
+	DEV_PM_QOS_MIN_FREQUENCY,
+	DEV_PM_QOS_MAX_FREQUENCY,
 	DEV_PM_QOS_FLAGS,
 };
 
 struct dev_pm_qos_request {
 	enum dev_pm_qos_req_type type;
 	union {
 		struct plist_node pnode;
 		struct pm_qos_flags_request flr;
+		struct freq_qos_request freq;
 	} data;
 	struct device *dev;
 };
 
 struct dev_pm_qos {
 	struct pm_qos_constraints resume_latency;
 	struct pm_qos_constraints latency_tolerance;
+	struct freq_constraints freq;
 	struct pm_qos_flags flags;
 	struct dev_pm_qos_request *resume_latency_req;
 	struct dev_pm_qos_request *latency_tolerance_req;
 	struct dev_pm_qos_request *flags_req;
 };
@@ -212,10 +218,14 @@ static inline s32 dev_pm_qos_read_value(struct device *dev,
 					enum dev_pm_qos_req_type type)
 {
 	switch (type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 		return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
+	case DEV_PM_QOS_MIN_FREQUENCY:
+		return PM_QOS_MIN_FREQUENCY_DEFAULT_VALUE;
+	case DEV_PM_QOS_MAX_FREQUENCY:
+		return PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
 	default:
 		WARN_ON(1);
 		return 0;
 	}
 }
-- 
2.17.1

[PATCH v10 09/11] PM / devfreq: Introduce get_freq_range helper

[Leonard Crestez]

Moving handling of min/max freq to a single function and call it from
update_devfreq and for printing min/max freq values in sysfs.

This changes the behavior of out-of-range min_freq/max_freq: clamping
is now done at evaluation time. This means that if an out-of-range
constraint is imposed by sysfs and it later becomes valid then it will
be enforced.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 108 +++++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 48 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ab12fd22aa08..ba3b53ee23fd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -96,10 +96,51 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
 		dev_pm_opp_put(opp);
 
 	return max_freq;
 }
 
+/**
+ * get_freq_range() - Get the current freq range
+ * @devfreq:	the devfreq instance
+ * @min_freq:	the min frequency
+ * @max_freq:	the max frequency
+ *
+ * This takes into consideration all constraints.
+ */
+static void get_freq_range(struct devfreq *devfreq,
+			   unsigned long *min_freq,
+			   unsigned long *max_freq)
+{
+	unsigned long *freq_table = devfreq->profile->freq_table;
+
+	lockdep_assert_held(&devfreq->lock);
+
+	/*
+	 * Initialize minimum/maximum frequency from freq table.
+	 * The devfreq drivers can initialize this in either ascending or
+	 * descending order and devfreq core supports both.
+	 */
+	if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
+		*min_freq = freq_table[0];
+		*max_freq = freq_table[devfreq->profile->max_state - 1];
+	} else {
+		*min_freq = freq_table[devfreq->profile->max_state - 1];
+		*max_freq = freq_table[0];
+	}
+
+	/* Apply constraints from sysfs */
+	*min_freq = max(*min_freq, devfreq->min_freq);
+	*max_freq = min(*max_freq, devfreq->max_freq);
+
+	/* Apply constraints from OPP interface */
+	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
+	*max_freq = min(*max_freq, devfreq->scaling_max_freq);
+
+	if (*min_freq > *max_freq)
+		*min_freq = *max_freq;
+}
+
 /**
  * devfreq_get_freq_level() - Lookup freq_table for the frequency
  * @devfreq:	the devfreq instance
  * @freq:	the target frequency
  */
@@ -348,20 +389,11 @@ int update_devfreq(struct devfreq *devfreq)
 
 	/* Reevaluate the proper frequency */
 	err = devfreq->governor->get_target_freq(devfreq, &freq);
 	if (err)
 		return err;
-
-	/*
-	 * 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);
+	get_freq_range(devfreq, &min_freq, &max_freq);
 
 	if (freq < min_freq) {
 		freq = min_freq;
 		flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
 	}
@@ -1292,40 +1324,28 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
 	mutex_lock(&df->lock);
-
-	if (value) {
-		if (value > df->max_freq) {
-			ret = -EINVAL;
-			goto unlock;
-		}
-	} else {
-		unsigned long *freq_table = df->profile->freq_table;
-
-		/* Get minimum frequency according to sorting order */
-		if (freq_table[0] < freq_table[df->profile->max_state - 1])
-			value = freq_table[0];
-		else
-			value = freq_table[df->profile->max_state - 1];
-	}
-
 	df->min_freq = value;
 	update_devfreq(df);
-	ret = count;
-unlock:
 	mutex_unlock(&df->lock);
-	return ret;
+
+	return count;
 }
 
 static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	struct devfreq *df = to_devfreq(dev);
+	unsigned long min_freq, max_freq;
 
-	return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
+	mutex_lock(&df->lock);
+	get_freq_range(df, &min_freq, &max_freq);
+	mutex_unlock(&df->lock);
+
+	return sprintf(buf, "%lu\n", min_freq);
 }
 
 static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
@@ -1337,40 +1357,32 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 	if (ret != 1)
 		return -EINVAL;
 
 	mutex_lock(&df->lock);
 
-	if (value) {
-		if (value < df->min_freq) {
-			ret = -EINVAL;
-			goto unlock;
-		}
-	} else {
-		unsigned long *freq_table = df->profile->freq_table;
-
-		/* Get maximum frequency according to sorting order */
-		if (freq_table[0] < freq_table[df->profile->max_state - 1])
-			value = freq_table[df->profile->max_state - 1];
-		else
-			value = freq_table[0];
-	}
+	if (!value)
+		value = ULONG_MAX;
 
 	df->max_freq = value;
 	update_devfreq(df);
-	ret = count;
-unlock:
 	mutex_unlock(&df->lock);
-	return ret;
+
+	return count;
 }
 static DEVICE_ATTR_RW(min_freq);
 
 static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	struct devfreq *df = to_devfreq(dev);
+	unsigned long min_freq, max_freq;
+
+	mutex_lock(&df->lock);
+	get_freq_range(df, &min_freq, &max_freq);
+	mutex_unlock(&df->lock);
 
-	return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
+	return sprintf(buf, "%lu\n", max_freq);
 }
 static DEVICE_ATTR_RW(max_freq);
 
 static ssize_t available_frequencies_show(struct device *d,
 					  struct device_attribute *attr,
-- 
2.17.1

[PATCH v10 10/11] PM / devfreq: Add PM QoS support

[Leonard Crestez]

Register notifiers with the PM QoS framework in order to respond to
requests for DEV_PM_QOS_MIN_FREQUENCY and DEV_PM_QOS_MAX_FREQUENCY.

No notifiers are added by this patch but PM QoS constraints can be
imposed externally (for example from other devices).

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/devfreq.c | 78 +++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h   |  5 +++
 2 files changed, 83 insertions(+)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index ba3b53ee23fd..776dec301a4e 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -22,15 +22,18 @@
 #include <linux/platform_device.h>
 #include <linux/list.h>
 #include <linux/printk.h>
 #include <linux/hrtimer.h>
 #include <linux/of.h>
+#include <linux/pm_qos.h>
 #include "governor.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/devfreq.h>
 
+#define HZ_PER_KHZ	1000
+
 static struct class *devfreq_class;
 
 /*
  * devfreq core provides delayed work based load monitoring helper
  * functions. Governors can use these or can implement their own
@@ -109,10 +112,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
 static void get_freq_range(struct devfreq *devfreq,
 			   unsigned long *min_freq,
 			   unsigned long *max_freq)
 {
 	unsigned long *freq_table = devfreq->profile->freq_table;
+	s32 qos_min_freq, qos_max_freq;
 
 	lockdep_assert_held(&devfreq->lock);
 
 	/*
 	 * Initialize minimum/maximum frequency from freq table.
@@ -125,10 +129,20 @@ static void get_freq_range(struct devfreq *devfreq,
 	} else {
 		*min_freq = freq_table[devfreq->profile->max_state - 1];
 		*max_freq = freq_table[0];
 	}
 
+	/* Apply constraints from PM QoS */
+	qos_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
+					     DEV_PM_QOS_MIN_FREQUENCY);
+	qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
+					     DEV_PM_QOS_MAX_FREQUENCY);
+	*min_freq = max(*min_freq, (unsigned long)HZ_PER_KHZ * qos_min_freq);
+	if (qos_max_freq != PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE)
+		*max_freq = min(*max_freq,
+				(unsigned long)HZ_PER_KHZ * qos_max_freq);
+
 	/* Apply constraints from sysfs */
 	*min_freq = max(*min_freq, devfreq->min_freq);
 	*max_freq = min(*max_freq, devfreq->max_freq);
 
 	/* Apply constraints from OPP interface */
@@ -606,24 +620,75 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
 			err);
 
 	return NOTIFY_OK;
 }
 
+/**
+ * qos_notifier_call() - Common handler for QoS constraints.
+ * @devfreq:    the devfreq instance.
+ */
+static int qos_notifier_call(struct devfreq *devfreq)
+{
+	int err;
+
+	mutex_lock(&devfreq->lock);
+	err = update_devfreq(devfreq);
+	mutex_unlock(&devfreq->lock);
+	if (err)
+		dev_err(devfreq->dev.parent,
+			"failed to update frequency from PM QoS (%d)\n",
+			err);
+
+	return NOTIFY_OK;
+}
+
+/**
+ * qos_min_notifier_call() - Callback for QoS min_freq changes.
+ * @nb:		Should be devfreq->nb_min
+ */
+static int qos_min_notifier_call(struct notifier_block *nb,
+					 unsigned long val, void *ptr)
+{
+	return qos_notifier_call(container_of(nb, struct devfreq, nb_min));
+}
+
+/**
+ * qos_max_notifier_call() - Callback for QoS max_freq changes.
+ * @nb:		Should be devfreq->nb_max
+ */
+static int qos_max_notifier_call(struct notifier_block *nb,
+					 unsigned long val, void *ptr)
+{
+	return qos_notifier_call(container_of(nb, struct devfreq, nb_max));
+}
+
 /**
  * devfreq_dev_release() - Callback for struct device to release the device.
  * @dev:	the devfreq device
  *
  * Remove devfreq from the list and release its resources.
  */
 static void devfreq_dev_release(struct device *dev)
 {
 	struct devfreq *devfreq = to_devfreq(dev);
+	int err;
 
 	mutex_lock(&devfreq_list_lock);
 	list_del(&devfreq->node);
 	mutex_unlock(&devfreq_list_lock);
 
+	err = dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_max,
+					 DEV_PM_QOS_MAX_FREQUENCY);
+	if (err)
+		dev_warn(dev->parent,
+			"Failed to remove max_freq notifier: %d\n", err);
+	err = dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_min,
+					 DEV_PM_QOS_MIN_FREQUENCY);
+	if (err)
+		dev_warn(dev->parent,
+			"Failed to remove min_freq notifier: %d\n", err);
+
 	if (devfreq->profile->exit)
 		devfreq->profile->exit(devfreq->dev.parent);
 
 	mutex_destroy(&devfreq->lock);
 	kfree(devfreq);
@@ -731,10 +796,22 @@ struct devfreq *devfreq_add_device(struct device *dev,
 				atomic_inc_return(&devfreq_no));
 	err = device_add(&devfreq->dev);
 	if (err)
 		goto err_dev;
 
+	devfreq->nb_min.notifier_call = qos_min_notifier_call;
+	err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
+				      DEV_PM_QOS_MIN_FREQUENCY);
+	if (err)
+		goto err_devfreq;
+
+	devfreq->nb_max.notifier_call = qos_max_notifier_call;
+	err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_max,
+				      DEV_PM_QOS_MAX_FREQUENCY);
+	if (err)
+		goto err_devfreq;
+
 	mutex_lock(&devfreq_list_lock);
 
 	governor = try_then_request_governor(devfreq->governor_name);
 	if (IS_ERR(governor)) {
 		dev_err(dev, "%s: Unable to find governor for the device\n",
@@ -758,10 +835,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 	return devfreq;
 
 err_init:
 	mutex_unlock(&devfreq_list_lock);
+err_devfreq:
 	devfreq_remove_device(devfreq);
 	return ERR_PTR(err);
 err_dev:
 	put_device(&devfreq->dev);
 err_out:
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index c3cbc15fdf08..dac0dffeabb4 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -134,10 +134,12 @@ struct devfreq_dev_profile {
  * @total_trans:	Number of devfreq transitions
  * @trans_table:	Statistics of devfreq transitions
  * @time_in_state:	Statistics of devfreq states
  * @last_stat_updated:	The last time stat updated
  * @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier
+ * @nb_min:		Notifier block for DEV_PM_QOS_MIN_FREQUENCY
+ * @nb_max:		Notifier block for DEV_PM_QOS_MAX_FREQUENCY
  *
  * This structure stores the devfreq information for a give device.
  *
  * Note that when a governor accesses entries in struct devfreq in its
  * functions except for the context of callbacks defined in struct
@@ -176,10 +178,13 @@ struct devfreq {
 	unsigned int *trans_table;
 	unsigned long *time_in_state;
 	unsigned long last_stat_updated;
 
 	struct srcu_notifier_head transition_notifier_list;
+
+	struct notifier_block nb_min;
+	struct notifier_block nb_max;
 };
 
 struct devfreq_freqs {
 	unsigned long old;
 	unsigned long new;
-- 
2.17.1

[PATCH v10 11/11] PM / devfreq: Use PM QoS for sysfs min/max_freq

[Leonard Crestez]

Switch the handling of min_freq and max_freq from sysfs to use the
dev_pm_qos_request interface.

Since PM QoS handles frequencies as kHz this change reduces the
precision of min_freq and max_freq. This shouldn't introduce problems
because frequencies which are not an integer number of kHz are likely
not an integer number of Hz either.

Try to ensure compatibility by rounding min values down and rounding
max values up.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Tested-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/devfreq/devfreq.c | 58 +++++++++++++++++++++++++++------------
 include/linux/devfreq.h   |  9 +++---
 2 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 776dec301a4e..ddf6a8ff454d 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -139,14 +139,10 @@ static void get_freq_range(struct devfreq *devfreq,
 	*min_freq = max(*min_freq, (unsigned long)HZ_PER_KHZ * qos_min_freq);
 	if (qos_max_freq != PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE)
 		*max_freq = min(*max_freq,
 				(unsigned long)HZ_PER_KHZ * qos_max_freq);
 
-	/* Apply constraints from sysfs */
-	*min_freq = max(*min_freq, devfreq->min_freq);
-	*max_freq = min(*max_freq, devfreq->max_freq);
-
 	/* Apply constraints from OPP interface */
 	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
 	*max_freq = min(*max_freq, devfreq->scaling_max_freq);
 
 	if (*min_freq > *max_freq)
@@ -688,10 +684,19 @@ static void devfreq_dev_release(struct device *dev)
 			"Failed to remove min_freq notifier: %d\n", err);
 
 	if (devfreq->profile->exit)
 		devfreq->profile->exit(devfreq->dev.parent);
 
+	err = dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
+	if (err)
+		dev_warn(dev->parent,
+			"Failed to remove max_freq request: %d\n", err);
+	err = dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
+	if (err)
+		dev_warn(dev->parent,
+			"Failed to remove min_freq request: %d\n", err);
+
 	mutex_destroy(&devfreq->lock);
 	kfree(devfreq);
 }
 
 /**
@@ -755,18 +760,26 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
 	if (!devfreq->scaling_min_freq) {
 		err = -EINVAL;
 		goto err_dev;
 	}
-	devfreq->min_freq = devfreq->scaling_min_freq;
 
 	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
 	if (!devfreq->scaling_max_freq) {
 		err = -EINVAL;
 		goto err_dev;
 	}
-	devfreq->max_freq = devfreq->scaling_max_freq;
+
+	err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
+				     DEV_PM_QOS_MIN_FREQUENCY, 0);
+	if (err < 0)
+		goto err_dev;
+	err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
+				     DEV_PM_QOS_MAX_FREQUENCY,
+				     PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
+	if (err < 0)
+		goto err_dev;
 
 	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	atomic_set(&devfreq->suspend_count, 0);
 
 	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
@@ -1401,14 +1414,15 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
-	mutex_lock(&df->lock);
-	df->min_freq = value;
-	update_devfreq(df);
-	mutex_unlock(&df->lock);
+	/* Round down to kHz for PM QoS */
+	ret = dev_pm_qos_update_request(&df->user_min_freq_req,
+					value / HZ_PER_KHZ);
+	if (ret < 0)
+		return ret;
 
 	return count;
 }
 
 static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
@@ -1433,18 +1447,28 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
-	mutex_lock(&df->lock);
-
-	if (!value)
-		value = ULONG_MAX;
+	/*
+	 * PM QoS frequencies are in kHz so we need to convert. Convert by
+	 * rounding upwards so that the acceptable interval never shrinks.
+	 *
+	 * For example if the user writes "666666666" to sysfs this value will
+	 * be converted to 666667 kHz and back to 666667000 Hz before an OPP
+	 * lookup, this ensures that an OPP of 666666666Hz is still accepted.
+	 *
+	 * A value of zero means "no limit".
+	 */
+	if (value)
+		value = DIV_ROUND_UP(value, HZ_PER_KHZ);
+	else
+		value = PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE;
 
-	df->max_freq = value;
-	update_devfreq(df);
-	mutex_unlock(&df->lock);
+	ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
+	if (ret < 0)
+		return ret;
 
 	return count;
 }
 static DEVICE_ATTR_RW(min_freq);
 
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index dac0dffeabb4..d4d1ec04aeea 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -11,10 +11,11 @@
 #define __LINUX_DEVFREQ_H__
 
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
 
 #define DEVFREQ_NAME_LEN 16
 
 /* DEVFREQ governor name */
 #define DEVFREQ_GOV_SIMPLE_ONDEMAND	"simple_ondemand"
@@ -121,12 +122,12 @@ struct devfreq_dev_profile {
  *		devfreq.nb to the corresponding register notifier call chain.
  * @work:	delayed work for load monitoring.
  * @previous_freq:	previously configured frequency value.
  * @data:	Private data of the governor. The devfreq framework does not
  *		touch this.
- * @min_freq:	Limit minimum frequency requested by user (0: none)
- * @max_freq:	Limit maximum frequency requested by user (0: none)
+ * @user_min_freq_req:	PM QoS minimum frequency request from user (via sysfs)
+ * @user_max_freq_req:	PM QoS maximum frequency request from user (via sysfs)
  * @scaling_min_freq:	Limit minimum frequency requested by OPP interface
  * @scaling_max_freq:	Limit maximum frequency requested by OPP interface
  * @stop_polling:	 devfreq polling status of a device.
  * @suspend_freq:	 frequency of a device set during suspend phase.
  * @resume_freq:	 frequency of a device set in resume phase.
@@ -161,12 +162,12 @@ struct devfreq {
 	unsigned long previous_freq;
 	struct devfreq_dev_status last_status;
 
 	void *data; /* private data for governors */
 
-	unsigned long min_freq;
-	unsigned long max_freq;
+	struct dev_pm_qos_request user_min_freq_req;
+	struct dev_pm_qos_request user_max_freq_req;
 	unsigned long scaling_min_freq;
 	unsigned long scaling_max_freq;
 	bool stop_polling;
 
 	unsigned long suspend_freq;
-- 
2.17.1

Re: [PATCH v10 06/11] PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs

[Chanwoo Choi]

Hi Leonard,

Why do you add the new patches on v10 (patch6/7/8)
in this series? If you think that need to update
the pm_qos core, you have to send the new patchset
on separate mail thread. It make the difficult
to merge the PM_QoS support of devfreq.

Please split out the patch6/7/8 from this series.

On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
> This allows dev_pm_qos to embed freq_qos structs, which is done in the
> next patch. Separate commit to make it easier to review.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index c35ff21e8a40..a8e1486e3200 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -47,25 +47,10 @@ struct pm_qos_request {
>  struct pm_qos_flags_request {
>  	struct list_head node;
>  	s32 flags;	/* Do not change to 64 bit */
>  };
>  
> -enum dev_pm_qos_req_type {
> -	DEV_PM_QOS_RESUME_LATENCY = 1,
> -	DEV_PM_QOS_LATENCY_TOLERANCE,
> -	DEV_PM_QOS_FLAGS,
> -};
> -
> -struct dev_pm_qos_request {
> -	enum dev_pm_qos_req_type type;
> -	union {
> -		struct plist_node pnode;
> -		struct pm_qos_flags_request flr;
> -	} data;
> -	struct device *dev;
> -};
> -
>  enum pm_qos_type {
>  	PM_QOS_UNITIALIZED,
>  	PM_QOS_MAX,		/* return the largest value */
>  	PM_QOS_MIN,		/* return the smallest value */
>  	PM_QOS_SUM		/* return the sum */
> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>  struct pm_qos_flags {
>  	struct list_head list;
>  	s32 effective_flags;	/* Do not change to 64 bit */
>  };
>  
> +
> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
> +
> +enum freq_qos_req_type {
> +	FREQ_QOS_MIN = 1,
> +	FREQ_QOS_MAX,
> +};
> +
> +struct freq_constraints {
> +	struct pm_qos_constraints min_freq;
> +	struct blocking_notifier_head min_freq_notifiers;
> +	struct pm_qos_constraints max_freq;
> +	struct blocking_notifier_head max_freq_notifiers;
> +};
> +
> +struct freq_qos_request {
> +	enum freq_qos_req_type type;
> +	struct plist_node pnode;
> +	struct freq_constraints *qos;
> +};
> +
> +
> +enum dev_pm_qos_req_type {
> +	DEV_PM_QOS_RESUME_LATENCY = 1,
> +	DEV_PM_QOS_LATENCY_TOLERANCE,
> +	DEV_PM_QOS_FLAGS,
> +};
> +
> +struct dev_pm_qos_request {
> +	enum dev_pm_qos_req_type type;
> +	union {
> +		struct plist_node pnode;
> +		struct pm_qos_flags_request flr;
> +	} data;
> +	struct device *dev;
> +};
> +
>  struct dev_pm_qos {
>  	struct pm_qos_constraints resume_latency;
>  	struct pm_qos_constraints latency_tolerance;
>  	struct pm_qos_flags flags;
>  	struct dev_pm_qos_request *resume_latency_req;
> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>  {
>  	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>  }
>  #endif
>  
> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
> -
> -enum freq_qos_req_type {
> -	FREQ_QOS_MIN = 1,
> -	FREQ_QOS_MAX,
> -};
> -
> -struct freq_constraints {
> -	struct pm_qos_constraints min_freq;
> -	struct blocking_notifier_head min_freq_notifiers;
> -	struct pm_qos_constraints max_freq;
> -	struct blocking_notifier_head max_freq_notifiers;
> -};
> -
> -struct freq_qos_request {
> -	enum freq_qos_req_type type;
> -	struct plist_node pnode;
> -	struct freq_constraints *qos;
> -};
> -
>  static inline int freq_qos_request_active(struct freq_qos_request *req)
>  {
>  	return !IS_ERR_OR_NULL(req->qos);
>  }
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v10 06/11] PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs

[Leonard Crestez]

On 01.11.2019 04:07, Chanwoo Choi wrote:
> Hi Leonard,
> 
> Why do you add the new patches on v10 (patch6/7/8)
> in this series? If you think that need to update
> the pm_qos core, you have to send the new patchset
> on separate mail thread. It make the difficult
> to merge the PM_QoS support of devfreq.
> 
> Please split out the patch6/7/8 from this series.

Unfortunately DEV_PM_QOS_MIN/MAX_FREQUENCY was removed when cpufreq 
switched away:

     https://patchwork.kernel.org/cover/11193021/

Support for freq limits in PM QoS needs to be restored, otherwise PM QoS 
for devfreq doesn't even compile. I posted the restoration separately:

     https://patchwork.kernel.org/cover/11212887/

I guess we can wait for Rafael to review that?

We could also consider other alternatives such as using "raw" PM QoS 
aggregation inside devfreq and asking all consumers to call 
devfreq-specific APIs for frequency constraints?

In the meantime I can post the devfreq cleanups in separately. would 
that help?

> On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
>> This allows dev_pm_qos to embed freq_qos structs, which is done in the
>> next patch. Separate commit to make it easier to review.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> ---
>>   include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>>   1 file changed, 38 insertions(+), 36 deletions(-)
>>
>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>> index c35ff21e8a40..a8e1486e3200 100644
>> --- a/include/linux/pm_qos.h
>> +++ b/include/linux/pm_qos.h
>> @@ -47,25 +47,10 @@ struct pm_qos_request {
>>   struct pm_qos_flags_request {
>>   	struct list_head node;
>>   	s32 flags;	/* Do not change to 64 bit */
>>   };
>>   
>> -enum dev_pm_qos_req_type {
>> -	DEV_PM_QOS_RESUME_LATENCY = 1,
>> -	DEV_PM_QOS_LATENCY_TOLERANCE,
>> -	DEV_PM_QOS_FLAGS,
>> -};
>> -
>> -struct dev_pm_qos_request {
>> -	enum dev_pm_qos_req_type type;
>> -	union {
>> -		struct plist_node pnode;
>> -		struct pm_qos_flags_request flr;
>> -	} data;
>> -	struct device *dev;
>> -};
>> -
>>   enum pm_qos_type {
>>   	PM_QOS_UNITIALIZED,
>>   	PM_QOS_MAX,		/* return the largest value */
>>   	PM_QOS_MIN,		/* return the smallest value */
>>   	PM_QOS_SUM		/* return the sum */
>> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>>   struct pm_qos_flags {
>>   	struct list_head list;
>>   	s32 effective_flags;	/* Do not change to 64 bit */
>>   };
>>   
>> +
>> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>> +
>> +enum freq_qos_req_type {
>> +	FREQ_QOS_MIN = 1,
>> +	FREQ_QOS_MAX,
>> +};
>> +
>> +struct freq_constraints {
>> +	struct pm_qos_constraints min_freq;
>> +	struct blocking_notifier_head min_freq_notifiers;
>> +	struct pm_qos_constraints max_freq;
>> +	struct blocking_notifier_head max_freq_notifiers;
>> +};
>> +
>> +struct freq_qos_request {
>> +	enum freq_qos_req_type type;
>> +	struct plist_node pnode;
>> +	struct freq_constraints *qos;
>> +};
>> +
>> +
>> +enum dev_pm_qos_req_type {
>> +	DEV_PM_QOS_RESUME_LATENCY = 1,
>> +	DEV_PM_QOS_LATENCY_TOLERANCE,
>> +	DEV_PM_QOS_FLAGS,
>> +};
>> +
>> +struct dev_pm_qos_request {
>> +	enum dev_pm_qos_req_type type;
>> +	union {
>> +		struct plist_node pnode;
>> +		struct pm_qos_flags_request flr;
>> +	} data;
>> +	struct device *dev;
>> +};
>> +
>>   struct dev_pm_qos {
>>   	struct pm_qos_constraints resume_latency;
>>   	struct pm_qos_constraints latency_tolerance;
>>   	struct pm_qos_flags flags;
>>   	struct dev_pm_qos_request *resume_latency_req;
>> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>   {
>>   	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>>   }
>>   #endif
>>   
>> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>> -
>> -enum freq_qos_req_type {
>> -	FREQ_QOS_MIN = 1,
>> -	FREQ_QOS_MAX,
>> -};
>> -
>> -struct freq_constraints {
>> -	struct pm_qos_constraints min_freq;
>> -	struct blocking_notifier_head min_freq_notifiers;
>> -	struct pm_qos_constraints max_freq;
>> -	struct blocking_notifier_head max_freq_notifiers;
>> -};
>> -
>> -struct freq_qos_request {
>> -	enum freq_qos_req_type type;
>> -	struct plist_node pnode;
>> -	struct freq_constraints *qos;
>> -};
>> -
>>   static inline int freq_qos_request_active(struct freq_qos_request *req)
>>   {
>>   	return !IS_ERR_OR_NULL(req->qos);
>>   }
>>   
>>
> 
> 

Re: [PATCH v10 06/11] PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs

[Chanwoo Choi]

Hi Leonard,

On 11/1/19 11:45 PM, Leonard Crestez wrote:
> On 01.11.2019 04:07, Chanwoo Choi wrote:
>> Hi Leonard,
>>
>> Why do you add the new patches on v10 (patch6/7/8)
>> in this series? If you think that need to update
>> the pm_qos core, you have to send the new patchset
>> on separate mail thread. It make the difficult
>> to merge the PM_QoS support of devfreq.
>>
>> Please split out the patch6/7/8 from this series.
> 
> Unfortunately DEV_PM_QOS_MIN/MAX_FREQUENCY was removed when cpufreq 
> switched away:
> 
>      https://patchwork.kernel.org/cover/11193021/
> 
> Support for freq limits in PM QoS needs to be restored, otherwise PM QoS 
> for devfreq doesn't even compile. I posted the restoration separately:
> 
>      https://patchwork.kernel.org/cover/11212887/
> 
> I guess we can wait for Rafael to review that?

I'm sorry for late reply.
Thanks for the explanation.

> 
> We could also consider other alternatives such as using "raw" PM QoS 
> aggregation inside devfreq and asking all consumers to call 
> devfreq-specific APIs for frequency constraints?

Actually, I don't want to make the separate devfreq-specific something
for PM QoS feature. If possible, I think that we need to reduce the redundant
or duplicate code in the linux kernel. Even if spend the long time
for updating or fixing the issue of PM QoS, we need to do them. Thanks.


> 
> In the meantime I can post the devfreq cleanups in separately. would 
> that help?

Yes, you better to send the devfreq cleanup patches separately.

> 
>> On 19. 11. 1. 오전 6:34, Leonard Crestez wrote:
>>> This allows dev_pm_qos to embed freq_qos structs, which is done in the
>>> next patch. Separate commit to make it easier to review.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> ---
>>>   include/linux/pm_qos.h | 74 ++++++++++++++++++++++--------------------
>>>   1 file changed, 38 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>>> index c35ff21e8a40..a8e1486e3200 100644
>>> --- a/include/linux/pm_qos.h
>>> +++ b/include/linux/pm_qos.h
>>> @@ -47,25 +47,10 @@ struct pm_qos_request {
>>>   struct pm_qos_flags_request {
>>>   	struct list_head node;
>>>   	s32 flags;	/* Do not change to 64 bit */
>>>   };
>>>   
>>> -enum dev_pm_qos_req_type {
>>> -	DEV_PM_QOS_RESUME_LATENCY = 1,
>>> -	DEV_PM_QOS_LATENCY_TOLERANCE,
>>> -	DEV_PM_QOS_FLAGS,
>>> -};
>>> -
>>> -struct dev_pm_qos_request {
>>> -	enum dev_pm_qos_req_type type;
>>> -	union {
>>> -		struct plist_node pnode;
>>> -		struct pm_qos_flags_request flr;
>>> -	} data;
>>> -	struct device *dev;
>>> -};
>>> -
>>>   enum pm_qos_type {
>>>   	PM_QOS_UNITIALIZED,
>>>   	PM_QOS_MAX,		/* return the largest value */
>>>   	PM_QOS_MIN,		/* return the smallest value */
>>>   	PM_QOS_SUM		/* return the sum */
>>> @@ -88,10 +73,48 @@ struct pm_qos_constraints {
>>>   struct pm_qos_flags {
>>>   	struct list_head list;
>>>   	s32 effective_flags;	/* Do not change to 64 bit */
>>>   };
>>>   
>>> +
>>> +#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>>> +#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>>> +
>>> +enum freq_qos_req_type {
>>> +	FREQ_QOS_MIN = 1,
>>> +	FREQ_QOS_MAX,
>>> +};
>>> +
>>> +struct freq_constraints {
>>> +	struct pm_qos_constraints min_freq;
>>> +	struct blocking_notifier_head min_freq_notifiers;
>>> +	struct pm_qos_constraints max_freq;
>>> +	struct blocking_notifier_head max_freq_notifiers;
>>> +};
>>> +
>>> +struct freq_qos_request {
>>> +	enum freq_qos_req_type type;
>>> +	struct plist_node pnode;
>>> +	struct freq_constraints *qos;
>>> +};
>>> +
>>> +
>>> +enum dev_pm_qos_req_type {
>>> +	DEV_PM_QOS_RESUME_LATENCY = 1,
>>> +	DEV_PM_QOS_LATENCY_TOLERANCE,
>>> +	DEV_PM_QOS_FLAGS,
>>> +};
>>> +
>>> +struct dev_pm_qos_request {
>>> +	enum dev_pm_qos_req_type type;
>>> +	union {
>>> +		struct plist_node pnode;
>>> +		struct pm_qos_flags_request flr;
>>> +	} data;
>>> +	struct device *dev;
>>> +};
>>> +
>>>   struct dev_pm_qos {
>>>   	struct pm_qos_constraints resume_latency;
>>>   	struct pm_qos_constraints latency_tolerance;
>>>   	struct pm_qos_flags flags;
>>>   	struct dev_pm_qos_request *resume_latency_req;
>>> @@ -253,31 +276,10 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>>   {
>>>   	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>>>   }
>>>   #endif
>>>   
>>> -#define FREQ_QOS_MIN_DEFAULT_VALUE	0
>>> -#define FREQ_QOS_MAX_DEFAULT_VALUE	(-1)
>>> -
>>> -enum freq_qos_req_type {
>>> -	FREQ_QOS_MIN = 1,
>>> -	FREQ_QOS_MAX,
>>> -};
>>> -
>>> -struct freq_constraints {
>>> -	struct pm_qos_constraints min_freq;
>>> -	struct blocking_notifier_head min_freq_notifiers;
>>> -	struct pm_qos_constraints max_freq;
>>> -	struct blocking_notifier_head max_freq_notifiers;
>>> -};
>>> -
>>> -struct freq_qos_request {
>>> -	enum freq_qos_req_type type;
>>> -	struct plist_node pnode;
>>> -	struct freq_constraints *qos;
>>> -};
>>> -
>>>   static inline int freq_qos_request_active(struct freq_qos_request *req)
>>>   {
>>>   	return !IS_ERR_OR_NULL(req->qos);
>>>   }
>>>   
>>>
>>
>>
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v10 01/11] PM / devfreq: Fix devfreq_notifier_call returning errno

[Chanwoo Choi]

On 11/1/19 6:34 AM, Leonard Crestez wrote:
> Notifier callbacks shouldn't return negative errno but one of the
> NOTIFY_OK/DONE/BAD values.
> 
> The OPP core will ignore return values from notifiers but returning a
> value that matches NOTIFY_STOP_MASK will stop the notification chain.
> 
> Fix by always returning NOTIFY_OK.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 323d43315d1e..b65faa1a2baa 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -548,30 +548,32 @@ EXPORT_SYMBOL(devfreq_interval_update);
>   */
>  static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>  				 void *devp)
>  {
>  	struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> -	int ret;
> +	int err = -EINVAL;
>  
>  	mutex_lock(&devfreq->lock);
>  
>  	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> -	if (!devfreq->scaling_min_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		return -EINVAL;
> -	}
> +	if (!devfreq->scaling_min_freq)
> +		goto out;
>  
>  	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> -	if (!devfreq->scaling_max_freq) {
> -		mutex_unlock(&devfreq->lock);
> -		return -EINVAL;
> -	}
> +	if (!devfreq->scaling_max_freq)
> +		goto out;
> +
> +	err = update_devfreq(devfreq);
>  
> -	ret = update_devfreq(devfreq);
> +out:
>  	mutex_unlock(&devfreq->lock);
> +	if (err)
> +		dev_err(devfreq->dev.parent,
> +			"failed to update frequency from OPP notifier (%d)\n",
> +			err);
>  
> -	return ret;
> +	return NOTIFY_OK;
>  }
>  
>  /**
>   * devfreq_dev_release() - Callback for struct device to release the device.
>   * @dev:	the devfreq device
> 

Applied it because it doesn't depend on the pm_qos feature.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v10 02/11] PM / devfreq: Set scaling_max_freq to max on OPP notifier error

[Chanwoo Choi]

On 11/1/19 6:34 AM, Leonard Crestez wrote:
> The devfreq_notifier_call functions will update scaling_min_freq and
> scaling_max_freq when the OPP table is updated.
> 
> If fetching the maximum frequency fails then scaling_max_freq remains
> set to zero which is confusing. Set to ULONG_MAX instead so we don't
> need special handling for this case in other places.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index b65faa1a2baa..715127f1cda5 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -557,12 +557,14 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>  	devfreq->scaling_min_freq = find_available_min_freq(devfreq);
>  	if (!devfreq->scaling_min_freq)
>  		goto out;
>  
>  	devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> -	if (!devfreq->scaling_max_freq)
> +	if (!devfreq->scaling_max_freq) {
> +		devfreq->scaling_max_freq = ULONG_MAX;
>  		goto out;
> +	}
>  
>  	err = update_devfreq(devfreq);
>  
>  out:
>  	mutex_unlock(&devfreq->lock);
> 

Applied it because it doesn't depend on the pm_qos feature.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v10 09/11] PM / devfreq: Introduce get_freq_range helper

[Chanwoo Choi]

On 11/1/19 6:34 AM, Leonard Crestez wrote:
> Moving handling of min/max freq to a single function and call it from
> update_devfreq and for printing min/max freq values in sysfs.
> 
> This changes the behavior of out-of-range min_freq/max_freq: clamping
> is now done at evaluation time. This means that if an out-of-range
> constraint is imposed by sysfs and it later becomes valid then it will
> be enforced.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 108 +++++++++++++++++++++-----------------
>  1 file changed, 60 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index ab12fd22aa08..ba3b53ee23fd 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -96,10 +96,51 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
>  		dev_pm_opp_put(opp);
>  
>  	return max_freq;
>  }
>  
> +/**
> + * get_freq_range() - Get the current freq range
> + * @devfreq:	the devfreq instance
> + * @min_freq:	the min frequency
> + * @max_freq:	the max frequency
> + *
> + * This takes into consideration all constraints.
> + */
> +static void get_freq_range(struct devfreq *devfreq,
> +			   unsigned long *min_freq,
> +			   unsigned long *max_freq)
> +{
> +	unsigned long *freq_table = devfreq->profile->freq_table;
> +
> +	lockdep_assert_held(&devfreq->lock);
> +
> +	/*
> +	 * Initialize minimum/maximum frequency from freq table.
> +	 * The devfreq drivers can initialize this in either ascending or
> +	 * descending order and devfreq core supports both.
> +	 */
> +	if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
> +		*min_freq = freq_table[0];
> +		*max_freq = freq_table[devfreq->profile->max_state - 1];
> +	} else {
> +		*min_freq = freq_table[devfreq->profile->max_state - 1];
> +		*max_freq = freq_table[0];
> +	}
> +
> +	/* Apply constraints from sysfs */
> +	*min_freq = max(*min_freq, devfreq->min_freq);
> +	*max_freq = min(*max_freq, devfreq->max_freq);
> +
> +	/* Apply constraints from OPP interface */
> +	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
> +	*max_freq = min(*max_freq, devfreq->scaling_max_freq);
> +
> +	if (*min_freq > *max_freq)
> +		*min_freq = *max_freq;
> +}
> +
>  /**
>   * devfreq_get_freq_level() - Lookup freq_table for the frequency
>   * @devfreq:	the devfreq instance
>   * @freq:	the target frequency
>   */
> @@ -348,20 +389,11 @@ int update_devfreq(struct devfreq *devfreq)
>  
>  	/* Reevaluate the proper frequency */
>  	err = devfreq->governor->get_target_freq(devfreq, &freq);
>  	if (err)
>  		return err;
> -
> -	/*
> -	 * 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);
> +	get_freq_range(devfreq, &min_freq, &max_freq);
>  
>  	if (freq < min_freq) {
>  		freq = min_freq;
>  		flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
>  	}
> @@ -1292,40 +1324,28 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
>  	ret = sscanf(buf, "%lu", &value);
>  	if (ret != 1)
>  		return -EINVAL;
>  
>  	mutex_lock(&df->lock);
> -
> -	if (value) {
> -		if (value > df->max_freq) {
> -			ret = -EINVAL;
> -			goto unlock;
> -		}
> -	} else {
> -		unsigned long *freq_table = df->profile->freq_table;
> -
> -		/* Get minimum frequency according to sorting order */
> -		if (freq_table[0] < freq_table[df->profile->max_state - 1])
> -			value = freq_table[0];
> -		else
> -			value = freq_table[df->profile->max_state - 1];
> -	}
> -
>  	df->min_freq = value;
>  	update_devfreq(df);
> -	ret = count;
> -unlock:
>  	mutex_unlock(&df->lock);
> -	return ret;
> +
> +	return count;
>  }
>  
>  static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
>  			     char *buf)
>  {
>  	struct devfreq *df = to_devfreq(dev);
> +	unsigned long min_freq, max_freq;
>  
> -	return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
> +	mutex_lock(&df->lock);
> +	get_freq_range(df, &min_freq, &max_freq);
> +	mutex_unlock(&df->lock);
> +
> +	return sprintf(buf, "%lu\n", min_freq);
>  }
>  
>  static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>  			      const char *buf, size_t count)
>  {
> @@ -1337,40 +1357,32 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>  	if (ret != 1)
>  		return -EINVAL;
>  
>  	mutex_lock(&df->lock);
>  
> -	if (value) {
> -		if (value < df->min_freq) {
> -			ret = -EINVAL;
> -			goto unlock;
> -		}
> -	} else {
> -		unsigned long *freq_table = df->profile->freq_table;
> -
> -		/* Get maximum frequency according to sorting order */
> -		if (freq_table[0] < freq_table[df->profile->max_state - 1])
> -			value = freq_table[df->profile->max_state - 1];
> -		else
> -			value = freq_table[0];
> -	}
> +	if (!value)
> +		value = ULONG_MAX;
>  
>  	df->max_freq = value;
>  	update_devfreq(df);
> -	ret = count;
> -unlock:
>  	mutex_unlock(&df->lock);
> -	return ret;
> +
> +	return count;
>  }
>  static DEVICE_ATTR_RW(min_freq);
>  
>  static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
>  			     char *buf)
>  {
>  	struct devfreq *df = to_devfreq(dev);
> +	unsigned long min_freq, max_freq;
> +
> +	mutex_lock(&df->lock);
> +	get_freq_range(df, &min_freq, &max_freq);
> +	mutex_unlock(&df->lock);
>  
> -	return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
> +	return sprintf(buf, "%lu\n", max_freq);
>  }
>  static DEVICE_ATTR_RW(max_freq);
>  
>  static ssize_t available_frequencies_show(struct device *d,
>  					  struct device_attribute *attr,
> 

Applied it because it doesn't depend on the pm_qos feature.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v10 09/11] PM / devfreq: Introduce get_freq_range helper

[Leonard Crestez]

On Mon, 2019-11-11 at 14:56 +0900, Chanwoo Choi wrote:
> On 11/1/19 6:34 AM, Leonard Crestez wrote:
> > Moving handling of min/max freq to a single function and call it from
> > update_devfreq and for printing min/max freq values in sysfs.
> > 
> > This changes the behavior of out-of-range min_freq/max_freq: clamping
> > is now done at evaluation time. This means that if an out-of-range
> > constraint is imposed by sysfs and it later becomes valid then it will
> > be enforced.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> > ---
> >  drivers/devfreq/devfreq.c | 108 +++++++++++++++++++++-----------------
> >  1 file changed, 60 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index ab12fd22aa08..ba3b53ee23fd 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -96,10 +96,51 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
> >  		dev_pm_opp_put(opp);
> >  
> >  	return max_freq;
> >  }
> >  
> > +/**
> > + * get_freq_range() - Get the current freq range
> > + * @devfreq:	the devfreq instance
> > + * @min_freq:	the min frequency
> > + * @max_freq:	the max frequency
> > + *
> > + * This takes into consideration all constraints.
> > + */
> > +static void get_freq_range(struct devfreq *devfreq,
> > +			   unsigned long *min_freq,
> > +			   unsigned long *max_freq)
> > +{
> > +	unsigned long *freq_table = devfreq->profile->freq_table;
> > +
> > +	lockdep_assert_held(&devfreq->lock);
> > +
> > +	/*
> > +	 * Initialize minimum/maximum frequency from freq table.
> > +	 * The devfreq drivers can initialize this in either ascending or
> > +	 * descending order and devfreq core supports both.
> > +	 */
> > +	if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
> > +		*min_freq = freq_table[0];
> > +		*max_freq = freq_table[devfreq->profile->max_state - 1];
> > +	} else {
> > +		*min_freq = freq_table[devfreq->profile->max_state - 1];
> > +		*max_freq = freq_table[0];
> > +	}
> > +
> > +	/* Apply constraints from sysfs */
> > +	*min_freq = max(*min_freq, devfreq->min_freq);
> > +	*max_freq = min(*max_freq, devfreq->max_freq);
> > +
> > +	/* Apply constraints from OPP interface */
> > +	*min_freq = max(*min_freq, devfreq->scaling_min_freq);
> > +	*max_freq = min(*max_freq, devfreq->scaling_max_freq);
> > +
> > +	if (*min_freq > *max_freq)
> > +		*min_freq = *max_freq;
> > +}
> > +
> >  /**
> >   * devfreq_get_freq_level() - Lookup freq_table for the frequency
> >   * @devfreq:	the devfreq instance
> >   * @freq:	the target frequency
> >   */
> > @@ -348,20 +389,11 @@ int update_devfreq(struct devfreq *devfreq)
> >  
> >  	/* Reevaluate the proper frequency */
> >  	err = devfreq->governor->get_target_freq(devfreq, &freq);
> >  	if (err)
> >  		return err;
> > -
> > -	/*
> > -	 * 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);
> > +	get_freq_range(devfreq, &min_freq, &max_freq);
> >  
> >  	if (freq < min_freq) {
> >  		freq = min_freq;
> >  		flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
> >  	}
> > @@ -1292,40 +1324,28 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
> >  	ret = sscanf(buf, "%lu", &value);
> >  	if (ret != 1)
> >  		return -EINVAL;
> >  
> >  	mutex_lock(&df->lock);
> > -
> > -	if (value) {
> > -		if (value > df->max_freq) {
> > -			ret = -EINVAL;
> > -			goto unlock;
> > -		}
> > -	} else {
> > -		unsigned long *freq_table = df->profile->freq_table;
> > -
> > -		/* Get minimum frequency according to sorting order */
> > -		if (freq_table[0] < freq_table[df->profile->max_state - 1])
> > -			value = freq_table[0];
> > -		else
> > -			value = freq_table[df->profile->max_state - 1];
> > -	}
> > -
> >  	df->min_freq = value;
> >  	update_devfreq(df);
> > -	ret = count;
> > -unlock:
> >  	mutex_unlock(&df->lock);
> > -	return ret;
> > +
> > +	return count;
> >  }
> >  
> >  static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
> >  			     char *buf)
> >  {
> >  	struct devfreq *df = to_devfreq(dev);
> > +	unsigned long min_freq, max_freq;
> >  
> > -	return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
> > +	mutex_lock(&df->lock);
> > +	get_freq_range(df, &min_freq, &max_freq);
> > +	mutex_unlock(&df->lock);
> > +
> > +	return sprintf(buf, "%lu\n", min_freq);
> >  }
> >  
> >  static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> >  			      const char *buf, size_t count)
> >  {
> > @@ -1337,40 +1357,32 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
> >  	if (ret != 1)
> >  		return -EINVAL;
> >  
> >  	mutex_lock(&df->lock);
> >  
> > -	if (value) {
> > -		if (value < df->min_freq) {
> > -			ret = -EINVAL;
> > -			goto unlock;
> > -		}
> > -	} else {
> > -		unsigned long *freq_table = df->profile->freq_table;
> > -
> > -		/* Get maximum frequency according to sorting order */
> > -		if (freq_table[0] < freq_table[df->profile->max_state - 1])
> > -			value = freq_table[df->profile->max_state - 1];
> > -		else
> > -			value = freq_table[0];
> > -	}
> > +	if (!value)
> > +		value = ULONG_MAX;
> >  
> >  	df->max_freq = value;
> >  	update_devfreq(df);
> > -	ret = count;
> > -unlock:
> >  	mutex_unlock(&df->lock);
> > -	return ret;
> > +
> > +	return count;
> >  }
> >  static DEVICE_ATTR_RW(min_freq);
> >  
> >  static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
> >  			     char *buf)
> >  {
> >  	struct devfreq *df = to_devfreq(dev);
> > +	unsigned long min_freq, max_freq;
> > +
> > +	mutex_lock(&df->lock);
> > +	get_freq_range(df, &min_freq, &max_freq);
> > +	mutex_unlock(&df->lock);
> >  
> > -	return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
> > +	return sprintf(buf, "%lu\n", max_freq);
> >  }
> >  static DEVICE_ATTR_RW(max_freq);
> >  
> >  static ssize_t available_frequencies_show(struct device *d,
> >  					  struct device_attribute *attr,
> > 
> 
> Applied it because it doesn't depend on the pm_qos feature.

Thanks, this will make it easier to move forward.

--
Regards,
Leonard