From 558642bccede3d0e6ffebe4106b0719e29b9e4a8 Mon Sep 17 00:00:00 2001 From: Tian Tao Date: Thu, 20 May 2021 15:34:58 +0800 Subject: [PATCH 1/8] PM: wakeirq: Set IRQF_NO_AUTOEN when requesting the IRQ request_irq() after setting IRQ_NOAUTOEN as below irq_set_status_flags(irq, IRQ_NOAUTOEN); request_irq(dev, irq...); can be replaced by request_irq() with IRQF_NO_AUTOEN flag. This change is just to simplify the code, no actual functional changes. Signed-off-by: Tian Tao Reviewed-by: Tony Lindgren [ rjw: Subject ] Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeirq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c index 8e021082dba8..3bad3266a2ad 100644 --- a/drivers/base/power/wakeirq.c +++ b/drivers/base/power/wakeirq.c @@ -182,7 +182,6 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq) wirq->dev = dev; wirq->irq = irq; - irq_set_status_flags(irq, IRQ_NOAUTOEN); /* Prevent deferred spurious wakeirqs with disable_irq_nosync() */ irq_set_status_flags(irq, IRQ_DISABLE_UNLAZY); @@ -192,7 +191,8 @@ int dev_pm_set_dedicated_wake_irq(struct device *dev, int irq) * so we use a threaded irq. */ err = request_threaded_irq(irq, NULL, handle_threaded_wake_irq, - IRQF_ONESHOT, wirq->name, wirq); + IRQF_ONESHOT | IRQF_NO_AUTOEN, + wirq->name, wirq); if (err) goto err_free_name; From 6be2408a1ef632a48149044d1757c80ab1096213 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Mon, 24 May 2021 17:30:10 +0800 Subject: [PATCH 2/8] PM: hibernate: fix spelling mistakes Fix some spelling mistakes in comments: corresonds ==> corresponds alocated ==> allocated unitialized ==> uninitialized Deompression ==> Decompression Signed-off-by: Zhen Lei Signed-off-by: Rafael J. Wysocki --- kernel/power/snapshot.c | 8 ++++---- kernel/power/swap.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 1a221dcb3c01..af507c8c895b 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -331,7 +331,7 @@ static void *chain_alloc(struct chain_allocator *ca, unsigned int size) * * Memory bitmap is a structure consisting of many linked lists of * objects. The main list's elements are of type struct zone_bitmap - * and each of them corresonds to one zone. For each zone bitmap + * and each of them corresponds to one zone. For each zone bitmap * object there is a list of objects of type struct bm_block that * represent each blocks of bitmap in which information is stored. * @@ -1500,7 +1500,7 @@ static struct memory_bitmap copy_bm; /** * swsusp_free - Free pages allocated for hibernation image. * - * Image pages are alocated before snapshot creation, so they need to be + * Image pages are allocated before snapshot creation, so they need to be * released after resume. */ void swsusp_free(void) @@ -2326,7 +2326,7 @@ static struct memory_bitmap *safe_highmem_bm; * (@nr_highmem_p points to the variable containing the number of highmem image * pages). The pages that are "safe" (ie. will not be overwritten when the * hibernation image is restored entirely) have the corresponding bits set in - * @bm (it must be unitialized). + * @bm (it must be uninitialized). * * NOTE: This function should not be called if there are no highmem image pages. */ @@ -2483,7 +2483,7 @@ static inline void free_highmem_data(void) {} /** * prepare_image - Make room for loading hibernation image. - * @new_bm: Unitialized memory bitmap structure. + * @new_bm: Uninitialized memory bitmap structure. * @bm: Memory bitmap with unsafe pages marked. * * Use @bm to mark the pages that will be overwritten in the process of diff --git a/kernel/power/swap.c b/kernel/power/swap.c index bea3cb8afa11..3cb89baebc79 100644 --- a/kernel/power/swap.c +++ b/kernel/power/swap.c @@ -1125,7 +1125,7 @@ struct dec_data { }; /** - * Deompression function that runs in its own thread. + * Decompression function that runs in its own thread. */ static int lzo_decompress_threadfn(void *data) { From c58e7ed28b4534ed073371843d03c433d6a9fe34 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 26 May 2021 12:22:51 -0400 Subject: [PATCH 3/8] PM: runtime: document common mistake with pm_runtime_get_sync() pm_runtime_get_sync(), contradictory to intuition, does not drop the runtime PM usage counter on errors which lead to several wrong usages in drivers (missing the put). pm_runtime_resume_and_get() was added as a better implementation so document the preference of using it, hoping it will stop bad patterns. Suggested-by: Marek Szyprowski Signed-off-by: Krzysztof Kozlowski [ rjw: Documentation change edits ] Signed-off-by: Rafael J. Wysocki --- Documentation/power/runtime_pm.rst | 6 +++++- include/linux/pm_runtime.h | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst index 18ae21bf7f92..b48cac5f9048 100644 --- a/Documentation/power/runtime_pm.rst +++ b/Documentation/power/runtime_pm.rst @@ -378,7 +378,11 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h: `int pm_runtime_get_sync(struct device *dev);` - increment the device's usage counter, run pm_runtime_resume(dev) and - return its result + return its result; + note that it does not drop the device's usage counter on errors, so + consider using pm_runtime_resume_and_get() instead of it, especially + if its return value is checked by the caller, as this is likely to + result in cleaner code. `int pm_runtime_get_if_in_use(struct device *dev);` - return -EINVAL if 'power.disable_depth' is nonzero; otherwise, if the diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 6c08a085367b..aab8b35e9f8a 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -380,6 +380,9 @@ static inline int pm_runtime_get(struct device *dev) * The possible return values of this function are the same as for * pm_runtime_resume() and the runtime PM usage counter of @dev remains * incremented in all cases, even if it returns an error code. + * Consider using pm_runtime_resume_and_get() instead of it, especially + * if its return value is checked by the caller, as this is likely to result + * in cleaner code. */ static inline int pm_runtime_get_sync(struct device *dev) { From 03466883a0fdb5c38f2907b027565b9f253688a8 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 8 Jun 2021 15:44:37 +0800 Subject: [PATCH 4/8] PM: sleep: remove trailing spaces and tabs Run the following command to find and remove the trailing spaces and tabs: $ find kernel/power/ -type f | xargs sed -r -i 's/[ \t]+$//' Signed-off-by: Zhen Lei Signed-off-by: Rafael J. Wysocki --- kernel/power/Kconfig | 12 ++++++------ kernel/power/process.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index 6bfe3ead10ad..a12779650f15 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -98,20 +98,20 @@ config PM_STD_PARTITION default "" help The default resume partition is the partition that the suspend- - to-disk implementation will look for a suspended disk image. + to-disk implementation will look for a suspended disk image. - The partition specified here will be different for almost every user. + The partition specified here will be different for almost every user. It should be a valid swap partition (at least for now) that is turned - on before suspending. + on before suspending. The partition specified can be overridden by specifying: - resume=/dev/ + resume=/dev/ - which will set the resume partition to the device specified. + which will set the resume partition to the device specified. Note there is currently not a way to specify which device to save the - suspended image to. It will simply pick the first available swap + suspended image to. It will simply pick the first available swap device. config PM_SLEEP diff --git a/kernel/power/process.c b/kernel/power/process.c index 50cc63534486..37401c99b7d7 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * drivers/power/process.c - Functions for starting/stopping processes on + * drivers/power/process.c - Functions for starting/stopping processes on * suspend transitions. * * Originally from swsusp. From 480f0de68caddfe336b8cc0c74a40328779940d3 Mon Sep 17 00:00:00 2001 From: Zhen Lei Date: Tue, 8 Jun 2021 16:13:14 +0800 Subject: [PATCH 5/8] PM: hibernate: remove leading spaces before tabs 1) Run the following command to find and remove the leading spaces before tabs: $ find kernel/power/ -type f | xargs sed -r -i 's/^[ ]+\t/\t/' 2) Manually check and correct if necessary Signed-off-by: Zhen Lei Signed-off-by: Rafael J. Wysocki --- kernel/power/snapshot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index af507c8c895b..f7a986078213 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -1146,7 +1146,7 @@ int create_basic_memory_bitmaps(void) Free_second_object: kfree(bm2); Free_first_bitmap: - memory_bm_free(bm1, PG_UNSAFE_CLEAR); + memory_bm_free(bm1, PG_UNSAFE_CLEAR); Free_first_object: kfree(bm1); return -ENOMEM; From 5a2bd1b1c64e1ac5627db3767ac465f18606315c Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 8 Jun 2021 11:02:48 +0200 Subject: [PATCH 6/8] PM: runtime: Improve path in rpm_idle() when no callback When pm_runtime_no_callbacks() has been called for a struct device to set the dev->power.no_callbacks flag for it, it enables rpm_idle() to take a slightly quicker path by assuming that a ->runtime_idle() callback would have returned 0 to indicate success. A device that does not have the dev->power.no_callbacks flag set for it, may still be missing a corresponding ->runtime_idle() callback, in which case the slower path in rpm_idle() is taken. Let's improve the behaviour for this case, by aligning code to the quicker path. Signed-off-by: Ulf Hansson Acked-by: Alan Stern Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index b570848d23e0..68bebbf81347 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -446,7 +446,10 @@ static int rpm_idle(struct device *dev, int rpmflags) /* Pending requests need to be canceled. */ dev->power.request = RPM_REQ_NONE; - if (dev->power.no_callbacks) + callback = RPM_GET_CALLBACK(dev, runtime_idle); + + /* If no callback assume success. */ + if (!callback || dev->power.no_callbacks) goto out; /* Carry out an asynchronous or a synchronous idle notification. */ @@ -462,10 +465,7 @@ static int rpm_idle(struct device *dev, int rpmflags) dev->power.idle_notification = true; - callback = RPM_GET_CALLBACK(dev, runtime_idle); - - if (callback) - retval = __rpm_callback(callback, dev); + retval = __rpm_callback(callback, dev); dev->power.idle_notification = false; wake_up_all(&dev->power.wait_queue); From 63d00be69348fda431ae59aba6af268a5cf5058e Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 8 Jun 2021 11:02:49 +0200 Subject: [PATCH 7/8] PM: runtime: Allow unassigned ->runtime_suspend|resume callbacks We are currently allowing ->runtime_idle() callbacks to be unassigned without returning an error code from rpm_idle(). This has been useful to avoid boilerplate code in drivers. Let's take this approach a step further, by allowing also unassigned ->runtime_suspend|resume() callbacks. In this way, a consumer/supplier device link can be used to let a consumer device be power managed through its supplier device, without requiring assigned ->runtime_suspend|resume() callbacks for the consumer device, for example. Signed-off-by: Ulf Hansson Acked-by: Alan Stern Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 68bebbf81347..8a66eaf731e4 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -345,7 +345,7 @@ static void rpm_suspend_suppliers(struct device *dev) static int __rpm_callback(int (*cb)(struct device *), struct device *dev) __releases(&dev->power.lock) __acquires(&dev->power.lock) { - int retval, idx; + int retval = 0, idx; bool use_links = dev->power.links_count > 0; if (dev->power.irq_safe) { @@ -373,7 +373,8 @@ static int __rpm_callback(int (*cb)(struct device *), struct device *dev) } } - retval = cb(dev); + if (cb) + retval = cb(dev); if (dev->power.irq_safe) { spin_lock(&dev->power.lock); @@ -484,9 +485,6 @@ static int rpm_callback(int (*cb)(struct device *), struct device *dev) { int retval; - if (!cb) - return -ENOSYS; - if (dev->power.memalloc_noio) { unsigned int noio_flag; From 4ec4f059088b48585c337328e05fa930c64d1ba8 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 Jun 2021 12:06:10 +0200 Subject: [PATCH 8/8] PM: runtime: Clarify documentation when callbacks are unassigned Recent changes to the PM core allows ->runtime_suspend|resume callbacks to be unassigned. In the earlier behaviour the PM core would return -ENOSYS, when trying to runtime resume a device, for example. Let's update the documentation to clarify this. Signed-off-by: Ulf Hansson Acked-by: Alan Stern Signed-off-by: Rafael J. Wysocki --- Documentation/power/runtime_pm.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst index b48cac5f9048..d6bf84f061f4 100644 --- a/Documentation/power/runtime_pm.rst +++ b/Documentation/power/runtime_pm.rst @@ -831,6 +831,15 @@ or driver about runtime power changes. Instead, the driver for the device's parent must take responsibility for telling the device's driver when the parent's power state changes. +Note that, in some cases it may not be desirable for subsystems/drivers to call +pm_runtime_no_callbacks() for their devices. This could be because a subset of +the runtime PM callbacks needs to be implemented, a platform dependent PM +domain could get attached to the device or that the device is power managed +through a supplier device link. For these reasons and to avoid boilerplate code +in subsystems/drivers, the PM core allows runtime PM callbacks to be +unassigned. More precisely, if a callback pointer is NULL, the PM core will act +as though there was a callback and it returned 0. + 9. Autosuspend, or automatically-delayed suspends =================================================