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@xxxxxxxxx>
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
