diff mbox series

[3/5] PM: core: Add new *_PM_OPS macros, deprecate old ones

Message ID 20211207002102.26414-4-paul@crapouillou.net
State New
Headers show
Series [1/5] r8169: Avoid misuse of pm_ptr() macro | expand

Commit Message

Paul Cercueil Dec. 7, 2021, 12:21 a.m. UTC
This commit introduces the following macros:
SYSTEM_SLEEP_PM_OPS()
LATE_SYSTEM_SLEEP_PM_OPS()
NOIRQ_SYSTEM_SLEEP_PM_OPS()
RUNTIME_PM_OPS()

These new macros are very similar to their SET_*_PM_OPS() equivalent.
They however differ in the fact that the callbacks they set will always
be seen as referenced by the compiler. This means that the callback
functions don't need to be wrapped with a #ifdef CONFIG_PM guard, or
tagged with __maybe_unused, to prevent the compiler from complaining
about unused static symbols. The compiler will then simply evaluate at
compile time whether or not these symbols are dead code.

The callbacks that are only useful with CONFIG_PM_SLEEP is enabled, are
now also wrapped with a new pm_sleep_ptr() macro, which is inspired from
pm_ptr(). This is needed for drivers that use different callbacks for
sleep and runtime PM, to handle the case where CONFIG_PM is set and
CONFIG_PM_SLEEP is not.

This commit also deprecates the following macros:
SIMPLE_DEV_PM_OPS()
UNIVERSAL_DEV_PM_OPS()

And introduces the following macros:
DEFINE_SIMPLE_DEV_PM_OPS()
DEFINE_UNIVERSAL_DEV_PM_OPS()

These macros are similar to the functions they were created to replace,
with the following differences:
- They use the new macros introduced above, and as such always reference
  the provided callback functions;
- They are not tagged with __maybe_unused. They are meant to be used
  with pm_ptr() or pm_sleep_ptr() for DEFINE_UNIVERSAL_DEV_PM_OPS() and
  DEFINE_SIMPLE_DEV_PM_OPS() respectively.
- They declare the symbol static, since every driver seems to do that
  anyway; and if a non-static use-case is needed an indirection pointer
  could be used.

The point of this change, is to progressively switch from a code model
where PM callbacks are all protected behind CONFIG_PM guards, to a code
model where the PM callbacks are always seen by the compiler, but
discarded if not used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 include/linux/pm.h | 74 +++++++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 24 deletions(-)

Comments

Jonathan Cameron Dec. 16, 2021, 11:09 a.m. UTC | #1
On Tue,  7 Dec 2021 00:21:00 +0000
Paul Cercueil <paul@crapouillou.net> wrote:

> This commit introduces the following macros:
> SYSTEM_SLEEP_PM_OPS()
> LATE_SYSTEM_SLEEP_PM_OPS()
> NOIRQ_SYSTEM_SLEEP_PM_OPS()
> RUNTIME_PM_OPS()
> 
> These new macros are very similar to their SET_*_PM_OPS() equivalent.
> They however differ in the fact that the callbacks they set will always
> be seen as referenced by the compiler. This means that the callback
> functions don't need to be wrapped with a #ifdef CONFIG_PM guard, or
> tagged with __maybe_unused, to prevent the compiler from complaining
> about unused static symbols. The compiler will then simply evaluate at
> compile time whether or not these symbols are dead code.
> 
> The callbacks that are only useful with CONFIG_PM_SLEEP is enabled, are
> now also wrapped with a new pm_sleep_ptr() macro, which is inspired from
> pm_ptr(). This is needed for drivers that use different callbacks for
> sleep and runtime PM, to handle the case where CONFIG_PM is set and
> CONFIG_PM_SLEEP is not.
> 
> This commit also deprecates the following macros:
> SIMPLE_DEV_PM_OPS()
> UNIVERSAL_DEV_PM_OPS()
> 
> And introduces the following macros:
> DEFINE_SIMPLE_DEV_PM_OPS()
> DEFINE_UNIVERSAL_DEV_PM_OPS()
> 
> These macros are similar to the functions they were created to replace,
> with the following differences:
> - They use the new macros introduced above, and as such always reference
>   the provided callback functions;
> - They are not tagged with __maybe_unused. They are meant to be used
>   with pm_ptr() or pm_sleep_ptr() for DEFINE_UNIVERSAL_DEV_PM_OPS() and
>   DEFINE_SIMPLE_DEV_PM_OPS() respectively.
> - They declare the symbol static, since every driver seems to do that
>   anyway; and if a non-static use-case is needed an indirection pointer
>   could be used.

There are non static usecases e.g. drivers/iio/ad7606.c
where they are shared across a couple of different modules (typically when
we have a core / i2c / spi module split for a driver or similar).
As you say, there are ways of working around that.

