Re: [PATCH] ASoC: nau8825: Modify power management and add interrupt wakeup

[John Hsu <KCHSU0@nuvoton.com>]

Hi Mark,
This patch has cancel and only submit enhance codec suspend and resume 
patch.
Please review the following link for the patch. Many thanks.

http://mailman.alsa-project.org/pipermail/alsa-devel/2016-January/102924.html
[alsa-devel] [PATCH] ASoC: nau8825: fix interrupt fails and unstable 
after resume

On 1/8/2016 9:14 PM, Mark Brown wrote:
> On Fri, Jan 08, 2016 at 03:11:22PM +0800, John Hsu wrote:
>   
>> 1. Enhance codec suspend and resume sequence 2. Add interrupt wakeup 
>> function
>>     
>
> If this is two or more separate changes you should be sending two or more separate patches.  You also need a better changelog which describes what the patch is supposed to do, in what way is suspend and resume enhanced for example?
>
>   
>> +/**
>> + * nau8825_init_wakeup - set wakeup capability for codec
>> + *
>> + * @codec:  codec device component
>> + *
>> + * After this function done, codec can support system wakeup by button.
>> + */
>> +int nau8825_init_wakeup(struct snd_soc_codec *codec) {
>> +	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
>> +
>> +	device_init_wakeup(nau8825->dev, true);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nau8825_init_wakeup);
>>     
>
> You need a bit more explanation as to what's going on here.  Why is this not something the driver just does or if this should be done elsewhere why do we need the wrapper function?
>
>   
>> +int nau8825_irq_wakeup(struct snd_soc_codec *codec, int on) {
>> +	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
>>     
>
> Same thing here, this looks like an unclear interface.
>