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

[Rajeev kumar <rajeev-dlh.kumar@st.com>]

Hello Clausen,

On 3/23/2012 2:37 PM, Lars-Peter Clausen wrote:
>
>>>> +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.
>

Actually I was hoping for the same but not able to locate the code.
Ok, I will made the changes.

> 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.
>

Ok

Best Regards
Rajeev

>
>
> .
>