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

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

Hello Lars,

On 3/20/2012 11:27 PM, Lars-Peter Clausen wrote:
> On 03/20/2012 12:33 PM, Rajeev Kumar wrote:
>> +static const char *interface_mode_text[] = { "slave", "master"};
>
> Master/slave mode should be configured using the set_dai_fmt callback.
>
You mean to say with the help of clock direction. I think you can do it 
in either way.
By this,  I am giving full control to user for the selection.

>> +
>> +static struct snd_soc_dai_ops sta529_dai_ops = {
>
> const
>

Ok,

>> +	.hw_params	=	sta529_hw_params,
>> +	.set_fmt	=	sta529_set_dai_fmt,
>> +	.digital_mute	=	sta529_mute,
>> +};
>> +
>> +/* power down chip */
>> +static int sta529_remove(struct snd_soc_codec *codec)
>> +{
>> +	sta529_set_bias_level(codec, SND_SOC_BIAS_OFF);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sta529_suspend(struct snd_soc_codec *codec, pm_message_t state)
>
> The suspend callback doesn't take the state parameter anymore.
>

Oops,

>> +{
>> +	sta529_set_bias_level(codec, SND_SOC_BIAS_OFF);
>> +
>> +	return 0;
>> +}
>> +
>> +static int sta529_resume(struct snd_soc_codec *codec)
>> +{
>> +	snd_soc_cache_sync(codec);
>> +	sta529_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
>> +	sta529_set_bias_level(codec, codec->dapm.suspend_bias_level);
>> +
>> +	return 0;
>> +}
>> +
>> +struct snd_soc_codec_driver soc_codec_dev_sta529 = {
>
> A better name is be sta520_codec_driver

Ok,

> [...]
>> +
>> +static int sta529_i2c_remove(struct i2c_client *client)
> __devexit
>> +{
>> +	snd_soc_unregister_codec(&client->dev);
>> +	kfree(i2c_get_clientdata(client));
>> +	return 0;
>> +}
> river");

This comment is not clear to me , please explain.

Best Regards
Rajeev

>> +MODULE_AUTHOR("Rajeev Kumar<rajeev-dlh.kumar@st.com>");
>> +MODULE_LICENSE("GPL");
>
> .
>