Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210408205858.51751-1-masahiroy@kernel.org> <20210408205858.51751-3-masahiroy@kernel.org>
 <CAK8P3a3gzfZhNdp-_=HCxoMw385nGQeNOhhH8X+uyN=AMvHJ8Q@mail.gmail.com>
In-Reply-To: <CAK8P3a3gzfZhNdp-_=HCxoMw385nGQeNOhhH8X+uyN=AMvHJ8Q@mail.gmail.com>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Fri, 9 Apr 2021 12:17:04 +0900
Message-ID: <CAK7LNAR9+u5Q3Sf2jgBSUcpit6xbD_5_Hc-xr-9Ge_m_da-1CA@mail.gmail.com>
Subject: Re: [PATCH 2/2] pm: allow drivers to drop #ifdef and __maybe_unused
 from pm callbacks
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
        Paul Cercueil <paul@crapouillou.net>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux PM list <linux-pm@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Apr 9, 2021 at 6:30 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Apr 8, 2021 at 11:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Drivers typically surround suspend and resume callbacks with #ifdef
> > CONFIG_PM(_SLEEP) or mark them as __maybe_unused in order to avoid
> > -Wunused-const-variable warnings.
> >
> > With this commit, drivers will be able to remove #ifdef CONFIG_PM(_SLEEP)
> > and __maybe_unsed because unused functions are dropped by the compiler
> > instead of the preprocessor.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> I tried this before and could not get it to work right.
>
> >
> > -#ifdef CONFIG_PM_SLEEP
> > +#define pm_ptr(_ptr)           PTR_IF(IS_ENABLED(CONFIG_PM), _ptr)
> > +#define pm_sleep_ptr(_ptr)     PTR_IF(IS_ENABLED(CONFIG_PM_SLEEP), _ptr)
> > +
> >  #define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
> > -       .suspend = suspend_fn, \
> > -       .resume = resume_fn, \
> > -       .freeze = suspend_fn, \
> > -       .thaw = resume_fn, \
> > -       .poweroff = suspend_fn, \
> > -       .restore = resume_fn,
> > -#else
> > -#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn)
> > -#endif
> > +       .suspend  = pm_sleep_ptr(suspend_fn), \
> > +       .resume   = pm_sleep_ptr(resume_fn), \
> > +       .freeze   = pm_sleep_ptr(suspend_fn), \
> > +       .thaw     = pm_sleep_ptr(resume_fn), \
> > +       .poweroff = pm_sleep_ptr(suspend_fn), \
> > +       .restore  = pm_sleep_ptr(resume_fn),
>
> The problem that I think you inevitably hit is that you run into a missing
> declaration for any driver that still uses an #ifdef around a static
> function.
>
> The only way I can see us doing this is to create a new set of
> macros that behave like the version you propose here but leave
> the old macros in place until the last such #ifdef has been removed.
>
>        Arnd

Agh, you are right.
We cannot change all the callsites atomically due to the huge amount of users.

-- 
Best Regards
Masahiro Yamada