Message ID | 20230310140250.359147-1-arnd@kernel.org |
---|---|
State | New |
Headers | show |
Series | pinctrl: s32cc: fix !CONFIG_PM_SLEEP build error | expand |
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 >
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
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
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 = {