Re: [PATCH 2/2] ASoC: cs4270: add power management support

[Timur Tabi <timur@freescale.com>]

Daniel Mack wrote:

> From d2303f37695145018e07aad85200f06b24341648 Mon Sep 17 00:00:00 2001
> From: Daniel Mack <daniel@caiaq.de>
> Date: Thu, 2 Apr 2009 15:43:44 +0200
> Subject: [PATCH 2/2] ASoC: cs4270: add power management support

You forgot to add "v2" to the subject line.  And you'll need a v3 on the
next version of the patch, because ...

> +static int cs4270_i2c_resume(struct i2c_client *client)
> +{
> +	struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> +	struct snd_soc_codec *codec = &cs4270->codec;
> +	int reg;
> +
> +	/* In case the device was put to hard reset during sleep, we need to
> +	 * wait 500ns here before any I2C communication. */
> +	ndelay(500);
> +
> +	/* first restore the entire register cache ... */
> +	for (reg = CS4270_FIRSTREG; reg <= CS4270_LASTREG; reg++) {
> +		u8 val = snd_soc_read(codec, reg);
> +
> +		if (i2c_smbus_write_byte_data(client, reg, val)) {
> +			dev_err(codec->dev, "i2c write failed\n");
> +			return -EIO;
> +		}
> +	}
> +
> +	/* ... then disable the power-down bits */
> +	reg = snd_soc_read(codec, CS4270_PWRCTL);
> +	if (reg < 0)
> +		return reg;

... you forgot this "reg < 0" check. Also, ...

> +static int cs4270_i2c_suspend(struct i2c_client *client, pm_message_t mesg)
> +{
> +	struct cs4270_private *cs4270 = i2c_get_clientdata(client);
> +	struct snd_soc_codec *codec = &cs4270->codec;
> +	int reg = snd_soc_read(codec, CS4270_PWRCTL) | CS4270_PWRCTL_PDN_ALL;
> +	return snd_soc_write(codec, CS4270_PWRCTL, reg);
> +}

... you need a blank line above the "return"


> +
> +	reg &= ~CS4270_PWRCTL_PDN_ALL;
> +	return snd_soc_write(codec, CS4270_PWRCTL, reg);
> +}

And here.

-- 
Timur Tabi
Linux kernel developer at Freescale