From 494fd7b7ad10c33d3a7ff7d10b71b3ecad10474a Mon Sep 17 00:00:00 2001 From: Feng Kan Date: Tue, 10 Apr 2018 16:57:06 -0700 Subject: [PATCH 01/14] PM / core: fix deferred probe breaking suspend resume order When bridge and its endpoint is enumerated the devices are added to the dpm list. Afterward, the bridge defers probe when IOMMU is not ready. This causes the bridge to be moved to the end of the dpm list when deferred probe kicks in. The order of the dpm list for bridge and endpoint is reversed. Add reordering code to move the bridge and its children and consumers to the end of the pm list so the order for suspend and resume is not altered. The code also move device and its children and consumers to the tail of device_kset list if it is registered. Signed-off-by: Toan Le Signed-off-by: Feng Kan Reviewed-by: Greg Kroah-Hartman Signed-off-by: Rafael J. Wysocki --- drivers/base/base.h | 3 +++ drivers/base/core.c | 20 ++++++++++++++++++++ drivers/base/dd.c | 4 +--- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index d800de650fa5..a75c3025fb78 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -161,3 +161,6 @@ extern void device_links_driver_cleanup(struct device *dev); extern void device_links_no_driver(struct device *dev); extern bool device_links_busy(struct device *dev); extern void device_links_unbind_consumers(struct device *dev); + +/* device pm support */ +void device_pm_move_to_tail(struct device *dev); diff --git a/drivers/base/core.c b/drivers/base/core.c index b610816eb887..ad7b50897bcc 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -144,6 +144,26 @@ static int device_reorder_to_tail(struct device *dev, void *not_used) return 0; } +/** + * device_pm_move_to_tail - Move set of devices to the end of device lists + * @dev: Device to move + * + * This is a device_reorder_to_tail() wrapper taking the requisite locks. + * + * It moves the @dev along with all of its children and all of its consumers + * to the ends of the device_kset and dpm_list, recursively. + */ +void device_pm_move_to_tail(struct device *dev) +{ + int idx; + + idx = device_links_read_lock(); + device_pm_lock(); + device_reorder_to_tail(dev, NULL); + device_pm_unlock(); + device_links_read_unlock(idx); +} + /** * device_link_add - Create a link between two devices. * @consumer: Consumer end of the link. diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c9f54089429b..10454fe54482 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -122,9 +122,7 @@ static void deferred_probe_work_func(struct work_struct *work) * the list is a good order for suspend but deferred * probe makes that very unsafe. */ - device_pm_lock(); - device_pm_move_last(dev); - device_pm_unlock(); + device_pm_move_to_tail(dev); dev_dbg(dev, "Retrying from deferred list\n"); if (initcall_debug && !initcalls_done) From 147f297965533f500fa1e5617d70f76ed56db5cc Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 26 Apr 2018 16:36:27 -0500 Subject: [PATCH 02/14] PM / core: Remove unused initcall_debug_report() arguments Commit e8bca479c3f2 (PM / sleep: trace events for device PM callbacks) removed the only uses of "state" and "info" from initcall_debug_report(). Remove the now-unused arguments completely. Signed-off-by: Bjorn Helgaas Signed-off-by: Rafael J. Wysocki --- drivers/base/power/main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 02a497e7c785..85ef2af6dc92 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -207,8 +207,7 @@ static ktime_t initcall_debug_start(struct device *dev) } static void initcall_debug_report(struct device *dev, ktime_t calltime, - int error, pm_message_t state, - const char *info) + int error) { ktime_t rettime; s64 nsecs; @@ -454,7 +453,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev, trace_device_pm_callback_end(dev, error); suspend_report_result(cb, error); - initcall_debug_report(dev, calltime, error, state, info); + initcall_debug_report(dev, calltime, error); return error; } @@ -1671,7 +1670,7 @@ static int legacy_suspend(struct device *dev, pm_message_t state, trace_device_pm_callback_end(dev, error); suspend_report_result(cb, error); - initcall_debug_report(dev, calltime, error, state, info); + initcall_debug_report(dev, calltime, error); return error; } From 143711f0110637239f045a179e726b137b7caf59 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 26 Apr 2018 16:36:34 -0500 Subject: [PATCH 03/14] PM / core: Simplify initcall_debug_report() timing initcall_debug_report() always called ktime_get(), even if we didn't need the result. Change it so we only call it when we're going to use the result, and change initcall_debug_start() to follow the same style. Signed-off-by: Bjorn Helgaas Signed-off-by: Rafael J. Wysocki --- drivers/base/power/main.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 85ef2af6dc92..b32750d18b09 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -194,16 +194,13 @@ void device_pm_move_last(struct device *dev) static ktime_t initcall_debug_start(struct device *dev) { - ktime_t calltime = 0; + if (!pm_print_times_enabled) + return 0; - if (pm_print_times_enabled) { - pr_info("calling %s+ @ %i, parent: %s\n", - dev_name(dev), task_pid_nr(current), - dev->parent ? dev_name(dev->parent) : "none"); - calltime = ktime_get(); - } - - return calltime; + pr_info("calling %s+ @ %i, parent: %s\n", + dev_name(dev), task_pid_nr(current), + dev->parent ? dev_name(dev->parent) : "none"); + return ktime_get(); } static void initcall_debug_report(struct device *dev, ktime_t calltime, @@ -212,13 +209,14 @@ static void initcall_debug_report(struct device *dev, ktime_t calltime, ktime_t rettime; s64 nsecs; + if (!pm_print_times_enabled) + return; + rettime = ktime_get(); nsecs = (s64) ktime_to_ns(ktime_sub(rettime, calltime)); - if (pm_print_times_enabled) { - pr_info("call %s+ returned %d after %Ld usecs\n", dev_name(dev), - error, (unsigned long long)nsecs >> 10); - } + pr_info("call %s+ returned %d after %Ld usecs\n", dev_name(dev), + error, (unsigned long long)nsecs >> 10); } /** From 7f817ba942940df382de085f9b830f77411562f2 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 26 Apr 2018 16:36:41 -0500 Subject: [PATCH 04/14] PM / core: Use dev_printk() and symbols in suspend/resume diagnostics When we print diagnostic messages about suspend/resume, we have a device pointer, so use dev_printk() to match other device-related things. Add the function name, similar to initcall_debug output. E.g., - calling 0000:01:00.0+ @ 998, parent: 0000:00:1c.0 + pci 0000:01:00.0: calling @ 998, parent: 0000:00:1c.0 I wondered if this would break scripts/bootgraph.pl, but I don't think it will because bootgraph.pl doesn't add any timing information to $start{} after it sees "Write protecting the" or "Freeing unused kernel memory". Signed-off-by: Bjorn Helgaas Signed-off-by: Rafael J. Wysocki --- drivers/base/power/main.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index b32750d18b09..dceda0495abf 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -192,19 +192,19 @@ void device_pm_move_last(struct device *dev) list_move_tail(&dev->power.entry, &dpm_list); } -static ktime_t initcall_debug_start(struct device *dev) +static ktime_t initcall_debug_start(struct device *dev, void *cb) { if (!pm_print_times_enabled) return 0; - pr_info("calling %s+ @ %i, parent: %s\n", - dev_name(dev), task_pid_nr(current), - dev->parent ? dev_name(dev->parent) : "none"); + dev_info(dev, "calling %pF @ %i, parent: %s\n", cb, + task_pid_nr(current), + dev->parent ? dev_name(dev->parent) : "none"); return ktime_get(); } static void initcall_debug_report(struct device *dev, ktime_t calltime, - int error) + void *cb, int error) { ktime_t rettime; s64 nsecs; @@ -215,8 +215,8 @@ static void initcall_debug_report(struct device *dev, ktime_t calltime, rettime = ktime_get(); nsecs = (s64) ktime_to_ns(ktime_sub(rettime, calltime)); - pr_info("call %s+ returned %d after %Ld usecs\n", dev_name(dev), - error, (unsigned long long)nsecs >> 10); + dev_info(dev, "%pF returned %d after %Ld usecs\n", cb, error, + (unsigned long long)nsecs >> 10); } /** @@ -443,7 +443,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev, if (!cb) return 0; - calltime = initcall_debug_start(dev); + calltime = initcall_debug_start(dev, cb); pm_dev_dbg(dev, state, info); trace_device_pm_callback_start(dev, info, state.event); @@ -451,7 +451,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev, trace_device_pm_callback_end(dev, error); suspend_report_result(cb, error); - initcall_debug_report(dev, calltime, error); + initcall_debug_report(dev, calltime, cb, error); return error; } @@ -1661,14 +1661,14 @@ static int legacy_suspend(struct device *dev, pm_message_t state, int error; ktime_t calltime; - calltime = initcall_debug_start(dev); + calltime = initcall_debug_start(dev, cb); trace_device_pm_callback_start(dev, info, state.event); error = cb(dev, state); trace_device_pm_callback_end(dev, error); suspend_report_result(cb, error); - initcall_debug_report(dev, calltime, error); + initcall_debug_report(dev, calltime, cb, error); return error; } From 00ee22c28915d111ba415750a3311d7678fd1206 Mon Sep 17 00:00:00 2001 From: Mahendran Ganesh Date: Wed, 25 Apr 2018 18:59:31 +0800 Subject: [PATCH 05/14] PM / wakeup: Use seq_open() to show wakeup stats single_open() interface requires that the whole output must fit into a single buffer. This will lead to timeout when system memory is not in a good situation. This patch use seq_open() to show wakeup stats. This method need only one page, so timeout will not be observed. Signed-off-by: Ganesh Mahendran Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeup.c | 75 +++++++++++++++++++++++++++++-------- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index ea01621ed769..5872705432f8 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -1029,32 +1029,75 @@ static int print_wakeup_source_stats(struct seq_file *m, return 0; } -/** - * wakeup_sources_stats_show - Print wakeup sources statistics information. - * @m: seq_file to print the statistics into. - */ -static int wakeup_sources_stats_show(struct seq_file *m, void *unused) +static void *wakeup_sources_stats_seq_start(struct seq_file *m, + loff_t *pos) { struct wakeup_source *ws; - int srcuidx; + loff_t n = *pos; + int *srcuidx = m->private; - seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t" - "expire_count\tactive_since\ttotal_time\tmax_time\t" - "last_change\tprevent_suspend_time\n"); + if (n == 0) { + seq_puts(m, "name\t\tactive_count\tevent_count\twakeup_count\t" + "expire_count\tactive_since\ttotal_time\tmax_time\t" + "last_change\tprevent_suspend_time\n"); + } - srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) - print_wakeup_source_stats(m, ws); - srcu_read_unlock(&wakeup_srcu, srcuidx); + *srcuidx = srcu_read_lock(&wakeup_srcu); + list_for_each_entry_rcu(ws, &wakeup_sources, entry) { + if (n-- <= 0) + return ws; + } - print_wakeup_source_stats(m, &deleted_ws); + return NULL; +} + +static void *wakeup_sources_stats_seq_next(struct seq_file *m, + void *v, loff_t *pos) +{ + struct wakeup_source *ws = v; + struct wakeup_source *next_ws = NULL; + + ++(*pos); + + list_for_each_entry_continue_rcu(ws, &wakeup_sources, entry) { + next_ws = ws; + break; + } + + return next_ws; +} + +static void wakeup_sources_stats_seq_stop(struct seq_file *m, void *v) +{ + int *srcuidx = m->private; + + srcu_read_unlock(&wakeup_srcu, *srcuidx); +} + +/** + * wakeup_sources_stats_seq_show - Print wakeup sources statistics information. + * @m: seq_file to print the statistics into. + * @v: wakeup_source of each iteration + */ +static int wakeup_sources_stats_seq_show(struct seq_file *m, void *v) +{ + struct wakeup_source *ws = v; + + print_wakeup_source_stats(m, ws); return 0; } +static const struct seq_operations wakeup_sources_stats_seq_ops = { + .start = wakeup_sources_stats_seq_start, + .next = wakeup_sources_stats_seq_next, + .stop = wakeup_sources_stats_seq_stop, + .show = wakeup_sources_stats_seq_show, +}; + static int wakeup_sources_stats_open(struct inode *inode, struct file *file) { - return single_open(file, wakeup_sources_stats_show, NULL); + return seq_open_private(file, &wakeup_sources_stats_seq_ops, sizeof(int)); } static const struct file_operations wakeup_sources_stats_fops = { @@ -1062,7 +1105,7 @@ static const struct file_operations wakeup_sources_stats_fops = { .open = wakeup_sources_stats_open, .read = seq_read, .llseek = seq_lseek, - .release = single_release, + .release = seq_release_private, }; static int __init wakeup_sources_debugfs_init(void) From 2ef7c01c0cdb170142058c6d8fe0697aee4e4d7d Mon Sep 17 00:00:00 2001 From: Doug Berger Date: Wed, 25 Apr 2018 16:40:30 -0700 Subject: [PATCH 06/14] PM / wakeup: Only update last time for active wakeup sources When wakelock support was added, the wakeup_source_add() function was updated to set the last_time value of the wakeup source. This has the unintended side effect of producing confusing output from pm_print_active_wakeup_sources() when a wakeup source is added prior to a sleep that is blocked by a different wakeup source. The function pm_print_active_wakeup_sources() will search for the most recently active wakeup source when no active source is found. If a wakeup source is added after a different wakeup source blocks the system from going to sleep it may have a later last_time value than the blocking source and be output as the last active wakeup source even if it has never actually been active. It looks to me like the change to wakeup_source_add() was made to prevent the wakelock garbage collection from accidentally dropping a wakelock during the narrow window between adding the wakelock to the wakelock list in wakelock_lookup_add() and the activation of the wakeup source in pm_wake_lock(). This commit changes the behavior so that only the last_time of the wakeup source used by a wakelock is initialized prior to adding it to the wakeup source list. This preserves the meaning of the last_time value as the last time the wakeup source was active and allows a wakeup source that has never been active to have a last_time value of 0. Fixes: b86ff9820fd5 (PM / Sleep: Add user space interface for manipulating wakeup sources, v3) Signed-off-by: Doug Berger Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeup.c | 1 - kernel/power/wakelock.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 5872705432f8..36bad46c7c68 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -183,7 +183,6 @@ void wakeup_source_add(struct wakeup_source *ws) spin_lock_init(&ws->lock); timer_setup(&ws->timer, pm_wakeup_timer_fn, 0); ws->active = false; - ws->last_time = ktime_get(); spin_lock_irqsave(&events_lock, flags); list_add_rcu(&ws->entry, &wakeup_sources); diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c index dfba59be190b..4210152e56f0 100644 --- a/kernel/power/wakelock.c +++ b/kernel/power/wakelock.c @@ -188,6 +188,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len, return ERR_PTR(-ENOMEM); } wl->ws.name = wl->name; + wl->ws.last_time = ktime_get(); wakeup_source_add(&wl->ws); rb_link_node(&wl->node, parent, node); rb_insert_color(&wl->node, &wakelocks_tree); From abcab87587cc4d04647eb9aaa11b26eacb3f0c13 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 May 2018 10:46:33 +0200 Subject: [PATCH 07/14] PM / core: Drop internal unused inline functions for wakeups The inline versions of device_wakeup_arm|disarm_wake_irqs(), which are available while when CONFIG_PM is set and CONFIG_PM_SLEEP unset, are not being used, hence let's drop them. Signed-off-by: Ulf Hansson Acked-by: Tony Lindgren Signed-off-by: Rafael J. Wysocki --- drivers/base/power/power.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index 86e67e70b509..f38d178ca395 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -56,14 +56,6 @@ static inline void device_wakeup_detach_irq(struct device *dev) { } -static inline void device_wakeup_arm_wake_irqs(void) -{ -} - -static inline void device_wakeup_disarm_wake_irqs(void) -{ -} - #endif /* CONFIG_PM_SLEEP */ /* From ff5f078e20dc67811b94a3bce5dee23f668068d1 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 May 2018 10:46:34 +0200 Subject: [PATCH 08/14] PM / core: Drop unused internal inline functions for wakeirqs The inline versions of dev_pm_arm|disarm_wake_irq() and dev_pm_enable|disable_wake_irq_check() are not being used while CONFIG_PM is unset, hence let's drop them. Signed-off-by: Ulf Hansson Acked-by: Tony Lindgren Signed-off-by: Rafael J. Wysocki --- drivers/base/power/power.h | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index f38d178ca395..be409103fb62 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -93,23 +93,6 @@ static inline void wakeup_sysfs_remove(struct device *dev) {} static inline int pm_qos_sysfs_add(struct device *dev) { return 0; } static inline void pm_qos_sysfs_remove(struct device *dev) {} -static inline void dev_pm_arm_wake_irq(struct wake_irq *wirq) -{ -} - -static inline void dev_pm_disarm_wake_irq(struct wake_irq *wirq) -{ -} - -static inline void dev_pm_enable_wake_irq_check(struct device *dev, - bool can_change_status) -{ -} - -static inline void dev_pm_disable_wake_irq_check(struct device *dev) -{ -} - #endif #ifdef CONFIG_PM_SLEEP From b8e7ca205f32cf0a6a3ea56d807c938527f55269 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 May 2018 10:46:35 +0200 Subject: [PATCH 09/14] PM / core: Drop unused internal functions for pm_qos sysfs The functions pm_qos_sysfs_add|remove() are available as inline functions only while CONFIG_PM is unset, but are not being used. Likely they are a leftover from an earlier cleanup in the past, anyway let's drop them. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/power.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index be409103fb62..78bbb5da70af 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -90,8 +90,6 @@ static inline void dpm_sysfs_remove(struct device *dev) {} static inline void rpm_sysfs_remove(struct device *dev) {} static inline int wakeup_sysfs_add(struct device *dev) { return 0; } static inline void wakeup_sysfs_remove(struct device *dev) {} -static inline int pm_qos_sysfs_add(struct device *dev) { return 0; } -static inline void pm_qos_sysfs_remove(struct device *dev) {} #endif From 91eb88b027ecec00110f3c496899d389d2ab9850 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 May 2018 10:46:36 +0200 Subject: [PATCH 10/14] PM / core: Drop unused internal inline functions for sysfs The inline versions of rpm_sysfs_remove() and wakeup_sysfs_add|remove(), are not being used while CONFIG_PM is unset, hence let's drop them. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/power.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index 78bbb5da70af..c511def48b48 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -87,9 +87,6 @@ static inline void pm_runtime_remove(struct device *dev) {} static inline int dpm_sysfs_add(struct device *dev) { return 0; } static inline void dpm_sysfs_remove(struct device *dev) {} -static inline void rpm_sysfs_remove(struct device *dev) {} -static inline int wakeup_sysfs_add(struct device *dev) { return 0; } -static inline void wakeup_sysfs_remove(struct device *dev) {} #endif From 1d6442263400709f12586b4b23a75b6aad549140 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Mon, 21 May 2018 13:12:12 +0200 Subject: [PATCH 11/14] PM: wakeup: Use pr_debug() for the "aborting suspend" message The message printed by pm_wakeup_pending() on wakeup detection is not very useful if someone is not interested specifically in debugging wakeup, so turn it into a pm_debug() one. Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 36bad46c7c68..e1322788eaee 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -853,7 +853,7 @@ bool pm_wakeup_pending(void) spin_unlock_irqrestore(&events_lock, flags); if (ret) { - pr_info("PM: Wakeup pending, aborting suspend\n"); + pr_debug("PM: Wakeup pending, aborting suspend\n"); pm_print_active_wakeup_sources(); } From 1e8378619841ef1d621b130bbd3fc3b7e6739b50 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Fri, 18 May 2018 10:48:49 +0200 Subject: [PATCH 12/14] PM / runtime: Fixup reference counting of device link suppliers at probe In the driver core, before it invokes really_probe() it runtime resumes the suppliers for the device via calling pm_runtime_get_suppliers(), which also increases the runtime PM usage count for each of the available supplier. This makes sense, as to be able to allow the consumer device to be probed by its driver. However, if the driver decides to add a new supplier link during ->probe(), hence updating the list of suppliers, the following call to pm_runtime_put_suppliers(), invoked after really_probe() in the driver core, we get into trouble. More precisely, pm_runtime_put() gets called also for the new supplier(s), which is wrong as the driver core, didn't trigger pm_runtime_get_sync() to be called for it in the first place. In other words, the new supplier may be runtime suspended even in cases when it shouldn't. Fix this behaviour, by runtime resume suppliers according to the same conditions as managed by the runtime PM core, when runtime resume callbacks are being invoked. Additionally, don't try to runtime suspend any of the suppliers after really_probe(), but instead rely on that to happen via the consumer device, when it becomes runtime suspended. Fixes: 21d5c57b3726 (PM / runtime: Use device links) Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/dd.c | 3 +-- drivers/base/power/runtime.c | 27 +++------------------------ include/linux/pm_runtime.h | 6 ++---- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 10454fe54482..a41c91bfac0e 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -580,7 +580,7 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) pr_debug("bus: '%s': %s: matched device %s with driver %s\n", drv->bus->name, __func__, dev_name(dev), drv->name); - pm_runtime_get_suppliers(dev); + pm_runtime_resume_suppliers(dev); if (dev->parent) pm_runtime_get_sync(dev->parent); @@ -591,7 +591,6 @@ int driver_probe_device(struct device_driver *drv, struct device *dev) if (dev->parent) pm_runtime_put(dev->parent); - pm_runtime_put_suppliers(dev); return ret; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 8bef3cb2424d..6f4f50e196c1 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1563,37 +1563,16 @@ void pm_runtime_clean_up_links(struct device *dev) } /** - * pm_runtime_get_suppliers - Resume and reference-count supplier devices. + * pm_runtime_resume_suppliers - Resume supplier devices. * @dev: Consumer device. */ -void pm_runtime_get_suppliers(struct device *dev) +void pm_runtime_resume_suppliers(struct device *dev) { - struct device_link *link; int idx; idx = device_links_read_lock(); - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) - pm_runtime_get_sync(link->supplier); - - device_links_read_unlock(idx); -} - -/** - * pm_runtime_put_suppliers - Drop references to supplier devices. - * @dev: Consumer device. - */ -void pm_runtime_put_suppliers(struct device *dev) -{ - struct device_link *link; - int idx; - - idx = device_links_read_lock(); - - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) - if (link->flags & DL_FLAG_PM_RUNTIME) - pm_runtime_put(link->supplier); + rpm_get_suppliers(dev); device_links_read_unlock(idx); } diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index f0fc4700b6ff..db5dbbf7a48d 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -56,8 +56,7 @@ extern void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns); extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable); extern void pm_runtime_clean_up_links(struct device *dev); -extern void pm_runtime_get_suppliers(struct device *dev); -extern void pm_runtime_put_suppliers(struct device *dev); +extern void pm_runtime_resume_suppliers(struct device *dev); extern void pm_runtime_new_link(struct device *dev); extern void pm_runtime_drop_link(struct device *dev); @@ -173,8 +172,7 @@ static inline unsigned long pm_runtime_autosuspend_expiration( static inline void pm_runtime_set_memalloc_noio(struct device *dev, bool enable){} static inline void pm_runtime_clean_up_links(struct device *dev) {} -static inline void pm_runtime_get_suppliers(struct device *dev) {} -static inline void pm_runtime_put_suppliers(struct device *dev) {} +static inline void pm_runtime_resume_suppliers(struct device *dev) {} static inline void pm_runtime_new_link(struct device *dev) {} static inline void pm_runtime_drop_link(struct device *dev) {} From a0504aecba76baa1cddbc23512eb8be14df74cef Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 24 May 2018 10:33:36 +0200 Subject: [PATCH 13/14] PM / runtime: Drop usage count for suppliers at device link removal In the case consumer device is runtime resumed, while the link to the supplier is removed, the earlier call to pm_runtime_get_sync() made from rpm_get_suppliers() does not get properly balanced with a corresponding call to pm_runtime_put(). This leads to that suppliers remains to be runtime resumed forever, while they don't need to. Let's fix the behaviour by calling rpm_put_suppliers() when dropping a device link. Not that, since rpm_put_suppliers() checks the link->rpm_active flag, we can correctly avoid to call pm_runtime_put() in cases when we shouldn't. Reported-by: Todor Tomov Fixes: 21d5c57b3726 (PM / runtime: Use device links) Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 6f4f50e196c1..c6030f100c08 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1586,6 +1586,8 @@ void pm_runtime_new_link(struct device *dev) void pm_runtime_drop_link(struct device *dev) { + rpm_put_suppliers(dev); + spin_lock_irq(&dev->power.lock); WARN_ON(dev->power.links_count == 0); dev->power.links_count--; From 74cd8171c42fcb0a0d55575dd036239b7325a455 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 25 May 2018 12:33:10 +0200 Subject: [PATCH 14/14] PM / QoS: Drop redundant declaration of pm_qos_get_value() The extra forward declaration of pm_qos_get_value() is redundant, so drop it. Signed-off-by: Rafael J. Wysocki --- kernel/power/qos.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/power/qos.c b/kernel/power/qos.c index fa39092b7aea..86d72ffb811b 100644 --- a/kernel/power/qos.c +++ b/kernel/power/qos.c @@ -184,7 +184,6 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value) c->target_value = value; } -static inline int pm_qos_get_value(struct pm_qos_constraints *c); static int pm_qos_dbg_show_requests(struct seq_file *s, void *unused) { struct pm_qos_object *qos = (struct pm_qos_object *)s->private;