[PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[Nathan Chancellor]

After the switch to SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() and a subsequent
fix, stm32_rtc_{suspend,resume}() are unused when CONFIG_PM_SLEEP is not
set because SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is a no-op in that
configuration:

  drivers/rtc/rtc-stm32.c:904:12: error: 'stm32_rtc_resume' defined but not used [-Werror=unused-function]
    904 | static int stm32_rtc_resume(struct device *dev)
        |            ^~~~~~~~~~~~~~~~
  drivers/rtc/rtc-stm32.c:894:12: error: 'stm32_rtc_suspend' defined but not used [-Werror=unused-function]
    894 | static int stm32_rtc_suspend(struct device *dev)
        |            ^~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors

The non-"SET_" version of this macro, NOIRQ_SYSTEM_SLEEP_PM_OPS(), is
designed to handle this situation by only assigning the callbacks when
CONFIG_PM_SLEEP is set while allowing the functions to appear used to
the compiler. Switch to that macro to resolve the warnings. There is no
functional change with this, as SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is
defined using NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is set.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
I am not sure what to do about a Fixes: tag for this change. I am not
sure how Arnd triggered the error/warning in commit a69c610e13e2 ("rtc:
stm32: remove incorrect #ifdef check"), since from what I can tell,
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is only defined in terms of
NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is set, so I am not
sure how those functions could be absent in the source file but used in
NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is unset... I could be
missing something though.

However, I think this change should have been done as part of commit
fb9a7e5360dc ("rtc: stm32: change PM callbacks to "_noirq()"") because
the "SET_" macros are deprecated and that would have made Arnd's change
necessary so... assign an appropriate Fixes: tag based on that
information as you will :)
---
 drivers/rtc/rtc-stm32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 3ce4b3d08155..76753c71d92e 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -923,7 +923,7 @@ static int stm32_rtc_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops stm32_rtc_pm_ops = {
-	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_rtc_suspend, stm32_rtc_resume)
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_rtc_suspend, stm32_rtc_resume)
 };
 
 static struct platform_driver stm32_rtc_driver = {

---
base-commit: 4f3688dca15053555ade31a785a9c75837a64fb8
change-id: 20230815-rtc-stm32-unused-pm-funcs-90d84bc2c016

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>

[PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[Geert Uytterhoeven]

If CONFIG_PM_SLEEP=n (e.g. m68k/allmodconfig):

    drivers/rtc/rtc-stm32.c:904:12: error: ‘stm32_rtc_resume’ defined but not used [-Werror=unused-function]
      904 | static int stm32_rtc_resume(struct device *dev)
	  |            ^~~~~~~~~~~~~~~~
    drivers/rtc/rtc-stm32.c:894:12: error: ‘stm32_rtc_suspend’ defined but not used [-Werror=unused-function]
      894 | static int stm32_rtc_suspend(struct device *dev)
	  |            ^~~~~~~~~~~~~~~~~

Indeed, the previous change mixed up the semantics between
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() and NOIRQ_SYSTEM_SLEEP_PM_OPS().
Fix this by using the modern NOIRQ_SYSTEM_SLEEP_PM_OPS() instead.

Reported-by: noreply@ellerman.id.au
Fixes: a69c610e13e2b2de ("rtc: stm32: remove incorrect #ifdef check")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/rtc/rtc-stm32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 3ce4b3d08155b48d..76753c71d92eef3e 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -923,7 +923,7 @@ static int stm32_rtc_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops stm32_rtc_pm_ops = {
-	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_rtc_suspend, stm32_rtc_resume)
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(stm32_rtc_suspend, stm32_rtc_resume)
 };
 
 static struct platform_driver stm32_rtc_driver = {
-- 
2.34.1

Re: [PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[Arnd Bergmann]

On Wed, Aug 16, 2023, at 13:33, Geert Uytterhoeven wrote:
> If CONFIG_PM_SLEEP=n (e.g. m68k/allmodconfig):
>
>     drivers/rtc/rtc-stm32.c:904:12: error: ‘stm32_rtc_resume’ defined 
> but not used [-Werror=unused-function]
>       904 | static int stm32_rtc_resume(struct device *dev)
> 	  |            ^~~~~~~~~~~~~~~~
>     drivers/rtc/rtc-stm32.c:894:12: error: ‘stm32_rtc_suspend’ defined 
> but not used [-Werror=unused-function]
>       894 | static int stm32_rtc_suspend(struct device *dev)
> 	  |            ^~~~~~~~~~~~~~~~~
>
> Indeed, the previous change mixed up the semantics between
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() and NOIRQ_SYSTEM_SLEEP_PM_OPS().
> Fix this by using the modern NOIRQ_SYSTEM_SLEEP_PM_OPS() instead.
>
> Reported-by: noreply@ellerman.id.au
> Fixes: a69c610e13e2b2de ("rtc: stm32: remove incorrect #ifdef check")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

Re: [PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[Arnd Bergmann]

On Wed, Aug 16, 2023, at 00:16, Nathan Chancellor wrote:
> After the switch to SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() and a subsequent
> fix, stm32_rtc_{suspend,resume}() are unused when CONFIG_PM_SLEEP is not
> set because SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is a no-op in that
> configuration:
>
>   drivers/rtc/rtc-stm32.c:904:12: error: 'stm32_rtc_resume' defined but 
> not used [-Werror=unused-function]
>     904 | static int stm32_rtc_resume(struct device *dev)
>         |            ^~~~~~~~~~~~~~~~
>   drivers/rtc/rtc-stm32.c:894:12: error: 'stm32_rtc_suspend' defined 
> but not used [-Werror=unused-function]
>     894 | static int stm32_rtc_suspend(struct device *dev)
>         |            ^~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
>
> The non-"SET_" version of this macro, NOIRQ_SYSTEM_SLEEP_PM_OPS(), is
> designed to handle this situation by only assigning the callbacks when
> CONFIG_PM_SLEEP is set while allowing the functions to appear used to
> the compiler. Switch to that macro to resolve the warnings. There is no
> functional change with this, as SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is
> defined using NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is set.
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

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

> ---
> I am not sure what to do about a Fixes: tag for this change. I am not
> sure how Arnd triggered the error/warning in commit a69c610e13e2 ("rtc:
> stm32: remove incorrect #ifdef check"), since from what I can tell,
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is only defined in terms of
> NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is set, so I am not
> sure how those functions could be absent in the source file but used in
> NOIRQ_SYSTEM_SLEEP_PM_OPS() when CONFIG_PM_SLEEP is unset... I could be
> missing something though.

It was a mistake on my end: my randconfig tree has an experimental
patch to clean up all 13 users of SET_NOIRQ_SYSTEM_SLEEP_PM_OPS()
that actually rely on the old behavior and changing the definition
to be the same as NOIRQ_SYSTEM_SLEEP_PM_OPS. I should get back
to that series and actually send out patches towards removing
the deprecated helpers.

      Arnd

Re: [PATCH] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()

[Alexandre Belloni]

On Tue, 15 Aug 2023 15:16:41 -0700, Nathan Chancellor wrote:
> After the switch to SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() and a subsequent
> fix, stm32_rtc_{suspend,resume}() are unused when CONFIG_PM_SLEEP is not
> set because SET_NOIRQ_SYSTEM_SLEEP_PM_OPS() is a no-op in that
> configuration:
> 
>   drivers/rtc/rtc-stm32.c:904:12: error: 'stm32_rtc_resume' defined but not used [-Werror=unused-function]
>     904 | static int stm32_rtc_resume(struct device *dev)
>         |            ^~~~~~~~~~~~~~~~
>   drivers/rtc/rtc-stm32.c:894:12: error: 'stm32_rtc_suspend' defined but not used [-Werror=unused-function]
>     894 | static int stm32_rtc_suspend(struct device *dev)
>         |            ^~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
> 
> [...]

Applied, thanks!

[1/1] rtc: stm32: Use NOIRQ_SYSTEM_SLEEP_PM_OPS()
      commit: 2cf2a1acc6ebdffc6363de9156db8737f33c1803

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com