[PATCH] tlv320aic3x: headset/button press support

[PATCH] tlv320aic3x: headset/button press support

[Daniel Mack]

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

Hi,

This patch adds a function to setup headset and button press detection
modes.

Also, aic3x_headset_detected() returns the value from the current state's
register rather than the interrupt cause flag so it works without an 
interrupt as well.

Best regards,
Daniel


[-- Attachment #2: tlv320aic3x-headset.diff --]
[-- Type: text/x-diff, Size: 3136 bytes --]

Added a function to setup headset and button press detection modes. Also,
aic3x_headset_detected() returns the value from the current state's register
rather than the interrupt cause flag so it works without an interrupt as well.

Signed-off-by: Daniel Mack <daniel@caiaq.de>

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 2047c66..a41aa72 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1017,14 +1017,32 @@ int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio)
 }
 EXPORT_SYMBOL_GPL(aic3x_get_gpio);
 
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int val)
+{
+	u8 reg_a = val & 0xff;
+	if (reg_a & (3 << 5))
+		reg_a |= 0x80;
+
+	aic3x_write(codec, AIC3X_HEADSET_DETECT_CTRL_A, reg_a);
+}
+EXPORT_SYMBOL_GPL(aic3x_set_headset_detection);
+
 int aic3x_headset_detected(struct snd_soc_codec *codec)
 {
 	u8 val;
-	aic3x_read(codec, AIC3X_RT_IRQ_FLAGS_REG, &val);
-	return (val >> 2) & 1;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 4) & 1;
 }
 EXPORT_SYMBOL_GPL(aic3x_headset_detected);
 
+int aic3x_button_pressed(struct snd_soc_codec *codec)
+{
+	u8 val;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 5) & 1;
+}
+EXPORT_SYMBOL_GPL(aic3x_button_pressed);
+
 #define AIC3X_RATES	SNDRV_PCM_RATE_8000_96000
 #define AIC3X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
 			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index 7e982ac..2f36c03 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -39,7 +39,9 @@
 #define AIC3X_OVRF_STATUS_AND_PLLR_REG	11
 /* Audio codec digital filter control register */
 #define AIC3X_CODEC_DFILT_CTRL		12
-
+/* Headset/button press detection register */
+#define AIC3X_HEADSET_DETECT_CTRL_A	13
+#define AIC3X_HEADSET_DETECT_CTRL_B	14
 /* ADC PGA Gain control registers */
 #define LADC_VOL			15
 #define RADC_VOL			16
@@ -233,7 +235,28 @@ enum {
 
 void aic3x_set_gpio(struct snd_soc_codec *codec, int gpio, int state);
 int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio);
+
+/* headset detection / button API, logically OR the values
+ * on calls to aic3x_set_headset_detection(). */
+#define AIC3X_HEADSET_DETECT_OFF	(0 << 5)
+#define AIC3X_HEADSET_DETECT_STEREO	(1 << 5)
+#define AIC3X_HEADSET_DETECT_CELLULAR	(2 << 5)
+#define AIC3X_HEADSET_DETECT_BOTH	(3 << 5)
+
+#define AIC3X_HEADSET_DEBOUNCE_16MS	(0 << 2)
+#define AIC3X_HEADSET_DEBOUNCE_32MS	(1 << 2)
+#define AIC3X_HEADSET_DEBOUNCE_64MS	(2 << 2)
+#define AIC3X_HEADSET_DEBOUNCE_128MS	(3 << 2)
+#define AIC3X_HEADSET_DEBOUNCE_256MS	(4 << 2)
+
+#define AIC3X_BUTTON_DEBOUNCE_0MS	(0 << 0)
+#define AIC3X_BUTTON_DEBOUNCE_8MS	(1 << 0)
+#define AIC3X_BUTTON_DEBOUNCE_16MS	(2 << 0)
+#define AIC3X_BUTTON_DEBOUNCE_32MS	(3 << 0)
+
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int val);
 int aic3x_headset_detected(struct snd_soc_codec *codec);
+int aic3x_button_pressed(struct snd_soc_codec *codec);
 
 struct aic3x_setup_data {
 	int i2c_bus;

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] tlv320aic3x: headset/button press support

[Mark Brown]

On Wed, Nov 26, 2008 at 11:07:42PM +0100, Daniel Mack wrote:

> Hi,

As previously mentioned it'd be really helpful if you could submit
patches in the standard format in Documentation/SubmittingPatches.
Even if you need to use an attachment for the code pleasee don't submit
two separate changelogs as you currently do.

> +void aic3x_set_headset_detection(struct snd_soc_codec *codec, int val)
> +{
> +	u8 reg_a = val & 0xff;
> +	if (reg_a & (3 << 5))
> +		reg_a |= 0x80;

This is a little obscure...  Why are you munging the set value like
this?  Replacing (3 << 5) with the constants from the header may help.
Looking at the defines below I can't help but think that it'd be clearer
to pass in three parameters - what to detect and the two debounce times.

>  int aic3x_headset_detected(struct snd_soc_codec *codec)
>  {
>  	u8 val;
> -	aic3x_read(codec, AIC3X_RT_IRQ_FLAGS_REG, &val);
> -	return (val >> 2) & 1;
> +	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
> +	return (val >> 4) & 1;
>  }
>  EXPORT_SYMBOL_GPL(aic3x_headset_detected);

How is the interrupt deasserted?  Are you sure that the interrupt will
be cleared if you don't read the interrupt flags register?  May be worth
splitting out, it's not directly part of the rest of the patch.

> +
> +/* headset detection / button API, logically OR the values
> + * on calls to aic3x_set_headset_detection(). */

This could go in kerneldoc for the function as well.

> +#define AIC3X_HEADSET_DETECT_OFF	(0 << 5)
> +#define AIC3X_HEADSET_DETECT_STEREO	(1 << 5)
> +#define AIC3X_HEADSET_DETECT_CELLULAR	(2 << 5)
> +#define AIC3X_HEADSET_DETECT_BOTH	(3 << 5)

What exactly is CELLULAR?  A headset?

Re: [PATCH] tlv320aic3x: headset/button press support

[Daniel Mack]

On Thu, Nov 27, 2008 at 11:50:21AM +0000, Mark Brown wrote:
> > +void aic3x_set_headset_detection(struct snd_soc_codec *codec, int val)
> > +{
> > +	u8 reg_a = val & 0xff;
> > +	if (reg_a & (3 << 5))
> > +		reg_a |= 0x80;
> 
> This is a little obscure...  Why are you munging the set value like
> this?  Replacing (3 << 5) with the constants from the header may help.
> Looking at the defines below I can't help but think that it'd be clearer
> to pass in three parameters - what to detect and the two debounce times.

Ok, done in the new patch. There is one bit (0x80) which needs to be set
if any of 0x60 is given, hence the bit banging.

> >  int aic3x_headset_detected(struct snd_soc_codec *codec)
> >  {
> >  	u8 val;
> > -	aic3x_read(codec, AIC3X_RT_IRQ_FLAGS_REG, &val);
> > -	return (val >> 2) & 1;
> > +	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
> > +	return (val >> 4) & 1;
> >  }
> >  EXPORT_SYMBOL_GPL(aic3x_headset_detected);
> 
> How is the interrupt deasserted?  Are you sure that the interrupt will
> be cleared if you don't read the interrupt flags register? 

Seems to be, as it works for our device here. The point is that headset
insertions/removals do not neccessarily cause an interrupt, so the bit
in the flags registers won't be set. One good example is the power-up
condition where no event will be triggered but you still want to know
whether there is anything in the jack.

> > +#define AIC3X_HEADSET_DETECT_OFF	(0 << 5)
> > +#define AIC3X_HEADSET_DETECT_STEREO	(1 << 5)
> > +#define AIC3X_HEADSET_DETECT_CELLULAR	(2 << 5)
> > +#define AIC3X_HEADSET_DETECT_BOTH	(3 << 5)
> 
> What exactly is CELLULAR?  A headset?

There is different types of headsets supported by this chip (see page 42
of the datasheet), and this bits define which types the detection should
be enabled for.

Best regards,
Daniel

---

- Add aic3x_set_headset_detection() function to define the headset detection
  mode for tlv32aic3x chips
- added aic3x_button_pressed()
- Read from the real-time registers in aic3x_headset_detected() to query
  headset presence without an occured interrupt

Signed-off-by: Daniel Mack <daniel@caiaq.de>

 tlv320aic3x.c |   31 +++++++++++++++++++++++++++++--
 tlv320aic3x.h |   40 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 68 insertions(+), 3 deletions(-)


diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 2047c66..537bbbf 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1017,14 +1017,41 @@ int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio)
 }
 EXPORT_SYMBOL_GPL(aic3x_get_gpio);
 
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int detect,
+				 int headset_debounce, int button_debounce)
+{
+	u8 val;
+
+	val = ((detect & AIC3X_HEADSET_DETECT_MASK)
+		<< AIC3X_HEADSET_DETECT_SHIFT) |
+	      ((headset_debounce & AIC3X_HEADSET_DEBOUNCE_MASK)
+		<< AIC3X_HEADSET_DEBOUNCE_SHIFT) |
+	      ((button_debounce & AIC3X_BUTTON_DEBOUNCE_MASK)
+		<< AIC3X_BUTTON_DEBOUNCE_SHIFT);
+
+	if (detect & AIC3X_HEADSET_DETECT_MASK)
+		val |= AIC3X_HEADSET_DETECT_ENABLED;
+
+	aic3x_write(codec, AIC3X_HEADSET_DETECT_CTRL_A, val);
+}
+EXPORT_SYMBOL_GPL(aic3x_set_headset_detection);
+
 int aic3x_headset_detected(struct snd_soc_codec *codec)
 {
 	u8 val;
-	aic3x_read(codec, AIC3X_RT_IRQ_FLAGS_REG, &val);
-	return (val >> 2) & 1;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 4) & 1;
 }
 EXPORT_SYMBOL_GPL(aic3x_headset_detected);
 