So I guess it's a question of what feels more natural + common kernel
way of doing things.

I'll defer to your (+ anyone else who wishes to comment) judgement.

> 
> The point of this change, is to progressively switch from a code model
> where PM callbacks are all protected behind CONFIG_PM guards, to a code
> model where the PM callbacks are always seen by the compiler, but
> discarded if not used.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Great work btw. When the holiday season gets boring I'll redo my IIO
set to use this + maybe the rest of IIO...

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---
>  include/linux/pm.h | 74 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 50 insertions(+), 24 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index b88ac7dcf2a2..fc9691cb01b4 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -300,47 +300,59 @@ struct dev_pm_ops {
>  	int (*runtime_idle)(struct device *dev);
>  };
>  
> +#define SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +	.suspend = pm_sleep_ptr(suspend_fn), \
> +	.resume = pm_sleep_ptr(resume_fn), \
> +	.freeze = pm_sleep_ptr(suspend_fn), \
> +	.thaw = pm_sleep_ptr(resume_fn), \
> +	.poweroff = pm_sleep_ptr(suspend_fn), \
> +	.restore = pm_sleep_ptr(resume_fn),
> +
> +#define LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +	.suspend_late = pm_sleep_ptr(suspend_fn), \
> +	.resume_early = pm_sleep_ptr(resume_fn), \
> +	.freeze_late = pm_sleep_ptr(suspend_fn), \
> +	.thaw_early = pm_sleep_ptr(resume_fn), \
> +	.poweroff_late = pm_sleep_ptr(suspend_fn), \
> +	.restore_early = pm_sleep_ptr(resume_fn),
> +
> +#define NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +	.suspend_noirq = pm_sleep_ptr(suspend_fn), \
> +	.resume_noirq = pm_sleep_ptr(resume_fn), \
> +	.freeze_noirq = pm_sleep_ptr(suspend_fn), \
> +	.thaw_noirq = pm_sleep_ptr(resume_fn), \
> +	.poweroff_noirq = pm_sleep_ptr(suspend_fn), \
> +	.restore_noirq = pm_sleep_ptr(resume_fn),
> +
> +#define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> +	.runtime_suspend = suspend_fn, \
> +	.runtime_resume = resume_fn, \
> +	.runtime_idle = idle_fn,
> +
>  #ifdef CONFIG_PM_SLEEP
>  #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend = suspend_fn, \
> -	.resume = resume_fn, \
> -	.freeze = suspend_fn, \
> -	.thaw = resume_fn, \
> -	.poweroff = suspend_fn, \
> -	.restore = resume_fn,
> +	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #else
>  #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #endif
>  
>  #ifdef CONFIG_PM_SLEEP
>  #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_late = suspend_fn, \
> -	.resume_early = resume_fn, \
> -	.freeze_late = suspend_fn, \
> -	.thaw_early = resume_fn, \
> -	.poweroff_late = suspend_fn, \
> -	.restore_early = resume_fn,
> +	LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #else
>  #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #endif
>  
>  #ifdef CONFIG_PM_SLEEP
>  #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> -	.suspend_noirq = suspend_fn, \
> -	.resume_noirq = resume_fn, \
> -	.freeze_noirq = suspend_fn, \
> -	.thaw_noirq = resume_fn, \
> -	.poweroff_noirq = suspend_fn, \
> -	.restore_noirq = resume_fn,
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #else
>  #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
>  #endif
>  
>  #ifdef CONFIG_PM
>  #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> -	.runtime_suspend = suspend_fn, \
> -	.runtime_resume = resume_fn, \
> -	.runtime_idle = idle_fn,
> +	RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
>  #else
>  #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
>  #endif
> @@ -349,9 +361,9 @@ struct dev_pm_ops {
>   * Use this if you want to use the same suspend and resume callbacks for suspend
>   * to RAM and hibernation.
>   */
> -#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> -const struct dev_pm_ops __maybe_unused name = { \
> -	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +#define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> +static const struct dev_pm_ops name = { \
> +	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
>  }
>  
>  /*
> @@ -367,6 +379,19 @@ const struct dev_pm_ops __maybe_unused name = { \
>   * .resume_early(), to the same routines as .runtime_suspend() and
>   * .runtime_resume(), respectively (and analogously for hibernation).
>   */
> +#define DEFINE_UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
> +static const struct dev_pm_ops name = { \
> +	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +	RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
> +}
> +
> +/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
> +#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> +const struct dev_pm_ops __maybe_unused name = { \
> +	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> +}
> +
> +/* Deprecated. Use DEFINE_UNIVERSAL_DEV_PM_OPS() instead. */
>  #define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
>  const struct dev_pm_ops __maybe_unused name = { \
>  	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> @@ -374,6 +399,7 @@ const struct dev_pm_ops __maybe_unused name = { \
>  }
>  
>  #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
> +#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
>  
>  /*
>   * PM_EVENT_ messages
diff mbox series

Patch

diff --git a/include/linux/pm.h b/include/linux/pm.h
index b88ac7dcf2a2..fc9691cb01b4 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -300,47 +300,59 @@  struct dev_pm_ops {
 	int (*runtime_idle)(struct device *dev);
 };
 
+#define SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+	.suspend = pm_sleep_ptr(suspend_fn), \
+	.resume = pm_sleep_ptr(resume_fn), \
+	.freeze = pm_sleep_ptr(suspend_fn), \
+	.thaw = pm_sleep_ptr(resume_fn), \
+	.poweroff = pm_sleep_ptr(suspend_fn), \
+	.restore = pm_sleep_ptr(resume_fn),
+
+#define LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+	.suspend_late = pm_sleep_ptr(suspend_fn), \
+	.resume_early = pm_sleep_ptr(resume_fn), \
+	.freeze_late = pm_sleep_ptr(suspend_fn), \
+	.thaw_early = pm_sleep_ptr(resume_fn), \
+	.poweroff_late = pm_sleep_ptr(suspend_fn), \
+	.restore_early = pm_sleep_ptr(resume_fn),
+
+#define NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+	.suspend_noirq = pm_sleep_ptr(suspend_fn), \
+	.resume_noirq = pm_sleep_ptr(resume_fn), \
+	.freeze_noirq = pm_sleep_ptr(suspend_fn), \
+	.thaw_noirq = pm_sleep_ptr(resume_fn), \
+	.poweroff_noirq = pm_sleep_ptr(suspend_fn), \
+	.restore_noirq = pm_sleep_ptr(resume_fn),
+
+#define RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
+	.runtime_suspend = suspend_fn, \
+	.runtime_resume = resume_fn, \
+	.runtime_idle = idle_fn,
+
 #ifdef CONFIG_PM_SLEEP
 #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
-	.suspend = suspend_fn, \
-	.resume = resume_fn, \
-	.freeze = suspend_fn, \
-	.thaw = resume_fn, \
-	.poweroff = suspend_fn, \
-	.restore = resume_fn,
+	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #else
 #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
 #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
-	.suspend_late = suspend_fn, \
-	.resume_early = resume_fn, \
-	.freeze_late = suspend_fn, \
-	.thaw_early = resume_fn, \
-	.poweroff_late = suspend_fn, \
-	.restore_early = resume_fn,
+	LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #else
 #define SET_LATE_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #endif
 
 #ifdef CONFIG_PM_SLEEP
 #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
-	.suspend_noirq = suspend_fn, \
-	.resume_noirq = resume_fn, \
-	.freeze_noirq = suspend_fn, \
-	.thaw_noirq = resume_fn, \
-	.poweroff_noirq = suspend_fn, \
-	.restore_noirq = resume_fn,
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #else
 #define SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
 #endif
 
 #ifdef CONFIG_PM
 #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
-	.runtime_suspend = suspend_fn, \
-	.runtime_resume = resume_fn, \
-	.runtime_idle = idle_fn,
+	RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
 #else
 #define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn)
 #endif
@@ -349,9 +361,9 @@  struct dev_pm_ops {
  * Use this if you want to use the same suspend and resume callbacks for suspend
  * to RAM and hibernation.
  */
-#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
-const struct dev_pm_ops __maybe_unused name = { \
-	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+#define DEFINE_SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+static const struct dev_pm_ops name = { \
+	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
 }
 
 /*
@@ -367,6 +379,19 @@  const struct dev_pm_ops __maybe_unused name = { \
  * .resume_early(), to the same routines as .runtime_suspend() and
  * .runtime_resume(), respectively (and analogously for hibernation).
  */
+#define DEFINE_UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
+static const struct dev_pm_ops name = { \
+	SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+	RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
+}
+
+/* Deprecated. Use DEFINE_SIMPLE_DEV_PM_OPS() instead. */
+#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
+const struct dev_pm_ops __maybe_unused name = { \
+	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
+}
+
+/* Deprecated. Use DEFINE_UNIVERSAL_DEV_PM_OPS() instead. */
 #define UNIVERSAL_DEV_PM_OPS(name, suspend_fn, resume_fn, idle_fn) \
 const struct dev_pm_ops __maybe_unused name = { \
 	SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
@@ -374,6 +399,7 @@  const struct dev_pm_ops __maybe_unused name = { \
 }
 
 #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
+#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
 
 /*
  * PM_EVENT_ messages