| Message ID | 20140425081804.10258.8324.stgit@srivatsabhat.in.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 25 April 2014 13:48, Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> wrote: > diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c > @@ -269,8 +269,6 @@ static void longhaul_setstate(struct cpufreq_policy *policy, This routine has this code as well: mult = mults[mults_index & 0x1f]; if (mult == -1) return; speed = calc_speed(mult); if ((speed > highest_speed) || (speed < lowest_speed)) return; And so it might return without changing frequency and that's why I left this driver earlier for this change.. So, please return -EINVAL from here.. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/25/2014 02:07 PM, Viresh Kumar wrote: > On 25 April 2014 13:48, Srivatsa S. Bhat > <srivatsa.bhat@linux.vnet.ibm.com> wrote: > >> diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c > >> @@ -269,8 +269,6 @@ static void longhaul_setstate(struct cpufreq_policy *policy, > > This routine has this code as well: > > mult = mults[mults_index & 0x1f]; > if (mult == -1) > return; > speed = calc_speed(mult); > if ((speed > highest_speed) || (speed < lowest_speed)) > return; > > And so it might return without changing frequency and that's why I > left this driver earlier for this change.. So, please return -EINVAL > from here.. > Ok, I'll address this in the next version. Thanks for pointing this out! Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpufreq/longhaul.c b/drivers/cpufreq/longhaul.c index d00e5d1..9b6dd9a 100644 --- a/drivers/cpufreq/longhaul.c +++ b/drivers/cpufreq/longhaul.c @@ -269,8 +269,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,8 +383,6 @@ retry_loop: goto retry_loop; } } - /* Report true CPU frequency */ - cpufreq_freq_transition_end(policy, &freqs, 0); if (!bm_timeout) printk(KERN_INFO PFX "Warning: Timeout while waiting for " @@ -968,7 +964,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; } }
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-by: Meelis Roos <mroos@linux.ee> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> --- drivers/cpufreq/longhaul.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html