All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
@ 2016-04-29  8:15 John Hsu
  2016-04-29  8:15 ` [PATCH 2/2] ASoC: nau8825: cross talk suppression measurement function John Hsu
  2016-05-02 16:27 ` [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby Mark Brown
  0 siblings, 2 replies; 13+ messages in thread
From: John Hsu @ 2016-04-29  8:15 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, anatol.pomozov, YHCHuang, John Hsu, lgirdwood, benzh,
	CTLIN0, mhkuo, yong.zhi

The driver changes jack type detection interruption to non-clock archi-
tecture for less 1mW power saving. The architecture is called manual mode
jack detection. It has no hardware debounce, no jack type detection, but
only detecting jack insertion. After jack insertion, the driver will
switch to auto mode jack detection with internal clock which can detect
microphone, jack type and do hardware debounce.

The manual architecture has these main changes including codec initiation,
interruption, clock control, and power management. When codec initiation
or system resume, the clock is closed as jack insertion detection at man-
ual mode, and bypass debounce circuit. These configurations move to resume
setup function when setup bias level after resume.

When jack insertion detection happens, the manual mode turns off and make
configuration about jack type detection interruption at auto mode in auto
irq setup function which can detect microphone and jack type. The inter-
ruption will switch to manual mode again with clock free until jack ejec-
tion happens.

The system clock configuration adds clock disable option which can disable
internal VCO clock. Before the system clock change, there is an restric-
tion added to make sure clock disabled and not config any clock when no
headset connected.

In power management, we involve the solution about races and jack detec-
tion in resume from Ben Zhang in the following patch and list his comment.
[PATCH] ASoC: nau8825: Fix jack detection across suspend
"Jack plug status is rechecked at resume to handle plug/unplug
in S3 when the chip has no power."
"Suspend/resume callbacks are moved from the i2c dev_pm_ops to
snd_soc_codec_driver. soc_resume_deferred is a delayed work
which may trigger nau8825_set_bias_level. The bias change races
against dev_pm_ops, causing jack detection issues.
soc_resume_deferred ensures bias change and snd_soc_codec_driver
suspend/resume are sequenced correctly."

Change SAR widget to supply type which can prevent the codec keeping at
SND_SOC_BIAS_ON during suspend. The codec suspend function can just invoke
normally.

Before the system suspends, the driver turns off all interruptions. Keep
the interruption quiet before resume setup completes. The ADC channel will
be disabled which is needed for interruptions at audo mode.


Signed-off-by: John Hsu <KCHSU0@nuvoton.com>
Signed-off-by: Ben Zhang <benzh@chromium.org>
---
 sound/soc/codecs/nau8825.c | 281 +++++++++++++++++++++++++++++++++++----------
 sound/soc/codecs/nau8825.h |   9 +-
 2 files changed, 226 insertions(+), 64 deletions(-)

diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c
index f35019a..9386624 100644
--- a/sound/soc/codecs/nau8825.c
+++ b/sound/soc/codecs/nau8825.c
@@ -30,10 +30,16 @@
 
 #include "nau8825.h"
 
+
+#define NUVOTON_CODEC_DAI "nau8825-hifi"
+
 #define NAU_FREF_MAX 13500000
 #define NAU_FVCO_MAX 124000000
 #define NAU_FVCO_MIN 90000000
 
+static int nau8825_configure_sysclk(struct nau8825 *nau8825,
+		int clk_id, unsigned int freq);
+
 struct nau8825_fll {
 	int mclk_src;
 	int ratio;
@@ -368,9 +374,12 @@ static const struct snd_soc_dapm_widget nau8825_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("ADC Power", NAU8825_REG_ANALOG_ADC_2, 6, 0, NULL,
 		0),
 
-	/* ADC for button press detection */
-	SND_SOC_DAPM_ADC("SAR", NULL, NAU8825_REG_SAR_CTRL,
-		NAU8825_SAR_ADC_EN_SFT, 0),
+	/* ADC for button press detection. A dapm supply widget is used to
+	 * prevent dapm_power_widgets keeping the codec at SND_SOC_BIAS_ON
+	 * during suspend.
+	 */
+	SND_SOC_DAPM_SUPPLY("SAR", NAU8825_REG_SAR_CTRL,
+		NAU8825_SAR_ADC_EN_SFT, 0, NULL, 0),
 
 	SND_SOC_DAPM_PGA_S("ADACL", 2, NAU8825_REG_RDAC, 12, 0, NULL, 0),
 	SND_SOC_DAPM_PGA_S("ADACR", 2, NAU8825_REG_RDAC, 13, 0, NULL, 0),
@@ -614,8 +623,10 @@ int nau8825_enable_jack_detect(struct snd_soc_codec *codec,
 		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L,
 		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L);
 
-	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
-		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_EJECT_EN, 0);
+	/* Change jack type detection interruption to non-clock architecture
+	 * for power saving less 1mW. Move these configuration about inter-
+	 * ruption at auto mode to auto irq setup function.
+	 */
 
 	return 0;
 }
@@ -642,6 +653,22 @@ static void nau8825_restart_jack_detection(struct regmap *regmap)
 		NAU8825_JACK_DET_RESTART, 0);
 }
 
+static void nau8825_int_status_clear_all(struct regmap *regmap)
+{
+	int active_irq, clear_irq, i;
+
+	/* Reset the intrruption status from rightmost bit if the corres-
+	 * ponding irq event occurs.
+	 */
+	regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq);
+	for (i = 0; i < NAU8825_REG_DATA_LEN; i++) {
+		clear_irq = (0x1 << i);
+		if (active_irq & clear_irq)
+			regmap_write(regmap,
+				NAU8825_REG_INT_CLR_KEY_STATUS, clear_irq);
+	}
+}
+
 static void nau8825_eject_jack(struct nau8825 *nau8825)
 {
 	struct snd_soc_dapm_context *dapm = nau8825->dapm;
@@ -656,6 +683,67 @@ static void nau8825_eject_jack(struct nau8825 *nau8825)
 	regmap_update_bits(regmap, NAU8825_REG_HSD_CTRL, 0xf, 0xf);
 
 	snd_soc_dapm_sync(dapm);
+
+	/* Clear all interruption status */
+	nau8825_int_status_clear_all(regmap);
+
+	/* Mask all interruptions except jack insertion interruption */
+	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);
+
+	/* Enable insertion interruption only and bypass de-bounce circuit */
+	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
+		NAU8825_IRQ_OUTPUT_EN | NAU8825_IRQ_EJECT_EN |
+		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_INSERT_EN,
+		NAU8825_IRQ_OUTPUT_EN | NAU8825_IRQ_EJECT_EN |
+		NAU8825_IRQ_HEADSET_COMPLETE_EN);
+	regmap_update_bits(regmap, NAU8825_REG_JACK_DET_CTRL,
+		NAU8825_JACK_DET_DB_BYPASS, NAU8825_JACK_DET_DB_BYPASS);
+
+	/* Disable ADC needed for interruptions at audo mode */
+	regmap_update_bits(regmap, NAU8825_REG_ENA_CTRL,
+		NAU8825_ENABLE_ADC, 0);
+
+	/* Close clock for jack type detection at manual mode */
+	nau8825_configure_sysclk(nau8825, NAU8825_CLK_DIS, 0);
+}
+
+/* Enable audo mode interruptions with internal clock. */
+static void nau8825_setup_auto_irq(struct nau8825 *nau8825)
+{
+	struct regmap *regmap = nau8825->regmap;
+
+	/* Enable headset jack type detection complete interruption and
+	 * jack ejection interruption.
+	 */
+	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
+		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_EJECT_EN, 0);
+
+	/* Enable internal VCO needed for interruptions */
+	nau8825_configure_sysclk(nau8825, NAU8825_CLK_INTERNAL, 0);
+
+	/* Enable ADC needed for interruptions */
+	regmap_update_bits(regmap, NAU8825_REG_ENA_CTRL,
+		NAU8825_ENABLE_ADC, NAU8825_ENABLE_ADC);
+
+	/* Chip needs one FSCLK cycle in order to generate interruptions,
+	 * as we cannot guarantee one will be provided by the system. Turning
+	 * master mode on then off enables us to generate that FSCLK cycle
+	 * with a minimum of contention on the clock bus.
+	 */
+	regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2,
+		NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_MASTER);
+	regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2,
+		NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_SLAVE);
+
+	/* Not bypass de-bounce circuit */
+	regmap_update_bits(regmap, NAU8825_REG_JACK_DET_CTRL,
+		NAU8825_JACK_DET_DB_BYPASS, 0);
+
+	/* Unmask all interruptions */
+	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0);
+
+	/* Restart the jack detection process at auto mode */
+	nau8825_restart_jack_detection(regmap);
 }
 
 static int nau8825_button_decode(int value)
@@ -753,7 +841,10 @@ static irqreturn_t nau8825_interrupt(int irq, void *data)
 	struct regmap *regmap = nau8825->regmap;
 	int active_irq, clear_irq = 0, event = 0, event_mask = 0;
 
-	regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq);
+	if (regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq)) {
+		dev_err(nau8825->dev, "failed to clear interruption\n");
+		return IRQ_NONE;
+	}
 
 	if ((active_irq & NAU8825_JACK_EJECTION_IRQ_MASK) ==
 		NAU8825_JACK_EJECTION_DETECTED) {
@@ -789,6 +880,26 @@ static irqreturn_t nau8825_interrupt(int irq, void *data)
 
 		event_mask |= SND_JACK_HEADSET;
 		clear_irq = NAU8825_HEADSET_COMPLETION_IRQ;
+	} else if ((active_irq & NAU8825_JACK_INSERTION_IRQ_MASK) ==
+		NAU8825_JACK_INSERTION_DETECTED) {
+		/* One more step to check GPIO status directly. Thus, the
+		 * driver can confirm the real insertion interruption because
+		 * the intrruption at manual mode has bypassed debounce
+		 * circuit which can get rid of unstable status.
+		 */
+		if (nau8825_is_jack_inserted(regmap)) {
+			/* Turn off insertion interruption at manual mode */
+			regmap_update_bits(regmap,
+				NAU8825_REG_INTERRUPT_DIS_CTRL,
+				NAU8825_IRQ_INSERT_DIS,
+				NAU8825_IRQ_INSERT_DIS);
+			regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
+				NAU8825_IRQ_INSERT_EN, NAU8825_IRQ_INSERT_EN);
+			/* Enable interruption for jack type detection at audo
+			 * mode which can detect microphone and jack type.
+			 */
+			nau8825_setup_auto_irq(nau8825);
+		}
 	}
 
 	if (!clear_irq)
@@ -938,8 +1049,8 @@ static void nau8825_init_regs(struct nau8825 *nau8825)
 }
 
 static const struct regmap_config nau8825_regmap_config = {
-	.val_bits = 16,
-	.reg_bits = 16,
+	.val_bits = NAU8825_REG_DATA_LEN,
+	.reg_bits = NAU8825_REG_ADDR_LEN,
 
 	.max_register = NAU8825_REG_MAX,
 	.readable_reg = nau8825_readable_reg,
@@ -958,11 +1069,10 @@ static int nau8825_codec_probe(struct snd_soc_codec *codec)
 
 	nau8825->dapm = dapm;
 
-	/* Unmask interruptions. Handler uses dapm object so we can enable
-	 * interruptions only after dapm is fully initialized.
+	/* Change jack type detection interruption to non-clock architecture
+	 * for power saving less 1mW. Move these configuration about inter-
+	 * ruption at auto mode to auto irq setup function.
 	 */
-	regmap_write(nau8825->regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0);
-	nau8825_restart_jack_detection(nau8825->regmap);
 
 	return 0;
 }
@@ -1134,7 +1244,26 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
 	struct regmap *regmap = nau8825->regmap;
 	int ret;
 
+	if (!nau8825_is_jack_inserted(nau8825->regmap) &&
+		clk_id != NAU8825_CLK_DIS) {
+		/* For power saving less 1mW, the jack type detection inter-
+		 * ruption changes to non-clock architecture. Therefore, the
+		 * clock should be disabled and not allowed to config any clock
+		 * when no headset connected.
+		 */
+		dev_warn(nau8825->dev, "Souldn't enable any clock when no headset connected\n");
+		return 0;
+	}
+
 	switch (clk_id) {
+	case NAU8825_CLK_DIS:
+		/* Clock provided externally and disable internal VCO clock */
+		regmap_update_bits(regmap, NAU8825_REG_CLK_DIVIDER,
+			NAU8825_CLK_SRC_MASK, NAU8825_CLK_SRC_MCLK);
+		regmap_update_bits(regmap, NAU8825_REG_FLL6,
+			NAU8825_DCO_EN, 0);
+
+		break;
 	case NAU8825_CLK_MCLK:
 		regmap_update_bits(regmap, NAU8825_REG_CLK_DIVIDER,
 			NAU8825_CLK_SRC_MASK, NAU8825_CLK_SRC_MCLK);
@@ -1209,6 +1338,30 @@ static int nau8825_set_sysclk(struct snd_soc_codec *codec, int clk_id,
 	return nau8825_configure_sysclk(nau8825, clk_id, freq);
 }
 
+static int nau8825_resume_setup(struct nau8825 *nau8825)
+{
+	struct regmap *regmap = nau8825->regmap;
+
+	/* Close clock when jack type detection at manual mode */
+	nau8825_configure_sysclk(nau8825, NAU8825_CLK_DIS, 0);
+
+	/* Clear all interruption status */
+	nau8825_int_status_clear_all(regmap);
+
+	/* Enable insertion interruption only and bypass de-bounce circuit */
+	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
+		NAU8825_IRQ_OUTPUT_EN | NAU8825_IRQ_EJECT_EN |
+		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_INSERT_EN,
+		NAU8825_IRQ_OUTPUT_EN | NAU8825_IRQ_EJECT_EN |
+		NAU8825_IRQ_HEADSET_COMPLETE_EN);
+	regmap_update_bits(regmap, NAU8825_REG_JACK_DET_CTRL,
+		NAU8825_JACK_DET_DB_BYPASS, NAU8825_JACK_DET_DB_BYPASS);
+	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL,
+				NAU8825_IRQ_INSERT_DIS, 0);
+
+	return 0;
+}
+
 static int nau8825_set_bias_level(struct snd_soc_codec *codec,
 				   enum snd_soc_bias_level level)
 {
@@ -1238,11 +1391,22 @@ static int nau8825_set_bias_level(struct snd_soc_codec *codec,
 					"Failed to sync cache: %d\n", ret);
 				return ret;
 			}
