On Fri, Jan 03, 2014 at 02:05:03PM +0800, RongJun Ying wrote:
@@ -6,5 +6,8 @@ config SND_SIRF_SOC config SND_SOC_SIRF_I2S tristate
+config SND_SIRF_SOC_INNER
- tristate
config SND_SOC_SIRF_USP
Everything else is SND_SOC_SIRF_, please keep this consistent.
+static struct sirf_soc_inner_audio_reg_bits sirf_soc_inner_audio_reg_bits_prima2 = {
- 20, 21, 22, 23, 24, 25, 26, 27,
+};
+static struct sirf_soc_inner_audio_reg_bits sirf_soc_inner_audio_reg_bits_atlas6 = {
- 22, 23, 24, 25, 26, 27, 28, 29,
+};
Please used named initialisers to set these values rather than just a list of numbers, this isn't very easy to read.
+static const struct snd_kcontrol_new sirf_inner_input_mode_control =
- SOC_DAPM_ENUM("Route", sirf_inner_enum[0]);
Either use constants to reference the array or (better) use named variables for each enum. This is more readable and less error prone.
+static struct snd_kcontrol_new volume_controls_atlas6[] = {
- SOC_DOUBLE("Playback Volume", AUDIO_IC_CODEC_CTRL0, 21, 14,
0x7F, 0),- SOC_DOUBLE("Capture Volume", AUDIO_IC_CODEC_CTRL1, 16, 10,
0x3F, 0),
Please provide TLV information.
+static struct snd_kcontrol_new left_input_path_controls[] = {
- SOC_DAPM_SINGLE("Line left Switch", AUDIO_IC_CODEC_CTRL1, 6, 1, 0),
Line Left Switch.
- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
/* Enable capture power of codec*/val = sirfsoc_rtc_iobrg_readl(sinner_audio->sys_pwrc_reg_base +PWRC_PDN_CTRL_OFFSET);val |= (1 << AUDIO_POWER_EN_BIT);sirfsoc_rtc_iobrg_writel(val,sinner_audio->sys_pwrc_reg_base + PWRC_PDN_CTRL_OFFSET);break;- case SND_SOC_DAPM_POST_PMU:
/*After enable adc, Delay 200ms to avoid pop noise*/msleep(200);
No need to split these operations, just have one callback. Why is there no power down callback?
+static int hp_amp_left_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)+{
- struct snd_soc_codec *codec = w->codec;
- struct sirf_soc_inner_audio *sinner_audio = dev_get_drvdata(codec->dev);
- u32 val;
- val = snd_soc_read(codec, AUDIO_IC_CODEC_CTRL1);
- switch (event) {
- case SND_SOC_DAPM_PRE_PMU:
val |= (1 << sinner_audio->reg_bits->firdac_hsl_en_bits);break;- case SND_SOC_DAPM_POST_PMD:
val &= ~(1 << sinner_audio->reg_bits->firdac_hsl_en_bits);- default:
break;- }
- snd_soc_write(codec, AUDIO_IC_CODEC_CTRL1, val);
- return 0;
snd_soc_update_bits(). Can this not be represented as multiple widgets - remember that DAPM will coalesce writes as much as it can.
- case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
spin_lock(&sinner_audio->lock);if (playback) {
The lock only gets used in the trigger function, is it needed?
+static struct snd_soc_codec_driver soc_codec_device_sirf_inner_codec = {
- .probe = sirf_inner_codec_probe,
- .remove = sirf_inner_codec_remove,
- .read = sirf_inner_codec_reg_read,
- .write = sirf_inner_codec_reg_write,
Don't use custom read and write callbacks, use a MMIO regmap.