Re: [PATCH] power: supply: olpc_battery: remove unnecessary CONFIG_PM_SLEEP

[Coiby Xu]

Hi Hans,

Thank you for reviewing this patch!

On Thu, Oct 29, 2020 at 11:04:36AM +0100, Hans de Goede wrote:
> Hi,
>
> On 10/29/20 8:41 AM, Coiby Xu wrote:
> > SIMPLE_DEV_PM_OPS has already took good care of CONFIG_PM_CONFIG.
>
> No it does not, when CONFIG_PM_SLEEP is not set then the
> SET_SYSTEM_SLEEP_PM_OPS macro which SIMPLE_DEV_PM_OPS uses
> is a no-op, so nothing will reference xo15_sci_resume leading to
> a compiler warning when CONFIG_PM_SLEEP is not set.
>
> You could drop the ifdef and add __maybe_unused to the definition
> of xo15_sci_resume, but that feels like needless churn, best to
> just keep this as is IMHO.

Actually, this is a tree-wide change by some semi-automation scripts.
Thank you for pointing out the issue to prevent me from releasing
another ~150 emails to flood other mailing lists.

Currently there are 929 drivers has device PM callbacks,

$ grep -rI "\.pm = &" --include=*.c  ./|wc -l
929

I put all files having device PM callbacks into four categories
based on weather a file has CONFIG_PM_SLEEP or PM macro like
SET_SYSTEM_SLEEP_PM_OPS, here are the statistics,
  1. have both CONFIG_PM_SLEEP and PM_OPS macro: 213
  2. have CONFIG_PM_SLEEP but no PM_OPS macro: 19
  3. have PM macro but not CONFIG_PM_SLEEP: 347
  4. no PM macro or CONFIG_PM_SLEEP: 302

Some drivers which have PM macro but not CONFIG_PM_SLEEP like
sound/x86/intel_hdmi_audio.c indeed use __maybe_unused to eliminate
the compiling warning. In 2011, there's a patch proposing to remove
ONFIG_PM altogether but an objection was turning CONFIG_PM on would
increase the kernel size [1]. So __maybe_unused also have this issue.
(I made a mistake when I thought PM macros like SIMPLE_DEV_PM_OPS
didn't have this issue). What do you think? Btw, It's easy for me to
add CONFIG_PM_SLEEP for those drivers have PM macro but not
CONFIG_PM_SLEEP since I have already written the necessary automation
scripts.

[1] https://lists.linux-foundation.org/pipermail/linux-pm/2011-February/030215.html

> Also s/CONFIG_PM_CONFIG/CONFIG_PM_SLEEP/ in the commit message.

Thank you for pointing out the typo. I've written some scripts to
automate the whole process from changing code to submitting patches.
Somehow there is still this issue.

> Regards,
>
> Hans
>
>
> > Signed-off-by: Coiby Xu <coiby.xu@xxxxxxxxx>
> > ---
> >  arch/x86/platform/olpc/olpc-xo15-sci.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/x86/platform/olpc/olpc-xo15-sci.c b/arch/x86/platform/olpc/olpc-xo15-sci.c
> > index 85f4638764d6..716eefd735a4 100644
> > --- a/arch/x86/platform/olpc/olpc-xo15-sci.c
> > +++ b/arch/x86/platform/olpc/olpc-xo15-sci.c
> > @@ -192,7 +192,6 @@ static int xo15_sci_remove(struct acpi_device *device)
> >  	return 0;
> >  }
> >
> > -#ifdef CONFIG_PM_SLEEP
> >  static int xo15_sci_resume(struct device *dev)
> >  {
> >  	/* Enable all EC events */
> > @@ -204,7 +203,6 @@ static int xo15_sci_resume(struct device *dev)
> >
> >  	return 0;
> >  }
> > -#endif
> >
> >  static SIMPLE_DEV_PM_OPS(xo15_sci_pm, NULL, xo15_sci_resume);

--
Best regards,
Coiby