+
+			/* Setup codec configuration after resume */
+			nau8825_resume_setup(nau8825);
 		}
 
 		break;
 
 	case SND_SOC_BIAS_OFF:
+		/* Turn off all interruptions before system shutdown. Keep the
+		 * interruption quiet before resume setup completes.
+		 */
+		regmap_write(nau8825->regmap,
+			NAU8825_REG_INTERRUPT_DIS_CTRL, 0xffff);
+		/* Disable ADC needed for interruptions at audo mode */
+		regmap_update_bits(nau8825->regmap, NAU8825_REG_ENA_CTRL,
+			NAU8825_ENABLE_ADC, 0);
 		if (nau8825->mclk_freq)
 			clk_disable_unprepare(nau8825->mclk);
 
@@ -1252,12 +1416,50 @@ static int nau8825_set_bias_level(struct snd_soc_codec *codec,
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int nau8825_codec_suspend(struct snd_soc_codec *codec)
+{
+	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
+
+	disable_irq(nau8825->irq);
+	snd_soc_codec_force_bias_level(codec, SND_SOC_BIAS_OFF);
+	regcache_cache_only(nau8825->regmap, true);
+
+	return 0;
+}
+
+static int nau8825_codec_resume(struct snd_soc_codec *codec)
+{
+	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
+
+	regcache_cache_only(nau8825->regmap, false);
+	if (!nau8825_is_jack_inserted(nau8825->regmap)) {
+		/* Check the jack plug status directly. If the headset is un-
+		 * plugged during S3 when the chip has no power, there will
+		 * be no jack detection irq event after restarting jack detec-
+		 * tion, because the chip just thinks no headset has ever
+		 * been plugged in.
+		 */
+		nau8825_eject_jack(nau8825);
+		snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);
+	}
+	enable_irq(nau8825->irq);
+
+	return 0;
+}
+#else
+#define nau8825_codec_suspend NULL
+#define nau8825_codec_resume NULL
+#endif
+
 static struct snd_soc_codec_driver nau8825_codec_driver = {
 	.probe = nau8825_codec_probe,
 	.set_sysclk = nau8825_set_sysclk,
 	.set_pll = nau8825_set_pll,
 	.set_bias_level = nau8825_set_bias_level,
 	.suspend_bias_off = true,
+	.suspend = nau8825_codec_suspend,
+	.resume = nau8825_codec_resume,
 
 	.controls = nau8825_controls,
 	.num_controls = ARRAY_SIZE(nau8825_controls),
@@ -1351,29 +1553,13 @@ static int nau8825_read_device_properties(struct device *dev,
 
 static int nau8825_setup_irq(struct nau8825 *nau8825)
 {
-	struct regmap *regmap = nau8825->regmap;
 	int ret;
 
-	/* IRQ Output Enable */
-	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
-		NAU8825_IRQ_OUTPUT_EN, NAU8825_IRQ_OUTPUT_EN);
-
-	/* Enable internal VCO needed for interruptions */
-	nau8825_configure_sysclk(nau8825, NAU8825_CLK_INTERNAL, 0);
-
-	/* Enable ADC needed for interrupts */
-	regmap_update_bits(regmap, NAU8825_REG_ENA_CTRL,
-		NAU8825_ENABLE_ADC, NAU8825_ENABLE_ADC);
-
-	/* Chip needs one FSCLK cycle in order to generate interrupts,
-	 * as we cannot guarantee one will be provided by the system. Turning
-	 * master mode on then off enables us to generate that FSCLK cycle
-	 * with a minimum of contention on the clock bus.
+	/* Change jack type detection interruption to non-clock architecture
+	 * for power saving less 1mW. Because resume and initiation all need
+	 * to enable the mechanism, move these configuration to resume setup
+	 * function.
 	 */
-	regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2,
-		NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_MASTER);
-	regmap_update_bits(regmap, NAU8825_REG_I2S_PCM_CTRL2,
-		NAU8825_I2S_MS_MASK, NAU8825_I2S_MS_SLAVE);
 
 	ret = devm_request_threaded_irq(nau8825->dev, nau8825->irq, NULL,
 		nau8825_interrupt, IRQF_TRIGGER_LOW | IRQF_ONESHOT,
@@ -1442,36 +1628,6 @@ static int nau8825_i2c_remove(struct i2c_client *client)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int nau8825_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct nau8825 *nau8825 = dev_get_drvdata(dev);
-
-	disable_irq(client->irq);
-	regcache_cache_only(nau8825->regmap, true);
-	regcache_mark_dirty(nau8825->regmap);
-
-	return 0;
-}
-
-static int nau8825_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct nau8825 *nau8825 = dev_get_drvdata(dev);
-
-	regcache_cache_only(nau8825->regmap, false);
-	regcache_sync(nau8825->regmap);
-	enable_irq(client->irq);
-
-	return 0;
-}
-#endif
-
-static const struct dev_pm_ops nau8825_pm = {
-	SET_SYSTEM_SLEEP_PM_OPS(nau8825_suspend, nau8825_resume)
-};
-
 static const struct i2c_device_id nau8825_i2c_ids[] = {
 	{ "nau8825", 0 },
 	{ }
@@ -1498,7 +1654,6 @@ static struct i2c_driver nau8825_driver = {
 		.name = "nau8825",
 		.of_match_table = of_match_ptr(nau8825_of_ids),
 		.acpi_match_table = ACPI_PTR(nau8825_acpi_match),
-		.pm = &nau8825_pm,
 	},
 	.probe = nau8825_i2c_probe,
 	.remove = nau8825_i2c_remove,
diff --git a/sound/soc/codecs/nau8825.h b/sound/soc/codecs/nau8825.h
index 9e6cb62..1100385 100644
--- a/sound/soc/codecs/nau8825.h
+++ b/sound/soc/codecs/nau8825.h
@@ -93,6 +93,9 @@
 #define NAU8825_REG_CHARGE_PUMP_INPUT_READ		0x81
 #define NAU8825_REG_GENERAL_STATUS		0x82
 #define NAU8825_REG_MAX		NAU8825_REG_GENERAL_STATUS
+/* 16-bit control register address, and 16-bits control register data */
+#define NAU8825_REG_ADDR_LEN		16
+#define NAU8825_REG_DATA_LEN		16
 
 /* ENA_CTRL (0x1) */
 #define NAU8825_ENABLE_DACR_SFT	10
@@ -145,6 +148,7 @@
 
 /* JACK_DET_CTRL (0xd) */
 #define NAU8825_JACK_DET_RESTART	(1 << 9)
+#define NAU8825_JACK_DET_DB_BYPASS	(1 << 8)
 #define NAU8825_JACK_INSERT_DEBOUNCE_SFT	5
 #define NAU8825_JACK_INSERT_DEBOUNCE_MASK	(0x7 << NAU8825_JACK_INSERT_DEBOUNCE_SFT)
 #define NAU8825_JACK_EJECT_DEBOUNCE_SFT		2
@@ -157,6 +161,7 @@
 #define NAU8825_IRQ_KEY_RELEASE_EN (1 << 7)
 #define NAU8825_IRQ_KEY_SHORT_PRESS_EN (1 << 5)
 #define NAU8825_IRQ_EJECT_EN (1 << 2)
+#define NAU8825_IRQ_INSERT_EN (1 << 0)
 
 /* IRQ_STATUS (0x10) */
 #define NAU8825_HEADSET_COMPLETION_IRQ	(1 << 10)
@@ -177,6 +182,7 @@
 #define NAU8825_IRQ_KEY_RELEASE_DIS (1 << 7)
 #define NAU8825_IRQ_KEY_SHORT_PRESS_DIS (1 << 5)
 #define NAU8825_IRQ_EJECT_DIS (1 << 2)
+#define NAU8825_IRQ_INSERT_DIS (1 << 0)
 
 /* SAR_CTRL (0x13) */
 #define NAU8825_SAR_ADC_EN_SFT	12
@@ -333,7 +339,8 @@
 
 /* System Clock Source */
 enum {
-	NAU8825_CLK_MCLK = 0,
+	NAU8825_CLK_DIS = 0,
+	NAU8825_CLK_MCLK,
 	NAU8825_CLK_INTERNAL,
 	NAU8825_CLK_FLL_MCLK,
 	NAU8825_CLK_FLL_BLK,
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] ASoC: nau8825: cross talk suppression measurement function
  2016-04-29  8:15 [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby John Hsu
@ 2016-04-29  8:15 ` John Hsu
  2016-05-02 16:27 ` [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby Mark Brown
  1 sibling, 0 replies; 13+ messages in thread
From: John Hsu @ 2016-04-29  8:15 UTC (permalink / raw)
  To: broonie
  Cc: alsa-devel, anatol.pomozov, YHCHuang, John Hsu, lgirdwood, benzh,
	CTLIN0, mhkuo, yong.zhi

The cross talk measurement function can reduce cross talk across the JKTIP
HPL) and JKR1(HPR) outputs which measures the cross talk signal level to
determine what is the cross talk reduction gain. This system works by
sending a 23Hz -24dBV sine wave into the headset output DAC and through
the PGA. The output of the PGA is then connected to an internal current
sense which measures the attenuated 23Hz signal and passing the output to
an ADC which converts the measurement to a binary code. With two separated
measurement, one for JKR1(HPR) and the other JKTIP(HPL), measurement data
can be separated read in IMM_RMS_L for HSR and HSL after each measurement.

Thus, the measurement function has four states to complete whole sequence.
(1)Prepare state : Prepare the resource for detection and transfer to HPR
IMM stat to make JKR1(HPR) impedance measure.
(2)HPR IMM state : Read out orignal signal level of JKR1(HPR) and transfer
to HPL IMM state to make JKTIP(HPL) impedance measure.
(3)HPL IMM state : Read out cross talk signal level of JKTIP(HPL) and
transfer to IMM state to determine suppression sidetone gain.
(4)IMM state : Computes cross talk suppression sidetone gain with orignal
and cross talk signal level. Apply this gain and then restore codec con-
figuration. Then transfer to Done state for ending.

In order to get the cross talk suppression sidetone gain, we need the
function to compute log10 value and the result is round off to 3 decimal.
This function takes reference to dvb-math. The source code locates as the
following. "Linux/drivers/media/dvb-core/dvb_math.c"
Then, the orignal and cross talk signal vlues need to be characterized.
The sidetone value can be converted to decibel with the equation below.
sidetone = 20 * log (original signal level / crosstalk signal level)

Besides, the state machine for cross talk process needs interruptions to
trigger worked. We have the RMS intrruption enabled with the internal VCO
clock when headset connected. In the interrupt handler, the driver will
judge the headset is high impedance or not. If yes, the cross talk supp-
ression shouldn't apply and do nothing but relieve the protection raised
before. Otherwise, apply the cross talk suppression in the headset and
start the process.

Because the process spends a lot of time, there is an resource race issue
easily between the application and interruption. They will control codec
power and clock concurrently. In one situaiton, the jack is inserted when
playback, and then the application changes to headset device. The applica-
tion prepares the playback and interrupt handler raises work for cross
talk process together. For this case, the solution is that driver delays
soc jack report until cross talk process completes. The mechanism can
avoid application to do playback preparation before cross talk detection
is still working.
In another situaiton, the system suspends when playback. After resume, the
system restarts playback, and meanwhile jack detection restarts. The play-
back preparation and cross talk process triggered by interruptions happens
concurrently. For the case, the driver provides the semaphone to syn-
chronize the playback and interrupt handler. In order to avoid the play-
back interfered by cross talk process, the driver make the playback prepa-
ration halted until cross talk process finish. After codec resume, the
driver finds the codec dai is active, and then the driver raises the pro-
tection for cross talk function to avoid the playback recovers before
cross talk process finish.

The driver also provides cancel method to forcely cancel the cross talk
task and restores the configuration to original status. Before the codec
remove, ejection, or suspend, the driver is obliged to cancel the cross
talk detection process. It can reduce the risk of failure when quickly and
continually doing jack insertion and ejection.


Signed-off-by: John Hsu <KCHSU0@nuvoton.com>
---
 sound/soc/codecs/nau8825.c | 786 ++++++++++++++++++++++++++++++++++++++++++++-
 sound/soc/codecs/nau8825.h |  82 ++++-
 2 files changed, 865 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/nau8825.c b/sound/soc/codecs/nau8825.c
index 9386624..4e7f29d 100644
--- a/sound/soc/codecs/nau8825.c
+++ b/sound/soc/codecs/nau8825.c
@@ -18,6 +18,7 @@
 #include <linux/clk.h>
 #include <linux/acpi.h>
 #include <linux/math64.h>
+#include <linux/semaphore.h>
 
 #include <sound/initval.h>
 #include <sound/tlv.h>
@@ -37,6 +38,12 @@
 #define NAU_FVCO_MAX 124000000
 #define NAU_FVCO_MIN 90000000
 
+/* cross talk suppression detection */
+#define LOG10_MAGIC 646456993
+#define GAIN_AUGMENT 22500
+#define SIDETONE_BASE 207000
+
+
 static int nau8825_configure_sysclk(struct nau8825 *nau8825,
 		int clk_id, unsigned int freq);
 
@@ -162,6 +169,661 @@ static const struct reg_default nau8825_reg_defaults[] = {
 	{ NAU8825_REG_CHARGE_PUMP, 0x0 },
 };
 
+/* register backup table when cross talk detection */
+static struct reg_default nau8825_xtalk_baktab[] = {
+	{ NAU8825_REG_ADC_DGAIN_CTRL, 0 },
+	{ NAU8825_REG_HSVOL_CTRL, 0 },
+	{ NAU8825_REG_DACL_CTRL, 0 },
+	{ NAU8825_REG_DACR_CTRL, 0 },
+};
+
+static const unsigned short logtable[256] = {
+	0x0000, 0x0171, 0x02e0, 0x044e, 0x05ba, 0x0725, 0x088e, 0x09f7,
+	0x0b5d, 0x0cc3, 0x0e27, 0x0f8a, 0x10eb, 0x124b, 0x13aa, 0x1508,
+	0x1664, 0x17bf, 0x1919, 0x1a71, 0x1bc8, 0x1d1e, 0x1e73, 0x1fc6,
+	0x2119, 0x226a, 0x23ba, 0x2508, 0x2656, 0x27a2, 0x28ed, 0x2a37,
+	0x2b80, 0x2cc8, 0x2e0f, 0x2f54, 0x3098, 0x31dc, 0x331e, 0x345f,
+	0x359f, 0x36de, 0x381b, 0x3958, 0x3a94, 0x3bce, 0x3d08, 0x3e41,
+	0x3f78, 0x40af, 0x41e4, 0x4319, 0x444c, 0x457f, 0x46b0, 0x47e1,
+	0x4910, 0x4a3f, 0x4b6c, 0x4c99, 0x4dc5, 0x4eef, 0x5019, 0x5142,
+	0x526a, 0x5391, 0x54b7, 0x55dc, 0x5700, 0x5824, 0x5946, 0x5a68,
+	0x5b89, 0x5ca8, 0x5dc7, 0x5ee5, 0x6003, 0x611f, 0x623a, 0x6355,
+	0x646f, 0x6588, 0x66a0, 0x67b7, 0x68ce, 0x69e4, 0x6af8, 0x6c0c,
+	0x6d20, 0x6e32, 0x6f44, 0x7055, 0x7165, 0x7274, 0x7383, 0x7490,
+	0x759d, 0x76aa, 0x77b5, 0x78c0, 0x79ca, 0x7ad3, 0x7bdb, 0x7ce3,
+	0x7dea, 0x7ef0, 0x7ff6, 0x80fb, 0x81ff, 0x8302, 0x8405, 0x8507,
+	0x8608, 0x8709, 0x8809, 0x8908, 0x8a06, 0x8b04, 0x8c01, 0x8cfe,
+	0x8dfa, 0x8ef5, 0x8fef, 0x90e9, 0x91e2, 0x92db, 0x93d2, 0x94ca,
+	0x95c0, 0x96b6, 0x97ab, 0x98a0, 0x9994, 0x9a87, 0x9b7a, 0x9c6c,
+	0x9d5e, 0x9e4f, 0x9f3f, 0xa02e, 0xa11e, 0xa20c, 0xa2fa, 0xa3e7,
+	0xa4d4, 0xa5c0, 0xa6ab, 0xa796, 0xa881, 0xa96a, 0xaa53, 0xab3c,
+	0xac24, 0xad0c, 0xadf2, 0xaed9, 0xafbe, 0xb0a4, 0xb188, 0xb26c,
+	0xb350, 0xb433, 0xb515, 0xb5f7, 0xb6d9, 0xb7ba, 0xb89a, 0xb97a,
+	0xba59, 0xbb38, 0xbc16, 0xbcf4, 0xbdd1, 0xbead, 0xbf8a, 0xc065,
+	0xc140, 0xc21b, 0xc2f5, 0xc3cf, 0xc4a8, 0xc580, 0xc658, 0xc730,
+	0xc807, 0xc8de, 0xc9b4, 0xca8a, 0xcb5f, 0xcc34, 0xcd08, 0xcddc,
+	0xceaf, 0xcf82, 0xd054, 0xd126, 0xd1f7, 0xd2c8, 0xd399, 0xd469,
+	0xd538, 0xd607, 0xd6d6, 0xd7a4, 0xd872, 0xd93f, 0xda0c, 0xdad9,
+	0xdba5, 0xdc70, 0xdd3b, 0xde06, 0xded0, 0xdf9a, 0xe063, 0xe12c,
+	0xe1f5, 0xe2bd, 0xe385, 0xe44c, 0xe513, 0xe5d9, 0xe69f, 0xe765,
+	0xe82a, 0xe8ef, 0xe9b3, 0xea77, 0xeb3b, 0xebfe, 0xecc1, 0xed83,
+	0xee45, 0xef06, 0xefc8, 0xf088, 0xf149, 0xf209, 0xf2c8, 0xf387,
+	0xf446, 0xf505, 0xf5c3, 0xf680, 0xf73e, 0xf7fb, 0xf8b7, 0xf973,
+	0xfa2f, 0xfaea, 0xfba5, 0xfc60, 0xfd1a, 0xfdd4, 0xfe8e, 0xff47
+};
+
+static struct snd_soc_dai *nau8825_get_codec_dai(struct nau8825 *nau8825)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(nau8825->dapm);
+	struct snd_soc_component *component = &codec->component;
+	struct snd_soc_dai *codec_dai, *_dai;
+
+	list_for_each_entry_safe(codec_dai, _dai, &component->dai_list, list) {
+		if (!strncmp(codec_dai->name, NUVOTON_CODEC_DAI,
+			strlen(NUVOTON_CODEC_DAI)))
+			return codec_dai;
+	}
+	return NULL;
+}
+
+static bool nau8825_dai_is_active(struct nau8825 *nau8825)
+{
+	struct snd_soc_dai *codec_dai = nau8825_get_codec_dai(nau8825);
+
+	if (codec_dai) {
+		if (codec_dai->playback_active || codec_dai->capture_active)
+			return true;
+	}
+	return false;
+}
+
+/**
+ * nau8825_sema_acquire - acquire the semaphore of nau88l25
+ * @nau8825:  component to register the codec private data with
+ * @timeout: how long in jiffies to wait before failure or zero to wait
+ * until release
+ *
+ * Attempts to acquire the semaphore with number of jiffies. If no more
+ * tasks are allowed to acquire the semaphore, calling this function will
+ * put the task to sleep. If the semaphore is not released within the
+ * specified number of jiffies, this function returns.
+ * Acquires the semaphore without jiffies. If no more tasks are allowed
+ * to acquire the semaphore, calling this function will put the task to
+ * sleep until the semaphore is released.
+ * It returns if the semaphore was acquired.
+ */
+static void nau8825_sema_acquire(struct nau8825 *nau8825, long timeout)
+{
+	int ret;
+
+	if (timeout)
+		ret = down_timeout(&nau8825->xtalk_sem, timeout);
+	else
+		ret = down_interruptible(&nau8825->xtalk_sem);
+
+	if (ret < 0)
+		dev_warn(nau8825->dev, "Acquire semaphone fail\n");
+}
+
+/**
+ * nau8825_sema_release - release the semaphore of nau88l25
+ * @nau8825:  component to register the codec private data with
+ *
+ * Release the semaphore which may be called from any context and
+ * even by tasks which have never called down().
+ */
+static inline void nau8825_sema_release(struct nau8825 *nau8825)
+{
+	up(&nau8825->xtalk_sem);
+}
+
+/**
+ * nau8825_sema_reset - reset the semaphore for nau88l25
+ * @nau8825:  component to register the codec private data with
+ *
+ * Reset the counter of the semaphore. Call this function to restart
+ * a new round task management.
+ */
+static inline void nau8825_sema_reset(struct nau8825 *nau8825)
+{
+	nau8825->xtalk_sem.count = 1;
+}
+
+/**
+ * Ramp up the headphone volume change gradually to target level.
+ *
+ * @nau8825:  component to register the codec private data with
+ * @vol_from: the volume to start up
+ * @vol_to: the target volume
+ * @step: the volume span to move on
+ *
+ * The headphone volume is from 0dB to minimum -54dB and -1dB per step.
+ * If the volume changes sharp, there is a pop noise heard in headphone. We
+ * provide the function to ramp up the volume up or down by delaying 10ms
+ * per step.
+ */
+static void nau8825_hpvol_ramp(struct nau8825 *nau8825,
+	unsigned int vol_from, unsigned int vol_to, unsigned int step)
+{
+	unsigned int value, volume, ramp_up, from, to;
+
+	if (vol_from == vol_to || step == 0) {
+		return;
+	} else if (vol_from < vol_to) {
+		ramp_up = true;
+		from = vol_from;
+		to = vol_to;
+	} else {
+		ramp_up = false;
+		from = vol_to;
+		to = vol_from;
+	}
+	/* only handle volume from 0dB to minimum -54dB */
+	if (to > NAU8825_HP_VOL_MIN)
+		to = NAU8825_HP_VOL_MIN;
+
+	for (volume = from; volume < to; volume += step) {
+		if (ramp_up)
+			value = volume;
+		else
+			value = to - volume + from;
+		regmap_update_bits(nau8825->regmap, NAU8825_REG_HSVOL_CTRL,
+			NAU8825_HPL_VOL_MASK | NAU8825_HPR_VOL_MASK,
+			(value << NAU8825_HPL_VOL_SFT) | value);
+		usleep_range(10000, 10500);
+	}
+	if (ramp_up)
+		value = to;
+	else
+		value = from;
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_HSVOL_CTRL,
+		NAU8825_HPL_VOL_MASK | NAU8825_HPR_VOL_MASK,
+		(value << NAU8825_HPL_VOL_SFT) | value);
+}
+
+/**
+ * Computes log10 of a value; the result is round off to 3 decimal. This func-
+ * tion takes reference to dvb-math. The source code locates as the following.
+ * Linux/drivers/media/dvb-core/dvb_math.c
+ *
+ * return log10(value) * 1000
+ */
+static u32 nau8825_intlog10_dec3(u32 value)
+{
+	u32 msb, logentry, significand, interpolation, log10val;
+	u64 log2val;
+
+	/* first detect the msb (count begins at 0) */
+	msb = fls(value) - 1;
+	/**
+	 *      now we use a logtable after the following method:
+	 *
+	 *      log2(2^x * y) * 2^24 = x * 2^24 + log2(y) * 2^24
+	 *      where x = msb and therefore 1 <= y < 2
+	 *      first y is determined by shifting the value left
+	 *      so that msb is bit 31
+	 *              0x00231f56 -> 0x8C7D5800
+	 *      the result is y * 2^31 -> "significand"
+	 *      then the highest 9 bits are used for a table lookup
+	 *      the highest bit is discarded because it's always set
+	 *      the highest nine bits in our example are 100011000
+	 *      so we would use the entry 0x18
+	 */
+	significand = value << (31 - msb);
+	logentry = (significand >> 23) & 0xff;
+	/**
+	 *      last step we do is interpolation because of the
+	 *      limitations of the log table the error is that part of
+	 *      the significand which isn't used for lookup then we
+	 *      compute the ratio between the error and the next table entry
+	 *      and interpolate it between the log table entry used and the
+	 *      next one the biggest error possible is 0x7fffff
+	 *      (in our example it's 0x7D5800)
+	 *      needed value for next table entry is 0x800000
+	 *      so the interpolation is
+	 *      (error / 0x800000) * (logtable_next - logtable_current)
+	 *      in the implementation the division is moved to the end for
+	 *      better accuracy there is also an overflow correction if
+	 *      logtable_next is 256
+	 */
+	interpolation = ((significand & 0x7fffff) *
+		((logtable[(logentry + 1) & 0xff] -
+		logtable[logentry]) & 0xffff)) >> 15;
+
+	log2val = ((msb << 24) + (logtable[logentry] << 8) + interpolation);
+	/**
+	 *      log10(x) = log2(x) * log10(2)
+	 */
+	log10val = (log2val * LOG10_MAGIC) >> 31;
+	/**
+	 *      the result is round off to 3 decimal
+	 */
+	return log10val / ((1 << 24) / 1000);
+}
+
+/**
+ * computes cross talk suppression sidetone gain.
+ *
+ * @sig_org: orignal signal level
+ * @sig_cros: cross talk signal level
+ *
+ * The orignal and cross talk signal vlues need to be characterized.
+ * Once these values have been characterized, this sidetone value
+ * can be converted to decibel with the equation below.
+ * sidetone = 20 * log (original signal level / crosstalk signal level)
+ *
+ * return cross talk sidetone gain
+ */
+static u32 nau8825_xtalk_sidetone(u32 sig_org, u32 sig_cros)
+{
+	u32 gain, sidetone;
+
+	if (unlikely(sig_org == 0) || unlikely(sig_cros == 0)) {
+		WARN_ON(1);
+		return 0;
+	}
+
+	sig_org = nau8825_intlog10_dec3(sig_org);
+	sig_cros = nau8825_intlog10_dec3(sig_cros);
+	if (sig_org >= sig_cros)
+		gain = (sig_org - sig_cros) * 20 + GAIN_AUGMENT;
+	else
+		gain = (sig_cros - sig_org) * 20 + GAIN_AUGMENT;
+	sidetone = SIDETONE_BASE - gain * 2;
+	sidetone /= 1000;
+
+	return sidetone;
+}
+
+static int nau8825_xtalk_baktab_index_by_reg(unsigned int reg)
+{
+	int index;
+
+	for (index = 0; index < ARRAY_SIZE(nau8825_xtalk_baktab); index++)
+		if (nau8825_xtalk_baktab[index].reg == reg)
+			return index;
+	return -EINVAL;
+}
+
+static void nau8825_xtalk_backup(struct nau8825 *nau8825)
+{
+	int i;
+
+	/* Backup some register values to backup table */
+	for (i = 0; i < ARRAY_SIZE(nau8825_xtalk_baktab); i++)
+		regmap_read(nau8825->regmap, nau8825_xtalk_baktab[i].reg,
+				&nau8825_xtalk_baktab[i].def);
+}
+
+static void nau8825_xtalk_restore(struct nau8825 *nau8825)
+{
+	int i, volume;
+
+	/* Restore register values from backup table; When the driver restores
+	 * the headphone volumem, it needs recover to original level gradually
+	 * with 3dB per step for less pop noise.
+	 */
+	for (i = 0; i < ARRAY_SIZE(nau8825_xtalk_baktab); i++) {
+		if (nau8825_xtalk_baktab[i].reg == NAU8825_REG_HSVOL_CTRL) {
+			/* Ramping up the volume change to reduce pop noise */
+			volume = nau8825_xtalk_baktab[i].def &
+				NAU8825_HPR_VOL_MASK;
+			nau8825_hpvol_ramp(nau8825, 0, volume, 3);
+			continue;
+		}
+		regmap_write(nau8825->regmap, nau8825_xtalk_baktab[i].reg,
+				nau8825_xtalk_baktab[i].def);
+	}
+}
+
+static void nau8825_xtalk_prepare_dac(struct nau8825 *nau8825)
+{
+	/* Enable power of DAC path */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_ENA_CTRL,
+		NAU8825_ENABLE_DACR | NAU8825_ENABLE_DACL |
+		NAU8825_ENABLE_ADC | NAU8825_ENABLE_ADC_CLK |
+		NAU8825_ENABLE_DAC_CLK, NAU8825_ENABLE_DACR |
+		NAU8825_ENABLE_DACL | NAU8825_ENABLE_ADC |
+		NAU8825_ENABLE_ADC_CLK | NAU8825_ENABLE_DAC_CLK);
+	/* Prevent startup click by letting charge pump to ramp up and
+	 * change bump enable
+	 */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CHARGE_PUMP,
+		NAU8825_JAMNODCLOW | NAU8825_CHANRGE_PUMP_EN,
+		NAU8825_JAMNODCLOW | NAU8825_CHANRGE_PUMP_EN);
+	/* Enable clock sync of DAC and DAC clock */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_RDAC,
+		NAU8825_RDAC_EN | NAU8825_RDAC_CLK_EN |
+		NAU8825_RDAC_FS_BCLK_ENB,
+		NAU8825_RDAC_EN | NAU8825_RDAC_CLK_EN);
+	/* Power up output driver with 2 stage */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_POWER_UP_CONTROL,
+		NAU8825_POWERUP_INTEGR_R | NAU8825_POWERUP_INTEGR_L |
+		NAU8825_POWERUP_DRV_IN_R | NAU8825_POWERUP_DRV_IN_L,
+		NAU8825_POWERUP_INTEGR_R | NAU8825_POWERUP_INTEGR_L |
+		NAU8825_POWERUP_DRV_IN_R | NAU8825_POWERUP_DRV_IN_L);
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_POWER_UP_CONTROL,
+		NAU8825_POWERUP_HP_DRV_R | NAU8825_POWERUP_HP_DRV_L,
+		NAU8825_POWERUP_HP_DRV_R | NAU8825_POWERUP_HP_DRV_L);
+	/* HP outputs not shouted to ground  */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_HSD_CTRL,
+		NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L, 0);
+	/* Enable HP boost driver */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_BOOST,
+		NAU8825_HP_BOOST_DIS, NAU8825_HP_BOOST_DIS);
+	/* Enable class G compare path to supply 1.8V or 0.9V. */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CLASSG_CTRL,
+		NAU8825_CLASSG_LDAC_EN | NAU8825_CLASSG_RDAC_EN,
+		NAU8825_CLASSG_LDAC_EN | NAU8825_CLASSG_RDAC_EN);
+}
+
+static void nau8825_xtalk_prepare_adc(struct nau8825 *nau8825)
+{
+	/* Power up left ADC and raise 5dB than Vmid for Vref  */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_ANALOG_ADC_2,
+		NAU8825_POWERUP_ADCL | NAU8825_ADC_VREFSEL_MASK,
+		NAU8825_POWERUP_ADCL | NAU8825_ADC_VREFSEL_VMID_PLUS_0_5DB);
+}
+
+static void nau8825_xtalk_clock(struct nau8825 *nau8825)
+{
+	/* Recover FLL default value */
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL1, 0x0);
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL2, 0x3126);
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL3, 0x0008);
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL4, 0x0010);
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL5, 0x0);
+	regmap_write(nau8825->regmap, NAU8825_REG_FLL6, 0x6000);
+	/* Enable internal VCO clock for detection signal generated */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CLK_DIVIDER,
+		NAU8825_CLK_SRC_MASK, NAU8825_CLK_SRC_VCO);
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_FLL6, NAU8825_DCO_EN,
+		NAU8825_DCO_EN);
+	/* Given specific clock frequency of internal clock to
+	 * generate signal.
+	 */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CLK_DIVIDER,
+		NAU8825_CLK_MCLK_SRC_MASK, 0xf);
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_FLL1,
+		NAU8825_FLL_RATIO_MASK, 0x10);
+}
+
+static void nau8825_xtalk_prepare(struct nau8825 *nau8825)
+{
+	int volume, index;
+
+	/* Backup those registers changed by cross talk detection */
+	nau8825_xtalk_backup(nau8825);
+	/* Config IIS as master to output signal by codec */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_I2S_PCM_CTRL2,
+		NAU8825_I2S_MS_MASK | NAU8825_I2S_DRV_MASK |
+		NAU8825_I2S_BLK_DIV_MASK, NAU8825_I2S_MS_MASTER |
+		(0x2 << NAU8825_I2S_DRV_SFT) | 0x1);
+	/* Ramp up headphone volume to 0dB to get better performance and
+	 * avoid pop noise in headphone.
+	 */
+	index = nau8825_xtalk_baktab_index_by_reg(NAU8825_REG_HSVOL_CTRL);
+	if (index != -EINVAL) {
+		volume = nau8825_xtalk_baktab[index].def &
+				NAU8825_HPR_VOL_MASK;
+		nau8825_hpvol_ramp(nau8825, volume, 0, 3);
+	}
+	nau8825_xtalk_clock(nau8825);
+	nau8825_xtalk_prepare_dac(nau8825);
+	nau8825_xtalk_prepare_adc(nau8825);
+	/* Config channel path and digital gain */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_DACL_CTRL,
+		NAU8825_DACL_CH_SEL_MASK | NAU8825_DACL_CH_VOL_MASK,
+		NAU8825_DACL_CH_SEL_L | 0xab);
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_DACR_CTRL,
+		NAU8825_DACR_CH_SEL_MASK | NAU8825_DACR_CH_VOL_MASK,
+		NAU8825_DACR_CH_SEL_R | 0xab);
+	/* Config cross talk parameters and generate the 23Hz sine wave with
+	 * 1/16 full scale of signal level for impedance measurement.
+	 */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_IMM_MODE_CTRL,
+		NAU8825_IMM_THD_MASK | NAU8825_IMM_GEN_VOL_MASK |
+		NAU8825_IMM_CYC_MASK | NAU8825_IMM_DAC_SRC_MASK,
+		(0x9 << NAU8825_IMM_THD_SFT) | NAU8825_IMM_GEN_VOL_1_16th |
+		NAU8825_IMM_CYC_8192 | NAU8825_IMM_DAC_SRC_SIN);
+	/* RMS intrruption enable */
+	regmap_update_bits(nau8825->regmap,
+		NAU8825_REG_INTERRUPT_MASK, NAU8825_IRQ_RMS_EN, 0);
+	/* Power up left and right DAC */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CHARGE_PUMP,
+		NAU8825_POWER_DOWN_DACR | NAU8825_POWER_DOWN_DACL, 0);
+}
+
+static void nau8825_xtalk_clean_dac(struct nau8825 *nau8825)
+{
+	/* Disable HP boost driver */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_BOOST,
+		NAU8825_HP_BOOST_DIS, 0);
+	/* HP outputs shouted to ground  */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_HSD_CTRL,
+		NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L,
+		NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L);
+	/* Power down left and right DAC */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CHARGE_PUMP,
+		NAU8825_POWER_DOWN_DACR | NAU8825_POWER_DOWN_DACL,
+		NAU8825_POWER_DOWN_DACR | NAU8825_POWER_DOWN_DACL);
+	/* Enable the TESTDAC and  disable L/R HP impedance */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_BIAS_ADJ,
+		NAU8825_BIAS_HPR_IMP | NAU8825_BIAS_HPL_IMP |
+		NAU8825_BIAS_TESTDAC_EN, NAU8825_BIAS_TESTDAC_EN);
+	/* Power down output driver with 2 stage */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_POWER_UP_CONTROL,
+		NAU8825_POWERUP_HP_DRV_R | NAU8825_POWERUP_HP_DRV_L, 0);
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_POWER_UP_CONTROL,
+		NAU8825_POWERUP_INTEGR_R | NAU8825_POWERUP_INTEGR_L |
+		NAU8825_POWERUP_DRV_IN_R | NAU8825_POWERUP_DRV_IN_L, 0);
+	/* Disable clock sync of DAC and DAC clock */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_RDAC,
+		NAU8825_RDAC_EN | NAU8825_RDAC_CLK_EN, 0);
+	/* Disable charge pump ramp up function and change bump */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_CHARGE_PUMP,
+		NAU8825_JAMNODCLOW | NAU8825_CHANRGE_PUMP_EN, 0);
+	/* Disable power of DAC path */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_ENA_CTRL,
+		NAU8825_ENABLE_DACR | NAU8825_ENABLE_DACL |
+		NAU8825_ENABLE_ADC_CLK | NAU8825_ENABLE_DAC_CLK, 0);
+	if (!nau8825->irq)
+		regmap_update_bits(nau8825->regmap,
+			NAU8825_REG_ENA_CTRL, NAU8825_ENABLE_ADC, 0);
+}
+
+static void nau8825_xtalk_clean_adc(struct nau8825 *nau8825)
+{
+	/* Power down left ADC and restore voltage to Vmid */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_ANALOG_ADC_2,
+		NAU8825_POWERUP_ADCL | NAU8825_ADC_VREFSEL_MASK, 0);
+}
+
+static void nau8825_xtalk_clean(struct nau8825 *nau8825)
+{
+	/* Enable internal VCO needed for interruptions */
+	nau8825_configure_sysclk(nau8825, NAU8825_CLK_INTERNAL, 0);
+	nau8825_xtalk_clean_dac(nau8825);
+	nau8825_xtalk_clean_adc(nau8825);
+	/* Clear cross talk parameters and disable */
+	regmap_write(nau8825->regmap, NAU8825_REG_IMM_MODE_CTRL, 0);
+	/* RMS intrruption disable */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_INTERRUPT_MASK,
+		NAU8825_IRQ_RMS_EN, NAU8825_IRQ_RMS_EN);
+	/* Recover default value for IIS */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_I2S_PCM_CTRL2,
+		NAU8825_I2S_MS_MASK | NAU8825_I2S_DRV_MASK |
+		NAU8825_I2S_BLK_DIV_MASK, NAU8825_I2S_MS_SLAVE);
+	/* Restore value of specific register for cross talk */
+	nau8825_xtalk_restore(nau8825);
+}
+
+static void nau8825_xtalk_imm_start(struct nau8825 *nau8825, int vol)
+{
+	/* Apply ADC volume for better cross talk performance */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_ADC_DGAIN_CTRL,
+				NAU8825_ADC_DIG_VOL_MASK, vol);
+	/* Disables JKTIP(HPL) DAC channel for right to left measurement.
+	 * Do it before sending signal in order to erase pop noise.
+	 */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_BIAS_ADJ,
+		NAU8825_BIAS_TESTDACR_EN | NAU8825_BIAS_TESTDACL_EN,
+		NAU8825_BIAS_TESTDACL_EN);
+	switch (nau8825->xtalk_state) {
+	case NAU8825_XTALK_HPR_R2L:
+		/* Enable right headphone impedance */
+		regmap_update_bits(nau8825->regmap, NAU8825_REG_BIAS_ADJ,
+			NAU8825_BIAS_HPR_IMP | NAU8825_BIAS_HPL_IMP,
+			NAU8825_BIAS_HPR_IMP);
+		break;
+	case NAU8825_XTALK_HPL_R2L:
+		/* Enable left headphone impedance */
+		regmap_update_bits(nau8825->regmap, NAU8825_REG_BIAS_ADJ,
+			NAU8825_BIAS_HPR_IMP | NAU8825_BIAS_HPL_IMP,
+			NAU8825_BIAS_HPL_IMP);
+		break;
+	default:
+		break;
+	}
+	msleep(100);
+	/* Impedance measurement mode enable */
+	regmap_update_bits(nau8825->regmap, NAU8825_REG_IMM_MODE_CTRL,
+				NAU8825_IMM_EN, NAU8825_IMM_EN);
+}
+
+static void nau8825_xtalk_imm_stop(struct nau8825 *nau8825)
+{
+	/* Impedance measurement mode disable */
+	regmap_update_bits(nau8825->regmap,
+		NAU8825_REG_IMM_MODE_CTRL, NAU8825_IMM_EN, 0);
+}
+
+/* The cross talk measurement function can reduce cross talk across the
+ * JKTIP(HPL) and JKR1(HPR) outputs which measures the cross talk signal
+ * level to determine what cross talk reduction gain is. This system works by
+ * sending a 23Hz -24dBV sine wave into the headset output DAC and through
+ * the PGA. The output of the PGA is then connected to an internal current
+ * sense which measures the attenuated 23Hz signal and passing the output to
+ * an ADC which converts the measurement to a binary code. With two separated
+ * measurement, one for JKR1(HPR) and the other JKTIP(HPL), measurement data
+ * can be separated read in IMM_RMS_L for HSR and HSL after each measurement.
+ * Thus, the measurement function has four states to complete whole sequence.
+ * 1. Prepare state : Prepare the resource for detection and transfer to HPR
+ *     IMM stat to make JKR1(HPR) impedance measure.
+ * 2. HPR IMM state : Read out orignal signal level of JKR1(HPR) and transfer
+ *     to HPL IMM state to make JKTIP(HPL) impedance measure.
+ * 3. HPL IMM state : Read out cross talk signal level of JKTIP(HPL) and
+ *     transfer to IMM state to determine suppression sidetone gain.
+ * 4. IMM state : Computes cross talk suppression sidetone gain with orignal
+ *     and cross talk signal level. Apply this gain and then restore codec
+ *     configuration. Then transfer to Done state for ending.
+ */
+static void nau8825_xtalk_measure(struct nau8825 *nau8825)
+{
+	u32 sidetone;
+
+	switch (nau8825->xtalk_state) {
+	case NAU8825_XTALK_PREPARE:
+		/* In prepare state, set up clock, intrruption, DAC path, ADC
+		 * path and cross talk detection parameters for preparation.
+		 */
+		nau8825_xtalk_prepare(nau8825);
+		msleep(280);
+		/* Trigger right headphone impedance detection */
+		nau8825->xtalk_state = NAU8825_XTALK_HPR_R2L;
+		nau8825_xtalk_imm_start(nau8825, 0x00d2);
+		break;
+	case NAU8825_XTALK_HPR_R2L:
+		/* In right headphone IMM state, read out right headphone
+		 * impedance measure result, and then start up left side.
+		 */
+		regmap_read(nau8825->regmap, NAU8825_REG_IMM_RMS_L,
+			&nau8825->imp_rms[NAU8825_XTALK_HPR_R2L]);
+		dev_dbg(nau8825->dev, "HPR_R2L imm: %x\n",
+			nau8825->imp_rms[NAU8825_XTALK_HPR_R2L]);
+		/* Disable then re-enable IMM mode to update */
+		nau8825_xtalk_imm_stop(nau8825);
+		/* Trigger left headphone impedance detection */
+		nau8825->xtalk_state = NAU8825_XTALK_HPL_R2L;
+		nau8825_xtalk_imm_start(nau8825, 0x00ff);
+		break;
+	case NAU8825_XTALK_HPL_R2L:
+		/* In left headphone IMM state, read out left headphone
+		 * impedance measure result, and delay some time to wait
+		 * detection sine wave output finish. Then, we can calculate
+		 * the cross talk suppresstion side tone according to the L/R
+		 * headphone imedance.
+		 */
+		regmap_read(nau8825->regmap, NAU8825_REG_IMM_RMS_L,
+			&nau8825->imp_rms[NAU8825_XTALK_HPL_R2L]);
+		dev_dbg(nau8825->dev, "HPL_R2L imm: %x\n",
+			nau8825->imp_rms[NAU8825_XTALK_HPL_R2L]);
+		nau8825_xtalk_imm_stop(nau8825);
+		msleep(150);
+		nau8825->xtalk_state = NAU8825_XTALK_IMM;
+		break;
+	case NAU8825_XTALK_IMM:
+		/* In impedance measure state, the orignal and cross talk
+		 * signal level vlues are ready. The side tone gain is deter-
+		 * mined with these signal level. After all, restore codec
+		 * configuration.
+		 */
+		sidetone = nau8825_xtalk_sidetone(
+			nau8825->imp_rms[NAU8825_XTALK_HPR_R2L],
+			nau8825->imp_rms[NAU8825_XTALK_HPL_R2L]);
+		dev_dbg(nau8825->dev, "cross talk sidetone: %x\n", sidetone);
+		regmap_write(nau8825->regmap, NAU8825_REG_DAC_DGAIN_CTRL,
+					(sidetone << 8) | sidetone);
+		nau8825_xtalk_clean(nau8825);
+		nau8825->xtalk_state = NAU8825_XTALK_DONE;
+		break;
+	default:
+		break;
+	}
+}
+
+static void nau8825_xtalk_work(struct work_struct *work)
+{
+	struct nau8825 *nau8825 = container_of(
+		work, struct nau8825, xtalk_work);
+
+	nau8825_xtalk_measure(nau8825);
+	/* To determine the cross talk side tone gain when reach
+	 * the impedance measure state.
+	 */
+	if (nau8825->xtalk_state == NAU8825_XTALK_IMM)
+		nau8825_xtalk_measure(nau8825);
+
+	/* Delay jack report until cross talk detection process
+	 * completed. It can avoid application to do playback
+	 * preparation before cross talk detection is still working.
+	 * Meanwhile, the protection of the cross talk detection
+	 * is released.
+	 */
+	if (nau8825->xtalk_state == NAU8825_XTALK_DONE) {
+		snd_soc_jack_report(nau8825->jack, nau8825->xtalk_event,
+				nau8825->xtalk_event_mask);
+		nau8825_sema_release(nau8825);
+		nau8825->xtalk_protect = false;
+	}
+}
+
+static void nau8825_xtalk_cancel(struct nau8825 *nau8825)
+{
+	/* If the xtalk_protect is true, that means the process is still
+	 * on going. The driver forces to cancel the cross talk task and
+	 * restores the configuration to original status.
+	 */
+	if (nau8825->xtalk_protect) {
+		cancel_work_sync(&nau8825->xtalk_work);
+		nau8825_xtalk_clean(nau8825);
+	}
+	/* Reset parameters for cross talk suppression function */
+	nau8825_sema_reset(nau8825);
+	nau8825->xtalk_state = NAU8825_XTALK_DONE;
+	nau8825->xtalk_protect = false;
+}
+
 static bool nau8825_readable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -674,6 +1336,9 @@ static void nau8825_eject_jack(struct nau8825 *nau8825)
 	struct snd_soc_dapm_context *dapm = nau8825->dapm;
 	struct regmap *regmap = nau8825->regmap;
 
