| Message ID | 20201029074301.226644-1-coiby.xu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/25] ALSA: core: pcm: remove unnecessary CONFIG_PM_SLEEP | expand |
On Thu, Oct 29, 2020 at 08:48:55AM +0100, Takashi Iwai wrote: >On Thu, 29 Oct 2020 08:42:37 +0100, >Coiby Xu wrote: >> >> SET_SYSTEM_SLEEP_PM_OPS has already took good care of CONFIG_PM_CONFIG. >> >> Signed-off-by: Coiby Xu <coiby.xu@gmail.com> > >It caused compile warnings. Was it already addressed in general? > It hasn't been addressed in general. Thank you for the reminding! >Or we may use __maybe_unused attribute instead, but it's just a matter >of taste. > I'll add __maybe_unused in v2 since __maybe_unused should be preferred over CONFIG_PM_SLEEP according to Arnd Bergmann [1], > > By and large, drivers handle this by using a CONFIG_PM_SLEEP ifdef. > > > > Unless you can make an extremely convincing argument why not to do > > so here, I'd like you to handle it that way instead. > > [adding linux-pm to Cc] > > The main reason is that everyone gets the #ifdef wrong, I run into > half a dozen new build regressions with linux-next every week on > average, the typical problems being: > > - testing CONFIG_PM_SLEEP instead of CONFIG_PM, leading to an unused > function warning > - testing CONFIG_PM instead of CONFIG_PM_SLEEP, leading to a build > failure > - calling a function outside of the #ifdef only from inside an > otherwise correct #ifdef, again leading to an unused function > warning > - causing a warning inside of the #ifdef but only testing if that > is disabled, leading to a problem if the macro is set (this is > rare these days for CONFIG_PM as that is normally enabled) > > Using __maybe_unused avoids all of the above. [1] https://lore.kernel.org/patchwork/comment/919944/ > >thanks, > >Takashi > >> --- >> sound/core/pcm.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/sound/core/pcm.c b/sound/core/pcm.c >> index be5714f1bb58..5a281ac92958 100644 >> --- a/sound/core/pcm.c >> +++ b/sound/core/pcm.c >> @@ -599,7 +599,6 @@ static const struct attribute_group *pcm_dev_attr_groups[]; >> * PM callbacks: we need to deal only with suspend here, as the resume is >> * triggered either from user-space or the driver's resume callback >> */ >> -#ifdef CONFIG_PM_SLEEP >> static int do_pcm_suspend(struct device *dev) >> { >> struct snd_pcm_str *pstr = container_of(dev, struct snd_pcm_str, dev); >> @@ -608,7 +607,6 @@ static int do_pcm_suspend(struct device *dev) >> snd_pcm_suspend_all(pstr->pcm); >> return 0; >> } >> -#endif >> >> static const struct dev_pm_ops pcm_dev_pm_ops = { >> SET_SYSTEM_SLEEP_PM_OPS(do_pcm_suspend, NULL) >> -- >> 2.28.0 >> -- Best regards, Coiby
diff --git a/sound/core/pcm.c b/sound/core/pcm.c index be5714f1bb58..5a281ac92958 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -599,7 +599,6 @@ static const struct attribute_group *pcm_dev_attr_groups[]; * PM callbacks: we need to deal only with suspend here, as the resume is * triggered either from user-space or the driver's resume callback */ -#ifdef CONFIG_PM_SLEEP static int do_pcm_suspend(struct device *dev) { struct snd_pcm_str *pstr = container_of(dev, struct snd_pcm_str, dev); @@ -608,7 +607,6 @@ static int do_pcm_suspend(struct device *dev) snd_pcm_suspend_all(pstr->pcm); return 0; } -#endif static const struct dev_pm_ops pcm_dev_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(do_pcm_suspend, NULL)
SET_SYSTEM_SLEEP_PM_OPS has already took good care of CONFIG_PM_CONFIG. Signed-off-by: Coiby Xu <coiby.xu@gmail.com> --- sound/core/pcm.c | 2 -- 1 file changed, 2 deletions(-)