diff mbox

ASoC: da7218: Enable mic level detection reporting to user-space

Message ID d6102f65136a8457d7ca19a51be4a938f3146613.1449059324.git.Adam.Thomson.Opensource@diasemi.com (mailing list archive)
State New, archived
Headers show

Commit Message

Adam Thomson Dec. 3, 2015, 10:45 a.m. UTC 
This patch adds support to the codec driver to handle mic level
detect related IRQs, and report these to user-space using a uevent
variable.

Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
---
 sound/soc/codecs/da7218.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--
1.9.3

Comments

Takashi Iwai Dec. 3, 2015, 10:55 a.m. UTC | #1 
On Thu, 03 Dec 2015 11:45:50 +0100,
Adam Thomson wrote:
> 
> This patch adds support to the codec driver to handle mic level
> detect related IRQs, and report these to user-space using a uevent
> variable.

Is the uevent the best way for this?


thanks,

Takashi

> 
> Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> ---
>  sound/soc/codecs/da7218.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> index 4fee7ae..752ed04 100644
> --- a/sound/soc/codecs/da7218.c
> +++ b/sound/soc/codecs/da7218.c
> @@ -2202,6 +2202,16 @@ int da7218_hpldet(struct snd_soc_codec *codec, struct snd_soc_jack *jack)
>  }
>  EXPORT_SYMBOL_GPL(da7218_hpldet);
> 
> +static void da7218_micldet_irq(struct snd_soc_codec *codec)
> +{
> +	char *envp[] = {
> +		"EVENT=MIC_LEVEL_DETECT",
> +		NULL,
> +	};
> +
> +	kobject_uevent_env(&codec->dev->kobj, KOBJ_CHANGE, envp);
> +}
> +
>  static void da7218_hpldet_irq(struct snd_soc_codec *codec)
>  {
>  	struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec);
> @@ -2232,6 +2242,10 @@ static irqreturn_t da7218_irq_thread(int irq, void *data)
>  	if (!status)
>  		return IRQ_NONE;
> 
> +	/* Mic level detect */
> +	if (status & DA7218_LVL_DET_EVENT_MASK)
> +		da7218_micldet_irq(codec);
> +
>  	/* HP detect */
>  	if (status & DA7218_HPLDET_JACK_EVENT_MASK)
>  		da7218_hpldet_irq(codec);
> @@ -2936,11 +2950,6 @@ static int da7218_probe(struct snd_soc_codec *codec)
>  	}
> 
>  	if (da7218->irq) {
> -		/* Mask off mic level events, currently not handled */
> -		snd_soc_update_bits(codec, DA7218_EVENT_MASK,
> -				    DA7218_LVL_DET_EVENT_MSK_MASK,
> -				    DA7218_LVL_DET_EVENT_MSK_MASK);
> -
>  		ret = devm_request_threaded_irq(codec->dev, da7218->irq, NULL,
>  						da7218_irq_thread,
>  						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> --
> 1.9.3
> 
>
Adam Thomson Dec. 3, 2015, 11:15 a.m. UTC | #2 
On December 03, 2015 10:56, Takashi Iwai wrote:

> > This patch adds support to the codec driver to handle mic level
> > detect related IRQs, and report these to user-space using a uevent
> > variable.
> 
> Is the uevent the best way for this?
> 
> 
> thanks,
> 
> Takashi

Well originally I was using an input device mechanism to report to user-space,
albeit using KEY_VOICECOMMAND, which Mark mentioned wasn't correct for this
scenario, which was fair enough. He also mentioned that there had been a general
push back on using input devices so that code was dropped. See thread below:

https://lkml.org/lkml/2015/11/10/347

uevent seemed like the simplest solution to report this event, and allow
user-space to act based on it. As mentioned before though, I am open to
suggestions if there's a better way.

> >
> > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> > ---
> >  sound/soc/codecs/da7218.c | 19 ++++++++++++++-----
> >  1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> > index 4fee7ae..752ed04 100644
> > --- a/sound/soc/codecs/da7218.c
> > +++ b/sound/soc/codecs/da7218.c
> > @@ -2202,6 +2202,16 @@ int da7218_hpldet(struct snd_soc_codec *codec,
> struct snd_soc_jack *jack)
> >  }
> >  EXPORT_SYMBOL_GPL(da7218_hpldet);
> >
> > +static void da7218_micldet_irq(struct snd_soc_codec *codec)
> > +{
> > +	char *envp[] = {
> > +		"EVENT=MIC_LEVEL_DETECT",
> > +		NULL,
> > +	};
> > +
> > +	kobject_uevent_env(&codec->dev->kobj, KOBJ_CHANGE, envp);
> > +}
> > +
> >  static void da7218_hpldet_irq(struct snd_soc_codec *codec)
> >  {
> >  	struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec);
> > @@ -2232,6 +2242,10 @@ static irqreturn_t da7218_irq_thread(int irq, void
> *data)
> >  	if (!status)
> >  		return IRQ_NONE;
> >
> > +	/* Mic level detect */
> > +	if (status & DA7218_LVL_DET_EVENT_MASK)
> > +		da7218_micldet_irq(codec);
> > +
> >  	/* HP detect */
> >  	if (status & DA7218_HPLDET_JACK_EVENT_MASK)
> >  		da7218_hpldet_irq(codec);
> > @@ -2936,11 +2950,6 @@ static int da7218_probe(struct snd_soc_codec *codec)
> >  	}
> >
> >  	if (da7218->irq) {
> > -		/* Mask off mic level events, currently not handled */
> > -		snd_soc_update_bits(codec, DA7218_EVENT_MASK,
> > -				    DA7218_LVL_DET_EVENT_MSK_MASK,
> > -				    DA7218_LVL_DET_EVENT_MSK_MASK);
> > -
> >  		ret = devm_request_threaded_irq(codec->dev, da7218->irq, NULL,
> >  						da7218_irq_thread,
> >  						IRQF_TRIGGER_LOW |
> IRQF_ONESHOT,
> > --
> > 1.9.3
> >
> >
Takashi Iwai Dec. 3, 2015, 11:37 a.m. UTC | #3 
On Thu, 03 Dec 2015 12:15:41 +0100,
Opensource [Adam Thomson] wrote:
> 
> On December 03, 2015 10:56, Takashi Iwai wrote:
> 
> > > This patch adds support to the codec driver to handle mic level
> > > detect related IRQs, and report these to user-space using a uevent
> > > variable.
> > 
> > Is the uevent the best way for this?
> > 
> > 
> > thanks,
> > 
> > Takashi
> 
> Well originally I was using an input device mechanism to report to user-space,
> albeit using KEY_VOICECOMMAND, which Mark mentioned wasn't correct for this
> scenario, which was fair enough. He also mentioned that there had been a general
> push back on using input devices so that code was dropped. See thread below:
> 
> https://lkml.org/lkml/2015/11/10/347
> 
> uevent seemed like the simplest solution to report this event, and allow
> user-space to act based on it. As mentioned before though, I am open to
> suggestions if there's a better way.

I'm not strongly opposing to uevent as long as it's well considered.
But then please describe how it's notified properly and why it was
chosen.  The patch changelog has enough free room to write more
details, fortunately.  Tell more your story for Christmas :)

In general, you can use an ALSA control notification for this kind of
event.  But it also depends on the user-space side.  With alsa-lib,
it's pretty easy to use, while with tinyalsa that lacks of the notifier
handling, you'd need to implement more code there.

For a plug/unplug type event, we have a generic jack layer, but this
doesn't seem fitting perfectly, either.


Takashi

> 
> > >
> > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
> > > ---
> > >  sound/soc/codecs/da7218.c | 19 ++++++++++++++-----
> > >  1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
> > > index 4fee7ae..752ed04 100644
> > > --- a/sound/soc/codecs/da7218.c
> > > +++ b/sound/soc/codecs/da7218.c
> > > @@ -2202,6 +2202,16 @@ int da7218_hpldet(struct snd_soc_codec *codec,
> > struct snd_soc_jack *jack)
> > >  }
> > >  EXPORT_SYMBOL_GPL(da7218_hpldet);
> > >
> > > +static void da7218_micldet_irq(struct snd_soc_codec *codec)
> > > +{
> > > +	char *envp[] = {
> > > +		"EVENT=MIC_LEVEL_DETECT",
> > > +		NULL,
> > > +	};
> > > +
> > > +	kobject_uevent_env(&codec->dev->kobj, KOBJ_CHANGE, envp);
> > > +}
> > > +
> > >  static void da7218_hpldet_irq(struct snd_soc_codec *codec)
> > >  {
> > >  	struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec);
> > > @@ -2232,6 +2242,10 @@ static irqreturn_t da7218_irq_thread(int irq, void
> > *data)
> > >  	if (!status)
> > >  		return IRQ_NONE;
> > >
> > > +	/* Mic level detect */
> > > +	if (status & DA7218_LVL_DET_EVENT_MASK)
> > > +		da7218_micldet_irq(codec);
> > > +
> > >  	/* HP detect */
> > >  	if (status & DA7218_HPLDET_JACK_EVENT_MASK)
> > >  		da7218_hpldet_irq(codec);
> > > @@ -2936,11 +2950,6 @@ static int da7218_probe(struct snd_soc_codec *codec)
> > >  	}
> > >
> > >  	if (da7218->irq) {
> > > -		/* Mask off mic level events, currently not handled */
> > > -		snd_soc_update_bits(codec, DA7218_EVENT_MASK,
> > > -				    DA7218_LVL_DET_EVENT_MSK_MASK,
> > > -				    DA7218_LVL_DET_EVENT_MSK_MASK);
> > > -
> > >  		ret = devm_request_threaded_irq(codec->dev, da7218->irq, NULL,
> > >  						da7218_irq_thread,
> > >  						IRQF_TRIGGER_LOW |
> > IRQF_ONESHOT,
> > > --
> > > 1.9.3
> > >
> > >
Adam Thomson Dec. 3, 2015, 2:45 p.m. UTC | #4 
On December 03, 2015 11:37, Takashi Iwai wrote:

> > > > This patch adds support to the codec driver to handle mic level
> > > > detect related IRQs, and report these to user-space using a uevent
> > > > variable.
> > >
> > > Is the uevent the best way for this?
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> >
> > Well originally I was using an input device mechanism to report to user-space,
> > albeit using KEY_VOICECOMMAND, which Mark mentioned wasn't correct for this
> > scenario, which was fair enough. He also mentioned that there had been a general
> > push back on using input devices so that code was dropped. See thread below:
> >
> > https://lkml.org/lkml/2015/11/10/347
> >
> > uevent seemed like the simplest solution to report this event, and allow
> > user-space to act based on it. As mentioned before though, I am open to
> > suggestions if there's a better way.
> 
> I'm not strongly opposing to uevent as long as it's well considered.
> But then please describe how it's notified properly and why it was
> chosen.  The patch changelog has enough free room to write more
> details, fortunately.  Tell more your story for Christmas :)
> 
> In general, you can use an ALSA control notification for this kind of
> event.  But it also depends on the user-space side.  With alsa-lib,
> it's pretty easy to use, while with tinyalsa that lacks of the notifier
> handling, you'd need to implement more code there.
> 
> For a plug/unplug type event, we have a generic jack layer, but this
> doesn't seem fitting perfectly, either.

For ALSA control notification, this implies there's a control associated, but
for this scenario that's really not necessary. You just want to know when the
noise level captured at the mic has gone above a certain threshold and then act
accordingly (process audio capture stream). There's no real need I can see for
a control to read from. As for the jack layer, I agree that it doesn't really
fit.

Ok, will update the patch commit message to add more details on this, including
references to Santa Claus, reindeer with shining noses, and other festive
symbols. :)
diff mbox

