alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] ASoC: extend seq_notifier callback interface
@ 2015-08-06 23:15 yang.a.fang
  2015-08-06 23:15 ` [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new yang.a.fang
  2015-08-07 10:44 ` [PATCH 1/2] ASoC: extend seq_notifier callback interface Mark Brown
  0 siblings, 2 replies; 8+ messages in thread
From: yang.a.fang @ 2015-08-06 23:15 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Anish.Kumar, praveen.k.jain,
	denny.iriawan, Fang, Yang A, sathyanarayana.nujella,
	ullysses.a.eoff, kevin.strasser, dgreid

From: "Fang, Yang A" <yang.a.fang@intel.com>

This is needed for some codec driver to know whether

it is power up/down sequence call from dapm

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 include/sound/soc-dapm.h  |    2 +-
 include/sound/soc.h       |    2 +-
 sound/soc/codecs/wm5100.c |    2 +-
 sound/soc/codecs/wm8903.c |    2 +-
 sound/soc/codecs/wm8996.c |    2 +-
 sound/soc/soc-dapm.c      |    5 ++---
 6 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 37d95a8..1d017a3 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -607,7 +607,7 @@ struct snd_soc_dapm_context {
 	/* Go to BIAS_OFF in suspend if the DAPM context is idle */
 	unsigned int suspend_bias_off:1;
 	void (*seq_notifier)(struct snd_soc_dapm_context *,
-			     enum snd_soc_dapm_type, int);
+			     enum snd_soc_dapm_type, int, bool);
 
 	struct device *dev; /* from parent - for debug */
 	struct snd_soc_component *component; /* parent component */
diff --git a/include/sound/soc.h b/include/sound/soc.h
index add5097..0072e21 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -875,7 +875,7 @@ struct snd_soc_codec_driver {
 	bool suspend_bias_off;
 
 	void (*seq_notifier)(struct snd_soc_dapm_context *,
-			     enum snd_soc_dapm_type, int);
+			     enum snd_soc_dapm_type, int, bool);
 
 	bool ignore_pmdown_time;  /* Doesn't benefit from pmdown delay */
 };
diff --git a/sound/soc/codecs/wm5100.c b/sound/soc/codecs/wm5100.c
index 8120b0c..6f4e6c3 100644
--- a/sound/soc/codecs/wm5100.c
+++ b/sound/soc/codecs/wm5100.c
@@ -733,7 +733,7 @@ WM5100_MIXER_CONTROLS("LHPF4", WM5100_HPLP4MIX_INPUT_1_SOURCE),
 };
 
 static void wm5100_seq_notifier(struct snd_soc_dapm_context *dapm,
-				enum snd_soc_dapm_type event, int subseq)
+			enum snd_soc_dapm_type event, int subseq, bool power_up)
 {
 	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
 	struct wm5100_priv *wm5100 = snd_soc_codec_get_drvdata(codec);
diff --git a/sound/soc/codecs/wm8903.c b/sound/soc/codecs/wm8903.c
index b011253..12da3b1 100644
--- a/sound/soc/codecs/wm8903.c
+++ b/sound/soc/codecs/wm8903.c
@@ -280,7 +280,7 @@ static int wm8903_dcs_event(struct snd_soc_dapm_widget *w,
 #define WM8903_DCS_MODE_START_STOP 2
 
 static void wm8903_seq_notifier(struct snd_soc_dapm_context *dapm,
-				enum snd_soc_dapm_type event, int subseq)
+			enum snd_soc_dapm_type event, int subseq, bool power_up)
 {
 	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
 	struct wm8903_priv *wm8903 = snd_soc_codec_get_drvdata(codec);
diff --git a/sound/soc/codecs/wm8996.c b/sound/soc/codecs/wm8996.c
index 2a696db..f12a900 100644
--- a/sound/soc/codecs/wm8996.c
+++ b/sound/soc/codecs/wm8996.c
@@ -686,7 +686,7 @@ static void wait_for_dc_servo(struct snd_soc_codec *codec, u16 mask)
 }
 
 static void wm8996_seq_notifier(struct snd_soc_dapm_context *dapm,
-				enum snd_soc_dapm_type event, int subseq)
+			enum snd_soc_dapm_type event, int subseq, bool power_up)
 {
 	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
 	struct wm8996_priv *wm8996 = snd_soc_codec_get_drvdata(codec);
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 1992568..ebea2d3 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -1528,8 +1528,7 @@ static void dapm_seq_run(struct snd_soc_card *card,
 				for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
 					if (sort[i] == cur_sort)
 						cur_dapm->seq_notifier(cur_dapm,
-								       i,
-								       cur_subseq);
+						i, cur_subseq, power_up);
 			}
 
 			if (cur_dapm && w->dapm != cur_dapm)