+int aic3x_button_pressed(struct snd_soc_codec *codec)
+{
+	u8 val;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 5) & 1;
+}
+EXPORT_SYMBOL_GPL(aic3x_button_pressed);
+
 #define AIC3X_RATES	SNDRV_PCM_RATE_8000_96000
 #define AIC3X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
 			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index 7e982ac..d6736c0 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -39,7 +39,9 @@
 #define AIC3X_OVRF_STATUS_AND_PLLR_REG	11
 /* Audio codec digital filter control register */
 #define AIC3X_CODEC_DFILT_CTRL		12
-
+/* Headset/button press detection register */
+#define AIC3X_HEADSET_DETECT_CTRL_A	13
+#define AIC3X_HEADSET_DETECT_CTRL_B	14
 /* ADC PGA Gain control registers */
 #define LADC_VOL			15
 #define RADC_VOL			16
@@ -233,7 +235,43 @@ enum {
 
 void aic3x_set_gpio(struct snd_soc_codec *codec, int gpio, int state);
 int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio);
+
+/* headset detection / button API */
+enum {
+	AIC3X_HEADSET_DETECT_OFF	= 0,
+	AIC3X_HEADSET_DETECT_STEREO	= 1,
+	AIC3X_HEADSET_DETECT_CELLULAR   = 2,
+	AIC3X_HEADSET_DETECT_BOTH	= 3
+};
+
+enum {
+	AIC3X_HEADSET_DEBOUNCE_16MS	= 0,
+	AIC3X_HEADSET_DEBOUNCE_32MS	= 1,
+	AIC3X_HEADSET_DEBOUNCE_64MS	= 2,
+	AIC3X_HEADSET_DEBOUNCE_128MS	= 3,
+	AIC3X_HEADSET_DEBOUNCE_256MS	= 4,
+	AIC3X_HEADSET_DEBOUNCE_512MS	= 5
+};
+
+enum {
+	AIC3X_BUTTON_DEBOUNCE_0MS	= 0,
+	AIC3X_BUTTON_DEBOUNCE_8MS	= 1,
+	AIC3X_BUTTON_DEBOUNCE_16MS	= 2,
+	AIC3X_BUTTON_DEBOUNCE_32MS	= 3
+};
+
+#define AIC3X_HEADSET_DETECT_ENABLED	0x80
+#define AIC3X_HEADSET_DETECT_SHIFT	5
+#define AIC3X_HEADSET_DETECT_MASK	3
+#define AIC3X_HEADSET_DEBOUNCE_SHIFT	2
+#define AIC3X_HEADSET_DEBOUNCE_MASK	7
+#define AIC3X_BUTTON_DEBOUNCE_SHIFT 	0
+#define AIC3X_BUTTON_DEBOUNCE_MASK	3
+
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int detect,
+				 int headset_debounce, int button_debounce);
 int aic3x_headset_detected(struct snd_soc_codec *codec);
