Re: [PATCH v5 02/14] gpio: brcmstb: Use modern PM macros

Jonas Gorski
2025-11-24T14:20:13Z

On Mon, Nov 24, 2025 at 2:52 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Nov 24, 2025 at 2:40 PM Jonas Gorski <jonas.gorski@gmail.com> wrote:
> > On Mon, Nov 24, 2025 at 1:39 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> ...
>
> > > static const struct dev_pm_ops brcmstb_gpio_pm_ops = {
> > > - .suspend_noirq = brcmstb_gpio_suspend,
> > > - .resume_noirq = brcmstb_gpio_resume,
> > > + .suspend_noirq = pm_sleep_ptr(brcmstb_gpio_suspend),
> > > + .resume_noirq = pm_sleep_ptr(brcmstb_gpio_resume),
> > > };
>
> ...
>
> > > - .pm = &brcmstb_gpio_pm_ops,
> > > + .pm = pm_sleep_ptr(&brcmstb_gpio_pm_ops),
> >
> > won't this cause a "brcmstb_gpio_pm_ops is unused" compile warning for
> > !CONFIG_PM_SLEEP?
> >
> > You probably need to add a __maybe_unused to brcmstb_gpio_pm_ops
> > (which incidentally DEFINE_NOIRQ_DEV_PM_OPS() also doesn't set, but
> > all other *_DEV_PM_OPS() macros do).
>
> Shouldn't it be covered by the same trick as pm_sleep_ptr() does for functions?

pm_sleep_ptr() becomes NULL for !CONFIG_PM_SLEEP, so there is no
reference then anymore to brcmstb_gpio_pm_ops. You would need a
wrapper for brcmstb_gpio_pm_ops itself to conditionally define it to
avoid the warning, or add __maybe_unused to it to silence it.

Note how SIMPLE_DEV_PM_OPS() and UNIVERSAL_DEV_PM_OPS() tag the struct
with it (for that reason I assume).

Best regards,
Jonas