From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: rjw@rjwysocki.net, mroos@linux.ee, viresh.kumar@linaro.org
Cc: cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, srivatsa.bhat@linux.vnet.ibm.com
Subject: [PATCH v2 1/5] cpufreq, longhaul: Fix double invocation of cpufreq_freq_transition_begin/end
Date: Tue, 29 Apr 2014 00:24:09 +0530 [thread overview]
Message-ID: <20140428185403.28755.99738.stgit@srivatsabhat.in.ibm.com> (raw)
In-Reply-To: <20140428185331.28755.899.stgit@srivatsabhat.in.ibm.com>
During frequency transitions, the cpufreq core takes the responsibility of
invoking cpufreq_freq_transition_begin() and cpufreq_freq_transition_end()
for those cpufreq drivers that define the ->target_index callback but don't
set the ASYNC_NOTIFICATION flag.
The longhaul cpufreq driver falls under this category, but this driver was
invoking the _begin() and _end() APIs itself around frequency transitions,
which led to double invocation of the _begin() API. The _begin API makes
contending callers wait until the previous invocation is complete. Hence,
the longhaul driver ended up waiting on itself, leading to system hangs
during boot.
Fix this by removing the calls to the _begin() and _end() APIs from the
longhaul driver, since they rightly belong to the cpufreq core.
(Note that during module_exit(), the longhaul driver sets the frequency
without any help from the cpufreq core. So add explicit calls to the
_begin() and _end() APIs around that frequency transition alone, to take
care of that special case.)
Reported-and-Tested-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---
drivers/cpufreq/longhaul.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c
index d00e5d1..5c4369b 100644
--- a/drivers/cpufreq/longhaul.c
+++ b/drivers/cpufreq/longhaul.c
@@ -242,7 +242,7 @@ static void do_powersaver(int cx_address, unsigned int mults_index,
* Sets a new clock ratio.
*/
-static void longhaul_setstate(struct cpufreq_policy *policy,
+static int longhaul_setstate(struct cpufreq_policy *policy,
unsigned int table_index)
{
unsigned int mults_index;
@@ -258,10 +258,12 @@ static void longhaul_setstate(struct cpufreq_policy *policy,
/* Safety precautions */
mult = mults[mults_index & 0x1f];
if (mult == -1)
- return;
+ return -EINVAL;
+
speed = calc_speed(mult);
if ((speed > highest_speed) || (speed < lowest_speed))
- return;
+ return -EINVAL;
+
/* Voltage transition before frequency transition? */
if (can_scale_voltage && longhaul_index < table_index)
dir = 1;
@@ -269,8 +271,6 @@ static void longhaul_setstate(struct cpufreq_policy *policy,
freqs.old = calc_speed(longhaul_get_cpu_mult());
freqs.new = speed;
- cpufreq_freq_transition_begin(policy, &freqs);
-
pr_debug("Setting to FSB:%dMHz Mult:%d.%dx (%s)\n",
fsb, mult/10, mult%10, print_speed(speed/1000));
retry_loop:
@@ -385,12 +385,14 @@ retry_loop:
goto retry_loop;
}
}
- /* Report true CPU frequency */
- cpufreq_freq_transition_end(policy, &freqs, 0);
- if (!bm_timeout)
+ if (!bm_timeout) {
printk(KERN_INFO PFX "Warning: Timeout while waiting for "
"idle PCI bus.\n");
+ return -EBUSY;
+ }
+
+ return 0;
}
/*
@@ -631,9 +633,10 @@ static int longhaul_target(struct cpufreq_policy *policy,
unsigned int i;
unsigned int dir = 0;
u8 vid, current_vid;
+ int retval = 0;
if (!can_scale_voltage)
- longhaul_setstate(policy, table_index);
+ retval = longhaul_setstate(policy, table_index);
else {
/* On test system voltage transitions exceeding single
* step up or down were turning motherboard off. Both
@@ -648,7 +651,7 @@ static int longhaul_target(struct cpufreq_policy *policy,
while (i != table_index) {
vid = (longhaul_table[i].driver_data >> 8) & 0x1f;
if (vid != current_vid) {
- longhaul_setstate(policy, i);
+ retval = longhaul_setstate(policy, i);
current_vid = vid;
msleep(200);
}
@@ -657,10 +660,11 @@ static int longhaul_target(struct cpufreq_policy *policy,
else
i--;
}
- longhaul_setstate(policy, table_index);
+ retval = longhaul_setstate(policy, table_index);
}
+
longhaul_index = table_index;
- return 0;
+ return retval;
}
@@ -968,7 +972,15 @@ static void __exit longhaul_exit(void)
for (i = 0; i < numscales; i++) {
if (mults[i] == maxmult) {
+ struct cpufreq_freqs freqs;
+
+ freqs.old = policy->cur;
+ freqs.new = longhaul_table[i].frequency;
+ freqs.flags = 0;
+
+ cpufreq_freq_transition_begin(policy, &freqs);
longhaul_setstate(policy, i);
+ cpufreq_freq_transition_end(policy, &freqs, 0);
break;
}
}
next prev parent reply other threads:[~2014-04-28 18:54 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-28 18:53 [PATCH v2 0/5] Cpufreq frequency serialization fixes Srivatsa S. Bhat
2014-04-28 18:54 ` Srivatsa S. Bhat [this message]
2014-04-28 18:54 ` [PATCH v2 2/5] cpufreq, powernow-k6: Fix incorrect comparison with max_multipler Srivatsa S. Bhat
2014-04-28 18:54 ` [PATCH v2 3/5] cpufreq, powernow-k6: Fix double invocation of cpufreq_freq_transition_begin/end Srivatsa S. Bhat
2014-04-28 18:54 ` [PATCH v2 4/5] cpufreq, powernow-k7: " Srivatsa S. Bhat
2014-04-28 18:55 ` [PATCH v2 5/5] cpufreq: Catch double invocations " Srivatsa S. Bhat
2014-04-29 4:51 ` Viresh Kumar
2014-04-29 4:55 ` Viresh Kumar
2014-04-29 6:16 ` Srivatsa S. Bhat
2014-04-29 6:49 ` Viresh Kumar
2014-04-29 7:35 ` Srivatsa S. Bhat
2014-04-29 8:04 ` Viresh Kumar
2014-04-29 8:10 ` Srivatsa S. Bhat
2014-04-29 6:08 ` Srivatsa S. Bhat
2014-04-28 20:57 ` [PATCH v2 0/5] Cpufreq frequency serialization fixes Meelis Roos
2014-04-28 23:47 ` Rafael J. Wysocki
2014-04-29 5:59 ` Srivatsa S. Bhat
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=20140428185403.28755.99738.stgit@srivatsabhat.in.ibm.com \
--to=srivatsa.bhat@linux.vnet.ibm.com \
--cc=cpufreq@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mroos@linux.ee \
--cc=rjw@rjwysocki.net \
--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).