On Wed, May 04, 2011 at 09:44:58PM +0800, Lu Guanqun wrote:
uPD9976 is a complex codec, however this patch only provides basic playback functionality for headphone. More functionality will be added bit by bit in the following patches.
Signed-off-by: Lu Guanqun guanqun.lu@intel.com Signed-off-by: Wang Xingchao xingchao.wang@intel.com
+static inline unsigned int upd9976_read(struct snd_soc_codec *codec,
unsigned int reg)
+{
- u8 value = 0;
- int ret;
- ret = intel_scu_ipc_ioread8(reg, &value);
- if (ret)
pr_err("upd9976 read of 0x%x failed, error: %d\n", reg, ret);
- return value;
+}
dev_err() would be more preferable.
+static int upd9976_audio_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt) +{
- struct snd_soc_codec *codec = dai->codec;
- unsigned int mode, mask;
- mask = BIT(5) | BIT(4);
- mode = 0;
- switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
- case SND_SOC_DAIFMT_I2S:
mode |= BIT(4);
break;
- case SND_SOC_DAIFMT_RIGHT_J:
mode |= BIT(5);
break;
- case SND_SOC_DAIFMT_LEFT_J:
mode |= BIT(5) | BIT(4);
break;
- }
- mask |= BIT(7) | BIT(3);
- 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.
+static int upd9976_audio_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
+{
- struct snd_soc_codec *codec = dai->codec;
- unsigned int tmp;
- switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S16_LE:
tmp = 0x00;
break;
- case SNDRV_PCM_FORMAT_S24_LE:
tmp = 0x03;
break;
- case SNDRV_PCM_FORMAT_S18_3LE:
tmp = 0x01;
break;
- case SNDRV_PCM_FORMAT_S20_3LE:
tmp = 0x02;
break;
- default:
return -EINVAL;
- }
- snd_soc_update_bits(codec, UPD9976_AUDIOPORT1,
BIT(2)|BIT(1)|BIT(0), tmp);
- 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.
+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.
+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?
+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, Dimitris