[alsa-devel] [PATCH] ASoC: add RT5640 CODEC driver

[Mark Brown]

On Tue, Jun 11, 2013 at 01:10:16PM +0800, bardliao at realtek.com wrote:

> Signed-off-by: Bard Liao <bardliao at realtek.com>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>

I don't think that signoff for Stephen is right...

> +static int rt5640_set_bias_level(struct snd_soc_codec *codec,
> +			enum snd_soc_bias_level level)
> +{
> +	struct rt5640_priv *rt5640 = snd_soc_codec_get_drvdata(codec);
> +	switch (level) {
> +	case SND_SOC_BIAS_STANDBY:
> +		if (SND_SOC_BIAS_OFF == codec->dapm.bias_level) {
> +			regcache_cache_only(rt5640->regmap, false);

You mark the device cache only in suspend so for symmetry this should
really be in _BIAS_OFF, especially since the device is idle_bias_off so
it'll be able to power off without a full suspend.

> +	if (rt5640->pdata.ldo1_en) {
> +		ret = devm_gpio_request_one(&i2c->dev, rt5640->pdata.ldo1_en,
> +					    GPIOF_OUT_INIT_HIGH,
> +					    "RT5640 LDO1_EN");
> +		if (ret < 0) {
> +			dev_err(&i2c->dev, "Failed to request LDO1_EN %d: %d\n",
> +				rt5640->pdata.ldo1_en, ret);
> +			return ret;
> +		}
> +		msleep(400);
> +	}

I'd expect to see some handling fo the LDO over suspend but that can be
dealt with in a followup as previously discussed.  Ideally we'd be able
to power off when in _BIAS_OFF too but if it's going to take 400ms to
ramp the LDO (which does seem high) that might be an issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20130611/9464dc20/attachment-0001.sig>