+	/* Force to cancel the cross talk detection process */
+	nau8825_xtalk_cancel(nau8825);
+
 	snd_soc_dapm_disable_pin(dapm, "SAR");
 	snd_soc_dapm_disable_pin(dapm, "MICBIAS");
 	/* Detach 2kOhm Resistors from MICBIAS to MICGND1/2 */
@@ -776,6 +1441,11 @@ static int nau8825_jack_insert(struct nau8825 *nau8825)
 
 	regmap_read(regmap, NAU8825_REG_GENERAL_STATUS, &jack_status_reg);
 	mic_detected = (jack_status_reg >> 10) & 3;
+	/* The JKSLV and JKR2 all detected in high impedance headset */
+	if (mic_detected == 0x3)
+		nau8825->high_imped = true;
+	else
+		nau8825->high_imped = false;
 
 	switch (mic_detected) {
 	case 0:
@@ -873,6 +1543,33 @@ static irqreturn_t nau8825_interrupt(int irq, void *data)
 	} else if (active_irq & NAU8825_HEADSET_COMPLETION_IRQ) {
 		if (nau8825_is_jack_inserted(regmap)) {
 			event |= nau8825_jack_insert(nau8825);
+			if (!nau8825->high_imped) {
+				/* Apply the cross talk suppression in the
+				 * headset without high impedance.
+				 */
+				if (!nau8825->xtalk_protect) {
+					/* Raise protection for cross talk de-
+					 * tection if no protection before.
+					 * The driver has to cancel the pro-
+					 * cess and restore changes if process
+					 * is ongoing when ejection.
+					 */
+					nau8825->xtalk_protect = true;
+					nau8825_sema_acquire(nau8825, 0);
+				}
+				/* Startup cross talk detection process */
+				nau8825->xtalk_state = NAU8825_XTALK_PREPARE;
+				schedule_work(&nau8825->xtalk_work);
+			} else {
+				/* The cross talk suppression shouldn't apply
+				 * in the headset with high impedance. Thus,
+				 * relieve the protection raised before.
+				 */
+				if (nau8825->xtalk_protect) {
+					nau8825_sema_release(nau8825);
+					nau8825->xtalk_protect = false;
+				}
+			}
 		} else {
 			dev_warn(nau8825->dev, "Headset completion IRQ fired but no headset connected\n");
 			nau8825_eject_jack(nau8825);
@@ -880,6 +1577,17 @@ static irqreturn_t nau8825_interrupt(int irq, void *data)
 
 		event_mask |= SND_JACK_HEADSET;
 		clear_irq = NAU8825_HEADSET_COMPLETION_IRQ;
+		/* Record the interruption report event for driver to report
+		 * the event later. The jack report will delay until cross
+		 * talk detection process is done.
+		 */
+		if (nau8825->xtalk_state == NAU8825_XTALK_PREPARE) {
+			nau8825->xtalk_event = event;
+			nau8825->xtalk_event_mask = event_mask;
+		}
+	} else if (active_irq & NAU8825_IMPEDANCE_MEAS_IRQ) {
+		schedule_work(&nau8825->xtalk_work);
+		clear_irq = NAU8825_IMPEDANCE_MEAS_IRQ;
 	} else if ((active_irq & NAU8825_JACK_INSERTION_IRQ_MASK) ==
 		NAU8825_JACK_INSERTION_DETECTED) {
 		/* One more step to check GPIO status directly. Thus, the
@@ -907,7 +1615,12 @@ static irqreturn_t nau8825_interrupt(int irq, void *data)
 	/* clears the rightmost interruption */
 	regmap_write(regmap, NAU8825_REG_INT_CLR_KEY_STATUS, clear_irq);
 
-	if (event_mask)
+	/* Delay jack report until cross talk detection is done. It can avoid
+	 * application to do playback preparation when cross talk detection
+	 * process is still working. Otherwise, the resource like clock and
+	 * power will be issued by them at the same time and conflict happens.
+	 */
+	if (event_mask && nau8825->xtalk_state == NAU8825_XTALK_DONE)
 		snd_soc_jack_report(nau8825->jack, event, event_mask);
 
 	return IRQ_HANDLED;
@@ -1077,6 +1790,16 @@ static int nau8825_codec_probe(struct snd_soc_codec *codec)
 	return 0;
 }
 
+static int nau8825_codec_remove(struct snd_soc_codec *codec)
+{
+	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
+
+	/* Cancel and reset cross tak suppresstion detection funciton */
+	nau8825_xtalk_cancel(nau8825);
+
+	return 0;
+}
+
 /**
  * nau8825_calc_fll_param - Calculate FLL parameters.
  * @fll_in: external clock provided to codec.
@@ -1265,12 +1988,21 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
 
 		break;
 	case NAU8825_CLK_MCLK:
+		/* Acquire the semaphone to synchronize the playback and
+		 * interrupt handler. In order to avoid the playback inter-
+		 * fered by cross talk process, the driver make the playback
+		 * preparation halted until cross talk process finish.
+		 */
+		nau8825_sema_acquire(nau8825, 2 * HZ);
 		regmap_update_bits(regmap, NAU8825_REG_CLK_DIVIDER,
 			NAU8825_CLK_SRC_MASK, NAU8825_CLK_SRC_MCLK);
 		regmap_update_bits(regmap, NAU8825_REG_FLL6, NAU8825_DCO_EN, 0);
 		/* MCLK not changed by clock tree */
 		regmap_update_bits(regmap, NAU8825_REG_CLK_DIVIDER,
 			NAU8825_CLK_MCLK_SRC_MASK, 0);
+		/* Release the semaphone. */
+		nau8825_sema_release(nau8825);
+
 		ret = nau8825_mclk_prepare(nau8825, freq);
 		if (ret)
 			return ret;
@@ -1295,16 +2027,34 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
 
 		break;
 	case NAU8825_CLK_FLL_MCLK:
+		/* Acquire the semaphone to synchronize the playback and
+		 * interrupt handler. In order to avoid the playback inter-
+		 * fered by cross talk process, the driver make the playback
+		 * preparation halted until cross talk process finish.
+		 */
+		nau8825_sema_acquire(nau8825, 2 * HZ);
 		regmap_update_bits(regmap, NAU8825_REG_FLL3,
 			NAU8825_FLL_CLK_SRC_MASK, NAU8825_FLL_CLK_SRC_MCLK);
+		/* Release the semaphone. */
+		nau8825_sema_release(nau8825);
+
 		ret = nau8825_mclk_prepare(nau8825, freq);
 		if (ret)
 			return ret;
 
 		break;
 	case NAU8825_CLK_FLL_BLK:
+		/* Acquire the semaphone to synchronize the playback and
+		 * interrupt handler. In order to avoid the playback inter-
+		 * fered by cross talk process, the driver make the playback
+		 * preparation halted until cross talk process finish.
+		 */
+		nau8825_sema_acquire(nau8825, 2 * HZ);
 		regmap_update_bits(regmap, NAU8825_REG_FLL3,
 			NAU8825_FLL_CLK_SRC_MASK, NAU8825_FLL_CLK_SRC_BLK);
+		/* Release the semaphone. */
+		nau8825_sema_release(nau8825);
+
 		if (nau8825->mclk_freq) {
 			clk_disable_unprepare(nau8825->mclk);
 			nau8825->mclk_freq = 0;
@@ -1312,8 +2062,17 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
 
 		break;
 	case NAU8825_CLK_FLL_FS:
+		/* Acquire the semaphone to synchronize the playback and
+		 * interrupt handler. In order to avoid the playback inter-
+		 * fered by cross talk process, the driver make the playback
+		 * preparation halted until cross talk process finish.
+		 */
+		nau8825_sema_acquire(nau8825, 2 * HZ);
 		regmap_update_bits(regmap, NAU8825_REG_FLL3,
 			NAU8825_FLL_CLK_SRC_MASK, NAU8825_FLL_CLK_SRC_FS);
+		/* Release the semaphone. */
+		nau8825_sema_release(nau8825);
+
 		if (nau8825->mclk_freq) {
 			clk_disable_unprepare(nau8825->mclk);
 			nau8825->mclk_freq = 0;
@@ -1399,6 +2158,8 @@ static int nau8825_set_bias_level(struct snd_soc_codec *codec,
 		break;
 
 	case SND_SOC_BIAS_OFF:
+		/* Cancel and reset cross talk detection funciton */
+		nau8825_xtalk_cancel(nau8825);
 		/* Turn off all interruptions before system shutdown. Keep the
 		 * interruption quiet before resume setup completes.
 		 */
@@ -1433,7 +2194,20 @@ static int nau8825_codec_resume(struct snd_soc_codec *codec)
 	struct nau8825 *nau8825 = snd_soc_codec_get_drvdata(codec);
 
 	regcache_cache_only(nau8825->regmap, false);
-	if (!nau8825_is_jack_inserted(nau8825->regmap)) {
+	if (nau8825_is_jack_inserted(nau8825->regmap)) {
+		/* If the jack is inserted, we need to check whether the play-
+		 * back is active before suspend. If active, the driver has to
+		 * raise the protection for cross talk function to avoid the
+		 * playback recovers before cross talk process finish. Other-
+		 * wise, the playback will be interfered by cross talk func-
+		 * tion. It is better to apply hardware related parameters
+		 * before starting playback or record.
+		 */
+		if (nau8825_dai_is_active(nau8825)) {
+			nau8825->xtalk_protect = true;
+			nau8825_sema_acquire(nau8825, 0);
+		}
+	} else {
 		/* Check the jack plug status directly. If the headset is un-
 		 * plugged during S3 when the chip has no power, there will
 		 * be no jack detection irq event after restarting jack detec-
@@ -1454,6 +2228,7 @@ static int nau8825_codec_resume(struct snd_soc_codec *codec)
 
 static struct snd_soc_codec_driver nau8825_codec_driver = {
 	.probe = nau8825_codec_probe,
+	.remove = nau8825_codec_remove,
 	.set_sysclk = nau8825_set_sysclk,
 	.set_pll = nau8825_set_pll,
 	.set_bias_level = nau8825_set_bias_level,
@@ -1597,6 +2372,13 @@ static int nau8825_i2c_probe(struct i2c_client *i2c,
 		return PTR_ERR(nau8825->regmap);
 	nau8825->dev = dev;
 	nau8825->irq = i2c->irq;
+	/* Initiate parameters, semaphone and work queue which are needed in
+	 * cross talk suppression measurment function.
+	 */
+	nau8825->xtalk_state = NAU8825_XTALK_DONE;
+	nau8825->xtalk_protect = false;
+	sema_init(&nau8825->xtalk_sem, 1);
+	INIT_WORK(&nau8825->xtalk_work, nau8825_xtalk_work);
 
 	nau8825_print_device_properties(nau8825);
 
diff --git a/sound/soc/codecs/nau8825.h b/sound/soc/codecs/nau8825.h
index 1100385..188c769 100644
--- a/sound/soc/codecs/nau8825.h
+++ b/sound/soc/codecs/nau8825.h
@@ -101,8 +101,13 @@
 #define NAU8825_ENABLE_DACR_SFT	10
 #define NAU8825_ENABLE_DACR	(1 << NAU8825_ENABLE_DACR_SFT)
 #define NAU8825_ENABLE_DACL_SFT	9
+#define NAU8825_ENABLE_DACL		(1 << NAU8825_ENABLE_DACL_SFT)
 #define NAU8825_ENABLE_ADC_SFT	8
 #define NAU8825_ENABLE_ADC		(1 << NAU8825_ENABLE_ADC_SFT)
+#define NAU8825_ENABLE_ADC_CLK_SFT	7
+#define NAU8825_ENABLE_ADC_CLK	(1 << NAU8825_ENABLE_ADC_CLK_SFT)
+#define NAU8825_ENABLE_DAC_CLK_SFT	6
+#define NAU8825_ENABLE_DAC_CLK	(1 << NAU8825_ENABLE_DAC_CLK_SFT)
 #define NAU8825_ENABLE_SAR_SFT	1
 
 /* CLK_DIVIDER (0x3) */
@@ -158,6 +163,7 @@
 /* INTERRUPT_MASK (0xf) */
 #define NAU8825_IRQ_OUTPUT_EN (1 << 11)
 #define NAU8825_IRQ_HEADSET_COMPLETE_EN (1 << 10)
+#define NAU8825_IRQ_RMS_EN (1 << 8)
 #define NAU8825_IRQ_KEY_RELEASE_EN (1 << 7)
 #define NAU8825_IRQ_KEY_SHORT_PRESS_EN (1 << 5)
 #define NAU8825_IRQ_EJECT_EN (1 << 2)
@@ -232,10 +238,13 @@
 
 /* I2S_PCM_CTRL2 (0x1d) */
 #define NAU8825_I2S_TRISTATE	(1 << 15) /* 0 - normal mode, 1 - Hi-Z output */
+#define NAU8825_I2S_DRV_SFT	12
+#define NAU8825_I2S_DRV_MASK	(0x3 << NAU8825_I2S_DRV_SFT)
 #define NAU8825_I2S_MS_SFT	3
 #define NAU8825_I2S_MS_MASK	(1 << NAU8825_I2S_MS_SFT)
 #define NAU8825_I2S_MS_MASTER	(1 << NAU8825_I2S_MS_SFT)
 #define NAU8825_I2S_MS_SLAVE	(0 << NAU8825_I2S_MS_SFT)
+#define NAU8825_I2S_BLK_DIV_MASK	0x7
 
 /* ADC_RATE (0x2b) */
 #define NAU8825_ADC_SYNC_DOWN_SFT	0
@@ -254,28 +263,72 @@
 #define NAU8825_DAC_OVERSAMPLE_128	2
 #define NAU8825_DAC_OVERSAMPLE_32	4
 
+/* ADC_DGAIN_CTRL (0x30) */
+#define NAU8825_ADC_DIG_VOL_MASK	0xff
+
 /* MUTE_CTRL (0x31) */
 #define NAU8825_DAC_ZERO_CROSSING_EN	(1 << 9)
 #define NAU8825_DAC_SOFT_MUTE	(1 << 9)
 
 /* HSVOL_CTRL (0x32) */
 #define NAU8825_HP_MUTE	(1 << 15)
+#define NAU8825_HP_MUTE_AUTO	(1 << 14)
+#define NAU8825_HPL_MUTE	(1 << 13)
+#define NAU8825_HPR_MUTE	(1 << 12)
+#define NAU8825_HPL_VOL_SFT	6
+#define NAU8825_HPL_VOL_MASK	(0x3f << NAU8825_HPL_VOL_SFT)
+#define NAU8825_HPR_VOL_SFT	0
+#define NAU8825_HPR_VOL_MASK	(0x3f << NAU8825_HPR_VOL_SFT)
+#define NAU8825_HP_VOL_MIN	0x36
 
 /* DACL_CTRL (0x33) */
 #define NAU8825_DACL_CH_SEL_SFT	9
 #define NAU8825_DACL_CH_SEL_MASK (0x1 << NAU8825_DACL_CH_SEL_SFT)
 #define NAU8825_DACL_CH_SEL_L    (0x0 << NAU8825_DACL_CH_SEL_SFT)
 #define NAU8825_DACL_CH_SEL_R    (0x1 << NAU8825_DACL_CH_SEL_SFT)
+#define NAU8825_DACL_CH_VOL_MASK	0xff
 
 /* DACR_CTRL (0x34) */
 #define NAU8825_DACR_CH_SEL_SFT	9
 #define NAU8825_DACR_CH_SEL_MASK (0x1 << NAU8825_DACR_CH_SEL_SFT)
 #define NAU8825_DACR_CH_SEL_L    (0x0 << NAU8825_DACR_CH_SEL_SFT)
 #define NAU8825_DACR_CH_SEL_R    (0x1 << NAU8825_DACR_CH_SEL_SFT)
+#define NAU8825_DACR_CH_VOL_MASK	0xff
+
+/* IMM_MODE_CTRL (0x4C) */
+#define NAU8825_IMM_THD_SFT		8
+#define NAU8825_IMM_THD_MASK		(0x3f << NAU8825_IMM_THD_SFT)
+#define NAU8825_IMM_GEN_VOL_SFT	6
+#define NAU8825_IMM_GEN_VOL_MASK	(0x3 << NAU8825_IMM_GEN_VOL_SFT)
+#define NAU8825_IMM_GEN_VOL_1_2nd	(0x0 << NAU8825_IMM_GEN_VOL_SFT)
+#define NAU8825_IMM_GEN_VOL_1_4th	(0x1 << NAU8825_IMM_GEN_VOL_SFT)
+#define NAU8825_IMM_GEN_VOL_1_8th	(0x2 << NAU8825_IMM_GEN_VOL_SFT)
+#define NAU8825_IMM_GEN_VOL_1_16th	(0x3 << NAU8825_IMM_GEN_VOL_SFT)
+
+#define NAU8825_IMM_CYC_SFT		4
+#define NAU8825_IMM_CYC_MASK		(0x3 << NAU8825_IMM_CYC_SFT)
+#define NAU8825_IMM_CYC_1024		(0x0 << NAU8825_IMM_CYC_SFT)
+#define NAU8825_IMM_CYC_2048		(0x1 << NAU8825_IMM_CYC_SFT)
+#define NAU8825_IMM_CYC_4096		(0x2 << NAU8825_IMM_CYC_SFT)
+#define NAU8825_IMM_CYC_8192		(0x3 << NAU8825_IMM_CYC_SFT)
+#define NAU8825_IMM_EN			(1 << 3)
+#define NAU8825_IMM_DAC_SRC_MASK	0x7
+#define NAU8825_IMM_DAC_SRC_BIQ	0x0
+#define NAU8825_IMM_DAC_SRC_DRC	0x1
+#define NAU8825_IMM_DAC_SRC_MIX	0x2
+#define NAU8825_IMM_DAC_SRC_SIN	0x3
 
 /* CLASSG_CTRL (0x50) */
 #define NAU8825_CLASSG_TIMER_SFT	8
 #define NAU8825_CLASSG_TIMER_MASK	(0x3f << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_1ms	(0x1 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_2ms	(0x2 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_8ms	(0x4 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_16ms	(0x8 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_32ms	(0x10 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_TIMER_64ms	(0x20 << NAU8825_CLASSG_TIMER_SFT)
+#define NAU8825_CLASSG_LDAC_EN		(0x1 << 2)
+#define NAU8825_CLASSG_RDAC_EN		(0x1 << 1)
 #define NAU8825_CLASSG_EN		(1 << 0)
 
 /* I2C_DEVICE_ID (0x58) */
@@ -284,7 +337,12 @@
 #define NAU8825_SOFTWARE_ID_NAU8825	0x0
 
 /* BIAS_ADJ (0x66) */
-#define NAU8825_BIAS_TESTDAC_EN	(0x3 << 8)
+#define NAU8825_BIAS_HPR_IMP		(1 << 15)
+#define NAU8825_BIAS_HPL_IMP		(1 << 14)
+#define NAU8825_BIAS_TESTDAC_SFT	8
+#define NAU8825_BIAS_TESTDAC_EN	(0x3 << NAU8825_BIAS_TESTDAC_SFT)
+#define NAU8825_BIAS_TESTDACR_EN	(0x2 << NAU8825_BIAS_TESTDAC_SFT)
+#define NAU8825_BIAS_TESTDACL_EN	(0x1 << NAU8825_BIAS_TESTDAC_SFT)
 #define NAU8825_BIAS_VMID	(1 << 6)
 #define NAU8825_BIAS_VMID_SEL_SFT	4
 #define NAU8825_BIAS_VMID_SEL_MASK	(3 << NAU8825_BIAS_VMID_SEL_SFT)
@@ -303,6 +361,11 @@
 #define NAU8825_POWERUP_ADCL	(1 << 6)
 
 /* RDAC (0x73) */
+#define NAU8825_RDAC_FS_BCLK_ENB	(1 << 15)
+#define NAU8825_RDAC_EN_SFT		12
+#define NAU8825_RDAC_EN		(0x3 << NAU8825_RDAC_EN_SFT)
+#define NAU8825_RDAC_CLK_EN_SFT	8
+#define NAU8825_RDAC_CLK_EN		(0x3 << NAU8825_RDAC_CLK_EN_SFT)
 #define NAU8825_RDAC_CLK_DELAY_SFT	4
 #define NAU8825_RDAC_CLK_DELAY_MASK	(0x7 << NAU8825_RDAC_CLK_DELAY_SFT)
 #define NAU8825_RDAC_VREF_SFT	2
@@ -347,12 +410,23 @@ enum {
 	NAU8825_CLK_FLL_FS,
 };
 
+/* Cross talk detection state */
+enum {
+	NAU8825_XTALK_PREPARE = 0,
+	NAU8825_XTALK_HPR_R2L,
+	NAU8825_XTALK_HPL_R2L,
+	NAU8825_XTALK_IMM,
+	NAU8825_XTALK_DONE,
+};
+
 struct nau8825 {
 	struct device *dev;
 	struct regmap *regmap;
 	struct snd_soc_dapm_context *dapm;
 	struct snd_soc_jack *jack;
 	struct clk *mclk;
+	struct work_struct xtalk_work;
+	struct semaphore xtalk_sem;
 	int irq;
 	int mclk_freq; /* 0 - mclk is disabled */
 	int button_pressed;
@@ -371,6 +445,12 @@ struct nau8825 {
 	int key_debounce;
 	int jack_insert_debounce;
 	int jack_eject_debounce;
+	int high_imped;
+	int xtalk_state;
+	int xtalk_event;
+	int xtalk_event_mask;
+	bool xtalk_protect;
+	int imp_rms[NAU8825_XTALK_IMM];
 };
 
 int nau8825_enable_jack_detect(struct snd_soc_codec *codec,
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-04-29  8:15 [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby John Hsu
  2016-04-29  8:15 ` [PATCH 2/2] ASoC: nau8825: cross talk suppression measurement function John Hsu
@ 2016-05-02 16:27 ` Mark Brown
  2016-05-03  9:20   ` John Hsu
  1 sibling, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-05-02 16:27 UTC (permalink / raw)
  To: John Hsu
  Cc: alsa-devel, anatol.pomozov, YHCHuang, lgirdwood, benzh, CTLIN0,
	mhkuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 2904 bytes --]

On Fri, Apr 29, 2016 at 04:15:17PM +0800, John Hsu wrote:

> @@ -614,8 +623,10 @@ int nau8825_enable_jack_detect(struct snd_soc_codec *codec,
>  		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L,
>  		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L);
>  
> -	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
> -		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_EJECT_EN, 0);
> +	/* Change jack type detection interruption to non-clock architecture
> +	 * for power saving less 1mW. Move these configuration about inter-
> +	 * ruption at auto mode to auto irq setup function.
> +	 */
>  

This comment is about the change you're making to the code rather than
something that should be in the code.

> +	/* Clear all interruption status */
> +	nau8825_int_status_clear_all(regmap);
> +
> +	/* Mask all interruptions except jack insertion interruption */
> +	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);

So if any other interrupts occur then things will break...

> -	regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq);
> +	if (regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq)) {
> +		dev_err(nau8825->dev, "failed to clear interruption\n");
> +		return IRQ_NONE;
> +	}

This is a read, not a write, and it's better to report the error code if
the read failed.  This should probably be a separate patch.

>  static const struct regmap_config nau8825_regmap_config = {
> -	.val_bits = 16,
> -	.reg_bits = 16,
> +	.val_bits = NAU8825_REG_DATA_LEN,
> +	.reg_bits = NAU8825_REG_ADDR_LEN,

This appears to be an unrelated change which it's not clear helps the
reader, these defines only seem to be used in htis one place.

> @@ -1134,7 +1244,26 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
>  	struct regmap *regmap = nau8825->regmap;
>  	int ret;
>  
> +	if (!nau8825_is_jack_inserted(nau8825->regmap) &&
> +		clk_id != NAU8825_CLK_DIS) {
> +		/* For power saving less 1mW, the jack type detection inter-
> +		 * ruption changes to non-clock architecture. Therefore, the
> +		 * clock should be disabled and not allowed to config any clock
> +		 * when no headset connected.
> +		 */
> +		dev_warn(nau8825->dev, "Souldn't enable any clock when no headset connected\n");
> +		return 0;
> +	}

This is ignoring the attempt to set up a clock but returning success
which is going to break things, printing the warning is dubious (a
system could be built without detection for example, or a speaker driver
connected) but probably OK in itself but the fact that we don't tell the
caller may make things worse.

> +		nau8825_eject_jack(nau8825);
> +		snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);
> +	}
> +	enable_irq(nau8825->irq);

The interrupt is optional (that bug appears to be already present in the
driver but should be fixed).

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-02 16:27 ` [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby Mark Brown
@ 2016-05-03  9:20   ` John Hsu
  2016-05-04 16:39     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: John Hsu @ 2016-05-03  9:20 UTC (permalink / raw)
  To: Mark Brown
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi

Hi

On 5/3/2016 12:27 AM, Mark Brown wrote:
> On Fri, Apr 29, 2016 at 04:15:17PM +0800, John Hsu wrote:
>
>   
>> @@ -614,8 +623,10 @@ int nau8825_enable_jack_detect(struct snd_soc_codec *codec,
>>  		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L,
>>  		NAU8825_HSD_AUTO_MODE | NAU8825_SPKR_DWN1R | NAU8825_SPKR_DWN1L);
>>  
>> -	regmap_update_bits(regmap, NAU8825_REG_INTERRUPT_MASK,
>> -		NAU8825_IRQ_HEADSET_COMPLETE_EN | NAU8825_IRQ_EJECT_EN, 0);
>> +	/* Change jack type detection interruption to non-clock architecture
>> +	 * for power saving less 1mW. Move these configuration about inter-
>> +	 * ruption at auto mode to auto irq setup function.
>> +	 */
>>  
>>     
>
> This comment is about the change you're making to the code rather than
> something that should be in the code.
>
>   
I see and remove it.
>> +	/* Clear all interruption status */
>> +	nau8825_int_status_clear_all(regmap);
>> +
>> +	/* Mask all interruptions except jack insertion interruption */
>> +	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);
>>     
>
> So if any other interrupts occur then things will break...
>
>   
The codec only has headset output and its function works when headset is
connected. When headset is not connected, the driver only permit insertion
interruption to happen. After insertion, the internal clock of codec turns
on and all other interruptions just will be enabled. Without clock, the
codec can detect jack insertion only.
>> -	regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq);
>> +	if (regmap_read(regmap, NAU8825_REG_IRQ_STATUS, &active_irq)) {
>> +		dev_err(nau8825->dev, "failed to clear interruption\n");
>> +		return IRQ_NONE;
>> +	}
>>     
>
> This is a read, not a write, and it's better to report the error code if
> the read failed.  This should probably be a separate patch.
>
>   
OK, that will be separated to other patch.
>>  static const struct regmap_config nau8825_regmap_config = {
>> -	.val_bits = 16,
>> -	.reg_bits = 16,
>> +	.val_bits = NAU8825_REG_DATA_LEN,
>> +	.reg_bits = NAU8825_REG_ADDR_LEN,
>>     
>
> This appears to be an unrelated change which it's not clear helps the
> reader, these defines only seem to be used in htis one place.
>
>   
We have to check every bits of irq status register value in funciton
nau8825_int_status_clear_all. The definition NAU8825_REG_DATA_LEN is used
in the function to setup the boundary of for loop. There is also register
value width here. Thus, I use the definition in it.

>> @@ -1134,7 +1244,26 @@ static int nau8825_configure_sysclk(struct nau8825 *nau8825, int clk_id,
>>  	struct regmap *regmap = nau8825->regmap;
>>  	int ret;
>>  
>> +	if (!nau8825_is_jack_inserted(nau8825->regmap) &&
>> +		clk_id != NAU8825_CLK_DIS) {
>> +		/* For power saving less 1mW, the jack type detection inter-
>> +		 * ruption changes to non-clock architecture. Therefore, the
>> +		 * clock should be disabled and not allowed to config any clock
>> +		 * when no headset connected.
>> +		 */
>> +		dev_warn(nau8825->dev, "Souldn't enable any clock when no headset connected\n");
>> +		return 0;
>> +	}
>>     
>
> This is ignoring the attempt to set up a clock but returning success
> which is going to break things, printing the warning is dubious (a
> system could be built without detection for example, or a speaker driver
> connected) but probably OK in itself but the fact that we don't tell the
> caller may make things worse.
>
>   
For clear expression, we should print error message and return error to
caller. Is it right?
>> +		nau8825_eject_jack(nau8825);
>> +		snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);
>> +	}
>> +	enable_irq(nau8825->irq);
>>     
>
> The interrupt is optional (that bug appears to be already present in the
> driver but should be fixed).
>   
If headset ejection happened when system suspend. After resume, the codec
won't detect the change and no report to the kernel. The application does
not aware the device change and still outputs stream to the headset device.
For the issue, the driver has to notice the application of ejection ever
happened in suspend. Let the application to change its device correctly.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-03  9:20   ` John Hsu
@ 2016-05-04 16:39     ` Mark Brown
  2016-05-06  7:41       ` John Hsu
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-05-04 16:39 UTC (permalink / raw)
  To: John Hsu
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 2206 bytes --]

On Tue, May 03, 2016 at 05:20:00PM +0800, John Hsu wrote:
> On 5/3/2016 12:27 AM, Mark Brown wrote:
> > On Fri, Apr 29, 2016 at 04:15:17PM +0800, John Hsu wrote:

> > > +	/* Mask all interruptions except jack insertion interruption */
> > > +	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);

> > So if any other interrupts occur then things will break...

> The codec only has headset output and its function works when headset is
> connected. When headset is not connected, the driver only permit insertion
> interruption to happen. After insertion, the internal clock of codec turns
> on and all other interruptions just will be enabled. Without clock, the
> codec can detect jack insertion only.

People do surprising things with devices - they may not wire up the
headphone detection for some reason, or may connect some external
circuit.

> > This is ignoring the attempt to set up a clock but returning success
> > which is going to break things, printing the warning is dubious (a
> > system could be built without detection for example, or a speaker driver
> > connected) but probably OK in itself but the fact that we don't tell the
> > caller may make things worse.

> For clear expression, we should print error message and return error to
> caller. Is it right?

It'd be better to just accept the configuration but what you suggest is
less bad than just completely ignoring the problem.

> > > +		nau8825_eject_jack(nau8825);
> > > +		snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);
> > > +	}
> > > +	enable_irq(nau8825->irq);

> > The interrupt is optional (that bug appears to be already present in the
> > driver but should be fixed).

> If headset ejection happened when system suspend. After resume, the codec
> won't detect the change and no report to the kernel. The application does
> not aware the device change and still outputs stream to the headset device.
> For the issue, the driver has to notice the application of ejection ever
> happened in suspend. Let the application to change its device correctly.

This does not address the issue at all.  The interrupt is optional, it
may not have been wired up and the probe function handles that case
gracefully.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-04 16:39     ` Mark Brown
@ 2016-05-06  7:41       ` John Hsu
  2016-05-06 18:18         ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: John Hsu @ 2016-05-06  7:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi

Hi,

On 5/5/2016 12:39 AM, Mark Brown wrote:
> On Tue, May 03, 2016 at 05:20:00PM +0800, John Hsu wrote:
>   
>> On 5/3/2016 12:27 AM, Mark Brown wrote:
>>     
>>> On Fri, Apr 29, 2016 at 04:15:17PM +0800, John Hsu wrote:
>>>       
>
>   
>>>> +	/* Mask all interruptions except jack insertion interruption */
>>>> +	regmap_write(regmap, NAU8825_REG_INTERRUPT_DIS_CTRL, 0xfffe);
>>>>         
>
>   
>>> So if any other interrupts occur then things will break...
>>>       
>
>   
>> The codec only has headset output and its function works when headset is
>> connected. When headset is not connected, the driver only permit insertion
>> interruption to happen. After insertion, the internal clock of codec turns
>> on and all other interruptions just will be enabled. Without clock, the
>> codec can detect jack insertion only.
>>     
>
> People do surprising things with devices - they may not wire up the
> headphone detection for some reason, or may connect some external
> circuit.
>
>   

I see. Thanks for your reminder. I'll enable insertion interruption but
disable ejection interruption here when headset ejection.

>>> This is ignoring the attempt to set up a clock but returning success
>>> which is going to break things, printing the warning is dubious (a
>>> system could be built without detection for example, or a speaker driver
>>> connected) but probably OK in itself but the fact that we don't tell the
>>> caller may make things worse.
>>>       
>
>   
>> For clear expression, we should print error message and return error to
>> caller. Is it right?
>>     
>
> It'd be better to just accept the configuration but what you suggest is
> less bad than just completely ignoring the problem.
>
>   

The codec needs internal clock for interruption at auto mode. Therefor, the
system clock will go back to internal clock after playback to end. But we
don't want this happened when jack is ejected already. We expected no in-
ternal clock when no headset connected; but the system will turn on it when
playback finish. For the reason, the driver adds error check to avoid this
situation happened.


>>>> +		nau8825_eject_jack(nau8825);
>>>> +		snd_soc_jack_report(nau8825->jack, 0, SND_JACK_HEADSET);
>>>> +	}
>>>> +	enable_irq(nau8825->irq);
>>>>         
>
>   
>>> The interrupt is optional (that bug appears to be already present in the
>>> driver but should be fixed).
>>>       
>
>   
>> If headset ejection happened when system suspend. After resume, the codec
>> won't detect the change and no report to the kernel. The application does
>> not aware the device change and still outputs stream to the headset device.
>> For the issue, the driver has to notice the application of ejection ever
>> happened in suspend. Let the application to change its device correctly.
>>     
>
> This does not address the issue at all.  The interrupt is optional, it
> may not have been wired up and the probe function handles that case
> gracefully.
>   

The ejection interruption will turn on when resume for the issue. Let the
probe function to handle it.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-06  7:41       ` John Hsu
@ 2016-05-06 18:18         ` Mark Brown
  2016-05-09  2:57           ` John Hsu
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-05-06 18:18 UTC (permalink / raw)
  To: John Hsu
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 1411 bytes --]

On Fri, May 06, 2016 at 03:41:42PM +0800, John Hsu wrote:
> On 5/5/2016 12:39 AM, Mark Brown wrote:
> > On Tue, May 03, 2016 at 05:20:00PM +0800, John Hsu wrote:

> > > For clear expression, we should print error message and return error to
> > > caller. Is it right?

> > It'd be better to just accept the configuration but what you suggest is
> > less bad than just completely ignoring the problem.

> The codec needs internal clock for interruption at auto mode. Therefor, the
> system clock will go back to internal clock after playback to end. But we
> don't want this happened when jack is ejected already. We expected no in-
> ternal clock when no headset connected; but the system will turn on it when
> playback finish. For the reason, the driver adds error check to avoid this
> situation happened.

I'm not sure I fully follow the above explanation.  I appreciate that
power consumption is not going to be optimal when the clock is provided
and the chip is idle but does it actually stop anything working?

> > This does not address the issue at all.  The interrupt is optional, it
> > may not have been wired up and the probe function handles that case
> > gracefully.

> The ejection interruption will turn on when resume for the issue. Let the
> probe function to handle it.

I don't see how the probe function can handle the fact that the resume
function is unconditionally calling enable_irq()?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-06 18:18         ` Mark Brown
@ 2016-05-09  2:57           ` John Hsu
  2016-05-09 16:35             ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: John Hsu @ 2016-05-09  2:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi

Hi,

On 5/7/2016 2:18 AM, Mark Brown wrote:
> On Fri, May 06, 2016 at 03:41:42PM +0800, John Hsu wrote:
>   
>> On 5/5/2016 12:39 AM, Mark Brown wrote:
>>     
>>> On Tue, May 03, 2016 at 05:20:00PM +0800, John Hsu wrote:
>>>       
>
>   
>>>> For clear expression, we should print error message and return error to
>>>> caller. Is it right?
>>>>         
>
>   
>>> It'd be better to just accept the configuration but what you suggest is
>>> less bad than just completely ignoring the problem.
>>>       
>
>   
>> The codec needs internal clock for interruption at auto mode. Therefor, the
>> system clock will go back to internal clock after playback to end. But we
>> don't want this happened when jack is ejected already. We expected no in-
>> ternal clock when no headset connected; but the system will turn on it when
>> playback finish. For the reason, the driver adds error check to avoid this
>> situation happened.
>>     
>
> I'm not sure I fully follow the above explanation.  I appreciate that
> power consumption is not going to be optimal when the clock is provided
> and the chip is idle but does it actually stop anything working?
>
>   

The conditional check for the situation is limited. Only when jack dis-
connect, the clock id just will be restricted. Do you think it is better
to control by machine driver and codec driver not to add any restriction?

>>> This does not address the issue at all.  The interrupt is optional, it
>>> may not have been wired up and the probe function handles that case
>>> gracefully.
>>>       
>
>   
>> The ejection interruption will turn on when resume for the issue. Let the
>> probe function to handle it.
>>     
>
> I don't see how the probe function can handle the fact that the resume
> function is unconditionally calling enable_irq()?
>   

Maybe I'm not very clear about what the probe function means. Could you
tell me more detail? The codec resumption recovers the interruption func-
tion because the function turns off when suspension. The interruption is
managed in resume setup function after resumption. The driver will enable
the insertion and ejection interruptions here. Let the codec to detect the
event and do report instead of manual check the jack status.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-09  2:57           ` John Hsu
@ 2016-05-09 16:35             ` Mark Brown
  2016-05-10  3:18               ` John Hsu
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-05-09 16:35 UTC (permalink / raw)
  To: John Hsu
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 1475 bytes --]

On Mon, May 09, 2016 at 10:57:43AM +0800, John Hsu wrote:
> On 5/7/2016 2:18 AM, Mark Brown wrote:

> > I'm not sure I fully follow the above explanation.  I appreciate that
> > power consumption is not going to be optimal when the clock is provided
> > and the chip is idle but does it actually stop anything working?

> The conditional check for the situation is limited. Only when jack dis-
> connect, the clock id just will be restricted. Do you think it is better
> to control by machine driver and codec driver not to add any restriction?

Well, the machine driver has to cope anyway.  What's not clear to me is
if the device *has* to use the internal clock when doing accessory
detection or if it's just lower power.

> > I don't see how the probe function can handle the fact that the resume
> > function is unconditionally calling enable_irq()?

> Maybe I'm not very clear about what the probe function means. Could you

The probe function is the function that runs on device probe, usually
with probe() in the name.  The relevant one here is nau8825_i2c_probe().

> tell me more detail? The codec resumption recovers the interruption func-
> tion because the function turns off when suspension. The interruption is
> managed in resume setup function after resumption. The driver will enable
> the insertion and ejection interruptions here. Let the codec to detect the
> event and do report instead of manual check the jack status.

There may not *be* an interrupt.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-09 16:35             ` Mark Brown
@ 2016-05-10  3:18               ` John Hsu
  2016-05-11 17:15                 ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: John Hsu @ 2016-05-10  3:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi

On 5/10/2016 12:35 AM, Mark Brown wrote:
> On Mon, May 09, 2016 at 10:57:43AM +0800, John Hsu wrote:
>   
>> On 5/7/2016 2:18 AM, Mark Brown wrote:
>>     
>
>   
>>> I'm not sure I fully follow the above explanation.  I appreciate that
>>> power consumption is not going to be optimal when the clock is provided
>>> and the chip is idle but does it actually stop anything working?
>>>       
>
>   
>> The conditional check for the situation is limited. Only when jack dis-
>> connect, the clock id just will be restricted. Do you think it is better
>> to control by machine driver and codec driver not to add any restriction?
>>     
>
> Well, the machine driver has to cope anyway.  What's not clear to me is
> if the device *has* to use the internal clock when doing accessory
> detection or if it's just lower power.
>
>   
If the codec only does accessory insertion detection, the driver can setup
it and doesn't need any clock. That can make lower power. But if the codec
wants to do advanced jack detection like mic or impedance, the driver needs
the internal clock to setup auto detection. Thus, when no headset connected
yet, we use the solution without internal clock for power saving. When we
want to do advanced detection, we use the solution with internal clock.

>>> I don't see how the probe function can handle the fact that the resume
>>> function is unconditionally calling enable_irq()?
>>>       
>
>   
>> Maybe I'm not very clear about what the probe function means. Could you
>>     
>
> The probe function is the function that runs on device probe, usually
> with probe() in the name.  The relevant one here is nau8825_i2c_probe().
>
>   

I see. Thank you.

>> tell me more detail? The codec resumption recovers the interruption func-
>> tion because the function turns off when suspension. The interruption is
>> managed in resume setup function after resumption. The driver will enable
>> the insertion and ejection interruptions here. Let the codec to detect the
>> event and do report instead of manual check the jack status.
>>     
>
> There may not *be* an interrupt.
>   

For the condition, the driver makes setup to detect insertion or ejection
only, and triggers detection manually. The codec can detect the accessory
status and send the interruption out.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-10  3:18               ` John Hsu
@ 2016-05-11 17:15                 ` Mark Brown
  2016-05-12  3:54                   ` John Hsu
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2016-05-11 17:15 UTC (permalink / raw)
  To: John Hsu
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 1201 bytes --]

On Tue, May 10, 2016 at 11:18:03AM +0800, John Hsu wrote:
> On 5/10/2016 12:35 AM, Mark Brown wrote:

> > Well, the machine driver has to cope anyway.  What's not clear to me is
> > if the device *has* to use the internal clock when doing accessory
> > detection or if it's just lower power.

> If the codec only does accessory insertion detection, the driver can setup
> it and doesn't need any clock. That can make lower power. But if the codec
> wants to do advanced jack detection like mic or impedance, the driver needs
> the internal clock to setup auto detection. Thus, when no headset connected
> yet, we use the solution without internal clock for power saving. When we
> want to do advanced detection, we use the solution with internal clock.

I'm afraid this still leaves me none the wiser, sorry.  If this
switching to the internal clock is essential to the device operation
then I'd expect it to be made transparent to callers so it should
happen transparently rather than appearing via set_sysclk().  If it's
not and it's just a performance optimisation then erroring out is
definitely excessive but if the optimisation can be implemented
transparently then it might be nice to do that.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-11 17:15                 ` Mark Brown
@ 2016-05-12  3:54                   ` John Hsu
  2016-05-12 10:58                     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: John Hsu @ 2016-05-12  3:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi

On 5/12/2016 1:15 AM, Mark Brown wrote:
> On Tue, May 10, 2016 at 11:18:03AM +0800, John Hsu wrote:
>   
>> On 5/10/2016 12:35 AM, Mark Brown wrote:
>>     
>
>   
>>> Well, the machine driver has to cope anyway.  What's not clear to me is
>>> if the device *has* to use the internal clock when doing accessory
>>> detection or if it's just lower power.
>>>       
>
>   
>> If the codec only does accessory insertion detection, the driver can setup
>> it and doesn't need any clock. That can make lower power. But if the codec
>> wants to do advanced jack detection like mic or impedance, the driver needs
>> the internal clock to setup auto detection. Thus, when no headset connected
>> yet, we use the solution without internal clock for power saving. When we
>> want to do advanced detection, we use the solution with internal clock.
>>     
>
> I'm afraid this still leaves me none the wiser, sorry.  If this
> switching to the internal clock is essential to the device operation
> then I'd expect it to be made transparent to callers so it should
> happen transparently rather than appearing via set_sysclk().  If it's
> not and it's just a performance optimisation then erroring out is
> definitely excessive but if the optimisation can be implemented
> transparently then it might be nice to do that.
>   

In the driver patch, the internal clock switching is transparently
done by driver when the codec runs advanced jack detection. Our
purpose is to prevent the machine turns on the internal clock by
itself when no headset connected. That will not affect function
work but make more power consumption. Maybe we can change to clock
disabled quietly when the machine turns on the internal clock when
no headset connected. Is it the right way?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby
  2016-05-12  3:54                   ` John Hsu
@ 2016-05-12 10:58                     ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2016-05-12 10:58 UTC (permalink / raw)
  To: John Hsu
  Cc: AP MS30 Linux ALSA, anatol.pomozov, AC30 YHChuang, lgirdwood,
	benzh, AC30 CTLin0, MS40 MHKuo, yong.zhi


[-- Attachment #1.1: Type: text/plain, Size: 1161 bytes --]

On Thu, May 12, 2016 at 11:54:54AM +0800, John Hsu wrote:
> On 5/12/2016 1:15 AM, Mark Brown wrote:

> > I'm afraid this still leaves me none the wiser, sorry.  If this
> > switching to the internal clock is essential to the device operation
> > then I'd expect it to be made transparent to callers so it should
> > happen transparently rather than appearing via set_sysclk().  If it's
> > not and it's just a performance optimisation then erroring out is
> > definitely excessive but if the optimisation can be implemented
> > transparently then it might be nice to do that.

> In the driver patch, the internal clock switching is transparently
> done by driver when the codec runs advanced jack detection. Our
> purpose is to prevent the machine turns on the internal clock by
> itself when no headset connected. That will not affect function
> work but make more power consumption. Maybe we can change to clock
> disabled quietly when the machine turns on the internal clock when
> no headset connected. Is it the right way?

Yes, that seems good.  Printing a warning is also fine because it does
increase power consumption but erroring out seems excessive.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-05-12 10:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-29  8:15 [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby John Hsu
2016-04-29  8:15 ` [PATCH 2/2] ASoC: nau8825: cross talk suppression measurement function John Hsu
2016-05-02 16:27 ` [PATCH 1/2] ASoC: nau8825: non-clock jack detection for power saving at standby Mark Brown
2016-05-03  9:20   ` John Hsu
2016-05-04 16:39     ` Mark Brown
2016-05-06  7:41       ` John Hsu
2016-05-06 18:18         ` Mark Brown
2016-05-09  2:57           ` John Hsu
2016-05-09 16:35             ` Mark Brown
2016-05-10  3:18               ` John Hsu
2016-05-11 17:15                 ` Mark Brown
2016-05-12  3:54                   ` John Hsu
2016-05-12 10:58                     ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.