[PATCH] ASoC: Add GPIO support for jack reporting interface

[PATCH] ASoC: Add GPIO support for jack reporting interface

[Lopez Cruz, Misael]

Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
The gpio support exports two new functions: snd_soc_jack_add_gpios and
snd_soc_jack_free_gpios.

Client drivers using gpio feature must pass an array of jack_gpio pins
belonging to a specific jack to the snd_soc_jack_add_gpios function. The
framework will request the gpio, set the data direction and request irq.
The framework will update power status of related jack_pins when an event on
the gpio pins comes according to the reporting bits defined for each gpio.

All gpio resources allocated when adding jack_gpio pins for a particular
jack can be released using snd_soc_jack_free_gpios function.

Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
---
 include/sound/soc.h  |   27 +++++++++++
 sound/soc/soc-jack.c |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 68d8149..ad0466b 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -16,6 +16,8 @@
 #include <linux/platform_device.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/control.h>
@@ -168,6 +170,7 @@ struct soc_enum;
 struct snd_soc_ac97_ops;
 struct snd_soc_jack;
 struct snd_soc_jack_pin;
+struct snd_soc_jack_gpio;
 
 typedef int (*hw_write_t)(void *,const char* ,int);
 typedef int (*hw_read_t)(void *,char* ,int);
@@ -194,6 +197,9 @@ int snd_soc_jack_new(struct snd_soc_card *card, const char *id, int type,
 void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask);
 int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 			  struct snd_soc_jack_pin *pins);
+int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
+			  struct snd_soc_jack_gpio *gpios);
+void snd_soc_jack_free_gpios(struct snd_soc_jack *jack);
 
 /* codec IO */
 #define snd_soc_read(codec, reg) codec->read(codec, reg)
@@ -262,10 +268,31 @@ struct snd_soc_jack_pin {
 	bool invert;
 };
 
+/**
+ * struct snd_soc_jack_gpio - Describes a gpio pin for jack detection
+ *
+ * @gpio:         gpio number
+ * @name:         gpio name
+ * @report:       value to report when jack detected
+ * @invert:       report presence in low state
+ * @debouce_time: debouce time in ms
+ */
+struct snd_soc_jack_gpio {
+	unsigned int gpio;
+	const char *name;
+	int report;
+	int invert;
+	int debounce_time;
+	struct snd_soc_jack *jack;
+	struct work_struct work;
+};
+
 struct snd_soc_jack {
 	struct snd_jack *jack;
 	struct snd_soc_card *card;
 	struct list_head pins;
+	struct snd_soc_jack_gpio *gpios;
+	int gpio_count;
 	int status;
 };
 
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 8cc00c3..0e9ee28 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -14,6 +14,10 @@
 #include <sound/jack.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/delay.h>
 
 /**
  * snd_soc_jack_new - Create a new jack
@@ -136,3 +140,122 @@ int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_jack_add_pins);
+
+/* gpio detect */
+void snd_soc_jack_gpio_detect(struct snd_soc_jack_gpio *gpio)
+{
+	struct snd_soc_jack *jack = gpio->jack;
+	int enable;
+	int report;
+
+	if (gpio->debounce_time > 0)
+		mdelay(gpio->debounce_time);
+
+	enable = gpio_get_value(gpio->gpio);
+	if (gpio->invert)
+		enable != enable;
+
+	if (enable)
+		report = gpio->report;
+	else
+		report = 0;
+
+	snd_soc_jack_report(jack, report, gpio->report);
+}
+
+/* irq handler for gpio pin */
+static irqreturn_t gpio_handler(int irq, void *data)
+{
+	struct snd_soc_jack_gpio *gpio = data;
+
+	return IRQ_RETVAL(schedule_work(&gpio->work));
+}
+
+/* gpio work */
+static void gpio_work(struct work_struct *work)
+{
+	struct snd_soc_jack_gpio *gpio;
+
+	gpio = container_of(work, struct snd_soc_jack_gpio, work);
+	snd_soc_jack_gpio_detect(gpio);
+}
+
+/**
+ * snd_soc_jack_add_gpios - Associate GPIO pins with an ASoC jack
+ *
+ * @jack:  ASoC jack
+ * @count: number of pins
+ * @gpios: array of gpio pins
+ *
+ * This function will request gpio, set data direction and request irq
+ * for each gpio in the array. Valid gpio pins references will be saved.
+ */
+int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
+			struct snd_soc_jack_gpio *gpios)
+{
+	int i, ret = 0;
+
+	/* Link gpio array with the soc_jack */
+	if (count > 0)
+		jack->gpios = gpios;
+
+	for (i = 0; i < count; i++) {
+		if (!gpio_is_valid(gpios[i].gpio)) {
+			printk(KERN_ERR "Invalid gpio %d\n",
+				gpios[i].gpio);
+			return -EINVAL;
+		}
+		if (!gpios[i].name) {
+			printk(KERN_ERR "No name for gpio %d\n",
+				gpios[i].gpio);
+			return -EINVAL;
+		}
+
+		ret = gpio_request(gpios[i].gpio, gpios[i].name);
+		if (ret)
+			return ret;
+
+		ret = gpio_direction_input(gpios[i].gpio);
+		if (ret)
+			break;
+
+		ret = request_irq(gpio_to_irq(gpios[i].gpio),
+				gpio_handler,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				jack->card->dev->driver->name,
+				&gpios[i]);
+		if (ret)
+			break;
+
+		INIT_WORK(&gpios[i].work, gpio_work);
+		gpios[i].jack = jack;
+	}
+
+	if (ret)
+		gpio_free(gpios[i].gpio);
+
+	jack->gpio_count = i;	/* gpios allocated properly */
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_jack_add_gpios);
+
+/**
+ * snd_soc_jack_free_gpios - Release GPIO pins' resources of an ASoC jack
+ *
+ * @jack:  ASoC jack
+ * @count: number of pins
+ *
+ * Release gpio and irq resources for gpio pins associated with an ASoC jack.
+ */
+void snd_soc_jack_free_gpios(struct snd_soc_jack *jack)
+{
+	struct snd_soc_jack_gpio *gpios = jack->gpios;
+	int i;
+
+	for (i = 0; i < jack->gpio_count; i++) {
+		free_irq(gpio_to_irq(gpios[i].gpio), &gpios[i]);
+		gpio_free(gpios[i].gpio);
+	}
+}
+EXPORT_SYMBOL_GPL(snd_soc_jack_free_gpios);
-- 
1.5.4.3

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Takashi Iwai]

At Mon, 2 Mar 2009 19:05:28 -0600,
Lopez Cruz, Misael wrote:
> 
> Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
> The gpio support exports two new functions: snd_soc_jack_add_gpios and
> snd_soc_jack_free_gpios.
> 
> Client drivers using gpio feature must pass an array of jack_gpio pins
> belonging to a specific jack to the snd_soc_jack_add_gpios function. The
> framework will request the gpio, set the data direction and request irq.
> The framework will update power status of related jack_pins when an event on
> the gpio pins comes according to the reporting bits defined for each gpio.
> 
> All gpio resources allocated when adding jack_gpio pins for a particular
> jack can be released using snd_soc_jack_free_gpios function.
> 
> Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>

I prefer GPIO stuff built in conditionally.
The jack layer is used also for non-ASoC codes, and GPIO isn't always
present.

