[alsa-devel] [PATCH 1/4] ASoC:rt5670:Add runtime PM support

[Mark Brown]

On Thu, Nov 06, 2014 at 12:23:51PM +0800, bardliao at realtek.com wrote:

> +#ifdef CONFIG_PM_RUNTIME
> +static void rt5670_use_internal_clk(struct rt5670_priv *rt5670)
> +{
> +	struct snd_soc_codec *codec = rt5670->codec;
> +
> +	if (!codec ||
> +	   (0 == rt5670->sysclk && RT5670_SCLK_S_RCCLK == rt5670->sysclk_src))

Normal kernel coding style is to put the variable before the constant.

> +		return;
> +
> +	snd_soc_update_bits(codec, RT5670_GLB_CLK,
> +			RT5670_SCLK_SRC_MASK, RT5670_SCLK_SRC_RCCLK);

Instead of using snd_soc_ here use regmap and then you don't need to
worry if the card is registered.

> +static int rt5670_runtime_resume(struct device *dev)
> +{
> +	return 0;
> +}

This should be undoing the change done on suspend otherwise we'll be
stuck on the internal clock after resuming.  Alternatively if something
will undo that elsewhere then presumably we don't need to use runtime PM
to select the internal clock anyway?

If this were an empty function it should be removed.

> +#define RT5670_PM_OPS	rt5670_pm
> +#else
> +#define RT5670_PM_OPS	NULL
> +#endif /* CONFIG_PM_RUNTIME */
> +
>  static struct i2c_driver rt5670_i2c_driver = {
>  	.driver = {
>  		.name = "rt5670",
>  		.owner = THIS_MODULE,
> +		.pm = &RT5670_PM_OPS,
>  	},

Usual idiom is to include the & in the #define for the PM active case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20141106/a189f284/attachment.sig>