diff mbox

[v2] ASoC: rt286: add jack detection enable/disable APIs and export

Message ID 1423056843-26490-1-git-send-email-yang.jie@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jie, Yang Feb. 4, 2015, 1:34 p.m. UTC 
Some platforms, e.g. WSB, don't need jack detection when
system is in Suspend, for power save reason.

Here add jack detection enable/disable APIs and export them, when
disabled, it will ignore interrupt, and disable LDO1, which is
used for jack detection when headphone is plugged in.

Signed-off-by: Jie Yang <yang.jie@intel.com>
Reviewed-by: Bard Liao <bardliao@realtek.com>
---
 sound/soc/codecs/rt286.c | 24 ++++++++++++++++++++++++
 sound/soc/codecs/rt286.h |  2 ++
 2 files changed, 26 insertions(+)

Comments

Mark Brown Feb. 4, 2015, 8:26 p.m. UTC | #1 
On Wed, Feb 04, 2015 at 09:34:03PM +0800, Jie Yang wrote:

> +int rt286_disable_jack_detection(struct snd_soc_codec *codec)
> +{
> +	struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
> +
> +	disable_irq(rt286->i2c->irq);
> +
> +	snd_soc_dapm_disable_pin(&rt286->codec->dapm, "LDO1");
> +	snd_soc_dapm_sync(&rt286->codec->dapm);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rt286_disable_jack_detection);

So, now I look at this again I'm wondering why this isn't writing to any
registers - looking at the code we run on probe there's a register write
if we're passed an interrupt which looks awfully like we're enabling
jack detect.  Probably we wouldn't need to do the rather odd disabling
of the IRQ if we did that, but perhaps it's required.

I think there's actually problems with the existing jack detection code
in the driver, I'd expect that the initial enabling of jack detect would
be done when the machine driver says it wants jack detect by passing in
the jack to report for.
Jie, Yang Feb. 5, 2015, 2:07 a.m. UTC | #2 
> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Thursday, February 05, 2015 4:26 AM
> To: Jie, Yang
> Cc: alsa-devel@alsa-project.org; bardliao@realtek.com; Girdwood, Liam R
> Subject: Re: [PATCH v2] ASoC: rt286: add jack detection enable/disable APIs and
> export
> 
> On Wed, Feb 04, 2015 at 09:34:03PM +0800, Jie Yang wrote:
> 
> > +int rt286_disable_jack_detection(struct snd_soc_codec *codec) {
> > +	struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
> > +
> > +	disable_irq(rt286->i2c->irq);
> > +
> > +	snd_soc_dapm_disable_pin(&rt286->codec->dapm, "LDO1");
> > +	snd_soc_dapm_sync(&rt286->codec->dapm);
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rt286_disable_jack_detection);
> 
> So, now I look at this again I'm wondering why this isn't writing to any registers
> - looking at the code we run on probe there's a register write if we're passed an
> interrupt which looks awfully like we're enabling jack detect.  Probably we
> wouldn't need to do the rather odd disabling of the IRQ if we did that, but
> perhaps it's required.
[Keyon] yes, it's required to disable jack detection, otherwise, pin LDO1 will be force enabled again. So here I just disable the IRQ to make the code change smallest.

> 
> I think there's actually problems with the existing jack detection code in the
> driver, I'd expect that the initial enabling of jack detect would be done when the
> machine driver says it wants jack detect by passing in the jack to report for.
[Keyon] I agree that it may be more reasonable if we can pass jack(similar to rt286_mic_detect(), can use jack=NULL for disable) from this machine driver, then add checking this jack in handler rt286_irq(), don't call rt286_jack_detect() and snd_soc_jack_report() when the jack is NULL.
What do you think about this solution?
Mark Brown Feb. 5, 2015, 6:16 p.m. UTC | #3 
On Thu, Feb 05, 2015 at 02:07:35AM +0000, Jie, Yang wrote:

Again, please fix your mailer to word wrap within paragraphs - your
mails are very difficult to read.

> [Keyon] I agree that it may be more reasonable if we can pass
> jack(similar to rt286_mic_detect(), can use jack=NULL for disable)
> from this machine driver, then add checking this jack in handler
> rt286_irq(), don't call rt286_jack_detect() and snd_soc_jack_report()
> when the jack is NULL.  What do you think about this solution?

That sounds fine.  You should be able to just call the jack API with
NULL jacks, it's set up to handle this since it's such a common case.
diff mbox

Patch

diff --git a/sound/soc/codecs/rt286.c b/sound/soc/codecs/rt286.c
index 8104d22..9ea01d0 100644
--- a/sound/soc/codecs/rt286.c
+++ b/sound/soc/codecs/rt286.c
@@ -400,6 +400,30 @@  int rt286_mic_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack)
 }
 EXPORT_SYMBOL_GPL(rt286_mic_detect);
 
+int rt286_enable_jack_detection(struct snd_soc_codec *codec)
+{
+	struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
+
+	if (rt286->jack->status | SND_JACK_HEADPHONE)
+		snd_soc_dapm_force_enable_pin(&rt286->codec->dapm, "LDO1");
+	snd_soc_dapm_sync(&rt286->codec->dapm);
+	enable_irq(rt286->i2c->irq);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rt286_enable_jack_detection);
+
+int rt286_disable_jack_detection(struct snd_soc_codec *codec)
+{
+	struct rt286_priv *rt286 = snd_soc_codec_get_drvdata(codec);
+
+	disable_irq(rt286->i2c->irq);
+
+	snd_soc_dapm_disable_pin(&rt286->codec->dapm, "LDO1");
+	snd_soc_dapm_sync(&rt286->codec->dapm);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rt286_disable_jack_detection);
+
 static int is_mclk_mode(struct snd_soc_dapm_widget *source,
 			 struct snd_soc_dapm_widget *sink)
 {
diff --git a/sound/soc/codecs/rt286.h b/sound/soc/codecs/rt286.h
index b539b73..c958182 100644
--- a/sound/soc/codecs/rt286.h
+++ b/sound/soc/codecs/rt286.h
@@ -193,6 +193,8 @@  enum {
 };
 
 int rt286_mic_detect(struct snd_soc_codec *codec, struct snd_soc_jack *jack);
+int rt286_enable_jack_detection(struct snd_soc_codec *codec);
+int rt286_disable_jack_detection(struct snd_soc_codec *codec);
 
 #endif /* __RT286_H__ */