@@ -1591,7 +1590,7 @@ static void dapm_seq_run(struct snd_soc_card *card,
 		for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
 			if (sort[i] == cur_sort)
 				cur_dapm->seq_notifier(cur_dapm,
-						       i, cur_subseq);
+						       i, cur_subseq, power_up);
 	}
 
 	list_for_each_entry(d, &card->dapm_list, list) {
-- 
1.7.9.5

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

* [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new
  2015-08-06 23:15 [PATCH 1/2] ASoC: extend seq_notifier callback interface yang.a.fang
@ 2015-08-06 23:15 ` yang.a.fang
  2015-08-07 10:43   ` Mark Brown
  2015-08-07 21:08   ` [PATCH v2] " yang.a.fang
  2015-08-07 10:44 ` [PATCH 1/2] ASoC: extend seq_notifier callback interface Mark Brown
  1 sibling, 2 replies; 8+ messages in thread
From: yang.a.fang @ 2015-08-06 23:15 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Anish.Kumar, praveen.k.jain,
	denny.iriawan, Fang, Yang A, sathyanarayana.nujella,
	ullysses.a.eoff, kevin.strasser, dgreid

From: "Fang, Yang A" <yang.a.fang@intel.com>

audio route when there is an existing active audio route(s).

This patch fixed recording noise issue while playback is active.

We have some registers which require the device to be in full shutdown
or to enter full shutdown before the register settings will take effect.
Currently the driver is not shutting down the device when a new audio
route is created. If a new audio route is made active while there is
already an active audio route, then the required register sequencing is
violated. A hardware shutdown toggle when creating a new audio route
corrects the sequencing error. The device must remain in hardware
shutdown for 40ms to allow the internal hardware core to fully shutdown.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
---
 sound/soc/codecs/max98090.c |   46 +++++++++++++++++++++++++++++++++++++------
 sound/soc/codecs/max98090.h |    1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index 91c9f9f..1516c6b 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -850,6 +850,19 @@ static int max98090_micinput_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		max98090->shdn_pending = TRUE;
+
+	return 0;
+
+}
+
 static const char *mic1_mux_text[] = { "IN12", "IN56" };
 
 static SOC_ENUM_SINGLE_DECL(mic1_mux_enum,
@@ -1158,9 +1171,11 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("SDOEN", M98090_REG_IO_CONFIGURATION,
 		M98090_SDOEN_SHIFT, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY("DMICL_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICL_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICL_SHIFT, 0, max98090_shdn_event,
+			SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("DMICR_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICR_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICR_SHIFT, 0, max98090_shdn_event,
+			 SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("AHPF", M98090_REG_FILTER_CONFIG,
 		M98090_AHPF_SHIFT, 0, NULL, 0),
 
@@ -1205,10 +1220,12 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 		&max98090_right_adc_mixer_controls[0],
 		ARRAY_SIZE(max98090_right_adc_mixer_controls)),
 
-	SND_SOC_DAPM_ADC("ADCL", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADLEN_SHIFT, 0),
-	SND_SOC_DAPM_ADC("ADCR", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADREN_SHIFT, 0),
+	SND_SOC_DAPM_ADC_E("ADCL", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADLEN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
+	SND_SOC_DAPM_ADC_E("ADCR", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADREN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
 
 	SND_SOC_DAPM_AIF_OUT("AIFOUTL", "HiFi Capture", 0,
 		SND_SOC_NOPM, 0, 0),
@@ -2539,9 +2556,26 @@ static int max98090_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
+	enum snd_soc_dapm_type event, int subseq, bool power_up)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (max98090->shdn_pending && power_up) {
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, 0);
+		msleep(40);
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, M98090_SHDNN_MASK);
+		max98090->shdn_pending = FALSE;
+	}
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_max98090 = {
 	.probe   = max98090_probe,
 	.remove  = max98090_remove,
+	.seq_notifier = max98090_seq_notifier,
 	.set_bias_level = max98090_set_bias_level,
 };
 
diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h
index 21ff743..bc610d9 100644
--- a/sound/soc/codecs/max98090.h
+++ b/sound/soc/codecs/max98090.h
@@ -1543,6 +1543,7 @@ struct max98090_priv {
 	unsigned int pa2en;
 	unsigned int sidetone;
 	bool master;
+	bool shdn_pending;
 };
 
 int max98090_mic_detect(struct snd_soc_codec *codec,
-- 
1.7.9.5

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

* Re: [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new
  2015-08-06 23:15 ` [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new yang.a.fang
@ 2015-08-07 10:43   ` Mark Brown
  2015-08-07 20:50     ` Fang, Yang A
  2015-08-07 21:08   ` [PATCH v2] " yang.a.fang
  1 sibling, 1 reply; 8+ messages in thread
From: Mark Brown @ 2015-08-07 10:43 UTC (permalink / raw)
  To: yang.a.fang
  Cc: alsa-devel, srinivas.sripathi, Anish.Kumar, praveen.k.jain,
	lgirdwood, denny.iriawan, sathyanarayana.nujella,
	ullysses.a.eoff, kevin.strasser, dgreid


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

On Thu, Aug 06, 2015 at 04:15:45PM -0700, yang.a.fang@intel.com wrote:

> +static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
> +				 struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
> +	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
> +
> +	if (event & SND_SOC_DAPM_POST_PMU)
> +		max98090->shdn_pending = TRUE;

TRUE?  Please use normal C99 booleans like we normally do in the kernel.

> +static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
> +	enum snd_soc_dapm_type event, int subseq, bool power_up)
> +{
> +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
> +	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
> +
> +	if (max98090->shdn_pending && power_up) {
> +		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
> +				M98090_SHDNN_MASK, 0);
> +		msleep(40);
> +		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
> +				M98090_SHDNN_MASK, M98090_SHDNN_MASK);
> +		max98090->shdn_pending = FALSE;
> +	}
> +}

Why did you change the API for this?  As far as I can tell power_up is
redundant here, we already know we're powering things up because we got
a _PMU event, the main effect here appears to be to just run this at the
next sequence point after we've powered up a widget with a shutdown
event.  The general concept with the seq_notifier is that we record
things we need to do in the per-widget events and then implement them in
the seq_notifier and I'm not seeing anything else going on here.

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

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



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

* Re: [PATCH 1/2] ASoC: extend seq_notifier callback interface
  2015-08-06 23:15 [PATCH 1/2] ASoC: extend seq_notifier callback interface yang.a.fang
  2015-08-06 23:15 ` [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new yang.a.fang
@ 2015-08-07 10:44 ` Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2015-08-07 10:44 UTC (permalink / raw)
  To: yang.a.fang
  Cc: alsa-devel, srinivas.sripathi, Anish.Kumar, praveen.k.jain,
	lgirdwood, denny.iriawan, sathyanarayana.nujella,
	ullysses.a.eoff, kevin.strasser, dgreid


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

On Thu, Aug 06, 2015 at 04:15:44PM -0700, yang.a.fang@intel.com wrote:
> From: "Fang, Yang A" <yang.a.fang@intel.com>
> 
> This is needed for some codec driver to know whether
> 
> it is power up/down sequence call from dapm

This isn't a good changelog - it's not enough to say what the change is,
you should (especially for a core change like this) be explaining why
the change is needed.  For example in this case why do CODEC drivers
need this information?

You've also got a random blank line in there.

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

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



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

* Re: [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new
  2015-08-07 10:43   ` Mark Brown
@ 2015-08-07 20:50     ` Fang, Yang A
  0 siblings, 0 replies; 8+ messages in thread
From: Fang, Yang A @ 2015-08-07 20:50 UTC (permalink / raw)
  To: 'Mark Brown'
  Cc: alsa-devel, Sripathi, Srinivas, Anish.Kumar, Jain, Praveen K,
	lgirdwood, Iriawan, Denny, Nujella, Sathyanarayana, Eoff,
	Ullysses A, kevin.strasser, dgreid



> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Friday, August 07, 2015 3:43 AM
> To: Fang, Yang A
> Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org;
> dgreid@chromium.org; Nujella, Sathyanarayana;
> kevin.strasser@linux.intel.com; Sripathi, Srinivas; Iriawan, Denny; Jain,
> Praveen K; Anish.Kumar@maximintegrated.com; Eoff, Ullysses A
> Subject: Re: [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing
> when configuring a new
> 
> On Thu, Aug 06, 2015 at 04:15:45PM -0700, yang.a.fang@intel.com wrote:
> 
> > +static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
> > +				 struct snd_kcontrol *kcontrol, int event) {
> > +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
> > +	struct max98090_priv *max98090 =
> snd_soc_codec_get_drvdata(codec);
> > +
> > +	if (event & SND_SOC_DAPM_POST_PMU)
> > +		max98090->shdn_pending = TRUE;
> 
> TRUE?  Please use normal C99 booleans like we normally do in the kernel.
I will fix this
> 
> > +static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
> > +	enum snd_soc_dapm_type event, int subseq, bool power_up) {
> > +	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
> > +	struct max98090_priv *max98090 =
> snd_soc_codec_get_drvdata(codec);
> > +
> > +	if (max98090->shdn_pending && power_up) {
> > +		snd_soc_update_bits(codec,
> M98090_REG_DEVICE_SHUTDOWN,
> > +				M98090_SHDNN_MASK, 0);
> > +		msleep(40);
> > +		snd_soc_update_bits(codec,
> M98090_REG_DEVICE_SHUTDOWN,
> > +				M98090_SHDNN_MASK,
> M98090_SHDNN_MASK);
> > +		max98090->shdn_pending = FALSE;
> > +	}
> > +}
> 
> Why did you change the API for this?  As far as I can tell power_up is
> redundant here, we already know we're powering things up because we got
> a _PMU event, the main effect here appears to be to just run this at the next
> sequence point after we've powered up a widget with a shutdown event.
> The general concept with the seq_notifier is that we record things we need
> to do in the per-widget events and then implement them in the seq_notifier
> and I'm not seeing anything else going on here.
Thanks Mark.   Right , _PMU already indicates it is power_up event.. I will drop
This API change.

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

* [PATCH v2] ASoC: max98090: Enforce correct device sequencing when configuring a new
  2015-08-06 23:15 ` [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new yang.a.fang
  2015-08-07 10:43   ` Mark Brown
@ 2015-08-07 21:08   ` yang.a.fang
  2015-08-07 21:41     ` Anish Kumar
  2015-08-10 12:00     ` Applied "ASoC: max98090: Fix sequencing when starting additional routes" to the asoc tree Mark Brown
  1 sibling, 2 replies; 8+ messages in thread
From: yang.a.fang @ 2015-08-07 21:08 UTC (permalink / raw)
  To: broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, Anish.Kumar, praveen.k.jain,
	denny.iriawan, Fang, Yang A, sathyanarayana.nujella,
	ullysses.a.eoff, kevin.strasser, dgreid

From: "Fang, Yang A" <yang.a.fang@intel.com>

audio route when there is an existing active audio route(s).

This patch fixed recording noise issue while playback is active.

We have some registers which require the device to be in full shutdown
or to enter full shutdown before the register settings will take effect.
Currently the driver is not shutting down the device when a new audio
route is created. If a new audio route is made active while there is
already an active audio route, then the required register sequencing is
violated. A hardware shutdown toggle when creating a new audio route
corrects the sequencing error. The device must remain in hardware
shutdown for 40ms to allow the internal hardware core to fully shutdown.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
---
 sound/soc/codecs/max98090.c |   46 +++++++++++++++++++++++++++++++++++++------
 sound/soc/codecs/max98090.h |    1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index 2d69fca..584aab8 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -777,6 +777,19 @@ static int max98090_micinput_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		max98090->shdn_pending = true;
+
+	return 0;
+
+}
+
 static const char *mic1_mux_text[] = { "IN12", "IN56" };
 
 static SOC_ENUM_SINGLE_DECL(mic1_mux_enum,
@@ -1085,9 +1098,11 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("SDOEN", M98090_REG_IO_CONFIGURATION,
 		M98090_SDOEN_SHIFT, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY("DMICL_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICL_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICL_SHIFT, 0, max98090_shdn_event,
+			SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("DMICR_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICR_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICR_SHIFT, 0, max98090_shdn_event,
+			 SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("AHPF", M98090_REG_FILTER_CONFIG,
 		M98090_AHPF_SHIFT, 0, NULL, 0),
 
@@ -1132,10 +1147,12 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 		&max98090_right_adc_mixer_controls[0],
 		ARRAY_SIZE(max98090_right_adc_mixer_controls)),
 
-	SND_SOC_DAPM_ADC("ADCL", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADLEN_SHIFT, 0),
-	SND_SOC_DAPM_ADC("ADCR", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADREN_SHIFT, 0),
+	SND_SOC_DAPM_ADC_E("ADCL", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADLEN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
+	SND_SOC_DAPM_ADC_E("ADCR", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADREN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
 
 	SND_SOC_DAPM_AIF_OUT("AIFOUTL", "HiFi Capture", 0,
 		SND_SOC_NOPM, 0, 0),
@@ -2466,9 +2483,26 @@ static int max98090_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
+	enum snd_soc_dapm_type event, int subseq)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (max98090->shdn_pending) {
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, 0);
+		msleep(40);
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, M98090_SHDNN_MASK);
+		max98090->shdn_pending = false;
+	}
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_max98090 = {
 	.probe   = max98090_probe,
 	.remove  = max98090_remove,
+	.seq_notifier = max98090_seq_notifier,
 	.set_bias_level = max98090_set_bias_level,
 };
 
diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h
index 21ff743..bc610d9 100644
--- a/sound/soc/codecs/max98090.h
+++ b/sound/soc/codecs/max98090.h
@@ -1543,6 +1543,7 @@ struct max98090_priv {
 	unsigned int pa2en;
 	unsigned int sidetone;
 	bool master;
+	bool shdn_pending;
 };
 
 int max98090_mic_detect(struct snd_soc_codec *codec,
-- 
1.7.9.5

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

* Re: [PATCH v2] ASoC: max98090: Enforce correct device sequencing when configuring a new
  2015-08-07 21:08   ` [PATCH v2] " yang.a.fang
@ 2015-08-07 21:41     ` Anish Kumar
  2015-08-10 12:00     ` Applied "ASoC: max98090: Fix sequencing when starting additional routes" to the asoc tree Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Anish Kumar @ 2015-08-07 21:41 UTC (permalink / raw)
  To: yang.a.fang, broonie, lgirdwood
  Cc: alsa-devel, srinivas.sripathi, praveen.k.jain, denny.iriawan,
	sathyanarayana.nujella, ullysses.a.eoff, kevin.strasser, dgreid

Acked-by: anish kumar <anish.kumar@maximintegrated.com>
________________________________________
From: yang.a.fang@intel.com [yang.a.fang@intel.com]
Sent: Friday, August 07, 2015 2:08 PM
To: broonie@kernel.org; lgirdwood@gmail.com
Cc: alsa-devel@alsa-project.org; dgreid@chromium.org; sathyanarayana.nujella@intel.com; kevin.strasser@linux.intel.com; srinivas.sripathi@intel.com; denny.iriawan@intel.com; praveen.k.jain@intel.com; Anish Kumar; ullysses.a.eoff@intel.com; Fang, Yang A
Subject: [PATCH v2] ASoC: max98090: Enforce correct device sequencing when configuring a new

From: "Fang, Yang A" <yang.a.fang@intel.com>

audio route when there is an existing active audio route(s).

This patch fixed recording noise issue while playback is active.

We have some registers which require the device to be in full shutdown
or to enter full shutdown before the register settings will take effect.
Currently the driver is not shutting down the device when a new audio
route is created. If a new audio route is made active while there is
already an active audio route, then the required register sequencing is
violated. A hardware shutdown toggle when creating a new audio route
corrects the sequencing error. The device must remain in hardware
shutdown for 40ms to allow the internal hardware core to fully shutdown.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
---
 sound/soc/codecs/max98090.c |   46 +++++++++++++++++++++++++++++++++++++------
 sound/soc/codecs/max98090.h |    1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index 2d69fca..584aab8 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -777,6 +777,19 @@ static int max98090_micinput_event(struct snd_soc_dapm_widget *w,
        return 0;
 }

+static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
+                                struct snd_kcontrol *kcontrol, int event)
+{
+       struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+       struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+       if (event & SND_SOC_DAPM_POST_PMU)
+               max98090->shdn_pending = true;
+
+       return 0;
+
+}
+
 static const char *mic1_mux_text[] = { "IN12", "IN56" };

 static SOC_ENUM_SINGLE_DECL(mic1_mux_enum,
@@ -1085,9 +1098,11 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
        SND_SOC_DAPM_SUPPLY("SDOEN", M98090_REG_IO_CONFIGURATION,
                M98090_SDOEN_SHIFT, 0, NULL, 0),
        SND_SOC_DAPM_SUPPLY("DMICL_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-                M98090_DIGMICL_SHIFT, 0, NULL, 0),
+                M98090_DIGMICL_SHIFT, 0, max98090_shdn_event,
+                       SND_SOC_DAPM_POST_PMU),
        SND_SOC_DAPM_SUPPLY("DMICR_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-                M98090_DIGMICR_SHIFT, 0, NULL, 0),
+                M98090_DIGMICR_SHIFT, 0, max98090_shdn_event,
+                        SND_SOC_DAPM_POST_PMU),
        SND_SOC_DAPM_SUPPLY("AHPF", M98090_REG_FILTER_CONFIG,
                M98090_AHPF_SHIFT, 0, NULL, 0),

@@ -1132,10 +1147,12 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
                &max98090_right_adc_mixer_controls[0],
                ARRAY_SIZE(max98090_right_adc_mixer_controls)),

-       SND_SOC_DAPM_ADC("ADCL", NULL, M98090_REG_INPUT_ENABLE,
-               M98090_ADLEN_SHIFT, 0),
-       SND_SOC_DAPM_ADC("ADCR", NULL, M98090_REG_INPUT_ENABLE,
-               M98090_ADREN_SHIFT, 0),
+       SND_SOC_DAPM_ADC_E("ADCL", NULL, M98090_REG_INPUT_ENABLE,
+               M98090_ADLEN_SHIFT, 0, max98090_shdn_event,
+               SND_SOC_DAPM_POST_PMU),
+       SND_SOC_DAPM_ADC_E("ADCR", NULL, M98090_REG_INPUT_ENABLE,
+               M98090_ADREN_SHIFT, 0, max98090_shdn_event,
+               SND_SOC_DAPM_POST_PMU),

        SND_SOC_DAPM_AIF_OUT("AIFOUTL", "HiFi Capture", 0,
                SND_SOC_NOPM, 0, 0),
@@ -2466,9 +2483,26 @@ static int max98090_remove(struct snd_soc_codec *codec)
        return 0;
 }

+static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
+       enum snd_soc_dapm_type event, int subseq)
+{
+       struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
+       struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+       if (max98090->shdn_pending) {
+               snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+                               M98090_SHDNN_MASK, 0);
+               msleep(40);
+               snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+                               M98090_SHDNN_MASK, M98090_SHDNN_MASK);
+               max98090->shdn_pending = false;
+       }
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_max98090 = {
        .probe   = max98090_probe,
        .remove  = max98090_remove,
+       .seq_notifier = max98090_seq_notifier,
        .set_bias_level = max98090_set_bias_level,
 };

diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h
index 21ff743..bc610d9 100644
--- a/sound/soc/codecs/max98090.h
+++ b/sound/soc/codecs/max98090.h
@@ -1543,6 +1543,7 @@ struct max98090_priv {
        unsigned int pa2en;
        unsigned int sidetone;
        bool master;
+       bool shdn_pending;
 };

 int max98090_mic_detect(struct snd_soc_codec *codec,
--
1.7.9.5

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

* Applied "ASoC: max98090: Fix sequencing when starting additional routes" to the asoc tree
  2015-08-07 21:08   ` [PATCH v2] " yang.a.fang
  2015-08-07 21:41     ` Anish Kumar
@ 2015-08-10 12:00     ` Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2015-08-10 12:00 UTC (permalink / raw)
  To: Fang Yang A, Sathyanarayana Nujella, Anish Kumar, Mark Brown; +Cc: alsa-devel

The patch

   ASoC: max98090: Fix sequencing when starting additional routes

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From e5b94083d0996b4c69674c8a5563c2eb272557ba Mon Sep 17 00:00:00 2001
From: "Fang, Yang A" <yang.a.fang@intel.com>
Date: Fri, 7 Aug 2015 14:08:15 -0700
Subject: [PATCH] ASoC: max98090: Fix sequencing when starting additional
 routes

Enforce correct device sequencing when configuring a new
audio route when there is an existing active audio route(s).

This patch fixed recording noise issue while playback is active.

We have some registers which require the device to be in full shutdown
or to enter full shutdown before the register settings will take effect.
Currently the driver is not shutting down the device when a new audio
route is created. If a new audio route is made active while there is
already an active audio route, then the required register sequencing is
violated. A hardware shutdown toggle when creating a new audio route
corrects the sequencing error. The device must remain in hardware
shutdown for 40ms to allow the internal hardware core to fully shutdown.

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
Signed-off-by: Sathyanarayana Nujella <sathyanarayana.nujella@intel.com>
Acked-by: Anish Kumar <anish.kumar@maximintegrated.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/max98090.c | 46 +++++++++++++++++++++++++++++++++++++++------
 sound/soc/codecs/max98090.h |  1 +
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/max98090.c b/sound/soc/codecs/max98090.c
index 78268f05..2a4c2e1 100644
--- a/sound/soc/codecs/max98090.c
+++ b/sound/soc/codecs/max98090.c
@@ -850,6 +850,19 @@ static int max98090_micinput_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int max98090_shdn_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		max98090->shdn_pending = true;
+
+	return 0;
+
+}
+
 static const char *mic1_mux_text[] = { "IN12", "IN56" };
 
 static SOC_ENUM_SINGLE_DECL(mic1_mux_enum,
@@ -1158,9 +1171,11 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("SDOEN", M98090_REG_IO_CONFIGURATION,
 		M98090_SDOEN_SHIFT, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY("DMICL_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICL_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICL_SHIFT, 0, max98090_shdn_event,
+			SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("DMICR_ENA", M98090_REG_DIGITAL_MIC_ENABLE,
-		 M98090_DIGMICR_SHIFT, 0, NULL, 0),
+		 M98090_DIGMICR_SHIFT, 0, max98090_shdn_event,
+			 SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SUPPLY("AHPF", M98090_REG_FILTER_CONFIG,
 		M98090_AHPF_SHIFT, 0, NULL, 0),
 
@@ -1205,10 +1220,12 @@ static const struct snd_soc_dapm_widget max98090_dapm_widgets[] = {
 		&max98090_right_adc_mixer_controls[0],
 		ARRAY_SIZE(max98090_right_adc_mixer_controls)),
 
-	SND_SOC_DAPM_ADC("ADCL", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADLEN_SHIFT, 0),
-	SND_SOC_DAPM_ADC("ADCR", NULL, M98090_REG_INPUT_ENABLE,
-		M98090_ADREN_SHIFT, 0),
+	SND_SOC_DAPM_ADC_E("ADCL", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADLEN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
+	SND_SOC_DAPM_ADC_E("ADCR", NULL, M98090_REG_INPUT_ENABLE,
+		M98090_ADREN_SHIFT, 0, max98090_shdn_event,
+		SND_SOC_DAPM_POST_PMU),
 
 	SND_SOC_DAPM_AIF_OUT("AIFOUTL", "HiFi Capture", 0,
 		SND_SOC_NOPM, 0, 0),
@@ -2536,9 +2553,26 @@ static int max98090_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+static void max98090_seq_notifier(struct snd_soc_dapm_context *dapm,
+	enum snd_soc_dapm_type event, int subseq)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(dapm);
+	struct max98090_priv *max98090 = snd_soc_codec_get_drvdata(codec);
+
+	if (max98090->shdn_pending) {
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, 0);
+		msleep(40);
+		snd_soc_update_bits(codec, M98090_REG_DEVICE_SHUTDOWN,
+				M98090_SHDNN_MASK, M98090_SHDNN_MASK);
+		max98090->shdn_pending = false;
+	}
+}
+
 static struct snd_soc_codec_driver soc_codec_dev_max98090 = {
 	.probe   = max98090_probe,
 	.remove  = max98090_remove,
+	.seq_notifier = max98090_seq_notifier,
 	.set_bias_level = max98090_set_bias_level,
 };
 
diff --git a/sound/soc/codecs/max98090.h b/sound/soc/codecs/max98090.h
index 21ff743f..bc610d9 100644
--- a/sound/soc/codecs/max98090.h
+++ b/sound/soc/codecs/max98090.h
@@ -1543,6 +1543,7 @@ struct max98090_priv {
 	unsigned int pa2en;
 	unsigned int sidetone;
 	bool master;
+	bool shdn_pending;
 };
 
 int max98090_mic_detect(struct snd_soc_codec *codec,
-- 
2.5.0

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

end of thread, other threads:[~2015-08-10 12:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-06 23:15 [PATCH 1/2] ASoC: extend seq_notifier callback interface yang.a.fang
2015-08-06 23:15 ` [PATCH 2/2] ASoC: max98090: Enforce correct device sequencing when configuring a new yang.a.fang
2015-08-07 10:43   ` Mark Brown
2015-08-07 20:50     ` Fang, Yang A
2015-08-07 21:08   ` [PATCH v2] " yang.a.fang
2015-08-07 21:41     ` Anish Kumar
2015-08-10 12:00     ` Applied "ASoC: max98090: Fix sequencing when starting additional routes" to the asoc tree Mark Brown
2015-08-07 10:44 ` [PATCH 1/2] ASoC: extend seq_notifier callback interface Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).