[1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error
diff mbox

Message ID 880838f666b591f5ffb0de1e39df5e3734846246.1434020423.git.zidan.wang@freescale.com
State New
Headers show

Commit Message

Zidan Wang June 11, 2015, 11:14 a.m. UTC
It will playback from speaker in the first 2 seconds, then switch to
headphone. Steps to reproduce this issue:
1. plug out headphone and playback a wav.
2. stop playback and wait for at least 5 seconds, then
   plug in headphone and playback a wav.

Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
---
 sound/soc/codecs/wm8960.c | 73 +++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 40 deletions(-)

Comments

Mark Brown June 11, 2015, 4:20 p.m. UTC | #1
On Thu, Jun 11, 2015 at 07:14:33PM +0800, Zidan Wang wrote:
> It will playback from speaker in the first 2 seconds, then switch to
> headphone. Steps to reproduce this issue:
> 1. plug out headphone and playback a wav.
> 2. stop playback and wait for at least 5 seconds, then
>    plug in headphone and playback a wav.

I'd really like to understand the logic behind this in more detail -
what is the actual problem here and how does this fix it?  You're moving
the clock management from the bias level setting to the stream startup
and teardown which doesn't seem directly related, if anything it seems
like it'd be making things worse since it reduces the proportion of the
time where the clock is enabled.  My guess is that the jack detection
needs MCLK enabling.
Zidan Wang June 12, 2015, 6:32 a.m. UTC | #2
On Thu, Jun 11, 2015 at 05:20:28PM +0100, Mark Brown wrote:
> On Thu, Jun 11, 2015 at 07:14:33PM +0800, Zidan Wang wrote:
> > It will playback from speaker in the first 2 seconds, then switch to
> > headphone. Steps to reproduce this issue:
> > 1. plug out headphone and playback a wav.
> > 2. stop playback and wait for at least 5 seconds, then
> >    plug in headphone and playback a wav.
> 
> I'd really like to understand the logic behind this in more detail -
> what is the actual problem here and how does this fix it?  You're moving
> the clock management from the bias level setting to the stream startup
> and teardown which doesn't seem directly related, if anything it seems
> like it'd be making things worse since it reduces the proportion of the
> time where the clock is enabled.  My guess is that the jack detection
> needs MCLK enabling.

I select RINPUT3/JD3 pins as headphone jack detect inputs. I found that
it would detected as speaker sometimes even if headphone jack is plugged in.
And it would be reproduced everytime when i followed below steps:
1. Unplug headphone jack and playback a wav. It will playback from speaker.
2. Stop playback and wait for at least 5 seconds, then plug in headphone jack
   and playback a wav. I will playback from speaker in the first 1-2 seconds,
   and then switch to headphone.

I suspect the codec need some time to prepare jack detect function after SYSCLK
enable. When Unplug headphone jack and playback, it will detect as speaker. After
this playback, the mclk will be disabled, and the detect function will be disabled
too. So when you plug in the headphone jack in this moment, the codec can't detect
as headphone. And when playback a wav, it will playback from speaker. After the
headphone jack detect function works, it will switch to headphone.

I have done some test today, and found that after mclk enable, we should delay
at least 150ms to prepare the jack detect function. So no need to move mclk
enable to startup.

Best Regards,
Zidan Wang
Mark Brown June 12, 2015, 10:33 a.m. UTC | #3
On Fri, Jun 12, 2015 at 02:32:33PM +0800, Zidan Wang wrote:

> I have done some test today, and found that after mclk enable, we should delay
> at least 150ms to prepare the jack detect function. So no need to move mclk
> enable to startup.

No, this makes no sense - jack detection is not related to playback at
all.  The system should be able to detect if something is connected at
any time.  What this says is that you need to keep MCLK enabled while
jack detection is available.
Zidan Wang June 16, 2015, 9:30 a.m. UTC | #4
On Fri, Jun 12, 2015 at 11:33:49AM +0100, Mark Brown wrote:
> On Fri, Jun 12, 2015 at 02:32:33PM +0800, Zidan Wang wrote:
> 
> > I have done some test today, and found that after mclk enable, we should delay
> > at least 150ms to prepare the jack detect function. So no need to move mclk
> > enable to startup.
> 
> No, this makes no sense - jack detection is not related to playback at
> all.  The system should be able to detect if something is connected at
> any time.  What this says is that you need to keep MCLK enabled while
> jack detection is available.

This may be the wm8960 codec hardware issue. In generally, we will connect the headphone detect
output to the CPU GPIO and let CPU to deal with the headphone jack detect event. The wm8960 have
the feature to auto detect headphone jack itself, so in order to save CPU pin, we connect the
headphone detect output to the codec headphone jack detect pin(RINPUT3/JD2).

This issue is abnormal. Anyone else who using the community driver meet the same issue?

Best Regards,
Zidan Wang

Patch
diff mbox

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 761418f..729205f 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -491,6 +491,34 @@  static int wm8960_add_widgets(struct snd_soc_codec *codec)
 	return 0;
 }
 
