diff mbox series

[v2,6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Message ID 20240404071350.4242-7-linux.amoon@gmail.com
State New
Headers show
Series [v2,1/6] usb: ehci-exynos: Use devm_clk_get_enabled() helpers | expand

Commit Message

Anand Moon April 4, 2024, 7:13 a.m. UTC
Use the new PM macros for the suspend and resume functions to be
automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
they will simply be discarded by the compiler.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
   disabled.
---
 drivers/usb/dwc3/dwc3-exynos.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Thinh Nguyen April 9, 2024, 1:54 a.m. UTC | #1
On Thu, Apr 04, 2024, Anand Moon wrote:
> Use the new PM macros for the suspend and resume functions to be
> automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> they will simply be discarded by the compiler.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
>    disabled.

The compiler discards the code, yet we still need __maybe_unused?

BR,
Thinh

> ---
>  drivers/usb/dwc3/dwc3-exynos.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
Anand Moon April 10, 2024, 5:17 a.m. UTC | #2
Hi Thinh,

On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> On Thu, Apr 04, 2024, Anand Moon wrote:
> > Use the new PM macros for the suspend and resume functions to be
> > automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> > without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> > they will simply be discarded by the compiler.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
> >    disabled.
>
> The compiler discards the code, yet we still need __maybe_unused?
>
Earlier version had not added this since but I removed the
guard.CONFIG_PM_SLEEP.
added __maybe_unused just to safeguard the function.

I have tried to build with config by disabling CONFIG_PM and CONFIG_PM_SLEEP
but could get the warning compilation by adding flag W=1
-Werror=unused-function.

diff --git a/arch/arm/configs/exynos_defconfig
b/arch/arm/configs/exynos_defconfig
index 7ad48fdda1da..43110e42076e 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -29,8 +29,19 @@ CONFIG_ARM_EXYNOS_CPUIDLE=y
 CONFIG_VFP=y
 CONFIG_NEON=y
 CONFIG_KERNEL_MODE_NEON=y
-CONFIG_PM_DEBUG=y
-CONFIG_PM_ADVANCED_DEBUG=y
+CONFIG_PM_SLEEP=n
+CONFIG_PM_SLEEP_SMP=n
+# CONFIG_PM_AUTOSLEEP is not set
+# CONFIG_PM_USERSPACE_AUTOSLEEP is not set
+# CONFIG_PM_WAKELOCKS is not set
+CONFIG_PM=n
+CONFIG_PM_DEBUG=n
+CONFIG_PM_ADVANCED_DEBUG=n
+# CONFIG_PM_TEST_SUSPEND is not set
+# CONFIG_PM_SLEEP_DEBUG=n
+#
+CONFIG_PM_DEBUG=n
+CONFIG_PM_ADVANCED_DEBUG=n
 CONFIG_ENERGY_MODEL=y
 CONFIG_KALLSYMS_ALL=y
 CONFIG_MODULES=y

But since these CONFIG_PM and CONFIG_PM_SLEEP cannot be disabled,
I am not getting any warning related to these functions.

Do you want me to remove __maybe_unused ?

alarm@archl-xu4b:~/linux-exynos-5.y-devel$ ./buildkernelexynos-arch.sh
arch/arm/configs/exynos_defconfig:43:warning: override: reassigning to
symbol PM_DEBUG
arch/arm/configs/exynos_defconfig:44:warning: override: reassigning to
symbol PM_ADVANCED_DEBUG
#
# configuration written to .config
#
  SYNC    include/config/auto.conf
  CALL    scripts/checksyscalls.sh
  CC      arch/arm/mach-exynos/pm.o

> BR,
> Thinh
>

Thanks
-Anand
Thinh Nguyen April 10, 2024, 11:26 p.m. UTC | #3
On Wed, Apr 10, 2024, Anand Moon wrote:
> Hi Thinh,
> 
> On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> >
> > On Thu, Apr 04, 2024, Anand Moon wrote:
> > > Use the new PM macros for the suspend and resume functions to be
> > > automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> > > without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> > > they will simply be discarded by the compiler.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
> > >    disabled.
> >
> > The compiler discards the code, yet we still need __maybe_unused?
> >
> Earlier version had not added this since but I removed the
> guard.CONFIG_PM_SLEEP.
> added __maybe_unused just to safeguard the function.
> 
> I have tried to build with config by disabling CONFIG_PM and CONFIG_PM_SLEEP
> but could get the warning compilation by adding flag W=1
> -Werror=unused-function.
> 

<snip>

> 
> But since these CONFIG_PM and CONFIG_PM_SLEEP cannot be disabled,
> I am not getting any warning related to these functions.
> 
> Do you want me to remove __maybe_unused ?
> 

The warning was there as expected. You should to use it along with
DEFINE_SIMPLE_DEV_PM_OPS(). Let me know if you still see the same
warning.

BR,
Thinh
Anand Moon April 11, 2024, 7:50 a.m. UTC | #4
Hi Thinh,

On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> On Wed, Apr 10, 2024, Anand Moon wrote:
> > Hi Thinh,
> >
> > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> > >
> > > On Thu, Apr 04, 2024, Anand Moon wrote:
> > > > Use the new PM macros for the suspend and resume functions to be
> > > > automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> > > > without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> > > > they will simply be discarded by the compiler.
> > > >
> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > ---
> > > > v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
> > > >    disabled.
> > >
> > > The compiler discards the code, yet we still need __maybe_unused?
> > >
> > Earlier version had not added this since but I removed the
> > guard.CONFIG_PM_SLEEP.
> > added __maybe_unused just to safeguard the function.
> >
> > I have tried to build with config by disabling CONFIG_PM and CONFIG_PM_SLEEP
> > but could get the warning compilation by adding flag W=1
> > -Werror=unused-function.
> >
>
> <snip>
>
> >
> > But since these CONFIG_PM and CONFIG_PM_SLEEP cannot be disabled,
> > I am not getting any warning related to these functions.
> >
> > Do you want me to remove __maybe_unused ?
> >
>
> The warning was there as expected. You should to use it along with
> DEFINE_SIMPLE_DEV_PM_OPS(). Let me know if you still see the same
> warning.
>

But the warnings are related to the following macros

#define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)

#define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
#define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))

So if we can disable CONFIG_PM and CONFIG_PM_SLEEP options
the relevant function with the above macro will be set to NULL.
in this case, the compiler will discard the function in SET_SYSTEM_SLEEP_PM_OPS

> BR,
> Thinh

Thanks
-Anand
Thinh Nguyen April 11, 2024, 10:21 p.m. UTC | #5
On Thu, Apr 11, 2024, Anand Moon wrote:
> Hi Thinh,
> 
> On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> >
> > On Wed, Apr 10, 2024, Anand Moon wrote:
> > > Hi Thinh,
> > >
> > > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> > > >
> > > > On Thu, Apr 04, 2024, Anand Moon wrote:
> > > > > Use the new PM macros for the suspend and resume functions to be
> > > > > automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> > > > > without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> > > > > they will simply be discarded by the compiler.
> > > > >
> > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > > ---
> > > > > v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
> > > > >    disabled.
> > > >
> > > > The compiler discards the code, yet we still need __maybe_unused?
> > > >
> > > Earlier version had not added this since but I removed the
> > > guard.CONFIG_PM_SLEEP.
> > > added __maybe_unused just to safeguard the function.
> > >
> > > I have tried to build with config by disabling CONFIG_PM and CONFIG_PM_SLEEP
> > > but could get the warning compilation by adding flag W=1
> > > -Werror=unused-function.
> > >
> >
> > <snip>
> >
> > >
> > > But since these CONFIG_PM and CONFIG_PM_SLEEP cannot be disabled,
> > > I am not getting any warning related to these functions.
> > >
> > > Do you want me to remove __maybe_unused ?
> > >
> >
> > The warning was there as expected. You should to use it along with
> > DEFINE_SIMPLE_DEV_PM_OPS(). Let me know if you still see the same
> > warning.
> >
> 
> But the warnings are related to the following macros
> 
> #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
> 
> #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
> #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
> 
> So if we can disable CONFIG_PM and CONFIG_PM_SLEEP options
> the relevant function with the above macro will be set to NULL.
> in this case, the compiler will discard the function in SET_SYSTEM_SLEEP_PM_OPS
> 

There are differences if you use SET_SYSTEM_SLEEP_PM_OPS vs the new
macro. It's noted in this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1a3c7bb088266fa2db017be299f91f1c1894c857

Please try it out.

Thanks,
Thinh
Anand Moon April 12, 2024, 5:32 a.m. UTC | #6
Hi Thinh,

On Fri, 12 Apr 2024 at 03:52, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
>
> On Thu, Apr 11, 2024, Anand Moon wrote:
> > Hi Thinh,
> >
> > On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> > >
> > > On Wed, Apr 10, 2024, Anand Moon wrote:
> > > > Hi Thinh,
> > > >
> > > > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <Thinh.Nguyen@synopsys.com> wrote:
> > > > >
> > > > > On Thu, Apr 04, 2024, Anand Moon wrote:
> > > > > > Use the new PM macros for the suspend and resume functions to be
> > > > > > automatically dropped by the compiler when CONFIG_PM_SLEEP are disabled,
> > > > > > without having to use #ifdef guards. If CONFIG_PM_SLEEP unused,
> > > > > > they will simply be discarded by the compiler.
> > > > > >
> > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > > > ---
> > > > > > v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM is
> > > > > >    disabled.
> > > > >
> > > > > The compiler discards the code, yet we still need __maybe_unused?
> > > > >
> > > > Earlier version had not added this since but I removed the
> > > > guard.CONFIG_PM_SLEEP.
> > > > added __maybe_unused just to safeguard the function.
> > > >
> > > > I have tried to build with config by disabling CONFIG_PM and CONFIG_PM_SLEEP
> > > > but could get the warning compilation by adding flag W=1
> > > > -Werror=unused-function.
> > > >
> > >
> > > <snip>
> > >
> > > >
> > > > But since these CONFIG_PM and CONFIG_PM_SLEEP cannot be disabled,
> > > > I am not getting any warning related to these functions.
> > > >
> > > > Do you want me to remove __maybe_unused ?
> > > >
> > >
> > > The warning was there as expected. You should to use it along with
> > > DEFINE_SIMPLE_DEV_PM_OPS(). Let me know if you still see the same
> > > warning.
> > >
> >
> > But the warnings are related to the following macros
> >
> > #define PTR_IF(cond, ptr) ((cond) ? (ptr) : NULL)
> >
> > #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
> > #define pm_sleep_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), (_ptr))
> >
> > So if we can disable CONFIG_PM and CONFIG_PM_SLEEP options
> > the relevant function with the above macro will be set to NULL.
> > in this case, the compiler will discard the function in SET_SYSTEM_SLEEP_PM_OPS
> >
>
> There are differences if you use SET_SYSTEM_SLEEP_PM_OPS vs the new
> macro. It's noted in this commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1a3c7bb088266fa2db017be299f91f1c1894c857
>
> Please try it out.

Thank you very much for this input

I will drop the __maybe_unused from my patch series.
and switch to use DEFINE_SIMPLE_DEV_PM_OPS macro
for suspend resume functions

>
> Thanks,
> Thinh

Thanks
-Anand
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 2d341f0e22a3..90259ad1d0d9 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -161,8 +161,7 @@  static const struct of_device_id exynos_dwc3_match[] = {
 };
 MODULE_DEVICE_TABLE(of, exynos_dwc3_match);
 
-#ifdef CONFIG_PM_SLEEP
-static int dwc3_exynos_suspend(struct device *dev)
+static int __maybe_unused dwc3_exynos_suspend(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i;
@@ -173,7 +172,7 @@  static int dwc3_exynos_suspend(struct device *dev)
 	return 0;
 }
 
-static int dwc3_exynos_resume(struct device *dev)
+static int __maybe_unused dwc3_exynos_resume(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);
 	int i, ret;
@@ -194,18 +193,13 @@  static const struct dev_pm_ops dwc3_exynos_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dwc3_exynos_suspend, dwc3_exynos_resume)
 };
 
-#define DEV_PM_OPS	(&dwc3_exynos_dev_pm_ops)
-#else
-#define DEV_PM_OPS	NULL
-#endif /* CONFIG_PM_SLEEP */
-
 static struct platform_driver dwc3_exynos_driver = {
 	.probe		= dwc3_exynos_probe,
 	.remove_new	= dwc3_exynos_remove,
 	.driver		= {
 		.name	= "exynos-dwc3",
 		.of_match_table = exynos_dwc3_match,
-		.pm	= DEV_PM_OPS,
+		.pm	= pm_sleep_ptr(&dwc3_exynos_dev_pm_ops),
 	},
 };