> +/* irq handler for gpio pin */
> +static irqreturn_t gpio_handler(int irq, void *data)
> +{
> +	struct snd_soc_jack_gpio *gpio = data;
> +
> +	return IRQ_RETVAL(schedule_work(&gpio->work));

Is this really correct?

> +int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> +			struct snd_soc_jack_gpio *gpios)
> +{
> +	int i, ret = 0;
> +
> +	/* Link gpio array with the soc_jack */
> +	if (count > 0)
> +		jack->gpios = gpios;
> +
> +	for (i = 0; i < count; i++) {
> +		if (!gpio_is_valid(gpios[i].gpio)) {
> +			printk(KERN_ERR "Invalid gpio %d\n",
> +				gpios[i].gpio);
> +			return -EINVAL;
> +		}
> +		if (!gpios[i].name) {
> +			printk(KERN_ERR "No name for gpio %d\n",
> +				gpios[i].gpio);
> +			return -EINVAL;
> +		}
> +
> +		ret = gpio_request(gpios[i].gpio, gpios[i].name);
> +		if (ret)
> +			return ret;
> +
> +		ret = gpio_direction_input(gpios[i].gpio);
> +		if (ret)
> +			break;
> +
> +		ret = request_irq(gpio_to_irq(gpios[i].gpio),
> +				gpio_handler,
> +				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +				jack->card->dev->driver->name,
> +				&gpios[i]);
> +		if (ret)
> +			break;
> +
> +		INIT_WORK(&gpios[i].work, gpio_work);
> +		gpios[i].jack = jack;
> +	}
> +
> +	if (ret)
> +		gpio_free(gpios[i].gpio);

This error path doesn't look good.  It seems leaking / keeping some
resources.


thanks,

Takashi

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Mark Brown]

On Mon, Mar 02, 2009 at 07:05:28PM -0600, Lopez Cruz, Misael wrote:

This is much better than last time, thanks!  Some issues do remain,
though (as well as those that Takashi mentioned):

> +int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> +			  struct snd_soc_jack_gpio *gpios);
> +void snd_soc_jack_free_gpios(struct snd_soc_jack *jack);

snd_soc_jack_free_gpios() shouild take the same arguments as the add...

>  struct snd_soc_jack {
>  	struct snd_jack *jack;
>  	struct snd_soc_card *card;
>  	struct list_head pins;
> +	struct snd_soc_jack_gpio *gpios;
> +	int gpio_count;
>  	int status;
>  };

...so (as previously discussed) you don't have this.  The jack shouldn't
need this information and it's only used to free things at the minute.

Consider how this will scale: if the jack detection method for every
codec were to add back pointers to the snd_soc_jack structure then you'd
rapidly find that the bulk of the structure is taken up with these back
pointers.

> +	enable = gpio_get_value(gpio->gpio);
> +	if (gpio->invert)
> +		enable != enable;

I think you mean enable = !enable here (I'm surprised the compiler
doesn't warn about this).

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Mark Brown]

On Tue, Mar 03, 2009 at 07:41:16AM +0100, Takashi Iwai wrote:

> I prefer GPIO stuff built in conditionally.
> The jack layer is used also for non-ASoC codes, and GPIO isn't always
> present.

This is the jack wrapper in sound/soc rather in the generic sound/core
version so it should be safe to have it in unconditionally (I'd be
surprised if an ASoC platform didn't have gpiolib support), though
there's no harm in making it conditional on gpiolib either.

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Takashi Iwai]

At Tue, 3 Mar 2009 11:16:40 +0000,
Mark Brown wrote:
> 
> On Tue, Mar 03, 2009 at 07:41:16AM +0100, Takashi Iwai wrote:
> 
> > I prefer GPIO stuff built in conditionally.
> > The jack layer is used also for non-ASoC codes, and GPIO isn't always
> > present.
> 
> This is the jack wrapper in sound/soc rather in the generic sound/core
> version so it should be safe to have it in unconditionally (I'd be
> surprised if an ASoC platform didn't have gpiolib support), though
> there's no harm in making it conditional on gpiolib either.

Ah, OK.  If the size doesn't matter much, I don't care, too.


thanks,

Takashi

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Lopez Cruz, Misael]

> > Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
> > The gpio support exports two new functions: snd_soc_jack_add_gpios and 
> > snd_soc_jack_free_gpios.

> I prefer GPIO stuff built in conditionally.
> The jack layer is used also for non-ASoC codes, and GPIO 
> isn't always present.

Is it ok if I enclose all gpio functionality with #ifdef CONFIG_GPIOLIB?
 
> > +/* irq handler for gpio pin */
> > +static irqreturn_t gpio_handler(int irq, void *data) {
> > +	struct snd_soc_jack_gpio *gpio = data;
> > +
> > +	return IRQ_RETVAL(schedule_work(&gpio->work));

> Is this really correct?

Changed to:

static irqreturn_t gpio_handler(int irq, void *data)
{
	struct snd_soc_jack_gpio *gpio = data;

	schedule_work(&gpio->work);
	return IRQ_HANDLED;
}

> > +int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> > +			struct snd_soc_jack_gpio *gpios)

> This error path doesn't look good.  It seems leaking / 
> keeping some resources.

Changed to:

int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
                        struct snd_soc_jack_gpio *gpios)
{
	int i, ret;

	for (i = 0; i < count; i++) {
		if (!gpio_is_valid(gpios[i].gpio)) {
			printk(KERN_ERR "Invalid gpio %d\n",
					gpios[i].gpio);
			return -EINVAL;
		}
		if (!gpios[i].name) {
			printk(KERN_ERR "No name for gpio %d\n",
					gpios[i].gpio);
			return -EINVAL;
		}

		ret = gpio_request(gpios[i].gpio, gpios[i].name);
		if (ret)
			return ret;

		ret = gpio_direction_input(gpios[i].gpio);
		if (ret) {
			gpio_free(gpios[i].gpio);
			return ret;
		}

		ret = request_irq(gpio_to_irq(gpios[i].gpio),
				gpio_handler,
				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
				jack->card->dev->driver->name,
				&gpios[i]);
		if (ret) {
			gpio_free(gpios[i].gpio);
			return ret;
		}

		INIT_WORK(&gpios[i].work, gpio_work);
		gpios[i].jack = jack;
	}

	return 0;
}

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Mark Brown]

On Tue, Mar 03, 2009 at 12:39:17PM -0600, Lopez Cruz, Misael wrote:

> > I prefer GPIO stuff built in conditionally.
> > The jack layer is used also for non-ASoC codes, and GPIO 
> > isn't always present.

> Is it ok if I enclose all gpio functionality with #ifdef CONFIG_GPIOLIB?

Yes, that's fine.

> > This error path doesn't look good.  It seems leaking / 
> > keeping some resources.

> Changed to:

> int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
>                         struct snd_soc_jack_gpio *gpios)
> {
> 	int i, ret;
> 
> 	for (i = 0; i < count; i++) {
> 		if (!gpio_is_valid(gpios[i].gpio)) {
> 			printk(KERN_ERR "Invalid gpio %d\n",
> 					gpios[i].gpio);
> 			return -EINVAL;
> 		}

This still leaks GPIOs and interrupts if one of these tests fails on a
GPIO after the first.

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Lopez Cruz, Misael]

> > > This error path doesn't look good.  It seems leaking / 
> > > keeping some resources.

> > Changed to:
> > int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> >                         struct snd_soc_jack_gpio *gpios)
> > {
> > 	int i, ret;
> > 
> > 	for (i = 0; i < count; i++) {
> > 		if (!gpio_is_valid(gpios[i].gpio)) {
> > 			printk(KERN_ERR "Invalid gpio %d\n",
> > 					gpios[i].gpio);
> > 			return -EINVAL;
> > 		}

> This still leaks GPIOs and interrupts if one of these tests fails on a
> GPIO after the first.

I was thinking more in that machine drivers could handle the returned
error, for example, calling snd_soc_jack_free_gpios.

ret = snd_soc_jack_add_gpios(jack, ARRAY_SIZE(my_array), my_array);
if (ret) {
	snd_soc_jack_free_gpios(jack, ARRAY_SIZE(my_array), my_array);
	return ret;
}

But if snd_soc_jack_add_gpios() has to take care of it, then I'll
add that.

[PATCH] ASoC: Add GPIO support for jack reporting interface

[Lopez Cruz, Misael]

Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
The gpio support exports two new functions: snd_soc_jack_add_gpios and
snd_soc_jack_free_gpios.

Client drivers using gpio feature must pass an array of jack_gpio pins
belonging to a specific jack to the snd_soc_jack_add_gpios function. The
framework will request the gpios, set the data direction and request irq.
The framework will update power status of related jack_pins when an event on
the gpio pins comes according to the reporting bits defined for each gpio.

All gpio resources allocated when adding jack_gpio pins can be released
using snd_soc_jack_free_gpios function.

Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
---
 include/sound/soc.h  |   32 ++++++++++++
 sound/soc/soc-jack.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+), 0 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 68d8149..e5b4fd0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -16,6 +16,8 @@
 #include <linux/platform_device.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/control.h>
@@ -168,6 +170,9 @@ struct soc_enum;
 struct snd_soc_ac97_ops;
 struct snd_soc_jack;
 struct snd_soc_jack_pin;
+#ifdef CONFIG_GPIOLIB
+struct snd_soc_jack_gpio;
+#endif
 
 typedef int (*hw_write_t)(void *,const char* ,int);
 typedef int (*hw_read_t)(void *,char* ,int);
@@ -194,6 +199,12 @@ int snd_soc_jack_new(struct snd_soc_card *card, const char *id, int type,
 void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask);
 int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 			  struct snd_soc_jack_pin *pins);
+#ifdef CONFIG_GPIOLIB
+int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
+			struct snd_soc_jack_gpio *gpios);
+void snd_soc_jack_free_gpios(struct snd_soc_jack *jack, int count,
+			struct snd_soc_jack_gpio *gpios);
+#endif
 
 /* codec IO */
 #define snd_soc_read(codec, reg) codec->read(codec, reg)
@@ -262,6 +273,27 @@ struct snd_soc_jack_pin {
 	bool invert;
 };
 
