[PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[Lopez Cruz, Misael]

Add headset jack detection for SDP3430 boards using SoC jack
reporting interface. Headset detection on SDP3430 board is
achieved through TWL4030 GPIO_2 pin.

Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
---
 sound/soc/omap/sdp3430.c |   49 ++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/sound/soc/omap/sdp3430.c b/sound/soc/omap/sdp3430.c
index 4eab4b4..b595aa4 100644
--- a/sound/soc/omap/sdp3430.c
+++ b/sound/soc/omap/sdp3430.c
@@ -28,6 +28,7 @@
 #include <sound/pcm.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <sound/jack.h>
 
 #include <asm/mach-types.h>
 #include <mach/hardware.h>
@@ -122,7 +123,7 @@ static int sdp3430_twl4030_init(struct snd_soc_codec *codec)
 	/* SDP3430 connected pins */
 	snd_soc_dapm_enable_pin(codec, "Ext Mic");
 	snd_soc_dapm_enable_pin(codec, "Ext Spk");
-	snd_soc_dapm_enable_pin(codec, "Headset Jack");
+	snd_soc_dapm_disable_pin(codec, "Headset Jack");
 
 	/* TWL4030 not connected pins */
 	snd_soc_dapm_nc_pin(codec, "AUXL");
@@ -144,6 +145,29 @@ static int sdp3430_twl4030_init(struct snd_soc_codec *codec)
 	return ret;
 }
 
+/* Headset jack */
+struct snd_soc_jack *hs_jack;
+
+/* Headset jack DAPM pins */
+struct snd_soc_jack_pin hs_jack_pins[] = {
+	{
+		.pin = "Headset Jack",
+		.mask = SND_JACK_HEADSET,
+		.invert = 0,
+	},
+};
+
+/* Headset jack gpios */
+struct snd_soc_jack_gpio hs_jack_gpios[] = {
+	{
+		.gpio = (OMAP_MAX_GPIO_LINES + 2),
+		.name = "hsdet-gpio",
+		.report = SND_JACK_HEADSET,
+		.invert = 0,
+		.debounce_time = 200,
+	},
+};
+
 /* Digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link sdp3430_dai = {
 	.name = "TWL4030",
@@ -194,8 +218,25 @@ static int __init sdp3430_soc_init(void)
 	if (ret)
 		goto err1;
 
-	return 0;
+	/* Headset jack detection */
+	hs_jack = kzalloc(sizeof(struct snd_soc_jack), GFP_KERNEL);
+	if (!hs_jack)
+		return -ENOMEM;
+
+	ret = snd_soc_jack_new(&snd_soc_sdp3430, "SDP3430 headset jack",
+				SND_JACK_HEADSET, hs_jack);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_jack_add_pins(hs_jack, ARRAY_SIZE(hs_jack_pins),
+				hs_jack_pins);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_jack_add_gpios(hs_jack, ARRAY_SIZE(hs_jack_gpios),
+				hs_jack_gpios);
 
+	return ret;
 err1:
 	printk(KERN_ERR "Unable to add platform device\n");
 	platform_device_put(sdp3430_snd_device);
@@ -206,6 +247,10 @@ module_init(sdp3430_soc_init);
 
 static void __exit sdp3430_soc_exit(void)
 {
+	snd_soc_jack_free_gpios(hs_jack, ARRAY_SIZE(hs_jack_gpios),
+				hs_jack_gpios);
+	kfree(hs_jack);
+
 	platform_device_unregister(sdp3430_snd_device);
 }
 module_exit(sdp3430_soc_exit);
-- 
1.5.4.3

Re: [PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[Mark Brown]

On Wed, Mar 04, 2009 at 11:39:12AM -0600, Lopez Cruz, Misael wrote:

> +/* Headset jack */
> +struct snd_soc_jack *hs_jack;

This (and all your other globals) should be declared static.

> +/* Headset jack DAPM pins */
> +struct snd_soc_jack_pin hs_jack_pins[] = {
> +	{
> +		.pin = "Headset Jack",
> +		.mask = SND_JACK_HEADSET,
> +		.invert = 0,
> +	},
> +};

No need to initialise things to zero, that's the default for static
variables.

> +	/* Headset jack detection */
> +	hs_jack = kzalloc(sizeof(struct snd_soc_jack), GFP_KERNEL);
> +	if (!hs_jack)
> +		return -ENOMEM;

If you declare hs_jack as a direct static variable you won't need to
kzalloc() it and save some error checking and cleanup code too :)

