| Message ID | f9d7b0d5424e4443597d2ed39bec3fedd2b10d1e.1387848958.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 26 December 2013 06:35, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: cpufreq: Clean up after a failing light-weight initialization > > If cpufreq_policy_restore() returns NULL during system resume, > __cpufreq_add_dev() should just fall back to the full initialization > instead of returning an error, because that may actually make things > work. Moreover, it should not leave stale fallback data behind after > it has failed to restore a previously existing policy. > > This change is based on Viresh Kumar's work. > > Reported-by: Bjørn Mork <bjorn@mork.no> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 26 December 2013 08:17, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 26 December 2013 06:35, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Subject: cpufreq: Clean up after a failing light-weight initialization >> >> If cpufreq_policy_restore() returns NULL during system resume, >> __cpufreq_add_dev() should just fall back to the full initialization >> instead of returning an error, because that may actually make things >> work. Moreover, it should not leave stale fallback data behind after >> it has failed to restore a previously existing policy. >> >> This change is based on Viresh Kumar's work. >> >> Reported-by: Bjørn Mork <bjorn@mork.no> >> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> --- > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> I think there is nothing much different in this patch compared to what Bjorn tested. So you can probably push that now and let him test linux-next later once he is back?
On 27 December 2013 15:27, Viresh Kumar <viresh.kumar@linaro.org> wrote: > I think there is nothing much different in this patch compared to what Bjorn > tested. So you can probably push that now and let him test linux-next later > once he is back? Just saw that you already pushed it. :)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 16d7b4a..0a48e71 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1016,16 +1016,24 @@ static int __cpufreq_add_dev(struct device *dev, struct subsys_interface *sif, read_unlock_irqrestore(&cpufreq_driver_lock, flags); #endif - if (frozen) + if (frozen) { /* Restore the saved policy when doing light-weight init */ policy = cpufreq_policy_restore(cpu); - else + + /* + * As we failed to resume cpufreq core last time, lets try to + * create a new policy. + */ + if (!policy) + frozen = false; + } + + if (!frozen) policy = cpufreq_policy_alloc(); if (!policy) goto nomem_out; - /* * In the resume path, since we restore a saved policy, the assignment * to policy->cpu is like an update of the existing policy, rather than @@ -1118,8 +1126,14 @@ err_get_freq: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); err_set_policy_cpu: - if (frozen) + if (frozen) { + /* + * Clear fallback data as we should try to make things work on + * next suspend/resume + */ + per_cpu(cpufreq_cpu_data_fallback, cpu) = NULL; cpufreq_policy_put_kobj(policy); + } cpufreq_policy_free(policy); nomem_out:
__cpufreq_add_dev() can fail sometimes while we are resuming our system. Currently we are clearing all sysfs nodes for cpufreq's failed policy as that could make userspace unstable. But if we suspend/resume again, we should atleast try to bring back those policies. This patch fixes this issue by clearing fallback data on failure and trying to allocate a new struct cpufreq_policy on second resume. Reported-and-tested-by: Bjørn Mork <bjorn@mork.no> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- These are sent again (earlier sent as reply to emails), so that people can give inputs if they have any. Tested on my thinkpad T420. drivers/cpufreq/cpufreq.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)