+/**
+ * struct snd_soc_jack_gpio - Describes a gpio pin for jack detection
+ *
+ * @gpio:         gpio number
+ * @name:         gpio name
+ * @report:       value to report when jack detected
+ * @invert:       report presence in low state
+ * @debouce_time: debouce time in ms
+ */
+#ifdef CONFIG_GPIOLIB
+struct snd_soc_jack_gpio {
+	unsigned int gpio;
+	const char *name;
+	int report;
+	int invert;
+	int debounce_time;
+	struct snd_soc_jack *jack;
+	struct work_struct work;
+};
+#endif
+
 struct snd_soc_jack {
 	struct snd_jack *jack;
 	struct snd_soc_card *card;
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 8cc00c3..15bb80f 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -14,6 +14,10 @@
 #include <sound/jack.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/workqueue.h>
+#include <linux/delay.h>
 
 /**
  * snd_soc_jack_new - Create a new jack
@@ -136,3 +140,128 @@ int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_jack_add_pins);
+
+#ifdef CONFIG_GPIOLIB
+/* gpio detect */
+void snd_soc_jack_gpio_detect(struct snd_soc_jack_gpio *gpio)
+{
+	struct snd_soc_jack *jack = gpio->jack;
+	int enable;
+	int report;
+
+	if (gpio->debounce_time > 0)
+		mdelay(gpio->debounce_time);
+
+	enable = gpio_get_value(gpio->gpio);
+	if (gpio->invert)
+		enable = !enable;
+
+	if (enable)
+		report = gpio->report;
+	else
+		report = 0;
+
+	snd_soc_jack_report(jack, report, gpio->report);
+}
+
+/* irq handler for gpio pin */
+static irqreturn_t gpio_handler(int irq, void *data)
+{
+	struct snd_soc_jack_gpio *gpio = data;
+
+	schedule_work(&gpio->work);
+
+	return IRQ_HANDLED;
+}
+
+/* gpio work */
+static void gpio_work(struct work_struct *work)
+{
+	struct snd_soc_jack_gpio *gpio;
+
+	gpio = container_of(work, struct snd_soc_jack_gpio, work);
+	snd_soc_jack_gpio_detect(gpio);
+}
+
+/**
+ * snd_soc_jack_add_gpios - Associate GPIO pins with an ASoC jack
+ *
+ * @jack:  ASoC jack
+ * @count: number of pins
+ * @gpios: array of gpio pins
+ *
+ * This function will request gpio, set data direction and request irq
+ * for each gpio in the array.
+ */
+int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
+			struct snd_soc_jack_gpio *gpios)
+{
+	int i, ret;
+
+	for (i = 0; i < count; i++) {
+		if (!gpio_is_valid(gpios[i].gpio)) {
+			printk(KERN_ERR "Invalid gpio %d\n",
+				gpios[i].gpio);
+			ret = -EINVAL;
+			goto undo;
+		}
+		if (!gpios[i].name) {
+			printk(KERN_ERR "No name for gpio %d\n",
+				gpios[i].gpio);
+			ret = -EINVAL;
+			goto undo;
+		}
+
+		ret = gpio_request(gpios[i].gpio, gpios[i].name);
+		if (ret)
+			goto undo;
+
+		ret = gpio_direction_input(gpios[i].gpio);
+		if (ret)
+			goto err;
+
+		ret = request_irq(gpio_to_irq(gpios[i].gpio),
+				gpio_handler,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				jack->card->dev->driver->name,
+				&gpios[i]);
+		if (ret)
+			goto err;
+
+		INIT_WORK(&gpios[i].work, gpio_work);
+		gpios[i].jack = jack;
+	}
+
+	return 0;
+
+err:
+	gpio_free(gpios[i].gpio);
+undo:
+	snd_soc_jack_free_gpios(jack, i, gpios);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_jack_add_gpios);
+
+/**
+ * snd_soc_jack_free_gpios - Release GPIO pins' resources of an ASoC jack
+ *
+ * @jack:  ASoC jack
+ * @count: number of pins
+ * @gpios: array of gpio pins
+ *
+ * Release gpio and irq resources for gpio pins associated with an ASoC jack.
+ */
+void snd_soc_jack_free_gpios(struct snd_soc_jack *jack, int count,
+			struct snd_soc_jack_gpio *gpios)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		free_irq(gpio_to_irq(gpios[i].gpio), &gpios[i]);
+		gpio_free(gpios[i].gpio);
+		gpios[i].jack = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(snd_soc_jack_free_gpios);
+#endif	/* CONFIG_GPIOLIB */
-- 
1.5.4.3

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Mark Brown]

On Tue, Mar 03, 2009 at 03:25:04PM -0600, Lopez Cruz, Misael wrote:
> Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
> The gpio support exports two new functions: snd_soc_jack_add_gpios and
> snd_soc_jack_free_gpios.

Applied, thanks.  However, I'll provide a follow on patch to...

> +undo:
> +	snd_soc_jack_free_gpios(jack, i, gpios);

change this to i + 1 - confusion due to zero based indexing.

Re: [PATCH] ASoC: Add GPIO support for jack reporting interface

[Mark Brown]

On Wed, Mar 04, 2009 at 02:43:14PM +0000, Mark Brown wrote:

> Applied, thanks.  However, I'll provide a follow on patch to...

> > +undo:
> > +	snd_soc_jack_free_gpios(jack, i, gpios);

> change this to i + 1 - confusion due to zero based indexing.

Eh, no I won't - there's no case where it's required.  Sorry about the
confusion.