[PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Randy Dunlap]

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)
 {

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Randy Dunlap]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Arnd Bergmann]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Paul Cercueil]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Arnd Bergmann]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Paul Cercueil]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Paul Cercueil]

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

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Jonathan Cameron]

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  
> 
> 

Re: [PATCH -next] misc: rtsx: fix build for CONFIG_PM not set

[Jonathan Cameron]

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