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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux