[alsa-devel] [PATCH V3 1/5] sound: asoc: Adding support for STA529 Audio Codec
rajeev
rajeev-dlh.kumar at st.com
Wed Apr 20 06:24:47 CEST 2011
Hi Mark
please find my answer below.
On 4/11/2011 8:26 PM, Mark Brown wrote:
> On Mon, Apr 11, 2011 at 11:00:00AM +0530, Rajeev Kumar wrote:
>
>> +static const char *op_mode_text[] = { "slave", "master"};
>
> What is op_mode? This sounds like it should be configured by
> set_dai_fmt()...
>
OK, I will change the name.
>> +static const struct snd_kcontrol_new sta529_new_snd_controls[] = {
>> + SOC_ENUM("pwm select", pwm_src_enum),
>> + SOC_ENUM("mode select", mode_src_enum),
>> +};
>
> ALSA control names are idiomatically things like "PWM Select" with
> capitalisation.
>
Using PWM Select
>> + /*store the label for powers down audio subsystem for suspend.This is
>> + ** used by soc core layer*/
>> + codec->bias_level = level;
>
> The formatting of this comment isn't terribly idiomatic.
>
OK
>> +static int sta529_probe(struct snd_soc_codec *codec)
>> +{
>> + struct sta529 *sta529 = snd_soc_codec_get_drvdata(codec);
>> + int ret;
>> +
>> + codec->hw_write = (hw_write_t)i2c_master_send;
>> + codec->hw_read = NULL;
>> + ret = snd_soc_codec_set_cache_io(codec, 8, 8, sta529->control_type);
>> + if (ret < 0) {
>> + dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
>> + return ret;
>> + }
>
> You shouldn't need to be assigning the I/O functions if you set the
> control type. If the device only supports I2C that can just be hard
> coded.
>
explained below.
>> +static int sta529_resume(struct snd_soc_codec *codec)
>> +{
>> + int i;
>> + u8 data[2];
>> + u8 *cache = codec->reg_cache;
>> +
>> + for (i = 0; i < ARRAY_SIZE(sta529_reg); i++) {
>> + data[0] = i;
>> + data[1] = cache[i];
>> + codec->hw_write(codec->control_data, data, 2);
>> + }
>
> It looks like you can use the standard cache sync implementation here?
> snd_soc_cache_sync().
> .
>
This patch set has been compiled and tested for Linux-kernel-version 2.6.38-rc4,
since SPEAr patches for this linux version are already under review in Russel King's
ARM branch.This version does not have snd_soc_cache_sync() function.
We plan to start porting the SPEAr patches on latest kernel after the earlier SPEAr patches
are ACKED. After the same, I will circulate the next SPEAr ASoC patch set containing changes
specific to this new kernel version.
Best Rgds
Rajeev
More information about the Alsa-devel
mailing list