* [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
@ 2022-02-26 22:24 Randy Dunlap
2022-02-27 12:04 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2022-02-26 22:24 UTC (permalink / raw)
To: linux-kernel
Cc: Randy Dunlap, Wei WANG, Kai-Heng Feng, Greg Kroah-Hartman,
Arnd Bergmann
When CONFG_WERROR=y and CONFIG_PM is not set, there are fatal build
errors, so surround these functions in an #ifdef CONFIG_PM block.
../drivers/misc/cardreader/rtsx_pcr.c:1057:13: error: ‘rtsx_enable_aspm’ defined but not used [-Werror=unused-function]
static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
miscread001.out:../drivers/misc/cardreader/rtsx_pcr.c:1065:13: error: ‘rtsx_comm_pm_power_saving’ defined but not used [-Werror=unused-function]
miscread001.out: static void rtsx_comm_pm_power_saving(struct rtsx_pcr *pcr)
../drivers/misc/cardreader/rtsx_pcr.c:1084:13: error: ‘rtsx_pm_power_saving’ defined but not used [-Werror=unused-function]
static void rtsx_pm_power_saving(struct rtsx_pcr *pcr)
Fixes: 597568e8df04 ("misc: rtsx: Rework runtime power management flow")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Wei WANG <wei_wang@realsil.com.cn>
Cc: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
---
drivers/misc/cardreader/rtsx_pcr.c | 2 ++
1 file changed, 2 insertions(+)
--- linux-next-20220225.orig/drivers/misc/cardreader/rtsx_pcr.c
+++ linux-next-20220225/drivers/misc/cardreader/rtsx_pcr.c
@@ -1054,6 +1054,7 @@ static int rtsx_pci_acquire_irq(struct r
return 0;
}
+#ifdef CONFIG_PM
static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
{
if (pcr->ops->set_aspm)
@@ -1085,6 +1086,7 @@ static void rtsx_pm_power_saving(struct
{
rtsx_comm_pm_power_saving(pcr);
}
+#endif
static void rtsx_base_force_power_down(struct rtsx_pcr *pcr)
{
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-26 22:24 [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set Randy Dunlap
@ 2022-02-27 12:04 ` Arnd Bergmann
2022-02-27 16:57 ` Randy Dunlap
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2022-02-27 12:04 UTC (permalink / raw)
To: Randy Dunlap
Cc: Linux Kernel Mailing List, Wei WANG, Kai-Heng Feng,
Greg Kroah-Hartman, Arnd Bergmann
On Sat, Feb 26, 2022 at 11:24 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> ---
> drivers/misc/cardreader/rtsx_pcr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-next-20220225.orig/drivers/misc/cardreader/rtsx_pcr.c
> +++ linux-next-20220225/drivers/misc/cardreader/rtsx_pcr.c
> @@ -1054,6 +1054,7 @@ static int rtsx_pci_acquire_irq(struct r
> return 0;
> }
>
> +#ifdef CONFIG_PM
> static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
> {
> if (pcr->ops->set_aspm)
> @@ -1085,6 +1086,7 @@ static void rtsx_pm_power_saving(struct
> {
> rtsx_comm_pm_power_saving(pcr);
> }
> +#endif
Now that we have DEFINE_SIMPLE_DEV_PM_OPS() etc, I think we should
no longer add more __maybe_unused annotations or #ifdef CONFIG_PM checks
but just use the new macros for any new files or whenever a warning like
this shows up.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 12:04 ` Arnd Bergmann
@ 2022-02-27 16:57 ` Randy Dunlap
2022-02-27 17:30 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Randy Dunlap @ 2022-02-27 16:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Kernel Mailing List, Wei WANG, Kai-Heng Feng,
Greg Kroah-Hartman
On 2/27/22 04:04, Arnd Bergmann wrote:
> On Sat, Feb 26, 2022 at 11:24 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
>> ---
>> drivers/misc/cardreader/rtsx_pcr.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> --- linux-next-20220225.orig/drivers/misc/cardreader/rtsx_pcr.c
>> +++ linux-next-20220225/drivers/misc/cardreader/rtsx_pcr.c
>> @@ -1054,6 +1054,7 @@ static int rtsx_pci_acquire_irq(struct r
>> return 0;
>> }
>>
>> +#ifdef CONFIG_PM
>> static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
>> {
>> if (pcr->ops->set_aspm)
>> @@ -1085,6 +1086,7 @@ static void rtsx_pm_power_saving(struct
>> {
>> rtsx_comm_pm_power_saving(pcr);
>> }
>> +#endif
>
> Now that we have DEFINE_SIMPLE_DEV_PM_OPS() etc, I think we should
> no longer add more __maybe_unused annotations or #ifdef CONFIG_PM checks
> but just use the new macros for any new files or whenever a warning like
> this shows up.
In this case it looks like DEFINE_RUNTIME_DEV_PM_OPS() is better.
Using DEFINE_SIMPLE_DEV_PM_OPS() still produces build warnings/errors
for unused functions. And I do see 4 drivers that are already using
DEFINE_RUNTIME_DEV_PM_OPS().
Patch coming right up.
thanks.
--
~Randy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 16:57 ` Randy Dunlap
@ 2022-02-27 17:30 ` Arnd Bergmann
2022-02-27 17:46 ` Paul Cercueil
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2022-02-27 17:30 UTC (permalink / raw)
To: Randy Dunlap
Cc: Arnd Bergmann, Linux Kernel Mailing List, Wei WANG, Kai-Heng Feng,
Greg Kroah-Hartman, Paul Cercueil, Rafael J. Wysocki,
Jonathan Cameron
On Sun, Feb 27, 2022 at 5:57 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> On 2/27/22 04:04, Arnd Bergmann wrote:
> > On Sat, Feb 26, 2022 at 11:24 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> >> ---
> >> drivers/misc/cardreader/rtsx_pcr.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> --- linux-next-20220225.orig/drivers/misc/cardreader/rtsx_pcr.c
> >> +++ linux-next-20220225/drivers/misc/cardreader/rtsx_pcr.c
> >> @@ -1054,6 +1054,7 @@ static int rtsx_pci_acquire_irq(struct r
> >> return 0;
> >> }
> >>
> >> +#ifdef CONFIG_PM
> >> static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
> >> {
> >> if (pcr->ops->set_aspm)
> >> @@ -1085,6 +1086,7 @@ static void rtsx_pm_power_saving(struct
> >> {
> >> rtsx_comm_pm_power_saving(pcr);
> >> }
> >> +#endif
> >
> > Now that we have DEFINE_SIMPLE_DEV_PM_OPS() etc, I think we should
> > no longer add more __maybe_unused annotations or #ifdef CONFIG_PM checks
> > but just use the new macros for any new files or whenever a warning like
> > this shows up.
>
> In this case it looks like DEFINE_RUNTIME_DEV_PM_OPS() is better.
> Using DEFINE_SIMPLE_DEV_PM_OPS() still produces build warnings/errors
> for unused functions. And I do see 4 drivers that are already using
> DEFINE_RUNTIME_DEV_PM_OPS().
>
> Patch coming right up.
DEFINE_RUNTIME_DEV_PM_OPS() only references the three runtime functions
(rtsx_pci_runtime_suspend, rtsx_pci_runtime_resume and rtsx_pci_runtime_idle)
but not the pm-sleep functions (rtsx_pci_suspend and rtsx_pci_resume), so your
second patch doesn't look correct either.
I see there is a _DEFINE_DEV_PM_OPS() helper that appears to do
what we want here, but I'm not sure this is considered an official API. Adding
Rafael, Paul and Jonathan to Cc for extra input. As the macros are still
fairly new, I suspect the idea was to add more as needed, so maybe should
add a DEFINE_DEV_PM_OPS() to wrap _DEFINE_DEV_PM_OPS()?
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 17:30 ` Arnd Bergmann
@ 2022-02-27 17:46 ` Paul Cercueil
2022-02-27 17:51 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2022-02-27 17:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Randy Dunlap, Linux Kernel Mailing List, Wei WANG, Kai-Heng Feng,
Greg Kroah-Hartman, Rafael J. Wysocki, Jonathan Cameron
Hi,
Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
<arnd@arndb.de> a écrit :
> On Sun, Feb 27, 2022 at 5:57 PM Randy Dunlap <rdunlap@infradead.org>
> wrote:
>> On 2/27/22 04:04, Arnd Bergmann wrote:
>> > On Sat, Feb 26, 2022 at 11:24 PM Randy Dunlap
>> <rdunlap@infradead.org> wrote:
>> >
>> >> ---
>> >> drivers/misc/cardreader/rtsx_pcr.c | 2 ++
>> >> 1 file changed, 2 insertions(+)
>> >>
>> >> --- linux-next-20220225.orig/drivers/misc/cardreader/rtsx_pcr.c
>> >> +++ linux-next-20220225/drivers/misc/cardreader/rtsx_pcr.c
>> >> @@ -1054,6 +1054,7 @@ static int rtsx_pci_acquire_irq(struct r
>> >> return 0;
>> >> }
>> >>
>> >> +#ifdef CONFIG_PM
>> >> static void rtsx_enable_aspm(struct rtsx_pcr *pcr)
>> >> {
>> >> if (pcr->ops->set_aspm)
>> >> @@ -1085,6 +1086,7 @@ static void rtsx_pm_power_saving(struct
>> >> {
>> >> rtsx_comm_pm_power_saving(pcr);
>> >> }
>> >> +#endif
>> >
>> > Now that we have DEFINE_SIMPLE_DEV_PM_OPS() etc, I think we should
>> > no longer add more __maybe_unused annotations or #ifdef CONFIG_PM
>> checks
>> > but just use the new macros for any new files or whenever a
>> warning like
>> > this shows up.
>>
>> In this case it looks like DEFINE_RUNTIME_DEV_PM_OPS() is better.
>> Using DEFINE_SIMPLE_DEV_PM_OPS() still produces build
>> warnings/errors
>> for unused functions. And I do see 4 drivers that are already using
>> DEFINE_RUNTIME_DEV_PM_OPS().
>>
>> Patch coming right up.
>
> DEFINE_RUNTIME_DEV_PM_OPS() only references the three runtime
> functions
> (rtsx_pci_runtime_suspend, rtsx_pci_runtime_resume and
> rtsx_pci_runtime_idle)
> but not the pm-sleep functions (rtsx_pci_suspend and
> rtsx_pci_resume), so your
> second patch doesn't look correct either.
>
> I see there is a _DEFINE_DEV_PM_OPS() helper that appears to do
> what we want here, but I'm not sure this is considered an official
> API. Adding
> Rafael, Paul and Jonathan to Cc for extra input. As the macros are
> still
> fairly new, I suspect the idea was to add more as needed, so maybe
> should
> add a DEFINE_DEV_PM_OPS() to wrap _DEFINE_DEV_PM_OPS()?
There could be a DEFINE_DEV_PM_OPS(), but I don't think that's really
needed - you can very well declare your struct dev_pm_ops without using
one of these macros. Just make sure to use the SYSTEM_SLEEP_PM_OPS /
RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the device.pm
pointer.
Cheers,
-Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 17:46 ` Paul Cercueil
@ 2022-02-27 17:51 ` Arnd Bergmann
2022-02-27 17:56 ` Paul Cercueil
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2022-02-27 17:51 UTC (permalink / raw)
To: Paul Cercueil
Cc: Arnd Bergmann, Randy Dunlap, Linux Kernel Mailing List, Wei WANG,
Kai-Heng Feng, Greg Kroah-Hartman, Rafael J. Wysocki,
Jonathan Cameron
On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil <paul@crapouillou.net> wrote:
> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
>
> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's really
> needed - you can very well declare your struct dev_pm_ops without using
> one of these macros. Just make sure to use the SYSTEM_SLEEP_PM_OPS /
> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the device.pm
> pointer.
Ah, of course, so it comes down to
s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
removing all the #ifdef an __maybe_unused annotations. The pm_ptr()
in driver.pm makes this slightly more optimized AFAICT, but has no
effect on behavior, right?
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 17:51 ` Arnd Bergmann
@ 2022-02-27 17:56 ` Paul Cercueil
2022-03-06 15:12 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2022-02-27 17:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Randy Dunlap, Linux Kernel Mailing List, Wei WANG, Kai-Heng Feng,
Greg Kroah-Hartman, Rafael J. Wysocki, Jonathan Cameron
Le dim., févr. 27 2022 at 18:51:38 +0100, Arnd Bergmann
<arnd@arndb.de> a écrit :
> On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil <paul@crapouillou.net>
> wrote:
>> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
>>
>> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's
>> really
>> needed - you can very well declare your struct dev_pm_ops without
>> using
>> one of these macros. Just make sure to use the SYSTEM_SLEEP_PM_OPS /
>> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the
>> device.pm
>> pointer.
>
> Ah, of course, so it comes down to
> s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
> removing all the #ifdef an __maybe_unused annotations. The pm_ptr()
> in driver.pm makes this slightly more optimized AFAICT, but has no
> effect on behavior, right?
The use of SYSTEM_SLEEP_PM_OPS makes sure that the callbacks are
dropped if the dev_pm_ops is dead code, and the pm_ptr() must be used
for the compiler to know that the dev_pm_ops is dead code.
-Paul
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-02-27 17:56 ` Paul Cercueil
@ 2022-03-06 15:12 ` Jonathan Cameron
2022-03-06 15:29 ` Paul Cercueil
2022-03-06 16:24 ` Jonathan Cameron
0 siblings, 2 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-03-06 15:12 UTC (permalink / raw)
To: Paul Cercueil
Cc: Arnd Bergmann, Randy Dunlap, Linux Kernel Mailing List, Wei WANG,
Kai-Heng Feng, Greg Kroah-Hartman, Rafael J. Wysocki,
jonathan.cameron
On Sun, 27 Feb 2022 17:56:31 +0000
Paul Cercueil <paul@crapouillou.net> wrote:
> Le dim., févr. 27 2022 at 18:51:38 +0100, Arnd Bergmann
> <arnd@arndb.de> a écrit :
> > On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil <paul@crapouillou.net>
> > wrote:
> >> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
> >>
> >> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's
> >> really
> >> needed - you can very well declare your struct dev_pm_ops without
> >> using
> >> one of these macros. Just make sure to use the SYSTEM_SLEEP_PM_OPS /
> >> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the
> >> device.pm
> >> pointer.
> >
> > Ah, of course, so it comes down to
> > s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
> > removing all the #ifdef an __maybe_unused annotations. The pm_ptr()
> > in driver.pm makes this slightly more optimized AFAICT, but has no
> > effect on behavior, right?
>
> The use of SYSTEM_SLEEP_PM_OPS makes sure that the callbacks are
> dropped if the dev_pm_ops is dead code, and the pm_ptr() must be used
> for the compiler to know that the dev_pm_ops is dead code.
>
> -Paul
>
>
Hi Paul,
We have one remaining case which is still ugly to do.
Where both SYSTEM_SLEEP_PM_OPS/RUNTIME_PM_OPS are set and
the dev_pm_ops structure is exported.
For that one we still need to expose #ifdef fun in the
drivers I think.
Any suggestions on a clean solution for that?
Currently I have this...
#ifdef CONFIG_PM
const struct dev_pm_ops bmc150_magn_pm_ops = {
SYSTEM_SLEEP_PM_OPS(...)
RUNTIME_PM_OPS(...)
};
EXPORT_SYMBOL_NS(bmc150_magn_pm_ops, IIO_BMC150_MAGN);
#else
static const __maybe_unused dev_pm_ops bmc150_magn_pm_ops = {
SYSTEM_SLEEP_PM_OPS(...)
RUNTIME_PM_OPS(...)
};
#endif
Not super clean but perhaps we do need
EXPORT_NS_DEV_PM_OPS
EXPORT_NS_GPL_DEV_PM_OPS
and potentially the non namespaced versions.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-03-06 15:12 ` Jonathan Cameron
@ 2022-03-06 15:29 ` Paul Cercueil
2022-03-06 17:57 ` Jonathan Cameron
2022-03-06 16:24 ` Jonathan Cameron
1 sibling, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2022-03-06 15:29 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Arnd Bergmann, Randy Dunlap, Linux Kernel Mailing List, Wei WANG,
Kai-Heng Feng, Greg Kroah-Hartman, Rafael J. Wysocki
Hi Jonathan,
Le dim., mars 6 2022 at 15:12:12 +0000, Jonathan Cameron
<Jonathan.Cameron@Huawei.com> a écrit :
> On Sun, 27 Feb 2022 17:56:31 +0000
> Paul Cercueil <paul@crapouillou.net> wrote:
>
>> Le dim., févr. 27 2022 at 18:51:38 +0100, Arnd Bergmann
>> <arnd@arndb.de> a écrit :
>> > On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil
>> <paul@crapouillou.net>
>> > wrote:
>> >> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
>> >>
>> >> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's
>> >> really
>> >> needed - you can very well declare your struct dev_pm_ops
>> without
>> >> using
>> >> one of these macros. Just make sure to use the
>> SYSTEM_SLEEP_PM_OPS /
>> >> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the
>> >> device.pm
>> >> pointer.
>> >
>> > Ah, of course, so it comes down to
>> > s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
>> > removing all the #ifdef an __maybe_unused annotations. The
>> pm_ptr()
>> > in driver.pm makes this slightly more optimized AFAICT, but has no
>> > effect on behavior, right?
>>
>> The use of SYSTEM_SLEEP_PM_OPS makes sure that the callbacks are
>> dropped if the dev_pm_ops is dead code, and the pm_ptr() must be
>> used
>> for the compiler to know that the dev_pm_ops is dead code.
>>
>> -Paul
>>
>>
>
> Hi Paul,
>
> We have one remaining case which is still ugly to do.
> Where both SYSTEM_SLEEP_PM_OPS/RUNTIME_PM_OPS are set and
> the dev_pm_ops structure is exported.
>
> For that one we still need to expose #ifdef fun in the
> drivers I think.
>
> Any suggestions on a clean solution for that?
Use the _EXPORT_DEV_PM_OPS() macro?
If you make it call __EXPORT_SYMBOL() (with two underscores instead of
one) you can specify the namespace as well. All you need then is a nice
wrapper macro in pm_runtime.h, that can be used in the driver.
Cheers,
-Paul
> Currently I have this...
>
> #ifdef CONFIG_PM
> const struct dev_pm_ops bmc150_magn_pm_ops = {
> SYSTEM_SLEEP_PM_OPS(...)
> RUNTIME_PM_OPS(...)
> };
> EXPORT_SYMBOL_NS(bmc150_magn_pm_ops, IIO_BMC150_MAGN);
> #else
> static const __maybe_unused dev_pm_ops bmc150_magn_pm_ops = {
> SYSTEM_SLEEP_PM_OPS(...)
> RUNTIME_PM_OPS(...)
> };
> #endif
> Not super clean but perhaps we do need
> EXPORT_NS_DEV_PM_OPS
> EXPORT_NS_GPL_DEV_PM_OPS
> and potentially the non namespaced versions.
>
> Thanks,
>
> Jonathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-03-06 15:12 ` Jonathan Cameron
2022-03-06 15:29 ` Paul Cercueil
@ 2022-03-06 16:24 ` Jonathan Cameron
1 sibling, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-03-06 16:24 UTC (permalink / raw)
To: Paul Cercueil
Cc: Arnd Bergmann, Randy Dunlap, Linux Kernel Mailing List, Wei WANG,
Kai-Heng Feng, Greg Kroah-Hartman, Rafael J. Wysocki
On Sun, 6 Mar 2022 15:12:12 +0000
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Sun, 27 Feb 2022 17:56:31 +0000
> Paul Cercueil <paul@crapouillou.net> wrote:
>
> > Le dim., févr. 27 2022 at 18:51:38 +0100, Arnd Bergmann
> > <arnd@arndb.de> a écrit :
> > > On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil <paul@crapouillou.net>
> > > wrote:
> > >> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
> > >>
> > >> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's
> > >> really
> > >> needed - you can very well declare your struct dev_pm_ops without
> > >> using
> > >> one of these macros. Just make sure to use the SYSTEM_SLEEP_PM_OPS /
> > >> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the
> > >> device.pm
> > >> pointer.
> > >
> > > Ah, of course, so it comes down to
> > > s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
> > > removing all the #ifdef an __maybe_unused annotations. The pm_ptr()
> > > in driver.pm makes this slightly more optimized AFAICT, but has no
> > > effect on behavior, right?
> >
> > The use of SYSTEM_SLEEP_PM_OPS makes sure that the callbacks are
> > dropped if the dev_pm_ops is dead code, and the pm_ptr() must be used
> > for the compiler to know that the dev_pm_ops is dead code.
> >
> > -Paul
> >
> >
>
> Hi Paul,
>
> We have one remaining case which is still ugly to do.
> Where both SYSTEM_SLEEP_PM_OPS/RUNTIME_PM_OPS are set and
> the dev_pm_ops structure is exported.
>
> For that one we still need to expose #ifdef fun in the
> drivers I think.
>
> Any suggestions on a clean solution for that?
>
> Currently I have this...
>
> #ifdef CONFIG_PM
> const struct dev_pm_ops bmc150_magn_pm_ops = {
> SYSTEM_SLEEP_PM_OPS(...)
> RUNTIME_PM_OPS(...)
> };
> EXPORT_SYMBOL_NS(bmc150_magn_pm_ops, IIO_BMC150_MAGN);
> #else
> static const __maybe_unused dev_pm_ops bmc150_magn_pm_ops = {
With this renamed to dummy_bmc150_magn_pm_ops to avoid
the previous declared warning I'll otherwise get. oops.
> SYSTEM_SLEEP_PM_OPS(...)
> RUNTIME_PM_OPS(...)
> };
> #endif
> Not super clean but perhaps we do need
> EXPORT_NS_DEV_PM_OPS
> EXPORT_NS_GPL_DEV_PM_OPS
> and potentially the non namespaced versions.
>
> Thanks,
>
> Jonathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set
2022-03-06 15:29 ` Paul Cercueil
@ 2022-03-06 17:57 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2022-03-06 17:57 UTC (permalink / raw)
To: Paul Cercueil
Cc: Arnd Bergmann, Randy Dunlap, Linux Kernel Mailing List, Wei WANG,
Kai-Heng Feng, Greg Kroah-Hartman, Rafael J. Wysocki
On Sun, 6 Mar 2022 15:29:17 +0000
Paul Cercueil <paul@crapouillou.net> wrote:
> Hi Jonathan,
>
> Le dim., mars 6 2022 at 15:12:12 +0000, Jonathan Cameron
> <Jonathan.Cameron@Huawei.com> a écrit :
> > On Sun, 27 Feb 2022 17:56:31 +0000
> > Paul Cercueil <paul@crapouillou.net> wrote:
> >
> >> Le dim., févr. 27 2022 at 18:51:38 +0100, Arnd Bergmann
> >> <arnd@arndb.de> a écrit :
> >> > On Sun, Feb 27, 2022 at 6:46 PM Paul Cercueil
> >> <paul@crapouillou.net>
> >> > wrote:
> >> >> Le dim., févr. 27 2022 at 18:30:16 +0100, Arnd Bergmann
> >> >>
> >> >> There could be a DEFINE_DEV_PM_OPS(), but I don't think that's
> >> >> really
> >> >> needed - you can very well declare your struct dev_pm_ops
> >> without
> >> >> using
> >> >> one of these macros. Just make sure to use the
> >> SYSTEM_SLEEP_PM_OPS /
> >> >> RUNTIME_PM_OPS macros for the callbacks and pm_ptr() for the
> >> >> device.pm
> >> >> pointer.
> >> >
> >> > Ah, of course, so it comes down to
> >> > s/SET_SYSTEM_SLEEP_PM_OPS/SYSTEM_SLEEP_PM_OPS/ while
> >> > removing all the #ifdef an __maybe_unused annotations. The
> >> pm_ptr()
> >> > in driver.pm makes this slightly more optimized AFAICT, but has no
> >> > effect on behavior, right?
> >>
> >> The use of SYSTEM_SLEEP_PM_OPS makes sure that the callbacks are
> >> dropped if the dev_pm_ops is dead code, and the pm_ptr() must be
> >> used
> >> for the compiler to know that the dev_pm_ops is dead code.
> >>
> >> -Paul
> >>
> >>
> >
> > Hi Paul,
> >
> > We have one remaining case which is still ugly to do.
> > Where both SYSTEM_SLEEP_PM_OPS/RUNTIME_PM_OPS are set and
> > the dev_pm_ops structure is exported.
> >
> > For that one we still need to expose #ifdef fun in the
> > drivers I think.
> >
> > Any suggestions on a clean solution for that?
>
> Use the _EXPORT_DEV_PM_OPS() macro?
>
> If you make it call __EXPORT_SYMBOL() (with two underscores instead of
> one) you can specify the namespace as well. All you need then is a nice
> wrapper macro in pm_runtime.h, that can be used in the driver.
Yup. The patch for adding namespace versions in the first place
did that.
https://lore.kernel.org/linux-iio/20220220181522.541718-1-jic23@kernel.org/T/#m5a3dc798606f88c10870a66e18b5b175e1a30243
I was just hoping we could maybe avoid adding a few more macros, particularly
ones that take lots of arguments like this one will.
We'll have something like 4 more macros
#define EXPORT_NS_GPL_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
runtime_resume_fn, idle_fn, sec, ns) \
_EXPORT_DEV_PM_OPS(name, suspend_fn, resume_fn, runtime_suspend_fn, \
runtime_resume_fn, idle_fn, "_gpl", ns)
#define EXPORT_NS_DEV_PM_OPS() ...
#define EXPORT_DEV_PM_OPS() ...
#define EXPORT_GPL_DEV_PM_OPS() ...
if we provide the same for non NS cases.
I'll roll a patch for next cycle hopefully building on the one referenced
above if Rafael picks that up for this merge window.
Jonathan
>
> Cheers,
> -Paul
>
> > Currently I have this...
> >
> > #ifdef CONFIG_PM
> > const struct dev_pm_ops bmc150_magn_pm_ops = {
> > SYSTEM_SLEEP_PM_OPS(...)
> > RUNTIME_PM_OPS(...)
> > };
> > EXPORT_SYMBOL_NS(bmc150_magn_pm_ops, IIO_BMC150_MAGN);
> > #else
> > static const __maybe_unused dev_pm_ops bmc150_magn_pm_ops = {
> > SYSTEM_SLEEP_PM_OPS(...)
> > RUNTIME_PM_OPS(...)
> > };
> > #endif
> > Not super clean but perhaps we do need
> > EXPORT_NS_DEV_PM_OPS
> > EXPORT_NS_GPL_DEV_PM_OPS
> > and potentially the non namespaced versions.
> >
> > Thanks,
> >
> > Jonathan
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-06 17:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-26 22:24 [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set Randy Dunlap
2022-02-27 12:04 ` Arnd Bergmann
2022-02-27 16:57 ` Randy Dunlap
2022-02-27 17:30 ` Arnd Bergmann
2022-02-27 17:46 ` Paul Cercueil
2022-02-27 17:51 ` Arnd Bergmann
2022-02-27 17:56 ` Paul Cercueil
2022-03-06 15:12 ` Jonathan Cameron
2022-03-06 15:29 ` Paul Cercueil
2022-03-06 17:57 ` Jonathan Cameron
2022-03-06 16:24 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox