Re: [PATCH 1/8] sound:asoc: Add support for STA529 Audio Codec

[Lars-Peter Clausen <lars@metafoo.de>]

>>> +static const char *pwm_mode_text[] = { "binary", "headphone", "ternary",
>>> +	"phase-shift"};
>>> +static const char *interface_mode_text[] = { "slave", "master"};
>>
>> ALSA controls always use capitalisation.
>>
> 
> you mean to say
> static const char *interface_mode_text[] = { "SLAVE", "MASTER"}
> 

No, only the first letter upper case.

> 
>>> +	sta529_set_bias_level(codec, SND_SOC_BIAS_PREPARE);
>>> +	mdelay(10);
>>
>> Absolutely no - why are you doing this?
>>
> 
> In probe I am putting codec in standby mode. so to come out of this 
> sta529_set_bias_level(codec, SND_SOC_BIAS_PREPARE) is called. Since 
> there are some transition time between STANDBY and ON/PREAPRE, so a 
> delay is introduced in the code.

The framework will take care of taking the CODEC out of standby mode when it
is required, so you shouldn't need it here. Also, if the delay is required
put it into the set_bias_level function.

> 
>>> +static int sta529_mute(struct snd_soc_dai *dai, int mute)
>>> +{
>>> +	struct snd_soc_codec *codec = dai->codec;
>>> +
>>> +	u8 mute_reg = snd_soc_read(codec, STA529_FFXCFG0)&  ~CODEC_MUTE_VAL;
>>> +
>>> +	if (mute)
>>> +		mute_reg |= CODEC_MUTE_VAL;
>>> +
>>> +	snd_soc_update_bits(codec, STA529_FFXCFG0, 0x80, 00);
>>
>> You're always setting the same value here...
>>
> 
> Oops, it should be
> snd_soc_update_bits(codec, STA529_FFXCFG0, 0x80, mute_reg);

If you use snd_soc_update_bits, you don't have to read the register first.