From 7179621023011f23f636b3e9fcc97c41aa9d6823 Mon Sep 17 00:00:00 2001 From: Abhilash Kesavan Date: Fri, 31 Oct 2014 18:09:33 +0530 Subject: [PATCH 1/5] cpufreq: cpufreq-dt: Fix arguments in clock failure error message Fix the swapped arguments in the clock failure dev_err. Signed-off-by: Abhilash Kesavan Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq-dt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/cpufreq/cpufreq-dt.c b/drivers/cpufreq/cpufreq-dt.c index 23aaf40cf37f..f657c571b18e 100644 --- a/drivers/cpufreq/cpufreq-dt.c +++ b/drivers/cpufreq/cpufreq-dt.c @@ -166,8 +166,8 @@ try_again: if (ret == -EPROBE_DEFER) dev_dbg(cpu_dev, "cpu%d clock not ready, retry\n", cpu); else - dev_err(cpu_dev, "failed to get cpu%d clock: %d\n", ret, - cpu); + dev_err(cpu_dev, "failed to get cpu%d clock: %d\n", cpu, + ret); } else { *cdev = cpu_dev; *creg = cpu_reg; From 09712f557b31838092e1f22a5f2dd131a843a3de Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 4 Nov 2014 17:05:25 +0100 Subject: [PATCH 2/5] cpufreq: Avoid crash in resume on SMP without OPP When resuming from s2ram on an SMP system without cpufreq operating points (e.g. there's no "operating-points" property for the CPU node in DT, or the platform doesn't use DT yet), the kernel crashes when bringing CPU 1 online: Enabling non-boot CPUs ... CPU1: Booted secondary processor Unable to handle kernel NULL pointer dereference at virtual address 0000003c pgd = ee5e6b00 [0000003c] *pgd=6e579003, *pmd=6e588003, *pte=00000000 Internal error: Oops: a07 [#1] SMP ARM Modules linked in: CPU: 0 PID: 1246 Comm: s2ram Tainted: G W 3.18.0-rc3-koelsch-01614-g0377af242bb175c8-dirty #589 task: eeec5240 ti: ee704000 task.ti: ee704000 PC is at __cpufreq_add_dev.isra.24+0x24c/0x77c LR is at __cpufreq_add_dev.isra.24+0x244/0x77c pc : [] lr : [] psr: 60000153 sp : ee705d48 ip : ee705d48 fp : ee705d84 r10: c04e0450 r9 : 00000000 r8 : 00000001 r7 : c05426a8 r6 : 00000001 r5 : 00000001 r4 : 00000000 r3 : 00000000 r2 : 00000000 r1 : 20000153 r0 : c0542734 Verify that policy is not NULL before dereferencing it to fix this. Signed-off-by: Geert Uytterhoeven Fixes: 8414809c6a1e (cpufreq: Preserve policy structure across suspend/resume) Cc: 3.12+ # 3.12+ Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 644b54e1e7d1..4473eba1d6b0 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1022,7 +1022,8 @@ static struct cpufreq_policy *cpufreq_policy_restore(unsigned int cpu) read_unlock_irqrestore(&cpufreq_driver_lock, flags); - policy->governor = NULL; + if (policy) + policy->governor = NULL; return policy; } From c16561e8df7a64764ef61f02221e98273add325a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 6 Nov 2014 00:37:08 +0100 Subject: [PATCH 3/5] PM / Domains: Change prototype for the attach and detach callbacks Convert the prototypes to return an int in order to support error handling in these callbacks. Also, as suggested by Dmitry Torokhov, pass the domain pointer for use inside the callbacks, and so that they match the existing power_on/power_off callbacks which currently take the domain pointer. Acked-by: Dmitry Torokhov Acked-by: Geert Uytterhoeven Signed-off-by: Ulf Hansson [ khilman: added domain as parameter to callbacks, as suggested by Dmitry ] Signed-off-by: Kevin Hilman Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 4 ++-- include/linux/pm_domain.h | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 40bc2f4072cc..b520687046d4 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, spin_unlock_irq(&dev->power.lock); if (genpd->attach_dev) - genpd->attach_dev(dev); + genpd->attach_dev(genpd, dev); mutex_lock(&gpd_data->lock); gpd_data->base.dev = dev; @@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd, genpd->max_off_time_changed = true; if (genpd->detach_dev) - genpd->detach_dev(dev); + genpd->detach_dev(genpd, dev); spin_lock_irq(&dev->power.lock); diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 73e938b7e937..b3ed7766a291 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -72,8 +72,10 @@ struct generic_pm_domain { bool max_off_time_changed; bool cached_power_down_ok; struct gpd_cpuidle_data *cpuidle_data; - void (*attach_dev)(struct device *dev); - void (*detach_dev)(struct device *dev); + int (*attach_dev)(struct generic_pm_domain *domain, + struct device *dev); + void (*detach_dev)(struct generic_pm_domain *domain, + struct device *dev); }; static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd) From 403b9636fe9f59124d1a437a297b330729061252 Mon Sep 17 00:00:00 2001 From: Dmitry Eremin-Solenikov Date: Sat, 8 Nov 2014 19:17:13 +0300 Subject: [PATCH 4/5] PM / sleep: Fix entering suspend-to-IDLE if no freeze_oops is set If no freeze_ops is set, trying to enter suspend-to-IDLE will cause a nice oops in platform_suspend_prepare_late(). Add respective checks to platform_suspend_prepare_late() and platform_resume_early() functions. Fixes: a8d46b9e4e48 (ACPI / sleep: Rework the handling of ACPI GPE wakeup ...) Signed-off-by: Dmitry Eremin-Solenikov Signed-off-by: Rafael J. Wysocki --- kernel/power/suspend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c index 4ca9a33ff620..c347e3ce3a55 100644 --- a/kernel/power/suspend.c +++ b/kernel/power/suspend.c @@ -146,7 +146,7 @@ static int platform_suspend_prepare(suspend_state_t state) static int platform_suspend_prepare_late(suspend_state_t state) { - return state == PM_SUSPEND_FREEZE && freeze_ops->prepare ? + return state == PM_SUSPEND_FREEZE && freeze_ops && freeze_ops->prepare ? freeze_ops->prepare() : 0; } @@ -164,7 +164,7 @@ static void platform_resume_noirq(suspend_state_t state) static void platform_resume_early(suspend_state_t state) { - if (state == PM_SUSPEND_FREEZE && freeze_ops->restore) + if (state == PM_SUSPEND_FREEZE && freeze_ops && freeze_ops->restore) freeze_ops->restore(); } From 67732cd34382066ae5df313b6dad65ab14b9735f Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 11 Nov 2014 11:07:08 +0100 Subject: [PATCH 5/5] PM / Domains: Fix initial default state of the need_restore flag The initial state of the device's need_restore flag should'nt depend on the current state of the PM domain. For example it should be perfectly valid to attach an inactive device to a powered PM domain. The pm_genpd_dev_need_restore() API allow us to update the need_restore flag to somewhat cope with such scenarios. Typically that should have been done from drivers/buses ->probe() since it's those that put the requirements on the value of the need_restore flag. Until recently, the Exynos SOCs were the only user of the pm_genpd_dev_need_restore() API, though invoking it from a centralized location while adding devices to their PM domains. Due to that Exynos now have swithed to the generic OF-based PM domain look-up, it's no longer possible to invoke the API from a centralized location. The reason is because devices are now added to their PM domains during the probe sequence. Commit "ARM: exynos: Move to generic PM domain DT bindings" did the switch for Exynos to the generic OF-based PM domain look-up, but it also removed the call to pm_genpd_dev_need_restore(). This caused a regression for some of the Exynos drivers. To handle things more properly in the generic PM domain, let's change the default initial value of the need_restore flag to reflect that the state is unknown. As soon as some of the runtime PM callbacks gets invoked, update the initial value accordingly. Moreover, since the generic PM domain is verifying that all devices are both runtime PM enabled and suspended, using pm_runtime_suspended() while pm_genpd_poweroff() is invoked from the scheduled work, we can be sure of that the PM domain won't be powering off while having active devices. Do note that, the generic PM domain can still only know about active devices which has been activated through invoking its runtime PM resume callback. In other words, buses/drivers using pm_runtime_set_active() during ->probe() will still suffer from a race condition, potentially probing a device without having its PM domain being powered. That issue will have to be solved using a different approach. This a log from the boot regression for Exynos5, which is being fixed in this patch. ------------[ cut here ]------------ WARNING: CPU: 0 PID: 308 at ../drivers/clk/clk.c:851 clk_disable+0x24/0x30() Modules linked in: CPU: 0 PID: 308 Comm: kworker/0:1 Not tainted 3.18.0-rc3-00569-gbd9449f-dirty #10 Workqueue: pm pm_runtime_work [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x70/0xbc) [] (dump_stack) from [] (warn_slowpath_common+0x64/0x88) [] (warn_slowpath_common) from [] (warn_slowpath_null+0x1c/0x24) [] (warn_slowpath_null) from [] (clk_disable+0x24/0x30) [] (clk_disable) from [] (gsc_runtime_suspend+0x128/0x160) [] (gsc_runtime_suspend) from [] (pm_generic_runtime_suspend+0x2c/0x38) [] (pm_generic_runtime_suspend) from [] (pm_genpd_default_save_state+0x2c/0x8c) [] (pm_genpd_default_save_state) from [] (pm_genpd_poweroff+0x224/0x3ec) [] (pm_genpd_poweroff) from [] (pm_genpd_runtime_suspend+0x9c/0xcc) [] (pm_genpd_runtime_suspend) from [] (__rpm_callback+0x2c/0x60) [] (__rpm_callback) from [] (rpm_callback+0x20/0x74) [] (rpm_callback) from [] (rpm_suspend+0xd4/0x43c) [] (rpm_suspend) from [] (pm_runtime_work+0x80/0x90) [] (pm_runtime_work) from [] (process_one_work+0x12c/0x314) [] (process_one_work) from [] (worker_thread+0x3c/0x4b0) [] (worker_thread) from [] (kthread+0xcc/0xe8) [] (kthread) from [] (ret_from_fork+0x14/0x3c) ---[ end trace 40cd58bcd6988f12 ]--- Fixes: a4a8c2c4962bb655 (ARM: exynos: Move to generic PM domain DT bindings) Reported-and-tested0by: Sylwester Nawrocki Reviewed-by: Sylwester Nawrocki Reviewed-by: Kevin Hilman Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 38 +++++++++++++++++++++++++++++++------ include/linux/pm_domain.h | 2 +- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index b520687046d4..fb83d4acd400 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -361,9 +361,19 @@ static int __pm_genpd_save_device(struct pm_domain_data *pdd, struct device *dev = pdd->dev; int ret = 0; - if (gpd_data->need_restore) + if (gpd_data->need_restore > 0) return 0; + /* + * If the value of the need_restore flag is still unknown at this point, + * we trust that pm_genpd_poweroff() has verified that the device is + * already runtime PM suspended. + */ + if (gpd_data->need_restore < 0) { + gpd_data->need_restore = 1; + return 0; + } + mutex_unlock(&genpd->lock); genpd_start_dev(genpd, dev); @@ -373,7 +383,7 @@ static int __pm_genpd_save_device(struct pm_domain_data *pdd, mutex_lock(&genpd->lock); if (!ret) - gpd_data->need_restore = true; + gpd_data->need_restore = 1; return ret; } @@ -389,12 +399,17 @@ static void __pm_genpd_restore_device(struct pm_domain_data *pdd, { struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd); struct device *dev = pdd->dev; - bool need_restore = gpd_data->need_restore; + int need_restore = gpd_data->need_restore; - gpd_data->need_restore = false; + gpd_data->need_restore = 0; mutex_unlock(&genpd->lock); genpd_start_dev(genpd, dev); + + /* + * Call genpd_restore_dev() for recently added devices too (need_restore + * is negative then). + */ if (need_restore) genpd_restore_dev(genpd, dev); @@ -603,6 +618,7 @@ static void genpd_power_off_work_fn(struct work_struct *work) static int pm_genpd_runtime_suspend(struct device *dev) { struct generic_pm_domain *genpd; + struct generic_pm_domain_data *gpd_data; bool (*stop_ok)(struct device *__dev); int ret; @@ -628,6 +644,16 @@ static int pm_genpd_runtime_suspend(struct device *dev) return 0; mutex_lock(&genpd->lock); + + /* + * If we have an unknown state of the need_restore flag, it means none + * of the runtime PM callbacks has been invoked yet. Let's update the + * flag to reflect that the current state is active. + */ + gpd_data = to_gpd_data(dev->power.subsys_data->domain_data); + if (gpd_data->need_restore < 0) + gpd_data->need_restore = 0; + genpd->in_progress++; pm_genpd_poweroff(genpd); genpd->in_progress--; @@ -1442,7 +1468,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, mutex_lock(&gpd_data->lock); gpd_data->base.dev = dev; list_add_tail(&gpd_data->base.list_node, &genpd->dev_list); - gpd_data->need_restore = genpd->status == GPD_STATE_POWER_OFF; + gpd_data->need_restore = -1; gpd_data->td.constraint_changed = true; gpd_data->td.effective_constraint_ns = -1; mutex_unlock(&gpd_data->lock); @@ -1546,7 +1572,7 @@ void pm_genpd_dev_need_restore(struct device *dev, bool val) psd = dev_to_psd(dev); if (psd && psd->domain_data) - to_gpd_data(psd->domain_data)->need_restore = val; + to_gpd_data(psd->domain_data)->need_restore = val ? 1 : 0; spin_unlock_irqrestore(&dev->power.lock, flags); } diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index b3ed7766a291..2e0e06daf8c0 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -106,7 +106,7 @@ struct generic_pm_domain_data { struct notifier_block nb; struct mutex lock; unsigned int refcount; - bool need_restore; + int need_restore; }; #ifdef CONFIG_PM_GENERIC_DOMAINS