+int wm8960_startup(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *codec_dai)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
+	int ret;
+
+	if (!IS_ERR(wm8960->mclk)) {
+		ret = clk_prepare_enable(wm8960->mclk);
+		if (ret) {
+			dev_err(codec->dev,
+				"Failed to enable MCLK: %d\n", ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+void wm8960_shutdown(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *codec_dai)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
+
+	if (!IS_ERR(wm8960->mclk))
+		clk_disable_unprepare(wm8960->mclk);
+}
+
 static int wm8960_set_dai_fmt(struct snd_soc_dai *codec_dai,
 		unsigned int fmt)
 {
@@ -702,38 +730,14 @@  static int wm8960_set_bias_level_out3(struct snd_soc_codec *codec,
 				      enum snd_soc_bias_level level)
 {
 	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
-	int ret;
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
 		break;
 
 	case SND_SOC_BIAS_PREPARE:
-		switch (snd_soc_codec_get_bias_level(codec)) {
-		case SND_SOC_BIAS_STANDBY:
-			if (!IS_ERR(wm8960->mclk)) {
-				ret = clk_prepare_enable(wm8960->mclk);
-				if (ret) {
-					dev_err(codec->dev,
-						"Failed to enable MCLK: %d\n",
-						ret);
-					return ret;
-				}
-			}
-
-			/* Set VMID to 2x50k */
-			snd_soc_update_bits(codec, WM8960_POWER1, 0x180, 0x80);
-			break;
-
-		case SND_SOC_BIAS_ON:
-			if (!IS_ERR(wm8960->mclk))
-				clk_disable_unprepare(wm8960->mclk);
-			break;
-
-		default:
-			break;
-		}
-
+		/* Set VMID to 2x50k */
+		snd_soc_update_bits(codec, WM8960_POWER1, 0x180, 0x80);
 		break;
 
 	case SND_SOC_BIAS_STANDBY:
@@ -780,7 +784,7 @@  static int wm8960_set_bias_level_capless(struct snd_soc_codec *codec,
 					 enum snd_soc_bias_level level)
 {
 	struct wm8960_priv *wm8960 = snd_soc_codec_get_drvdata(codec);
-	int reg, ret;
+	int reg;
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
@@ -821,22 +825,9 @@  static int wm8960_set_bias_level_capless(struct snd_soc_codec *codec,
 					    WM8960_VREF, WM8960_VREF);
 
 			msleep(100);
-
-			if (!IS_ERR(wm8960->mclk)) {
-				ret = clk_prepare_enable(wm8960->mclk);
-				if (ret) {
-					dev_err(codec->dev,
-						"Failed to enable MCLK: %d\n",
-						ret);
-					return ret;
-				}
-			}
 			break;
 
 		case SND_SOC_BIAS_ON:
-			if (!IS_ERR(wm8960->mclk))
-				clk_disable_unprepare(wm8960->mclk);
-
 			/* Enable anti-pop mode */
 			snd_soc_update_bits(codec, WM8960_APOP1,
 					    WM8960_POBCTRL | WM8960_SOFT_ST |
@@ -1059,6 +1050,8 @@  static int wm8960_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 	SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
 
 static const struct snd_soc_dai_ops wm8960_dai_ops = {
+	.startup = wm8960_startup,
+	.shutdown = wm8960_shutdown,
 	.hw_params = wm8960_hw_params,
 	.digital_mute = wm8960_mute,
 	.set_fmt = wm8960_set_dai_fmt,