Patch

diff --git a/sound/soc/codecs/da7218.c b/sound/soc/codecs/da7218.c
index 4fee7ae..752ed04 100644
--- a/sound/soc/codecs/da7218.c
+++ b/sound/soc/codecs/da7218.c
@@ -2202,6 +2202,16 @@  int da7218_hpldet(struct snd_soc_codec *codec, struct snd_soc_jack *jack)
 }
 EXPORT_SYMBOL_GPL(da7218_hpldet);

+static void da7218_micldet_irq(struct snd_soc_codec *codec)
+{
+	char *envp[] = {
+		"EVENT=MIC_LEVEL_DETECT",
+		NULL,
+	};
+
+	kobject_uevent_env(&codec->dev->kobj, KOBJ_CHANGE, envp);
+}
+
 static void da7218_hpldet_irq(struct snd_soc_codec *codec)
 {
 	struct da7218_priv *da7218 = snd_soc_codec_get_drvdata(codec);
@@ -2232,6 +2242,10 @@  static irqreturn_t da7218_irq_thread(int irq, void *data)
 	if (!status)
 		return IRQ_NONE;

+	/* Mic level detect */
+	if (status & DA7218_LVL_DET_EVENT_MASK)
+		da7218_micldet_irq(codec);
+
 	/* HP detect */
 	if (status & DA7218_HPLDET_JACK_EVENT_MASK)
 		da7218_hpldet_irq(codec);
@@ -2936,11 +2950,6 @@  static int da7218_probe(struct snd_soc_codec *codec)
 	}

 	if (da7218->irq) {
-		/* Mask off mic level events, currently not handled */
-		snd_soc_update_bits(codec, DA7218_EVENT_MASK,
-				    DA7218_LVL_DET_EVENT_MSK_MASK,
-				    DA7218_LVL_DET_EVENT_MSK_MASK);
-
 		ret = devm_request_threaded_irq(codec->dev, da7218->irq, NULL,
 						da7218_irq_thread,
 						IRQF_TRIGGER_LOW | IRQF_ONESHOT,