All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Brian Norris <briannorris@chromium.org>,
	Douglas Anderson <dianders@chromium.org>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Lee Jones <lee.jones@linaro.org>,
	Benson Leung <bleung@chromium.org>,
	Olof Johansson <olof@lixom.net>,
	Matthias Kaehlcke <mka@chromium.org>
Subject: [PATCH v5 02/12] PM / devfreq: Fix handling of min/max_freq == 0
Date: Tue,  3 Jul 2018 16:46:55 -0700	[thread overview]
Message-ID: <20180703234705.227473-3-mka@chromium.org> (raw)
In-Reply-To: <20180703234705.227473-1-mka@chromium.org>

Commit ab8f58ad72c4 ("PM / devfreq: Set min/max_freq when adding the
devfreq device") initializes df->min/max_freq with the min/max OPP when
the device is added. Later commit f1d981eaecf8 ("PM / devfreq: Use the
available min/max frequency") adds df->scaling_min/max_freq and the
following to the frequency adjustment code:

  max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);

With the current handling of min/max_freq this is incorrect:

Even though df->max_freq is now initialized to a value != 0 user space
can still set it to 0, in this case max_freq would be 0 instead of
df->scaling_max_freq as intended. In consequence the frequency adjustment
is not performed:

  if (max_freq && freq > max_freq) {
	freq = max_freq;

To fix this set df->min/max freq to the min/max OPP in max/max_freq_store,
when the user passes a value of 0. This also prevents df->max_freq from
being set below the min OPP when df->min_freq is 0, and similar for
min_freq. Since it is now guaranteed that df->min/max_freq can't be 0 the
checks for this case can be removed.

Fixes: f1d981eaecf8 ("PM / devfreq: Use the available min/max frequency")
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---
Changes in v5:
- none

Changes in v4:
- added 'Reviewed-by: Brian Norris <briannorris@chromium.org>' tag

Changes in v3:
- none

Changes in v2:
- handle freq tables sorted in ascending and descending order in
  min/max_freq_store()
- use same order for conditional statements in min/max_freq_store()
---
 drivers/devfreq/devfreq.c | 42 ++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 0057ef5b0a98..6f604f8b2b81 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -283,11 +283,11 @@ int update_devfreq(struct devfreq *devfreq)
 	max_freq = MIN(devfreq->scaling_max_freq, devfreq->max_freq);
 	min_freq = MAX(devfreq->scaling_min_freq, devfreq->min_freq);
 
-	if (min_freq && freq < min_freq) {
+	if (freq < min_freq) {
 		freq = min_freq;
 		flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
 	}
-	if (max_freq && freq > max_freq) {
+	if (freq > max_freq) {
 		freq = max_freq;
 		flags |= DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use LUB */
 	}
@@ -1122,18 +1122,27 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
 {
 	struct devfreq *df = to_devfreq(dev);
 	unsigned long value;
+	unsigned long *freq_table;
 	int ret;
-	unsigned long max;
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
 	mutex_lock(&df->lock);
-	max = df->max_freq;
-	if (value && max && value > max) {
-		ret = -EINVAL;
-		goto unlock;
+
+	if (value) {
+		if (value > df->max_freq) {
+			ret = -EINVAL;
+			goto unlock;
+		}
+	} else {
+		freq_table = df->profile->freq_table;
+		/* typical order is ascending, some drivers use descending */
+		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;
@@ -1157,18 +1166,27 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
 {
 	struct devfreq *df = to_devfreq(dev);
 	unsigned long value;
+	unsigned long *freq_table;
 	int ret;
-	unsigned long min;
 
 	ret = sscanf(buf, "%lu", &value);
 	if (ret != 1)
 		return -EINVAL;
 
 	mutex_lock(&df->lock);
-	min = df->min_freq;
-	if (value && min && value < min) {
-		ret = -EINVAL;
-		goto unlock;
+
+	if (value) {
+		if (value < df->min_freq) {
+			ret = -EINVAL;
+			goto unlock;
+		}
+	} else {
+		freq_table = df->profile->freq_table;
+		/* typical order is ascending, some drivers use descending */
+		if (freq_table[0] < freq_table[df->profile->max_state - 1])
+			value = freq_table[df->profile->max_state - 1];
+		else
+			value = freq_table[0];
 	}
 
 	df->max_freq = value;
-- 
2.18.0.203.gfac676dfb9-goog


  parent reply	other threads:[~2018-07-03 23:50 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-03 23:46 [PATCH v5 00/12] Add throttler driver for non-thermal throttling Matthias Kaehlcke
2018-07-03 23:46 ` [PATCH v5 01/12] PM / devfreq: Init user limits from OPP limits, not viceversa Matthias Kaehlcke
2018-07-03 23:46 ` Matthias Kaehlcke [this message]
2018-07-04  2:20   ` [PATCH v5 02/12] PM / devfreq: Fix handling of min/max_freq == 0 Chanwoo Choi
2018-07-06 16:36     ` Matthias Kaehlcke
2018-07-12  8:34       ` Chanwoo Choi
2018-07-03 23:46 ` [PATCH v5 03/12] PM / devfreq: Don't adjust to user limits in governors Matthias Kaehlcke
2018-07-04  2:27   ` Chanwoo Choi
2018-08-02 23:36   ` Matthias Kaehlcke
2018-08-03  0:03     ` Chanwoo Choi
2018-08-03  0:24       ` Matthias Kaehlcke
2018-08-03  0:43         ` Chanwoo Choi
2018-07-03 23:46 ` [PATCH v5 04/12] PM / devfreq: Add struct devfreq_policy Matthias Kaehlcke
2018-07-04  2:51   ` Chanwoo Choi
2018-07-06 17:07     ` Matthias Kaehlcke
2018-07-12  8:38       ` Chanwoo Choi
2018-08-03  0:04         ` Chanwoo Choi
2018-07-03 23:46 ` [PATCH v5 05/12] PM / devfreq: Add support for policy notifiers Matthias Kaehlcke
2018-07-04  6:41   ` Chanwoo Choi
2018-07-06 17:53     ` Matthias Kaehlcke
2018-07-12  8:44       ` Chanwoo Choi
2018-07-16 17:50         ` Matthias Kaehlcke
2018-07-31 19:39           ` Matthias Kaehlcke
2018-08-01  1:22             ` Chanwoo Choi
2018-08-01 17:08               ` Matthias Kaehlcke
2018-08-02  1:58                 ` Chanwoo Choi
2018-08-02 23:13                   ` Matthias Kaehlcke
2018-08-02 23:48                     ` Matthias Kaehlcke
2018-08-03  0:14                       ` Chanwoo Choi
2018-08-06 19:21                         ` Matthias Kaehlcke
2018-08-06 22:31                           ` Chanwoo Choi
2018-08-06 22:50                             ` Chanwoo Choi
2018-08-07  0:23                             ` Matthias Kaehlcke
2018-08-07  1:35                               ` Chanwoo Choi
2018-08-07 22:34                                 ` Matthias Kaehlcke
2018-08-02 23:56                     ` Chanwoo Choi
2018-08-06 18:46                       ` Matthias Kaehlcke
2018-08-06 22:16                         ` Chanwoo Choi
2018-07-03 23:46 ` [PATCH v5 06/12] PM / devfreq: Make update_devfreq() public Matthias Kaehlcke
2018-08-01  8:32   ` Chanwoo Choi
2018-07-03 23:47 ` [PATCH v5 07/12] PM / devfreq: export devfreq_class Matthias Kaehlcke
2018-07-04  5:30   ` Chanwoo Choi
2018-07-06 18:09     ` Matthias Kaehlcke
2018-07-12  9:08       ` Chanwoo Choi
2018-07-16 19:41         ` Matthias Kaehlcke
2018-07-31 19:29           ` Matthias Kaehlcke
2018-08-01  8:18             ` Chanwoo Choi
2018-08-01 17:18               ` Matthias Kaehlcke
2018-07-03 23:47 ` [PATCH v5 08/12] cpufreq: Add stub for cpufreq_update_policy() Matthias Kaehlcke
2018-07-04 10:41   ` Rafael J. Wysocki
2018-07-10 22:24     ` Matthias Kaehlcke
2018-07-04 10:44   ` Viresh Kumar
2018-07-03 23:47 ` [PATCH v5 09/12] dt-bindings: misc: add bindings for throttler Matthias Kaehlcke
2018-07-04 10:00   ` Viresh Kumar
2018-07-04 10:00     ` Viresh Kumar
2018-08-01  8:27   ` Chanwoo Choi
2018-08-01 17:39     ` Matthias Kaehlcke
2018-07-03 23:47 ` [PATCH v5 10/12] misc: throttler: Add core support for non-thermal throttling Matthias Kaehlcke
2018-07-03 23:47 ` [PATCH v5 11/12] misc: throttler: Add Chrome OS EC throttler Matthias Kaehlcke
2018-07-03 23:47 ` [PATCH v5 12/12] mfd: cros_ec: Add throttler sub-device Matthias Kaehlcke
2018-07-04  7:59   ` Lee Jones
     [not found] ` <CGME20180703234727epcas3p1b9f4a41b1f1714c8c059100d46b816dd@epcms1p5>
2018-07-04  2:24   ` [PATCH v5 01/12] PM / devfreq: Init user limits from OPP limits, not viceversa MyungJoo Ham
2018-07-04  2:24     ` MyungJoo Ham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180703234705.227473-3-mka@chromium.org \
    --to=mka@chromium.org \
    --cc=arnd@arndb.de \
    --cc=bleung@chromium.org \
    --cc=briannorris@chromium.org \
    --cc=cw00.choi@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=enric.balletbo@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kyungmin.park@samsung.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=myungjoo.ham@samsung.com \
    --cc=olof@lixom.net \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.