* [PATCH 1/4] PM / core: Introduce dpm_async_fn() helper
2019-03-16 4:59 [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Yangtao Li
@ 2019-03-16 4:59 ` Yangtao Li
2019-04-10 8:15 ` Rafael J. Wysocki
2019-03-16 4:59 ` [PATCH 2/4] PM / core: Introduce DEVICE_SUSPEND_FUNC() helper macro Yangtao Li
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Yangtao Li @ 2019-03-16 4:59 UTC (permalink / raw)
To: rjw, len.brown, pavel, gregkh; +Cc: linux-pm, linux-kernel, Yangtao Li
When we want to execute device pm functions asynchronously, we'll
do the following for the device:
1) reinit_completion(&dev->power.completion);
2) Check if the device enables asynchronous suspend.
3) If necessary, execute the corresponding function asynchronously.
There are a lot of such repeated operations here, in fact we can avoid
this. So introduce dpm_async_fn() to have better code readability and
reuse.
And use this function to do some cleanup.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
drivers/base/power/main.c | 62 +++++++++++++++------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index eddb54057ed6..cb44bb6b2b66 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -706,6 +706,19 @@ static bool is_async(struct device *dev)
&& !pm_trace_is_enabled();
}
+static bool dpm_async_fn(struct device *dev, async_func_t func)
+{
+ reinit_completion(&dev->power.completion);
+
+ if (is_async(dev)) {
+ get_device(dev);
+ async_schedule(func, dev);
+ return true;
+ }
+
+ return false;
+}
+
static void async_resume_noirq(void *data, async_cookie_t cookie)
{
struct device *dev = (struct device *)data;
@@ -732,13 +745,8 @@ void dpm_noirq_resume_devices(pm_message_t state)
* in case the starting of async threads is
* delayed by non-async resuming devices.
*/
- list_for_each_entry(dev, &dpm_noirq_list, power.entry) {
- reinit_completion(&dev->power.completion);
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_resume_noirq, dev);
- }
- }
+ list_for_each_entry(dev, &dpm_noirq_list, power.entry)
+ dpm_async_fn(dev, async_resume_noirq);
while (!list_empty(&dpm_noirq_list)) {
dev = to_device(dpm_noirq_list.next);
@@ -889,13 +897,8 @@ void dpm_resume_early(pm_message_t state)
* in case the starting of async threads is
* delayed by non-async resuming devices.
*/
- list_for_each_entry(dev, &dpm_late_early_list, power.entry) {
- reinit_completion(&dev->power.completion);
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_resume_early, dev);
- }
- }
+ list_for_each_entry(dev, &dpm_late_early_list, power.entry)
+ dpm_async_fn(dev, async_resume_early);
while (!list_empty(&dpm_late_early_list)) {
dev = to_device(dpm_late_early_list.next);
@@ -1053,13 +1056,8 @@ void dpm_resume(pm_message_t state)
pm_transition = state;
async_error = 0;
- list_for_each_entry(dev, &dpm_suspended_list, power.entry) {
- reinit_completion(&dev->power.completion);
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_resume, dev);
- }
- }
+ list_for_each_entry(dev, &dpm_suspended_list, power.entry)
+ dpm_async_fn(dev, async_resume);
while (!list_empty(&dpm_suspended_list)) {
dev = to_device(dpm_suspended_list.next);
@@ -1373,13 +1371,9 @@ static void async_suspend_noirq(void *data, async_cookie_t cookie)
static int device_suspend_noirq(struct device *dev)
{
- reinit_completion(&dev->power.completion);
-
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_suspend_noirq, dev);
+ if (dpm_async_fn(dev, async_suspend_noirq))
return 0;
- }
+
return __device_suspend_noirq(dev, pm_transition, false);
}
@@ -1576,13 +1570,8 @@ static void async_suspend_late(void *data, async_cookie_t cookie)
static int device_suspend_late(struct device *dev)
{
- reinit_completion(&dev->power.completion);
-
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_suspend_late, dev);
+ if (dpm_async_fn(dev, async_suspend_late))
return 0;
- }
return __device_suspend_late(dev, pm_transition, false);
}
@@ -1842,13 +1831,8 @@ static void async_suspend(void *data, async_cookie_t cookie)
static int device_suspend(struct device *dev)
{
- reinit_completion(&dev->power.completion);
-
- if (is_async(dev)) {
- get_device(dev);
- async_schedule_dev(async_suspend, dev);
+ if (dpm_async_fn(dev, async_suspend))
return 0;
- }
return __device_suspend(dev, pm_transition, false);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/4] PM / core: Introduce dpm_async_fn() helper
2019-03-16 4:59 ` [PATCH 1/4] PM / core: Introduce dpm_async_fn() helper Yangtao Li
@ 2019-04-10 8:15 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-04-10 8:15 UTC (permalink / raw)
To: Yangtao Li; +Cc: len.brown, pavel, gregkh, linux-pm, linux-kernel
On Saturday, March 16, 2019 5:59:25 AM CEST Yangtao Li wrote:
> When we want to execute device pm functions asynchronously, we'll
> do the following for the device:
>
> 1) reinit_completion(&dev->power.completion);
> 2) Check if the device enables asynchronous suspend.
> 3) If necessary, execute the corresponding function asynchronously.
>
> There are a lot of such repeated operations here, in fact we can avoid
> this. So introduce dpm_async_fn() to have better code readability and
> reuse.
>
> And use this function to do some cleanup.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
I'm queuing up this one, but the [2-4/4] are not convincing.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/4] PM / core: Introduce DEVICE_SUSPEND_FUNC() helper macro
2019-03-16 4:59 [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Yangtao Li
2019-03-16 4:59 ` [PATCH 1/4] PM / core: Introduce dpm_async_fn() helper Yangtao Li
@ 2019-03-16 4:59 ` Yangtao Li
2019-03-16 4:59 ` [PATCH 3/4] PM / core: Introduce ASYNC_RESUME_FUNC() " Yangtao Li
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Yangtao Li @ 2019-03-16 4:59 UTC (permalink / raw)
To: rjw, len.brown, pavel, gregkh; +Cc: linux-pm, linux-kernel, Yangtao Li
The devices_suspend_noirq, device_suspend_late, device_suspen functions
are basically the same. As we have seen:
static int device_xxx(struct device *dev)
{
if (dpm_async_fn(dev, async_xxx))
return 0;
return __device_xxx(dev, pm_transition, false);
}
The DEVICE_SUSPEND_FUNC() helper macro can decrease
code duplication.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
drivers/base/power/main.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index cb44bb6b2b66..6026bda5e787 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -719,6 +719,15 @@ static bool dpm_async_fn(struct device *dev, async_func_t func)
return false;
}
+#define DEVICE_SUSPEND_FUNC(__func, __name) \
+static int __func(struct device *dev) \
+{ \
+ if (dpm_async_fn(dev, async_ ## __name)) \
+ return 0; \
+ \
+ return __device_ ## __name(dev, pm_transition, false); \
+}
+
static void async_resume_noirq(void *data, async_cookie_t cookie)
{
struct device *dev = (struct device *)data;
@@ -1369,13 +1378,7 @@ static void async_suspend_noirq(void *data, async_cookie_t cookie)
put_device(dev);
}
-static int device_suspend_noirq(struct device *dev)
-{
- if (dpm_async_fn(dev, async_suspend_noirq))
- return 0;
-
- return __device_suspend_noirq(dev, pm_transition, false);
-}
+DEVICE_SUSPEND_FUNC(device_suspend_noirq, suspend_noirq);
void dpm_noirq_begin(void)
{
@@ -1568,13 +1571,7 @@ static void async_suspend_late(void *data, async_cookie_t cookie)
put_device(dev);
}
-static int device_suspend_late(struct device *dev)
-{
- if (dpm_async_fn(dev, async_suspend_late))
- return 0;
-
- return __device_suspend_late(dev, pm_transition, false);
-}
+DEVICE_SUSPEND_FUNC(device_suspend_late, suspend_late);
/**
* dpm_suspend_late - Execute "late suspend" callbacks for all devices.
@@ -1829,13 +1826,7 @@ static void async_suspend(void *data, async_cookie_t cookie)
put_device(dev);
}
-static int device_suspend(struct device *dev)
-{
- if (dpm_async_fn(dev, async_suspend))
- return 0;
-
- return __device_suspend(dev, pm_transition, false);
-}
+DEVICE_SUSPEND_FUNC(device_suspend, suspend);
/**
* dpm_suspend - Execute "suspend" callbacks for all non-sysdev devices.
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] PM / core: Introduce ASYNC_RESUME_FUNC() helper macro
2019-03-16 4:59 [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Yangtao Li
2019-03-16 4:59 ` [PATCH 1/4] PM / core: Introduce dpm_async_fn() helper Yangtao Li
2019-03-16 4:59 ` [PATCH 2/4] PM / core: Introduce DEVICE_SUSPEND_FUNC() helper macro Yangtao Li
@ 2019-03-16 4:59 ` Yangtao Li
2019-03-16 4:59 ` [PATCH 4/4] PM / core: Introduce ASYNC_SUSPEND_FUNC() " Yangtao Li
2019-03-18 12:20 ` [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Pavel Machek
4 siblings, 0 replies; 7+ messages in thread
From: Yangtao Li @ 2019-03-16 4:59 UTC (permalink / raw)
To: rjw, len.brown, pavel, gregkh; +Cc: linux-pm, linux-kernel, Yangtao Li
The async_resume_noirq, async_resume_early, async_resume functions
are basically the same. As we have seen:
static void async_xxx(void *data, async_cookie_t cookie)
{
struct device *dev = (struct device *)data;
int error;
error = device_xxx(dev, pm_transition, true);
if (error)
pm_dev_err(dev, pm_transition, " async", error);
put_device(dev);
}
The ASYNC_RESUME_FUNC() helper macro can decrease code duplication.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
drivers/base/power/main.c | 46 +++++++++++++--------------------------
1 file changed, 15 insertions(+), 31 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 6026bda5e787..d512bee9d9ca 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -728,18 +728,21 @@ static int __func(struct device *dev) \
return __device_ ## __name(dev, pm_transition, false); \
}
-static void async_resume_noirq(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = device_resume_noirq(dev, pm_transition, true);
- if (error)
- pm_dev_err(dev, pm_transition, " async", error);
-
- put_device(dev);
+#define ASYNC_RESUME_FUNC(__func, __name) \
+static void __func(void *data, async_cookie_t cookie) \
+{ \
+ struct device *dev = (struct device *)data; \
+ int error; \
+ \
+ error = device_ ## __name(dev, pm_transition, true); \
+ if (error) \
+ pm_dev_err(dev, pm_transition, " async", error); \
+ \
+ put_device(dev); \
}
+ASYNC_RESUME_FUNC(async_resume_noirq, resume_noirq);
+
void dpm_noirq_resume_devices(pm_message_t state)
{
struct device *dev;
@@ -876,17 +879,7 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
return error;
}
-static void async_resume_early(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = device_resume_early(dev, pm_transition, true);
- if (error)
- pm_dev_err(dev, pm_transition, " async", error);
-
- put_device(dev);
-}
+ASYNC_RESUME_FUNC(async_resume_early, resume_early);
/**
* dpm_resume_early - Execute "early resume" callbacks for all devices.
@@ -1035,16 +1028,7 @@ static int device_resume(struct device *dev, pm_message_t state, bool async)
return error;
}
-static void async_resume(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = device_resume(dev, pm_transition, true);
- if (error)
- pm_dev_err(dev, pm_transition, " async", error);
- put_device(dev);
-}
+ASYNC_RESUME_FUNC(async_resume, resume);
/**
* dpm_resume - Execute "resume" callbacks for non-sysdev devices.
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] PM / core: Introduce ASYNC_SUSPEND_FUNC() helper macro
2019-03-16 4:59 [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Yangtao Li
` (2 preceding siblings ...)
2019-03-16 4:59 ` [PATCH 3/4] PM / core: Introduce ASYNC_RESUME_FUNC() " Yangtao Li
@ 2019-03-16 4:59 ` Yangtao Li
2019-03-18 12:20 ` [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Pavel Machek
4 siblings, 0 replies; 7+ messages in thread
From: Yangtao Li @ 2019-03-16 4:59 UTC (permalink / raw)
To: rjw, len.brown, pavel, gregkh; +Cc: linux-pm, linux-kernel, Yangtao Li
The async_suspend_noirq, async_suspend_late, async_suspend functions
are basically the same. As we have seen:
static void async_xxx(void *data, async_cookie_t cookie)
{
struct device *dev = (struct device *)data;
int error;
error = __device_xxx(dev, pm_transition, true);
if (error) {
dpm_save_failed_dev(dev_name(dev));
pm_dev_err(dev, pm_transition, " async", error);
}
put_device(dev);
}
The ASYNC_SUSPEND_FUNC() helper macro can decrease code duplication.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
drivers/base/power/main.c | 55 ++++++++++++---------------------------
1 file changed, 16 insertions(+), 39 deletions(-)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index d512bee9d9ca..3882dc5fee9f 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1348,20 +1348,22 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a
return error;
}
-static void async_suspend_noirq(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = __device_suspend_noirq(dev, pm_transition, true);
- if (error) {
- dpm_save_failed_dev(dev_name(dev));
- pm_dev_err(dev, pm_transition, " async", error);
- }
-
- put_device(dev);
+#define ASYNC_SUSPEND_FUNC(__func, __name) \
+static void __func(void *data, async_cookie_t cookie) \
+{ \
+ struct device *dev = (struct device *)data; \
+ int error; \
+ \
+ error = __device_ ## __name(dev, pm_transition, true); \
+ if (error) { \
+ dpm_save_failed_dev(dev_name(dev)); \
+ pm_dev_err(dev, pm_transition, " async", error); \
+ } \
+ \
+ put_device(dev); \
}
+ASYNC_SUSPEND_FUNC(async_suspend_noirq, suspend_noirq);
DEVICE_SUSPEND_FUNC(device_suspend_noirq, suspend_noirq);
void dpm_noirq_begin(void)
@@ -1542,19 +1544,7 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as
return error;
}
-static void async_suspend_late(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = __device_suspend_late(dev, pm_transition, true);
- if (error) {
- dpm_save_failed_dev(dev_name(dev));
- pm_dev_err(dev, pm_transition, " async", error);
- }
- put_device(dev);
-}
-
+ASYNC_SUSPEND_FUNC(async_suspend_late, suspend_late);
DEVICE_SUSPEND_FUNC(device_suspend_late, suspend_late);
/**
@@ -1796,20 +1786,7 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async)
return error;
}
-static void async_suspend(void *data, async_cookie_t cookie)
-{
- struct device *dev = (struct device *)data;
- int error;
-
- error = __device_suspend(dev, pm_transition, true);
- if (error) {
- dpm_save_failed_dev(dev_name(dev));
- pm_dev_err(dev, pm_transition, " async", error);
- }
-
- put_device(dev);
-}
-
+ASYNC_SUSPEND_FUNC(async_suspend, suspend);
DEVICE_SUSPEND_FUNC(device_suspend, suspend);
/**
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] PM / core: Introduce some helper for better Code reuse
2019-03-16 4:59 [PATCH 0/4] PM / core: Introduce some helper for better Code reuse Yangtao Li
` (3 preceding siblings ...)
2019-03-16 4:59 ` [PATCH 4/4] PM / core: Introduce ASYNC_SUSPEND_FUNC() " Yangtao Li
@ 2019-03-18 12:20 ` Pavel Machek
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Machek @ 2019-03-18 12:20 UTC (permalink / raw)
To: Yangtao Li; +Cc: rjw, len.brown, gregkh, linux-pm, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]
On Sat 2019-03-16 00:59:24, Yangtao Li wrote:
> This patch set introduces some functions and macros that help reduce
> code duplication.
>
> Yangtao Li (4):
> PM / core: Introduce dpm_async_fn() helper
> PM / core: Introduce DEVICE_SUSPEND_FUNC() helper macro
> PM / core: Introduce ASYNC_RESUME_FUNC() helper macro
> PM / core: Introduce ASYNC_SUSPEND_FUNC() helper macro
>
> drivers/base/power/main.c | 182 ++++++++++++--------------------------
> 1 file changed, 59 insertions(+), 123 deletions(-)
I'm not a big fan. Yes, you got line count down. But no, I do not
think "beauty" of the macros makes it worth it.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread