From: "Jon Medhurst (Tixy)" <tixy@linaro.org>
To: Viresh Kumar <viresh.kumar@linaro.org>,
Sudeep Holla <sudeep.holla@arm.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Turquette <mturquette@baylibre.com>
Subject: [PATCH v2] cpufreq: arm_big_little: fix frequency check when bL switcher is active
Date: Wed, 21 Oct 2015 10:55:33 +0100 [thread overview]
Message-ID: <1445421333.2916.13.camel@linaro.org> (raw)
The check for correct frequency being set in bL_cpufreq_set_rate is
broken when the big.LITTLE switcher is active, for two reasons.
1. The 'new_rate' variable gets overwritten before the test by the
code calculating the frequency of the old cluster.
2. The frequency returned by bL_cpufreq_get_rate will be the virtual
frequency, not the actual one the intended version of new_rate contains.
This means the function always returns an error causing an endless
stream of: "cpufreq: __target_index: Failed to change cpu frequency: -5"
As the intent is to check for errors that clk_set_rate doesn't report
lets move the check to immediately after that and directly use
clk_get_rate, rather than the arm_big_little helpers which only confuse
matters. Also, update the comment to be hopefully clearer about the
purpose of the code.
Fixes: 0a95e630b49a ("cpufreq: arm_big_little: check if the frequency is set correctly")
Signed-off-by: Jon Medhurst <tixy@linaro.org>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
---
Changes since V1:
- Check rate using clk_get_rate rather than disabling check when bL
switcher active
Sudeep, I added your Ack from the last comment on the previous patch.
This final patch differs from what was discussed only in the commit
message and in source comment which is hopefully more clear and is
also satisfactory.
I've also added Michael Turquette's correct email to the CC this time,
rather than his old Linaro address which was bouncing.
drivers/cpufreq/arm_big_little.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c
index f1e42f8..c5d256c 100644
--- a/drivers/cpufreq/arm_big_little.c
+++ b/drivers/cpufreq/arm_big_little.c
@@ -149,6 +149,19 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
__func__, cpu, old_cluster, new_cluster, new_rate);
ret = clk_set_rate(clk[new_cluster], new_rate * 1000);
+ if (!ret) {
+ /*
+ * FIXME: clk_set_rate hasn't returned an error here however it
+ * may be that clk_change_rate failed due to hardware or
+ * firmware issues and wasn't able to report that due to the
+ * current design of the clk core layer. To work around this
+ * problem we will read back the clock rate and check it is
+ * correct. This needs to be removed once clk core is fixed.
+ */
+ if (clk_get_rate(clk[new_cluster]) != new_rate * 1000)
+ ret = -EIO;
+ }
+
if (WARN_ON(ret)) {
pr_err("clk_set_rate failed: %d, new cluster: %d\n", ret,
new_cluster);
@@ -189,15 +202,6 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
mutex_unlock(&cluster_lock[old_cluster]);
}
- /*
- * FIXME: clk_set_rate has to handle the case where clk_change_rate
- * can fail due to hardware or firmware issues. Until the clk core
- * layer is fixed, we can check here. In most of the cases we will
- * be reading only the cached value anyway. This needs to be removed
- * once clk core is fixed.
- */
- if (bL_cpufreq_get_rate(cpu) != new_rate)
- return -EIO;
return 0;
}
--
2.1.4
next reply other threads:[~2015-10-21 9:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-21 9:55 Jon Medhurst (Tixy) [this message]
2015-10-21 9:57 ` [PATCH v2] cpufreq: arm_big_little: fix frequency check when bL switcher is active Viresh Kumar
2015-10-30 10:48 ` Jon Medhurst (Tixy)
2015-10-31 2:34 ` Rafael J. Wysocki
2015-11-05 22:46 ` Rafael J. Wysocki
2015-10-26 12:42 ` Michael Turquette
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=1445421333.2916.13.camel@linaro.org \
--to=tixy@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=rjw@rjwysocki.net \
--cc=sudeep.holla@arm.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).