pinctrl: s32cc: fix !CONFIG_PM_SLEEP build error

Message ID 20230310140250.359147-1-arnd@kernel.org
State New
Headers
Series pinctrl: s32cc: fix !CONFIG_PM_SLEEP build error |

Commit Message

Arnd Bergmann March 10, 2023, 2:02 p.m. UTC
  From: Arnd Bergmann <arnd@arndb.de>

The declaration of s32_pinctrl_suspend/s32_pinctrl_resume is hidden
in an #ifdef, causing a compilation failure when CONFIG_PM_SLEEP is
disabled:

drivers/pinctrl/nxp/pinctrl-s32g2.c:754:38: error: 's32_pinctrl_suspend' undeclared here (not in a function); did you mean 's32_pinctrl_probe'?
drivers/pinctrl/nxp/pinctrl-s32g2.c:754:9: note: in expansion of macro 'SET_LATE_SYSTEM_SLEEP_PM_OPS'
  754 |         SET_LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Remove the bogus #ifdef and __maybe_unused annation on the global
functions, and instead use the proper LATE_SYSTEM_SLEEP_PM_OPS()
macro to pick set the function pointer.

As the function definition is still in the #ifdef block, this leads
to the correct code in all configurations.

Fixes: fd84aaa8173d ("pinctrl: add NXP S32 SoC family support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/pinctrl/nxp/pinctrl-s32.h   | 6 ++----
 drivers/pinctrl/nxp/pinctrl-s32cc.c | 4 ++--
 drivers/pinctrl/nxp/pinctrl-s32g2.c | 3 +--
 3 files changed, 5 insertions(+), 8 deletions(-)
  

Comments

Chester Lin March 10, 2023, 4:51 p.m. UTC | #1
Hi Arnd,

On Fri, Mar 10, 2023 at 03:02:35PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The declaration of s32_pinctrl_suspend/s32_pinctrl_resume is hidden
> in an #ifdef, causing a compilation failure when CONFIG_PM_SLEEP is
> disabled:
> 
> drivers/pinctrl/nxp/pinctrl-s32g2.c:754:38: error: 's32_pinctrl_suspend' undeclared here (not in a function); did you mean 's32_pinctrl_probe'?
> drivers/pinctrl/nxp/pinctrl-s32g2.c:754:9: note: in expansion of macro 'SET_LATE_SYSTEM_SLEEP_PM_OPS'
>   754 |         SET_LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend,
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 

Thanks for your patch and hope you don't mind if I ask a stupid question.

To reproduce this issue, I tried disabling the CONFIG_PM_SLEEP and then compile
linux-pinctrl's latest for-next branch [ef455e9e8f76] but I still can't see this
compilation failure. May I know details such as your configs, compiler and kernel-tree?

Here are my steps:

1. make defconfig
2. Set CONFIG_PINCTRL_S32G2=y and unset both CONFIG_SUSPEND & CONFIG_HIBERNATION
   so that CONFIG_PM_SLEEP vanished.
3. make oldconfig; make
4. Ensure that both s32_pinctrl_suspend and s32_pinctrl_resume are disappeared
   from System.map

I tried the following two gcc versions on both arm64 and x86_64 machines:

1) Native arm64:

Target: aarch64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,ada,go --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/7 --enable-ssp --disable-libssp --disable-libvtv --disable-libmpx --disable-libcc1 --disable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-7 --without-system-libunwind --enable-fix-cortex-a53-835769 --enable-fix-cortex-a53-843419 --build=aarch64-suse-linux --host=aarch64-suse-linux
Thread model: posix
gcc version 7.5.0 (SUSE Linux)

2) Cross-compilation on x86_64:

Using built-in specs.
COLLECT_GCC=aarch64-suse-linux-gcc
COLLECT_LTO_WRAPPER=/usr/lib64/gcc/aarch64-suse-linux/12/lto-wrapper
Target: aarch64-suse-linux
Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++ --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/12 --enable-ssp --disable-libssp --disable-libvtv --enable-cet=auto --disable-libcc1 --disable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/usr/aarch64-suse-linux/sys-root/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-12 --program-prefix=aarch64-suse-linux- --target=aarch64-suse-linux --disable-nls --with-sysroot=/usr/aarch64-suse-linux/sys-root --with-build-sysroot=/usr/aarch64-suse-linux/sys-root --with-build-time-tools=/usr/aarch64-suse-linux/bin --enable-fix-cortex-a53-835769 --enable-fix-cortex-a53-843419 --disable-libsanitizer --build=x86_64-suse-linux --host=x86_64-suse-linux
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.2.1 20220830 [revision e927d1cf141f221c5a32574bde0913307e140984] (SUSE Linux)

Regards,
Chester

