-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Thursday, December 19, 2013 3:21 AM To: Bard Liao Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; Flove; Oder Chiou; leon@leon.nu Subject: Re: [PATCH v3] ASoC: add RT286 CODEC driver
- if (rt286->pdata.irq_en)
rt286_index_update_bits(codec,NODE_ID_VENDOR_REGISTERS, 0x33, 0x0002, 0x0002);Why would the system ever want to supply an interrupt and not use it?
The codec support a configurable irq pin which can be disable or enable. Some customers may not use it. So, I add a flag in platform data to configure it.
- /* Set configuration default */
- rt286_write(codec, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3,
NODE_ID_MIC1, 0x00);- rt286_write(codec, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3,
NODE_ID_DMIC1, 0x00);- rt286_write(codec, AC_VERB_SET_CONFIG_DEFAULT_BYTES_3,
NODE_ID_DMIC2, 0x00);Why are the silicon defaults not adequate?
Actually, they are hardware dependency. I will add a configuration flag in platform data.
+static int rt286_hp_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event) {- struct snd_soc_codec *codec = w->codec;
- unsigned int val;
- switch (event) {
- case SND_SOC_DAPM_POST_PMU:
val = snd_soc_read(codec, NODE_ID_HP_OUT) & 0x8080;switch (val) {case 0x0:rt286_write(codec, AC_VERB_SET_AMP_GAIN_MUTE,NODE_ID_HP_OUT, 0xb000);break;case 0x8000:rt286_write(codec, AC_VERB_SET_AMP_GAIN_MUTE,NODE_ID_HP_OUT, 0x9000);break;case 0x0080:rt286_write(codec, AC_VERB_SET_AMP_GAIN_MUTE,NODE_ID_HP_OUT, 0xa000);break;}This is rather unclear. What is it doing?
SOC_DOUBLE_EXT("Headphone Playback Switch", NODE_ID_HP_OUT, 15, 8, 1, 1, rt286_playback_switch_get, rt286_playback_switch_put), We use NODE_ID_HP_OUT bit 15, 8 to store the mute/unmute status for headphone L/R channel. rt286_hp_event will mute both channels when power down. And it will read the status from RT286_HP_OUT to unmute the corresponding channel(s) when power on.
+static int rt286_set_bias_level(struct snd_soc_codec *codec,
enum snd_soc_bias_level level)+{
- switch (level) {
- case SND_SOC_BIAS_PREPARE:
if (SND_SOC_BIAS_STANDBY == codec->dapm.bias_level)rt286_write(codec, AC_VERB_SET_POWER_STATE,NODE_ID_AUDIO_FUNCTION_GROUP, AC_PWRST_D0);break;- case SND_SOC_BIAS_OFF:
rt286_write(codec, AC_VERB_SET_POWER_STATE,NODE_ID_AUDIO_FUNCTION_GROUP, AC_PWRST_D3);break;These aren't matched - you power up when leaving standby but only power down when you go to off. That's a bit odd and probably won't do the right thing and will leave the CODEC powered up all the time since you don't set idle_bias_off (though perhaps you should).
I will power off the codec in standby.
I'm also not seeing any code for interrupts even though there appears to be some code to initialise them in the driver.
I will add it along with jack detection function in another patch later.
Thanks.
------Please consider the environment before printing this e-mail.