[alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[Zidan Wang]

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(-)

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,
-- 
1.9.1

Re: [alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[Zidan Wang]

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

Re: [alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [alsa-devel][PATCH 1/4] ASoC: wm8960: codec mclk should be enabled early to avoid jack detect error

[Zidan Wang]

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

[alsa-devel][PATCH 2/4] ASoC: wm8960: support configure headphone jack detect pin and polarity from device tree

[Zidan Wang]

The ADCLRC/GPIO1, LINPUT3/JD2 and RINPUT3/JD3 pins can be selected as
headphone jack detect inputs to automatically disable the speaker output
and enable the headphone.

Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
---
 Documentation/devicetree/bindings/sound/wm8960.txt | 11 +++++++++++
 include/sound/wm8960.h                             |  1 +
 sound/soc/codecs/wm8960.c                          | 22 ++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/wm8960.txt b/Documentation/devicetree/bindings/sound/wm8960.txt
index 2deb8a3..4f1aa8b 100644
--- a/Documentation/devicetree/bindings/sound/wm8960.txt
+++ b/Documentation/devicetree/bindings/sound/wm8960.txt
@@ -21,6 +21,16 @@ Optional properties:
 	enabled and disabled together with HP_L and HP_R pins in response to jack
 	detect events.
 
+  - wlf,hp-det: ADCLRC/GPIO1, LINPUT3/JD2 and RINPUT3/JD3 pins can be selected as
+	headphone jack detect inputs to automatically disable the speaker output
+	and enable the headphone output.
+	hp-det = <hp-det-pin hp-det-polarity>;
+	hp-det-pin = 1: ADCLRC/GPIO1 used as detect pin
+	hp-det-pin = 2: LINPUT3/JD2 used as detect pin
+	hp-det-pin = 3: RINPUT3/JD3 used as detect pin
+	hp-det-polarity = 0: hp detect high for headphone
+	hp-det-polarity = 1: hp detect high for speaker
+
 Example:
 
 codec: wm8960@1a {
@@ -28,4 +38,5 @@ codec: wm8960@1a {
 	reg = <0x1a>;
 
 	wlf,shared-lrclk;
+	wlf,hp-det = <2 0>
 };
diff --git a/include/sound/wm8960.h b/include/sound/wm8960.h
index e8ce8ee..a787376 100644
--- a/include/sound/wm8960.h
+++ b/include/sound/wm8960.h
@@ -19,6 +19,7 @@ struct wm8960_data {
 	bool capless;  /* Headphone outputs configured in capless mode */
 
 	bool shared_lrclk;  /* DAC and ADC LRCLKs are wired together */
+	u32 hp_det[2]; /* headphone detect pin and polarity */
 };
 
 #endif
diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 729205f..c1be15c 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -1123,6 +1123,11 @@ static void wm8960_set_pdata_from_of(struct i2c_client *i2c,
 
 	if (of_property_read_bool(np, "wlf,shared-lrclk"))
 		pdata->shared_lrclk = true;
+
+	of_property_read_u32_array(np, "wlf,hp-det", pdata->hp_det, 2);
+
+	if (pdata->hp_det[0] > 3)
+		pdata->hp_det[0] = 0;
 }
 
 static int wm8960_i2c_probe(struct i2c_client *i2c,
@@ -1168,6 +1173,23 @@ static int wm8960_i2c_probe(struct i2c_client *i2c,
 		}
 	}
 
+	/*
+	 * ADCLRC/GPIO1 pin configured as GPIO1 to select jack
+	 * detect input function
+	 */
+	if (wm8960->pdata.hp_det[0] == 1)
+	        regmap_update_bits(wm8960->regmap, WM8960_IFACE2, 1<<6, 1<<6);
+
+	/* Enable headphone jack detect */
+	if (wm8960->pdata.hp_det[0] != 0) {
+		regmap_update_bits(wm8960->regmap, WM8960_ADDCTL2, 1<<6, 1<<6);
+		regmap_update_bits(wm8960->regmap, WM8960_ADDCTL2, 1<<5,
+				    wm8960->pdata.hp_det[1]<<5);
+		regmap_update_bits(wm8960->regmap, WM8960_ADDCTL4, 3<<2,
+				    wm8960->pdata.hp_det[0]<<2);
+		regmap_update_bits(wm8960->regmap, WM8960_ADDCTL1, 3, 3);
+	}
+
 	/* Latch the update bits */
 	regmap_update_bits(wm8960->regmap, WM8960_LINVOL, 0x100, 0x100);
 	regmap_update_bits(wm8960->regmap, WM8960_RINVOL, 0x100, 0x100);
-- 
1.9.1

Re: [alsa-devel][PATCH 2/4] ASoC: wm8960: support configure headphone jack detect pin and polarity from device tree

[Charles Keepax]

On Thu, Jun 11, 2015 at 07:14:34PM +0800, Zidan Wang wrote:
> The ADCLRC/GPIO1, LINPUT3/JD2 and RINPUT3/JD3 pins can be selected as
> headphone jack detect inputs to automatically disable the speaker output
> and enable the headphone.
> 
> Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> ---

This feels like this is missing some iteraction with DAPM, there
are DAPM widgets that represent both the Speaker and the
Headphone, in the case this feature is in use the power state
DAPM shows may no longer accurately reflects the actual power
state of the device.

That said I have no idea if the registers update on the chip to
reflect this change (my guess is that they don't). I am not
really sure what the correct way to handle this sort of hardware
output switching would be in DAPM.

Thanks,
Charles

Re: [alsa-devel][PATCH 2/4] ASoC: wm8960: support configure headphone jack detect pin and polarity from device tree

[Zidan Wang]

On Fri, Jun 12, 2015 at 01:50:05PM +0100, Charles Keepax wrote:
> On Thu, Jun 11, 2015 at 07:14:34PM +0800, Zidan Wang wrote:
> > The ADCLRC/GPIO1, LINPUT3/JD2 and RINPUT3/JD3 pins can be selected as
> > headphone jack detect inputs to automatically disable the speaker output
> > and enable the headphone.
> > 
> > Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> > ---
> 
> This feels like this is missing some iteraction with DAPM, there
> are DAPM widgets that represent both the Speaker and the
> Headphone, in the case this feature is in use the power state
> DAPM shows may no longer accurately reflects the actual power
> state of the device.
> 
> That said I have no idea if the registers update on the chip to
> reflect this change (my guess is that they don't). I am not
> really sure what the correct way to handle this sort of hardware
> output switching would be in DAPM.
> 
> Thanks,
> Charles
Yes, there are no registers represent that it is playback from headphone
or speaker. So the hardware should connect the headphone detect output 
to CPU GPIO pin instead of the codec headphone detect pin?

Best Regards,
Zidan Wang

[alsa-devel][PATCH 3/4] ASoC: wm8960: add kcontrol to select ADC data output

[Zidan Wang]

add kcontrol to select ADC data output.

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

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index c1be15c..62246f1 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -141,6 +141,12 @@ static const char *wm8960_3d_upper_cutoff[] = {"High", "Low"};
 static const char *wm8960_3d_lower_cutoff[] = {"Low", "High"};
 static const char *wm8960_alcfunc[] = {"Off", "Right", "Left", "Stereo"};
 static const char *wm8960_alcmode[] = {"ALC", "Limiter"};
+static const char *wm8960_adc_data_sel[] = {
+	"ldata = left ADC; rdata = right ADC",
+	"ldata = left ADC; rdata = left ADC",
+	"ldata = right ADC; rdata = right ADC",
+	"ldata = right ADC; rdata = left ADC",
+};
 
 static const struct soc_enum wm8960_enum[] = {
 	SOC_ENUM_SINGLE(WM8960_DACCTL1, 5, 4, wm8960_polarity),
@@ -149,6 +155,7 @@ static const struct soc_enum wm8960_enum[] = {
 	SOC_ENUM_SINGLE(WM8960_3D, 5, 2, wm8960_3d_lower_cutoff),
 	SOC_ENUM_SINGLE(WM8960_ALC1, 7, 4, wm8960_alcfunc),
 	SOC_ENUM_SINGLE(WM8960_ALC3, 8, 2, wm8960_alcmode),
+	SOC_ENUM_SINGLE(WM8960_ADDCTL1, 2, 4, wm8960_adc_data_sel),
 };
 
 static const int deemph_settings[] = { 0, 32000, 44100, 48000 };
@@ -279,6 +286,8 @@ SOC_SINGLE_TLV("Right Output Mixer Boost Bypass Volume",
 	       WM8960_BYPASS2, 4, 7, 1, bypass_tlv),
 SOC_SINGLE_TLV("Right Output Mixer RINPUT3 Volume",
 	       WM8960_ROUTMIX, 4, 7, 1, bypass_tlv),
+
+SOC_ENUM("ADC Data Output Select", wm8960_enum[6]),
 };
 
 static const struct snd_kcontrol_new wm8960_lin_boost[] = {
-- 
1.9.1

Re: [alsa-devel][PATCH 3/4] ASoC: wm8960: add kcontrol to select ADC data output

[Charles Keepax]

On Thu, Jun 11, 2015 at 07:14:35PM +0800, Zidan Wang wrote:
> add kcontrol to select ADC data output.
> 
> Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> ---
>  sound/soc/codecs/wm8960.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> index c1be15c..62246f1 100644
> --- a/sound/soc/codecs/wm8960.c
> +++ b/sound/soc/codecs/wm8960.c
> @@ -141,6 +141,12 @@ static const char *wm8960_3d_upper_cutoff[] = {"High", "Low"};
>  static const char *wm8960_3d_lower_cutoff[] = {"Low", "High"};
>  static const char *wm8960_alcfunc[] = {"Off", "Right", "Left", "Stereo"};
>  static const char *wm8960_alcmode[] = {"ALC", "Limiter"};
> +static const char *wm8960_adc_data_sel[] = {
> +	"ldata = left ADC; rdata = right ADC",
> +	"ldata = left ADC; rdata = left ADC",
> +	"ldata = right ADC; rdata = right ADC",
> +	"ldata = right ADC; rdata = left ADC",
> +};

I see the datasheet lists this as one field but it would probably
look nicer as two controls, one to select the left channel source
and one for the right channel source.

Thanks,
Charles

[alsa-devel][PATCH 4/4] ASoC: wm8960: the enum of "DAC Polarity" should be wm8960_enum[1]

[Zidan Wang]

the enum of "DAC Polarity" should be wm8960_enum[1].

Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
---
 sound/soc/codecs/wm8960.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
index 62246f1..d482c0f 100644
--- a/sound/soc/codecs/wm8960.c
+++ b/sound/soc/codecs/wm8960.c
@@ -254,7 +254,7 @@ SOC_SINGLE("PCM Playback -6dB Switch", WM8960_DACCTL1, 7, 1, 0),
 SOC_ENUM("ADC Polarity", wm8960_enum[0]),
 SOC_SINGLE("ADC High Pass Filter Switch", WM8960_DACCTL1, 0, 1, 0),
 
-SOC_ENUM("DAC Polarity", wm8960_enum[2]),
+SOC_ENUM("DAC Polarity", wm8960_enum[1]),
 SOC_SINGLE_BOOL_EXT("DAC Deemphasis Switch", 0,
 		    wm8960_get_deemph, wm8960_put_deemph),
 
-- 
1.9.1

Re: [alsa-devel][PATCH 4/4] ASoC: wm8960: the enum of "DAC Polarity" should be wm8960_enum[1]

[Charles Keepax]

On Thu, Jun 11, 2015 at 07:14:36PM +0800, Zidan Wang wrote:
> the enum of "DAC Polarity" should be wm8960_enum[1].
> 
> Signed-off-by: Zidan Wang <zidan.wang@freescale.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [alsa-devel][PATCH 4/4] ASoC: wm8960: the enum of "DAC Polarity" should be wm8960_enum[1]

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 133 bytes --]

On Thu, Jun 11, 2015 at 07:14:36PM +0800, Zidan Wang wrote:
> the enum of "DAC Polarity" should be wm8960_enum[1].

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]