+int aic3x_button_pressed(struct snd_soc_codec *codec);
 
 struct aic3x_setup_data {
 	int i2c_bus;

Re: [PATCH] tlv320aic3x: headset/button press support

[Mark Brown]

On Tue, Dec 02, 2008 at 05:37:54PM +0100, Daniel Mack wrote:
> On Thu, Nov 27, 2008 at 11:50:21AM +0000, Mark Brown wrote:

> > > +#define AIC3X_HEADSET_DETECT_CELLULAR	(2 << 5)
> > > +#define AIC3X_HEADSET_DETECT_BOTH	(3 << 5)

> > What exactly is CELLULAR?  A headset?

> There is different types of headsets supported by this chip (see page 42
> of the datasheet), and this bits define which types the detection should
> be enabled for.

I *belive* it's a headset but please submit a patch adding some text
documenting this, ideally people shouldn't have to refer to the
datasheet to use the driver.

> Best regards,
> Daniel
> 
> ---
> 
> - Add aic3x_set_headset_detection() function to define the headset detection
>   mode for tlv32aic3x chips
> - added aic3x_button_pressed()
> - Read from the real-time registers in aic3x_headset_detected() to query
>   headset presence without an occured interrupt
> 
> Signed-off-by: Daniel Mack <daniel@caiaq.de>

As I have repeatedly mentioned it would be very much better if you were
to do your submissions in the standard fashion which is documented in
Documentation/SubmittingPatches.  The fact that you don't do this makes
your patches more difficult to apply and doesn't encourage review.

Note in particular the --- above which causes your commit message to be
removed.

Re: [PATCH] tlv320aic3x: headset/button press support

[Daniel Mack]

Hi Ben,

On Tue, Dec 02, 2008 at 07:50:19PM +0000, Mark Brown wrote:
> > There is different types of headsets supported by this chip (see page 42
> > of the datasheet), and this bits define which types the detection should
> > be enabled for.
> 
> I *belive* it's a headset but please submit a patch adding some text
> documenting this, ideally people shouldn't have to refer to the
> datasheet to use the driver.

Ok, added some lines of comment in my new version. However, I strongly
believe that anyone who uses that chip needs to read the datasheet
carefully anyway as it is very versatile and full of pitfalls therefore.

> As I have repeatedly mentioned it would be very much better if you were
> to do your submissions in the standard fashion which is documented in
> Documentation/SubmittingPatches.

I'm switching over to git-format-patch now, hoping this tool does it
right. Sorry for the trouble, I didn't want to bother you.

Best regards,
Daniel

Re: [PATCH] tlv320aic3x: headset/button press support

[Daniel Mack]

- Add aic3x_set_headset_detection() function to define the headset
  detection mode for tlv32aic3x chips
- added aic3x_button_pressed()
- Read from the real-time registers in aic3x_headset_detected() to query
  headset presence without an occured interrupt


Signed-off-by: Daniel Mack <daniel@caiaq.de>
---

Compared to the version I've sent before, this one adds some lines of
comments for aic3x_set_headset_detection() in the header file.

 sound/soc/codecs/tlv320aic3x.c |   31 +++++++++++++++++++++++++-
 sound/soc/codecs/tlv320aic3x.h |   46 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 2047c66..537bbbf 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -1017,14 +1017,41 @@ int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio)
 }
 EXPORT_SYMBOL_GPL(aic3x_get_gpio);
 
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int detect,
+				 int headset_debounce, int button_debounce)
+{
+	u8 val;
+
+	val = ((detect & AIC3X_HEADSET_DETECT_MASK)
+		<< AIC3X_HEADSET_DETECT_SHIFT) |
+	      ((headset_debounce & AIC3X_HEADSET_DEBOUNCE_MASK)
+		<< AIC3X_HEADSET_DEBOUNCE_SHIFT) |
+	      ((button_debounce & AIC3X_BUTTON_DEBOUNCE_MASK)
+		<< AIC3X_BUTTON_DEBOUNCE_SHIFT);
+
+	if (detect & AIC3X_HEADSET_DETECT_MASK)
+		val |= AIC3X_HEADSET_DETECT_ENABLED;
+
+	aic3x_write(codec, AIC3X_HEADSET_DETECT_CTRL_A, val);
+}
+EXPORT_SYMBOL_GPL(aic3x_set_headset_detection);
+
 int aic3x_headset_detected(struct snd_soc_codec *codec)
 {
 	u8 val;
-	aic3x_read(codec, AIC3X_RT_IRQ_FLAGS_REG, &val);
-	return (val >> 2) & 1;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 4) & 1;
 }
 EXPORT_SYMBOL_GPL(aic3x_headset_detected);
 
