Le mer., nov. 24 2021 at 10:11:13 +0000, Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> a écrit :
On Wed, 24 Nov 2021 08:29:40 +0100
Arnd Bergmann <arnd@xxxxxxxx> wrote:
On Tue, Nov 23, 2021 at 11:11 PM Paul Cercueil
<paul@xxxxxxxxxxxxxxx> wrote:
>
> One word about the pm_ptr() macro. Right now it's defined as:
> #ifdef CONFIG_PM
> #define pm_ptr(_ptr) (_ptr)
> #else
> #define pm_ptr(_ptr) NULL
> #endif
>
> It could be possible to define it like this instead:
> #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
>
> The difference is that if !CONFIG_PM, in the first case the
(_ptr) is
> not visible by the compiler and the __maybe_unused is required,
while
> in the second case the (_ptr) is always visible by the compiler,
but
> discarded as dead code. The reason we'd want that is the same
reason we
> use IS_ENABLED() instead of macro guards; and you wouldn't need
the
> __maybe_unused attribute anywhere.
That sounds like a great idea. I see there are only 12 users of
pm_ptr at
the moment, so auditing all of these should not be a problem.
I gave it a brief look and found that we probably only need to fix
drivers/net/ethernet/realtek/r8169_main.c if we change the
definition.
Cool.
> The problem then is that the SET_*_PM_OPS macros are defined
> differently according to CONFIG_PM, so their definition would
need to
> be changed to use the (redefined) pm_ptr() macro and a
corresponding
> pm_sleep_ptr() macro. Unfortunately since the SET_*_PM_OPS macros
are
> used everywhere with code wrapped around #ifdef CONFIG_PM guards,
it
> wouldn't be easy to change them, and it would just be easier to
> introduce new macros.
Right, this is what we've discussed multiple times, and I think
everyone
agreed we should do this, but so far we could not come up with a
name
for the new macro, and changing the macro in place is not practical
unless
we change hundreds of drivers in the same way as the iio series
first.
Nasty indeed and I'm not sure how scriptable either as lots of subtle
variants
unfortunately.
I'm cynical - don't need a good name. *_OPS2 works fine for me as
long as
the docs are good.
We could just remove the SET_ prefix,
SET_SYSTEM_SLEEP_PM_OPS -> SYSTEM_SLEEP_PM_OPS,
Or would that be too confusing?
-Paul
