Re: [PATCH] ALSA: ac97: Switch to dev_pm_ops

[Lars-Peter Clausen <lars@metafoo.de>]

On 08/07/2015 12:55 AM, Dmitry Torokhov wrote:
[...]
>>  
>> -#ifdef CONFIG_PM
>> -static int wm97xx_suspend(struct device *dev, pm_message_t state)
>> +#ifdef CONFIG_PM_SLEEP
>> +static int wm97xx_suspend(struct device *dev)
> 
> While we are changing it please drop #ifdef CONFIG_PM_SLEEP and annotate
> suspend and resume with __maybe_unused.

We know that it is used when CONFIG_PM_SLEEP is defined and we know that it
is unused CONFIG_PM_SLEEP is not defined. Marking the function as
__maybe_unused will cause the compiler to not generate a warning when the
function is really unused. Making this explicit works much better.

> 
>>  {
>>  	struct wm97xx *wm = dev_get_drvdata(dev);
>>  	u16 reg;
>> @@ -799,9 +799,11 @@ static int wm97xx_resume(struct device *dev)
>>  	return 0;
>>  }
>>  
>> +static SIMPLE_DEV_PM_OPS(wm97xx_pm_ops, wm97xx_suspend, wm97xx_resume);
> 
> Pull this out of #ifdef block and kill entire #else/endif along with
> WM97XX_PM_OPS define: SIMPLE_DEV_PM_OPS will result in an empty
> structure if CONFIG_PM_SLEEP is not set.

It will create a struct dev_pm_ops full of NULLs. That's kind of
counterproductive to removing PM related data and functions from the kernel
if PM support is no enabled.

> 
>> +#define WM97XX_PM_OPS (&wm97xx_pm_ops)
>> +
>>  #else
>> -#define wm97xx_suspend		NULL
>> -#define wm97xx_resume		NULL
>> +#define WM97XX_PM_OPS NULL
>>  #endif
[...]