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