On Fri, May 31, 2013 at 03:04:59PM +0800, bardliao@realtek.com wrote:
From: Bard Liao bardliao@realtek.com
This looks pretty good, a few small things below and probably one actual bug.
- Use regmap_range_cfg to replace index read/write function.
- Remove I2S3 related code since there is no I2S3 in ALC5640.
- Remove Voice DSP related code since there is no Voice DSP in ALC5640.
- Remove MICBIAS2 since there is no MICBIAS2 in ALC5640.
- Change DMIC1/2 CLK to DMIC1/2 Power since it is for enable/disable DMIC1/2
- Modify some texts for consistent coding style.
- Merge sto adc l/r mux since it shares the same control bits.
- Other minor changes.
Put stuff like this after the --- so it doesn't end up in the commit log.
- switch (event) {
- case SND_SOC_DAPM_POST_PMU:
snd_soc_update_bits(codec, RT5640_PWR_DIG1, 0x0001, 0x0001);regmap_update_bits(rt5640->regmap, 0x1c, 0xf000, 0xf000);break;
It's a bit weird to be using snd_soc_update_bits() and regmap_update_bits()...
- SND_SOC_DAPM_SUPPLY("DMIC1 Power", SND_SOC_NOPM, 0, 0,
rt5640_set_dmic1_event,SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),- SND_SOC_DAPM_SUPPLY("DMIC2 Power", SND_SOC_NOPM, 0, 0,
rt5640_set_dmic2_event,SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
The enable bits could be listed here - this would be a bit more idiomatic, would save the PMD event and would enable the writes to be combined.
- case SND_SOC_BIAS_STANDBY:
if (SND_SOC_BIAS_OFF == codec->dapm.bias_level) {snd_soc_update_bits(codec, RT5640_PWR_ANLG1,RT5640_PWR_VREF1 | RT5640_PWR_MB |RT5640_PWR_BG | RT5640_PWR_VREF2,RT5640_PWR_VREF1 | RT5640_PWR_MB |RT5640_PWR_BG | RT5640_PWR_VREF2);mdelay(10);snd_soc_update_bits(codec, RT5640_PWR_ANLG1,RT5640_PWR_FV1 | RT5640_PWR_FV2,RT5640_PWR_FV1 | RT5640_PWR_FV2);regcache_cache_only(rt5640->regmap, false);regcache_sync(rt5640->regmap);
This looks wrong - you're writing to the device before you take it out of cache only mode so your delay in the above sequence will have no effect.
- val = snd_soc_read(codec, RT5640_VENDOR_ID2);
- if ((val != RT5640_DEVICE_ID)) {
dev_err(codec->dev,"Device with ID register %x is not rt5640/39\n", val);return -ENODEV;- }
- rt5640_reset(codec);
Do these in the I2C probe function before you register the CODEC, that way we don't try to start the sound card if we can't talk to the device.
- ret = regmap_register_patch(rt5640->regmap, init_list,
ARRAY_SIZE(init_list));- if (ret != 0)
dev_warn(codec->dev, "Failed to apply regmap patch: %d\n",ret);
This should also be in the I2C probe.