+int aic3x_button_pressed(struct snd_soc_codec *codec)
+{
+	u8 val;
+	aic3x_read(codec, AIC3X_HEADSET_DETECT_CTRL_B, &val);
+	return (val >> 5) & 1;
+}
+EXPORT_SYMBOL_GPL(aic3x_button_pressed);
+
 #define AIC3X_RATES	SNDRV_PCM_RATE_8000_96000
 #define AIC3X_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
 			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S32_LE)
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index 7e982ac..73e35b6 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -39,7 +39,9 @@
 #define AIC3X_OVRF_STATUS_AND_PLLR_REG	11
 /* Audio codec digital filter control register */
 #define AIC3X_CODEC_DFILT_CTRL		12
-
+/* Headset/button press detection register */
+#define AIC3X_HEADSET_DETECT_CTRL_A	13
+#define AIC3X_HEADSET_DETECT_CTRL_B	14
 /* ADC PGA Gain control registers */
 #define LADC_VOL			15
 #define RADC_VOL			16
@@ -233,7 +235,49 @@ enum {
 
 void aic3x_set_gpio(struct snd_soc_codec *codec, int gpio, int state);
 int aic3x_get_gpio(struct snd_soc_codec *codec, int gpio);
+
+/* headset detection / button API */
+
+/* The AIC3x supports detection of stereo headsets (GND + left + right signal)
+ * and cellular headsets (GND + speaker output + microphone input).
+ * It is recommended to enable MIC bias for this function to work properly.
+ * For more information, please refer to the datasheet. */
+enum {
+	AIC3X_HEADSET_DETECT_OFF	= 0,
+	AIC3X_HEADSET_DETECT_STEREO	= 1,
+	AIC3X_HEADSET_DETECT_CELLULAR   = 2,
+	AIC3X_HEADSET_DETECT_BOTH	= 3
+};
+
+enum {
+	AIC3X_HEADSET_DEBOUNCE_16MS	= 0,
+	AIC3X_HEADSET_DEBOUNCE_32MS	= 1,
+	AIC3X_HEADSET_DEBOUNCE_64MS	= 2,
+	AIC3X_HEADSET_DEBOUNCE_128MS	= 3,
+	AIC3X_HEADSET_DEBOUNCE_256MS	= 4,
+	AIC3X_HEADSET_DEBOUNCE_512MS	= 5
+};
+
+enum {
+	AIC3X_BUTTON_DEBOUNCE_0MS	= 0,
+	AIC3X_BUTTON_DEBOUNCE_8MS	= 1,
+	AIC3X_BUTTON_DEBOUNCE_16MS	= 2,
+	AIC3X_BUTTON_DEBOUNCE_32MS	= 3
+};
+
+#define AIC3X_HEADSET_DETECT_ENABLED	0x80
+#define AIC3X_HEADSET_DETECT_SHIFT	5
+#define AIC3X_HEADSET_DETECT_MASK	3
+#define AIC3X_HEADSET_DEBOUNCE_SHIFT	2
+#define AIC3X_HEADSET_DEBOUNCE_MASK	7
+#define AIC3X_BUTTON_DEBOUNCE_SHIFT 	0
+#define AIC3X_BUTTON_DEBOUNCE_MASK	3
+
+/* see the enums above for valid parameters to this function */
+void aic3x_set_headset_detection(struct snd_soc_codec *codec, int detect,
+				 int headset_debounce, int button_debounce);
 int aic3x_headset_detected(struct snd_soc_codec *codec);
+int aic3x_button_pressed(struct snd_soc_codec *codec);
 
 struct aic3x_setup_data {
 	int i2c_bus;
-- 
1.5.6.5