diff mbox series

[01/25] ALSA: core: pcm: remove unnecessary CONFIG_PM_SLEEP

Message ID 20201029074301.226644-1-coiby.xu@gmail.com
State New, archived
Headers show
Series [01/25] ALSA: core: pcm: remove unnecessary CONFIG_PM_SLEEP | expand

Commit Message

Coiby Xu Oct. 29, 2020, 7:42 a.m. UTC
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(-)

Comments

Coiby Xu Oct. 29, 2020, 2:37 p.m. UTC | #1
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 mbox series

Patch

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)