On Wed, May 04, 2011 at 10:46:00PM +0800, Dimitris Papastamos wrote:
On Wed, May 04, 2011 at 09:44:58PM +0800, Lu Guanqun wrote:
- if (ret)
pr_err("upd9976 read of 0x%x failed, error: %d\n", reg, ret);- return value;
+}
dev_err() would be more preferable.
OK.
- switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
- case SND_SOC_DAIFMT_CBM_CFM:
- case SND_SOC_DAIFMT_CBM_CFS:
mode |= BIT(7) | BIT(3);break;- }
- return snd_soc_update_bits(codec, UPD9976_AUDIOPORT1, mask, mode);
+}
I am not sure whether the BIT() macro is more confusing than helpful.
I was trying to be helpful. :) but if that confuses you.,I'm OK to use hex number.
- switch (params_rate(params)) {
- case 8000:
tmp = 0x00;break;- case 11025:
tmp = 0x01;break;- case 12000:
tmp = 0x02;break;- case 16000:
tmp = 0x03;break;- case 22050:
tmp = 0x04;break;- case 24000:
tmp = 0x05;break;- case 32000:
tmp = 0x07;break;- case 44100:
tmp = 0x08;break;- case 48000:
tmp = 0x09;break;- default:
return -EINVAL;- }
Looks fine, I'd rather use an array though.
OK. Array makes it less code.
+static int upd9976_set_bias_level(struct snd_soc_codec *codec,
enum snd_soc_bias_level level)+{
- switch (level) {
- case SND_SOC_BIAS_ON:
break;- case SND_SOC_BIAS_PREPARE:
if (codec->dapm.bias_level == SND_SOC_BIAS_STANDBY) {snd_soc_update_bits(codec, UPD9976_VAUDIOCNT,0x27, 0x27);snd_soc_update_bits(codec, UPD9976_VREFPLL,0x35, 0x35);}break;- case SND_SOC_BIAS_STANDBY:
snd_soc_write(codec, UPD9976_VAUDIOCNT, 0x25);snd_soc_write(codec, UPD9976_VREFPLL, 0x10);break;- case SND_SOC_BIAS_OFF:
snd_soc_write(codec, UPD9976_VREFPLL, 0);snd_soc_write(codec, UPD9976_VAUDIOCNT, 0x24);break;- }
Why not snd_soc_update_bits()? These should normally be DAPM widgets.
There's no DAPM widgets bound to these two registers. So I'm afraid it's OK to use snd_soc_write when it's been powered off totally.
+static int upd9976_codec_probe(struct snd_soc_codec *codec) +{
- upd9976_set_bias_level(codec, SND_SOC_BIAS_OFF);
- return 0;
+}
Why SND_SOC_BIAS_OFF and not SND_SOC_BIAS_STANDBY?
Try to use as little power as possible.
+static int upd9976_codec_remove(struct snd_soc_codec *codec) +{
- return 0;
+}
You can call upd9976_set_bias_level(codec, SND_SOC_BIAS_OFF) here.
Thanks for reminding this.