[PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[Paul Cercueil]

Hi,

This patchset reworks the pm_ptr() macro I introduced a few versions
ago, so that it is not conditionally defined.

It applies the same treatment to the *_PM_OPS macros. Instead of
modifying the existing ones, which would mean a 2000+ patch bomb, this
patchset introduce two new macros to replace the now deprecated
UNIVERSAL_DEV_PM_OPS() and SIMPLE_DEV_PM_OPS().

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

Patch [4/5] and [5/5] are just examples to illustrate the use of the new
macros. As such they don't really have to be merged at the same time as
the rest and can be delayed until a subsystem-wide patchset is proposed.

- Patch [4/5] modifies a driver that already used the pm_ptr() macro,
  but had to use the __maybe_unused flag to avoid compiler warnings;
- Patch [5/5] modifies a driver that used a #ifdef CONFIG_PM guard
  around its suspend/resume functions.

Paul Cercueil (5):
  r8169: Avoid misuse of pm_ptr() macro
  PM: core: Redefine pm_ptr() macro
  PM: core: Add new *_PM_OPS macros, deprecate old ones
  mmc: jz4740: Use the new PM macros
  mmc: mxc: Use the new PM macros

 drivers/mmc/host/jz4740_mmc.c             |  8 +--
 drivers/mmc/host/mxcmmc.c                 |  6 +-
 drivers/net/ethernet/realtek/r8169_main.c |  4 +-
 include/linux/pm.h                        | 80 +++++++++++++++--------
 4 files changed, 60 insertions(+), 38 deletions(-)

-- 
2.33.0

[PATCH 1/5] r8169: Avoid misuse of pm_ptr() macro

[Paul Cercueil]

The pm_ptr() macro should be used when the suspend and resume functions
can be compiled independently of the CONFIG_PM Kconfig option.

In the case of this driver, the suspend and resume functions are inside
a section protected by a #ifdef CONFIG_PM guard. Therefore pm_ptr()
should not be used.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: <nic_swsd@realtek.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>

---
 drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 86c44bc5f73f..6b81f95698f0 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -5441,7 +5441,9 @@ static struct pci_driver rtl8169_pci_driver = {
 	.probe		= rtl_init_one,
 	.remove		= rtl_remove_one,
 	.shutdown	= rtl_shutdown,
-	.driver.pm	= pm_ptr(&rtl8169_pm_ops),
+#ifdef CONFIG_PM
+	.driver.pm	= &rtl8169_pm_ops,
+#endif
 };
 
 module_pci_driver(rtl8169_pci_driver);
-- 
2.33.0

[PATCH 2/5] PM: core: Redefine pm_ptr() macro

[Paul Cercueil]

The pm_ptr() macro was previously conditionally defined, according to
the value of the CONFIG_PM option. This meant that the pointed structure
was either referenced (if CONFIG_PM was set), or never referenced (if
CONFIG_PM was not set), causing it to be detected as unused by the
compiler.

This worked fine, but required the __maybe_unused compiler attribute to
be used to every symbol pointed to by a pointer wrapped with pm_ptr().

We can do better. With this change, the pm_ptr() is now defined the
same, independently of the value of CONFIG_PM. It now uses the (?:)
ternary operator to conditionally resolve to its argument. Since the
condition is known at compile time, the compiler will then choose to
discard the unused symbols, which won't need to be tagged with
__maybe_unused anymore.

This pm_ptr() macro is usually used with pointers to dev_pm_ops
structures created with SIMPLE_DEV_PM_OPS() or similar macros. These do
use a __maybe_unused flag, which is now useless with this change, so it
later can be removed. However in the meantime it causes no harm, and all
the drivers still compile fine with the new pm_ptr() macro.

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

diff --git a/include/linux/pm.h b/include/linux/pm.h
index 1d8209c09686..b88ac7dcf2a2 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -373,11 +373,7 @@ const struct dev_pm_ops __maybe_unused name = { \
 	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
 }
 
-#ifdef CONFIG_PM
-#define pm_ptr(_ptr) (_ptr)
-#else
-#define pm_ptr(_ptr) NULL
-#endif
+#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
 
 /*
  * PM_EVENT_ messages
-- 
2.33.0

Re: [PATCH 2/5] PM: core: Redefine pm_ptr() macro

[Jonathan Cameron]

On Tue,  7 Dec 2021 00:20:59 +0000
Paul Cercueil <paul@crapouillou.net> wrote:

> The pm_ptr() macro was previously conditionally defined, according to
> the value of the CONFIG_PM option. This meant that the pointed structure
> was either referenced (if CONFIG_PM was set), or never referenced (if
> CONFIG_PM was not set), causing it to be detected as unused by the
> compiler.
> 
> This worked fine, but required the __maybe_unused compiler attribute to
> be used to every symbol pointed to by a pointer wrapped with pm_ptr().
> 
> We can do better. With this change, the pm_ptr() is now defined the
> same, independently of the value of CONFIG_PM. It now uses the (?:)
> ternary operator to conditionally resolve to its argument. Since the
> condition is known at compile time, the compiler will then choose to
> discard the unused symbols, which won't need to be tagged with
> __maybe_unused anymore.
> 
> This pm_ptr() macro is usually used with pointers to dev_pm_ops
> structures created with SIMPLE_DEV_PM_OPS() or similar macros. These do
> use a __maybe_unused flag, which is now useless with this change, so it
> later can be removed. However in the meantime it causes no harm, and all
> the drivers still compile fine with the new pm_ptr() macro.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>

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

> ---
>  include/linux/pm.h | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 1d8209c09686..b88ac7dcf2a2 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -373,11 +373,7 @@ const struct dev_pm_ops __maybe_unused name = { \
>  	SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
>  }
>  
> -#ifdef CONFIG_PM
> -#define pm_ptr(_ptr) (_ptr)
> -#else
> -#define pm_ptr(_ptr) NULL
> -#endif
> +#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
>  
>  /*
>   * PM_EVENT_ messages

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

[Paul Cercueil]

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(-)

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
-- 
2.33.0

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

[Jonathan Cameron]

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

[PATCH 4/5] mmc: jz4740: Use the new PM macros

[Paul Cercueil]

- Use DEFINE_SIMPLE_DEV_PM_OPS() instead of the SIMPLE_DEV_PM_OPS()
  macro. This makes it possible to remove the __maybe_unused flags on
  the callback functions.
- Since we only have callbacks for suspend/resume, we can conditionally
  compile the dev_pm_ops structure for when CONFIG_PM_SLEEP is enabled;
  so use the pm_sleep_ptr() macro instead of pm_ptr().

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/mmc/host/jz4740_mmc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/jz4740_mmc.c b/drivers/mmc/host/jz4740_mmc.c
index 80a2c270d502..bb612fce7ead 100644
--- a/drivers/mmc/host/jz4740_mmc.c
+++ b/drivers/mmc/host/jz4740_mmc.c
@@ -1103,17 +1103,17 @@ static int jz4740_mmc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused jz4740_mmc_suspend(struct device *dev)
+static int jz4740_mmc_suspend(struct device *dev)
 {
 	return pinctrl_pm_select_sleep_state(dev);
 }
 
-static int __maybe_unused jz4740_mmc_resume(struct device *dev)
+static int jz4740_mmc_resume(struct device *dev)
 {
 	return pinctrl_select_default_state(dev);
 }
 
-static SIMPLE_DEV_PM_OPS(jz4740_mmc_pm_ops, jz4740_mmc_suspend,
+DEFINE_SIMPLE_DEV_PM_OPS(jz4740_mmc_pm_ops, jz4740_mmc_suspend,
 	jz4740_mmc_resume);
 
 static struct platform_driver jz4740_mmc_driver = {
@@ -1123,7 +1123,7 @@ static struct platform_driver jz4740_mmc_driver = {
 		.name = "jz4740-mmc",
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 		.of_match_table = of_match_ptr(jz4740_mmc_of_match),
-		.pm = pm_ptr(&jz4740_mmc_pm_ops),
+		.pm = pm_sleep_ptr(&jz4740_mmc_pm_ops),
 	},
 };
 
-- 
2.33.0

[PATCH 5/5] mmc: mxc: Use the new PM macros

[Paul Cercueil]

Use DEFINE_SIMPLE_DEV_PM_OPS() instead of the SIMPLE_DEV_PM_OPS()
macro, along with using pm_sleep_ptr() as this driver doesn't handle
runtime PM. This makes it possible to remove the #ifdef CONFIG_PM
guard around the suspend/resume functions.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/mmc/host/mxcmmc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index 2fe6fcdbb1b3..98c218bd6669 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -1183,7 +1183,6 @@ static int mxcmci_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
 static int mxcmci_suspend(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
@@ -1210,9 +1209,8 @@ static int mxcmci_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-static SIMPLE_DEV_PM_OPS(mxcmci_pm_ops, mxcmci_suspend, mxcmci_resume);
+DEFINE_SIMPLE_DEV_PM_OPS(mxcmci_pm_ops, mxcmci_suspend, mxcmci_resume);
 
 static struct platform_driver mxcmci_driver = {
 	.probe		= mxcmci_probe,
@@ -1220,7 +1218,7 @@ static struct platform_driver mxcmci_driver = {
 	.driver		= {
 		.name		= DRIVER_NAME,
 		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
-		.pm	= &mxcmci_pm_ops,
+		.pm	= pm_sleep_ptr(&mxcmci_pm_ops),
 		.of_match_table	= mxcmci_of_match,
 	}
 };
-- 
2.33.0

Re: [PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[Arnd Bergmann]

On Tue, Dec 7, 2021 at 1:20 AM Paul Cercueil <paul@crapouillou.net> wrote:
>
> This patchset reworks the pm_ptr() macro I introduced a few versions
> ago, so that it is not conditionally defined.
>
> It applies the same treatment to the *_PM_OPS macros. Instead of
> modifying the existing ones, which would mean a 2000+ patch bomb, this
> patchset introduce two new macros to replace the now deprecated
> UNIVERSAL_DEV_PM_OPS() and SIMPLE_DEV_PM_OPS().
>
> The point of all of this, is to progressively switch from a code model
> where PM callbacks are all protected behind CONFIG_PM guards, to a code
> model where PM callbacks are always seen by the compiler, but discarded
> if not used.
>
> Patch [4/5] and [5/5] are just examples to illustrate the use of the new
> macros. As such they don't really have to be merged at the same time as
> the rest and can be delayed until a subsystem-wide patchset is proposed.
>
> - Patch [4/5] modifies a driver that already used the pm_ptr() macro,
>   but had to use the __maybe_unused flag to avoid compiler warnings;
> - Patch [5/5] modifies a driver that used a #ifdef CONFIG_PM guard
>   around its suspend/resume functions.

This is fantastic, I love the new naming and it should provide a great path
towards converting all drivers eventually. I've added the patches to
my randconfig test build box to see if something breaks, but otherwise
I think these are ready to get into linux-next, at least patches 1-3,
so subsystem
maintainers can start queuing up the conversion patches once the
initial set is merged.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[Rafael J. Wysocki]

On Tue, Dec 7, 2021 at 10:22 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Dec 7, 2021 at 1:20 AM Paul Cercueil <paul@crapouillou.net> wrote:
> >
> > This patchset reworks the pm_ptr() macro I introduced a few versions
> > ago, so that it is not conditionally defined.
> >
> > It applies the same treatment to the *_PM_OPS macros. Instead of
> > modifying the existing ones, which would mean a 2000+ patch bomb, this
> > patchset introduce two new macros to replace the now deprecated
> > UNIVERSAL_DEV_PM_OPS() and SIMPLE_DEV_PM_OPS().
> >
> > The point of all of this, is to progressively switch from a code model
> > where PM callbacks are all protected behind CONFIG_PM guards, to a code
> > model where PM callbacks are always seen by the compiler, but discarded
> > if not used.
> >
> > Patch [4/5] and [5/5] are just examples to illustrate the use of the new
> > macros. As such they don't really have to be merged at the same time as
> > the rest and can be delayed until a subsystem-wide patchset is proposed.
> >
> > - Patch [4/5] modifies a driver that already used the pm_ptr() macro,
> >   but had to use the __maybe_unused flag to avoid compiler warnings;
> > - Patch [5/5] modifies a driver that used a #ifdef CONFIG_PM guard
> >   around its suspend/resume functions.
>
> This is fantastic, I love the new naming and it should provide a great path
> towards converting all drivers eventually. I've added the patches to
> my randconfig test build box to see if something breaks, but otherwise
> I think these are ready to get into linux-next, at least patches 1-3,
> so subsystem
> maintainers can start queuing up the conversion patches once the
> initial set is merged.
>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Patches [0-3/5] applied as 5.17 material.

The mmc patches need ACKs, but I can take them too.

Re: [PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[Ulf Hansson]

On Fri, 17 Dec 2021 at 16:07, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Tue, Dec 7, 2021 at 10:22 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Dec 7, 2021 at 1:20 AM Paul Cercueil <paul@crapouillou.net> wrote:
> > >
> > > This patchset reworks the pm_ptr() macro I introduced a few versions
> > > ago, so that it is not conditionally defined.
> > >
> > > It applies the same treatment to the *_PM_OPS macros. Instead of
> > > modifying the existing ones, which would mean a 2000+ patch bomb, this
> > > patchset introduce two new macros to replace the now deprecated
> > > UNIVERSAL_DEV_PM_OPS() and SIMPLE_DEV_PM_OPS().
> > >
> > > The point of all of this, is to progressively switch from a code model
> > > where PM callbacks are all protected behind CONFIG_PM guards, to a code
> > > model where PM callbacks are always seen by the compiler, but discarded
> > > if not used.
> > >
> > > Patch [4/5] and [5/5] are just examples to illustrate the use of the new
> > > macros. As such they don't really have to be merged at the same time as
> > > the rest and can be delayed until a subsystem-wide patchset is proposed.
> > >
> > > - Patch [4/5] modifies a driver that already used the pm_ptr() macro,
> > >   but had to use the __maybe_unused flag to avoid compiler warnings;
> > > - Patch [5/5] modifies a driver that used a #ifdef CONFIG_PM guard
> > >   around its suspend/resume functions.
> >
> > This is fantastic, I love the new naming and it should provide a great path
> > towards converting all drivers eventually. I've added the patches to
> > my randconfig test build box to see if something breaks, but otherwise
> > I think these are ready to get into linux-next, at least patches 1-3,
> > so subsystem
> > maintainers can start queuing up the conversion patches once the
> > initial set is merged.
> >
> > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
>
> Patches [0-3/5] applied as 5.17 material.
>
> The mmc patches need ACKs, but I can take them too.

Sure, please add my ack for them!

Kind regards
Uffe

Re: [PATCH 0/5] Rework pm_ptr() and *_PM_OPS macros

[Rafael J. Wysocki]

On Fri, Dec 17, 2021 at 6:17 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Fri, 17 Dec 2021 at 16:07, Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Tue, Dec 7, 2021 at 10:22 AM Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Tue, Dec 7, 2021 at 1:20 AM Paul Cercueil <paul@crapouillou.net> wrote:
> > > >
> > > > This patchset reworks the pm_ptr() macro I introduced a few versions
> > > > ago, so that it is not conditionally defined.
> > > >
> > > > It applies the same treatment to the *_PM_OPS macros. Instead of
> > > > modifying the existing ones, which would mean a 2000+ patch bomb, this
> > > > patchset introduce two new macros to replace the now deprecated
> > > > UNIVERSAL_DEV_PM_OPS() and SIMPLE_DEV_PM_OPS().
> > > >
> > > > The point of all of this, is to progressively switch from a code model
> > > > where PM callbacks are all protected behind CONFIG_PM guards, to a code
> > > > model where PM callbacks are always seen by the compiler, but discarded
> > > > if not used.
> > > >
> > > > Patch [4/5] and [5/5] are just examples to illustrate the use of the new
> > > > macros. As such they don't really have to be merged at the same time as
> > > > the rest and can be delayed until a subsystem-wide patchset is proposed.
> > > >
> > > > - Patch [4/5] modifies a driver that already used the pm_ptr() macro,
> > > >   but had to use the __maybe_unused flag to avoid compiler warnings;
> > > > - Patch [5/5] modifies a driver that used a #ifdef CONFIG_PM guard
> > > >   around its suspend/resume functions.
> > >
> > > This is fantastic, I love the new naming and it should provide a great path
> > > towards converting all drivers eventually. I've added the patches to
> > > my randconfig test build box to see if something breaks, but otherwise
> > > I think these are ready to get into linux-next, at least patches 1-3,
> > > so subsystem
> > > maintainers can start queuing up the conversion patches once the
> > > initial set is merged.
> > >
> > > Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Patches [0-3/5] applied as 5.17 material.
> >
> > The mmc patches need ACKs, but I can take them too.
>
> Sure, please add my ack for them!

Both applied as 5.17 material with your ACKs, thanks!