2024-04-04 07:16:52

by Anand Moon

[permalink] [raw]
Subject: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

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 <[email protected]>
---
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(-)

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),
},
};

--
2.44.0



2024-04-09 01:55:14

by Thinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

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 <[email protected]>
> ---
> 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(-)
>

2024-04-10 05:17:39

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Hi Thinh,

On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <[email protected]> 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 <[email protected]>
> > ---
> > 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

2024-04-10 23:33:26

by Thinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

On Wed, Apr 10, 2024, Anand Moon wrote:
> Hi Thinh,
>
> On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <[email protected]> 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 <[email protected]>
> > > ---
> > > 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

2024-04-11 08:13:43

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Hi Thinh,

On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <[email protected]> wrote:
>
> On Wed, Apr 10, 2024, Anand Moon wrote:
> > Hi Thinh,
> >
> > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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

2024-04-11 22:22:33

by Thinh Nguyen

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

On Thu, Apr 11, 2024, Anand Moon wrote:
> Hi Thinh,
>
> On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <[email protected]> wrote:
> >
> > On Wed, Apr 10, 2024, Anand Moon wrote:
> > > Hi Thinh,
> > >
> > > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <[email protected]> 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 <[email protected]>
> > > > > ---
> > > > > 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

2024-04-12 05:33:04

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] usb: dwc3: exynos: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Hi Thinh,

On Fri, 12 Apr 2024 at 03:52, Thinh Nguyen <[email protected]> wrote:
>
> On Thu, Apr 11, 2024, Anand Moon wrote:
> > Hi Thinh,
> >
> > On Thu, 11 Apr 2024 at 04:56, Thinh Nguyen <[email protected]> wrote:
> > >
> > > On Wed, Apr 10, 2024, Anand Moon wrote:
> > > > Hi Thinh,
> > > >
> > > > On Tue, 9 Apr 2024 at 07:24, Thinh Nguyen <[email protected]> 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 <[email protected]>
> > > > > > ---
> > > > > > 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