> +	ret = snd_soc_jack_new(&snd_soc_sdp3430, "SDP3430 headset jack",
> +				SND_JACK_HEADSET, hs_jack);
> +	if (ret)
> +		return ret;

This leaks the jack.

Re: [PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[Lopez Cruz, Misael]

> > +	ret = snd_soc_jack_new(&snd_soc_sdp3430, "SDP3430 headset jack",
> > +				SND_JACK_HEADSET, hs_jack);
> > +	if (ret)
> > +		return ret;

> This leaks the jack.

If hs_jack is declared as a direct static variable instead, then no mem
leaks, isn't it? Or do you mean to clean the hs_jack if any error?

-Misa

[PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[Lopez Cruz, Misael]

Add headset jack detection for SDP3430 boards using SoC jack
reporting interface. Headset detection on SDP3430 board is
achieved through TWL4030 GPIO_2 pin.

Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
---
 sound/soc/omap/sdp3430.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/sound/soc/omap/sdp3430.c b/sound/soc/omap/sdp3430.c
index 4eab4b4..715c648 100644
--- a/sound/soc/omap/sdp3430.c
+++ b/sound/soc/omap/sdp3430.c
@@ -28,6 +28,7 @@
 #include <sound/pcm.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <sound/jack.h>
 
 #include <asm/mach-types.h>
 #include <mach/hardware.h>
@@ -122,7 +123,7 @@ static int sdp3430_twl4030_init(struct snd_soc_codec *codec)
 	/* SDP3430 connected pins */
 	snd_soc_dapm_enable_pin(codec, "Ext Mic");
 	snd_soc_dapm_enable_pin(codec, "Ext Spk");
-	snd_soc_dapm_enable_pin(codec, "Headset Jack");
+	snd_soc_dapm_disable_pin(codec, "Headset Jack");
 
 	/* TWL4030 not connected pins */
 	snd_soc_dapm_nc_pin(codec, "AUXL");
@@ -144,6 +145,27 @@ static int sdp3430_twl4030_init(struct snd_soc_codec *codec)
 	return ret;
 }
 
+/* Headset jack */
+static struct snd_soc_jack hs_jack;
+
+/* Headset jack detection DAPM pins */
+static struct snd_soc_jack_pin hs_jack_pins[] = {
+	{
+		.pin = "Headset Jack",
+		.mask = SND_JACK_HEADSET,
+	},
+};
+
+/* Headset jack detection gpios */
+static struct snd_soc_jack_gpio hs_jack_gpios[] = {
+	{
+		.gpio = (OMAP_MAX_GPIO_LINES + 2),
+		.name = "hsdet-gpio",
+		.report = SND_JACK_HEADSET,
+		.debounce_time = 200,
+	},
+};
+
 /* Digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link sdp3430_dai = {
 	.name = "TWL4030",
@@ -194,7 +216,21 @@ static int __init sdp3430_soc_init(void)
 	if (ret)
 		goto err1;
 
-	return 0;
+	/* Headset jack detection */
+	ret = snd_soc_jack_new(&snd_soc_sdp3430, "SDP3430 headset jack",
+				SND_JACK_HEADSET, &hs_jack);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins),
+				hs_jack_pins);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_jack_add_gpios(&hs_jack, ARRAY_SIZE(hs_jack_gpios),
+				hs_jack_gpios);
+
+	return ret;
 
 err1:
 	printk(KERN_ERR "Unable to add platform device\n");
@@ -206,6 +242,9 @@ module_init(sdp3430_soc_init);
 
 static void __exit sdp3430_soc_exit(void)
 {
+	snd_soc_jack_free_gpios(&hs_jack, ARRAY_SIZE(hs_jack_gpios),
+				hs_jack_gpios);
+
 	platform_device_unregister(sdp3430_snd_device);
 }
 module_exit(sdp3430_soc_exit);
-- 
1.5.4.3

Re: [PATCH 2/2] ASoC: Add headset jack detection for SDP3430 machine driver

[Mark Brown]

On Thu, Mar 05, 2009 at 11:32:31AM -0600, Lopez Cruz, Misael wrote:
> Add headset jack detection for SDP3430 boards using SoC jack
> reporting interface. Headset detection on SDP3430 board is
> achieved through TWL4030 GPIO_2 pin.

Applied, thanks.  One comment:

> +/* Headset jack detection gpios */
> +static struct snd_soc_jack_gpio hs_jack_gpios[] = {
> +	{
> +		.gpio = (OMAP_MAX_GPIO_LINES + 2),

It feels like there should be a nicer way of specifying the GPIO
number?