> Remove the bogus #ifdef and __maybe_unused annation on the global
> functions, and instead use the proper LATE_SYSTEM_SLEEP_PM_OPS()
> macro to pick set the function pointer.
> 
> As the function definition is still in the #ifdef block, this leads
> to the correct code in all configurations.
> 
> Fixes: fd84aaa8173d ("pinctrl: add NXP S32 SoC family support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/pinctrl/nxp/pinctrl-s32.h   | 6 ++----
>  drivers/pinctrl/nxp/pinctrl-s32cc.c | 4 ++--
>  drivers/pinctrl/nxp/pinctrl-s32g2.c | 3 +--
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
> index b6d530a62051..545bf16b988d 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32.h
> +++ b/drivers/pinctrl/nxp/pinctrl-s32.h
> @@ -68,8 +68,6 @@ struct s32_pinctrl_soc_info {
>  
>  int s32_pinctrl_probe(struct platform_device *pdev,
>  			struct s32_pinctrl_soc_info *info);
> -#ifdef CONFIG_PM_SLEEP
> -int __maybe_unused s32_pinctrl_resume(struct device *dev);
> -int __maybe_unused s32_pinctrl_suspend(struct device *dev);
> -#endif
> +int s32_pinctrl_resume(struct device *dev);
> +int s32_pinctrl_suspend(struct device *dev);
>  #endif /* __DRIVERS_PINCTRL_S32_H */
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> index 2c945523af80..e1da332433a3 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
> @@ -658,7 +658,7 @@ static bool s32_pinctrl_should_save(struct s32_pinctrl *ipctl,
>  	return false;
>  }
>  
> -int __maybe_unused s32_pinctrl_suspend(struct device *dev)
> +int s32_pinctrl_suspend(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct s32_pinctrl *ipctl = platform_get_drvdata(pdev);
> @@ -685,7 +685,7 @@ int __maybe_unused s32_pinctrl_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -int __maybe_unused s32_pinctrl_resume(struct device *dev)
> +int s32_pinctrl_resume(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct s32_pinctrl *ipctl = platform_get_drvdata(pdev);
> diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
> index 7dd0b4f8904d..5028f4adc389 100644
> --- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
> +++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
> @@ -751,8 +751,7 @@ static int s32g_pinctrl_probe(struct platform_device *pdev)
>  }
>  
>  static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
> -	SET_LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend,
> -				     s32_pinctrl_resume)
> +	LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend, s32_pinctrl_resume)
>  };
>  
>  static struct platform_driver s32g_pinctrl_driver = {
> -- 
> 2.39.2
>
  
Arnd Bergmann March 10, 2023, 5:01 p.m. UTC | #2
On Fri, Mar 10, 2023, at 17:51, Chester Lin wrote:
>
> Thanks for your patch and hope you don't mind if I ask a stupid question.
>
> To reproduce this issue, I tried disabling the CONFIG_PM_SLEEP and then 
> compile
> linux-pinctrl's latest for-next branch [ef455e9e8f76] but I still can't 
> see this
> compilation failure. May I know details such as your configs, compiler 
> and kernel-tree?
>
> Here are my steps:
>
> 1. make defconfig
> 2. Set CONFIG_PINCTRL_S32G2=y and unset both CONFIG_SUSPEND & CONFIG_HIBERNATION
>    so that CONFIG_PM_SLEEP vanished.
> 3. make oldconfig; make
> 4. Ensure that both s32_pinctrl_suspend and s32_pinctrl_resume are disappeared
>    from System.map
>
> I tried the following two gcc versions on both arm64 and x86_64 machines:

I only see the problem on arm64 on yesterday's and today's linux-next.
See my attached .config from a 'make randconfig build'

     Arnd
  
Linus Walleij March 13, 2023, 10:20 a.m. UTC | #3
On Fri, Mar 10, 2023 at 3:02 PM Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>
>
> The declaration of s32_pinctrl_suspend/s32_pinctrl_resume is hidden
> in an #ifdef, causing a compilation failure when CONFIG_PM_SLEEP is
> disabled:
>
> drivers/pinctrl/nxp/pinctrl-s32g2.c:754:38: error: 's32_pinctrl_suspend' undeclared here (not in a function); did you mean 's32_pinctrl_probe'?
> drivers/pinctrl/nxp/pinctrl-s32g2.c:754:9: note: in expansion of macro 'SET_LATE_SYSTEM_SLEEP_PM_OPS'
>   754 |         SET_LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend,
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Remove the bogus #ifdef and __maybe_unused annation on the global
> functions, and instead use the proper LATE_SYSTEM_SLEEP_PM_OPS()
> macro to pick set the function pointer.
>
> As the function definition is still in the #ifdef block, this leads
> to the correct code in all configurations.
>
> Fixes: fd84aaa8173d ("pinctrl: add NXP S32 SoC family support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

Yours,
Linus Walleij
  

Patch

diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
index b6d530a62051..545bf16b988d 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32.h
+++ b/drivers/pinctrl/nxp/pinctrl-s32.h
@@ -68,8 +68,6 @@  struct s32_pinctrl_soc_info {
 
 int s32_pinctrl_probe(struct platform_device *pdev,
 			struct s32_pinctrl_soc_info *info);
-#ifdef CONFIG_PM_SLEEP
-int __maybe_unused s32_pinctrl_resume(struct device *dev);
-int __maybe_unused s32_pinctrl_suspend(struct device *dev);
-#endif
+int s32_pinctrl_resume(struct device *dev);
+int s32_pinctrl_suspend(struct device *dev);
 #endif /* __DRIVERS_PINCTRL_S32_H */
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 2c945523af80..e1da332433a3 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -658,7 +658,7 @@  static bool s32_pinctrl_should_save(struct s32_pinctrl *ipctl,
 	return false;
 }
 
-int __maybe_unused s32_pinctrl_suspend(struct device *dev)
+int s32_pinctrl_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct s32_pinctrl *ipctl = platform_get_drvdata(pdev);
@@ -685,7 +685,7 @@  int __maybe_unused s32_pinctrl_suspend(struct device *dev)
 	return 0;
 }
 
-int __maybe_unused s32_pinctrl_resume(struct device *dev)
+int s32_pinctrl_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct s32_pinctrl *ipctl = platform_get_drvdata(pdev);
diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
index 7dd0b4f8904d..5028f4adc389 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
@@ -751,8 +751,7 @@  static int s32g_pinctrl_probe(struct platform_device *pdev)
 }
 
 static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
-	SET_LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend,
-				     s32_pinctrl_resume)
+	LATE_SYSTEM_SLEEP_PM_OPS(s32_pinctrl_suspend, s32_pinctrl_resume)
 };
 
 static struct platform_driver s32g_pinctrl_driver = {