ASoC: simple-card: Add mic and hp detect gpios.
diff mbox

Message ID 1412198720-2326-1-git-send-email-dgreid@chromium.org
State Accepted
Commit 3fe240326cc395c66eda0518b1945ea505afd1fc
Headers show

Commit Message

Dylan Reid Oct. 1, 2014, 9:25 p.m. UTC
Allow Headphone and Microphone jack detect gpios to be specified in
device tree.  This will allow a few systems including rk3288_max98090
to use simple-card instead of having their own board file.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 .../devicetree/bindings/sound/simple-card.txt      |  4 ++
 sound/soc/generic/simple-card.c                    | 73 ++++++++++++++++++++++
 2 files changed, 77 insertions(+)

Comments

Mark Brown Oct. 2, 2014, 3:53 p.m. UTC | #1
On Wed, Oct 01, 2014 at 02:25:20PM -0700, Dylan Reid wrote:
> Allow Headphone and Microphone jack detect gpios to be specified in
> device tree.  This will allow a few systems including rk3288_max98090
> to use simple-card instead of having their own board file.

Applied, thanks.
Lars-Peter Clausen Oct. 2, 2014, 4:25 p.m. UTC | #2
On 10/01/2014 11:25 PM, Dylan Reid wrote:
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index c2e9841..72d94b7 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -17,6 +17,10 @@ Optional properties:
>   					  source.
>   - simple-audio-card,mclk-fs             : Multiplication factor between stream rate and codec
>     					  mclk.
> +- simple-audio-card,hp_det_gpio		: Reference to GPIO that signals when
> +					  headphones are attached.
> +- simple-audio-card,mic_det_gpio	: Reference to GPIO that signals when

The code has this correct, the "_"s should be "-"s.

- Lars
Dylan Reid Oct. 2, 2014, 4:57 p.m. UTC | #3
On Thu, Oct 2, 2014 at 9:25 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 10/01/2014 11:25 PM, Dylan Reid wrote:
>>
>> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt
>> b/Documentation/devicetree/bindings/sound/simple-card.txt
>> index c2e9841..72d94b7 100644
>> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
>> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
>> @@ -17,6 +17,10 @@ Optional properties:
>>                                           source.
>>   - simple-audio-card,mclk-fs             : Multiplication factor between
>> stream rate and codec
>>                                           mclk.
>> +- simple-audio-card,hp_det_gpio                : Reference to GPIO that
>> signals when
>> +                                         headphones are attached.
>> +- simple-audio-card,mic_det_gpio       : Reference to GPIO that signals
>> when
>
>
> The code has this correct, the "_"s should be "-"s.

Indeed, my fault.  Sorry about that.  I will post a patch once the
first one propagates.

>
> - Lars
>
Geert Uytterhoeven Oct. 7, 2014, 12:32 p.m. UTC | #4
Hi Mark, Dylan,

On Thu, Oct 2, 2014 at 5:53 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 01, 2014 at 02:25:20PM -0700, Dylan Reid wrote:
>> Allow Headphone and Microphone jack detect gpios to be specified in
>> device tree.  This will allow a few systems including rk3288_max98090
>> to use simple-card instead of having their own board file.
>
> Applied, thanks.

Unfortunately there's no equivalent code for platform data, and the
uninitialized default of 0 for gpio_hp_det and gpio_mic_det doesn't
play well with asm-generic's gpio_is_valid():

static inline bool gpio_is_valid(int number)
{
        return number >= 0 && number < ARCH_NR_GPIOS;
}

Hence on r8a7740/armadillo-legacy, which uses platform devices instead of DT:

    sh-mobile-hdmi sh-mobile-hdmi: SH Mobile HDMI Audio Codec
    sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones
    sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Mic Jack

After that the kernel log is spammed ca. 7 times per second with:

    sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones

Reverting commit 3fe240326cc395c66 ("ASoC: simple-card: Add mic and
hp detect gpios.") fixes this.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Mark Brown Oct. 7, 2014, 12:38 p.m. UTC | #5
On Tue, Oct 07, 2014 at 02:32:57PM +0200, Geert Uytterhoeven wrote:

> Hence on r8a7740/armadillo-legacy, which uses platform devices instead of DT:

>     sh-mobile-hdmi sh-mobile-hdmi: SH Mobile HDMI Audio Codec
>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones
>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Mic Jack

> After that the kernel log is spammed ca. 7 times per second with:

>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones

> Reverting commit 3fe240326cc395c66 ("ASoC: simple-card: Add mic and
> hp detect gpios.") fixes this.

The fix here is to not allow 0 as a GPIO in the core code (which
should've been there already).
Geert Uytterhoeven Oct. 7, 2014, 1:10 p.m. UTC | #6
(re-added some context, CC Linus, Alexandre, linux-gpio)

On Tue, Oct 7, 2014 at 2:38 PM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Oct 07, 2014 at 02:32:57PM +0200, Geert Uytterhoeven wrote:
>> Unfortunately there's no equivalent code for platform data, and the
>> uninitialized default of 0 for gpio_hp_det and gpio_mic_det doesn't
>> play well with asm-generic's gpio_is_valid():
>>
>> static inline bool gpio_is_valid(int number)
>> {
>>         return number >= 0 && number < ARCH_NR_GPIOS;
>> }
>>
>> Hence on r8a7740/armadillo-legacy, which uses platform devices instead of DT:
>
>>     sh-mobile-hdmi sh-mobile-hdmi: SH Mobile HDMI Audio Codec
>>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones
>>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Mic Jack
>
>> After that the kernel log is spammed ca. 7 times per second with:
>
>>     sh-mobile-hdmi sh-mobile-hdmi: ASoC: DAPM unknown pin Headphones
>
>> Reverting commit 3fe240326cc395c66 ("ASoC: simple-card: Add mic and
>> hp detect gpios.") fixes this.
>
> The fix here is to not allow 0 as a GPIO in the core code (which
> should've been there already).

Unfortunately it's not there.

And it's not as simple as changing the definition of gpio_is_valid()
(crash in gpio_get_value()):

    gpiochip_add: GPIOs 0..211 (r8a7740_pfc) failed to register
    sh-pfc pfc-r8a7740: failed to init GPIO chip, ignoring...
    sh-pfc pfc-r8a7740: r8a7740_pfc support registered
    Unable to handle kernel NULL pointer dereference at virtual address 0000004c

Quoting Linus (https://lkml.org/lkml/2014/9/4/464):
"Fixing the old global GPIO numberspace API is a waste of time IMO".

Hence I've just sent a patch to initialize the GPIO numbers with -ENOENT.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Mark Brown Oct. 7, 2014, 4:36 p.m. UTC | #7
On Tue, Oct 07, 2014 at 03:10:01PM +0200, Geert Uytterhoeven wrote:
> On Tue, Oct 7, 2014 at 2:38 PM, Mark Brown <broonie@kernel.org> wrote:

> > The fix here is to not allow 0 as a GPIO in the core code (which
> > should've been there already).

> Unfortunately it's not there.

> And it's not as simple as changing the definition of gpio_is_valid()
> (crash in gpio_get_value()):

>     gpiochip_add: GPIOs 0..211 (r8a7740_pfc) failed to register
>     sh-pfc pfc-r8a7740: failed to init GPIO chip, ignoring...
>     sh-pfc pfc-r8a7740: r8a7740_pfc support registered
>     Unable to handle kernel NULL pointer dereference at virtual address 0000004c

Right, obviously it's not going to work if the platform actually uses 0
as a valid GPIO.

> Quoting Linus (https://lkml.org/lkml/2014/9/4/464):
> "Fixing the old global GPIO numberspace API is a waste of time IMO".

> Hence I've just sent a patch to initialize the GPIO numbers with -ENOENT.

I do think it's worth renumbering the platforms since it's *relatively*
little work per platform compared to completing the gpiod transition.
Alexandre Courbot Oct. 8, 2014, 7:05 a.m. UTC | #8
On Wed, Oct 8, 2014 at 1:36 AM, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Oct 07, 2014 at 03:10:01PM +0200, Geert Uytterhoeven wrote:
>> On Tue, Oct 7, 2014 at 2:38 PM, Mark Brown <broonie@kernel.org> wrote:
>
>> > The fix here is to not allow 0 as a GPIO in the core code (which
>> > should've been there already).
>
>> Unfortunately it's not there.
>
>> And it's not as simple as changing the definition of gpio_is_valid()
>> (crash in gpio_get_value()):
>
>>     gpiochip_add: GPIOs 0..211 (r8a7740_pfc) failed to register
>>     sh-pfc pfc-r8a7740: failed to init GPIO chip, ignoring...
>>     sh-pfc pfc-r8a7740: r8a7740_pfc support registered
>>     Unable to handle kernel NULL pointer dereference at virtual address 0000004c
>
> Right, obviously it's not going to work if the platform actually uses 0
> as a valid GPIO.
>
>> Quoting Linus (https://lkml.org/lkml/2014/9/4/464):
>> "Fixing the old global GPIO numberspace API is a waste of time IMO".
>
>> Hence I've just sent a patch to initialize the GPIO numbers with -ENOENT.
>
> I do think it's worth renumbering the platforms since it's *relatively*
> little work per platform compared to completing the gpiod transition.

But transition to gpiod is the way to ultimately fix this issue, as
well as many others. Not to mention that renumbering GPIOs will
certainly make a few users of the GPIO sysfs (another abomination,
agreed) unhappy. I can only recommend switching drivers to gpiod when
such issues are spotted.
Linus Walleij Oct. 8, 2014, 8:50 a.m. UTC | #9
On Wed, Oct 8, 2014 at 9:05 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Wed, Oct 8, 2014 at 1:36 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Tue, Oct 07, 2014 at 03:10:01PM +0200, Geert Uytterhoeven wrote:

>> Right, obviously it's not going to work if the platform actually uses 0
>> as a valid GPIO.
>>
>>> Quoting Linus (https://lkml.org/lkml/2014/9/4/464):
>>> "Fixing the old global GPIO numberspace API is a waste of time IMO".
>>
>>> Hence I've just sent a patch to initialize the GPIO numbers with -ENOENT.
>>
>> I do think it's worth renumbering the platforms since it's *relatively*
>> little work per platform compared to completing the gpiod transition.
>
> But transition to gpiod is the way to ultimately fix this issue, as
> well as many others. Not to mention that renumbering GPIOs will
> certainly make a few users of the GPIO sysfs (another abomination,
> agreed) unhappy. I can only recommend switching drivers to gpiod when
> such issues are spotted.

Yeah that is another issue ... we end up in catch 22 situations like
that, renumber the GPIOs, OK, then we break the ABI.

Admittedly that "ABI" is something people break all the time,
/sys/*gpioN just isnt what it should be, not stable at all.

I'm not against renumbering GPIO if it's minor effort, but if it
start to consume hundreds of hours and regressions and what not,
that time is better spent focusing on the gpiod transition.

Yours,
Linus Walleij
Mark Brown Oct. 8, 2014, 11:40 a.m. UTC | #10
On Wed, Oct 08, 2014 at 10:50:00AM +0200, Linus Walleij wrote:
> On Wed, Oct 8, 2014 at 9:05 AM, Alexandre Courbot <gnurou@gmail.com> wrote:

> > But transition to gpiod is the way to ultimately fix this issue, as
> > well as many others. Not to mention that renumbering GPIOs will
> > certainly make a few users of the GPIO sysfs (another abomination,
> > agreed) unhappy. I can only recommend switching drivers to gpiod when
> > such issues are spotted.

> Yeah that is another issue ... we end up in catch 22 situations like
> that, renumber the GPIOs, OK, then we break the ABI.

> Admittedly that "ABI" is something people break all the time,
> /sys/*gpioN just isnt what it should be, not stable at all.

> I'm not against renumbering GPIO if it's minor effort, but if it
> start to consume hundreds of hours and regressions and what not,
> that time is better spent focusing on the gpiod transition.

My guess is that it's relatively little work for most platforms - with
the systems I've done enough work on to notice everything is keyed off a
few defines in the header file.  Things tend to be worse in out of tree
code but mainline's generally been pretty good.
jianqun Oct. 22, 2014, 6:44 a.m. UTC | #11
? 10/02/2014 05:25 AM, Dylan Reid ??:
> Allow Headphone and Microphone jack detect gpios to be specified in
> device tree.  This will allow a few systems including rk3288_max98090
> to use simple-card instead of having their own board file.
>
> Signed-off-by: Dylan Reid <dgreid@chromium.org>
> ---
>  .../devicetree/bindings/sound/simple-card.txt      |  4 ++
>  sound/soc/generic/simple-card.c                    | 73 ++++++++++++++++++++++
>  2 files changed, 77 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index c2e9841..72d94b7 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -17,6 +17,10 @@ Optional properties:
>  					  source.
>  - simple-audio-card,mclk-fs             : Multiplication factor between stream rate and codec
>    					  mclk.
> +- simple-audio-card,hp_det_gpio		: Reference to GPIO that signals when
> +					  headphones are attached.
> +- simple-audio-card,mic_det_gpio	: Reference to GPIO that signals when
> +					  a microphone is attached.
>  
>  Optional subnodes:
>  
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 709ce67..fcb431f 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -10,10 +10,13 @@
>   */
>  #include <linux/clk.h>
>  #include <linux/device.h>
> +#include <linux/gpio.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_gpio.h>
>  #include <linux/platform_device.h>
>  #include <linux/string.h>
> +#include <sound/jack.h>
>  #include <sound/simple_card.h>
>  #include <sound/soc-dai.h>
>  #include <sound/soc.h>
> @@ -25,6 +28,8 @@ struct simple_card_data {
>  		struct asoc_simple_dai codec_dai;
>  	} *dai_props;
>  	unsigned int mclk_fs;
> +	int gpio_hp_det;
> +	int gpio_mic_det;
>  	struct snd_soc_dai_link dai_link[];	/* dynamically allocated */
>  };
>  
> @@ -54,6 +59,32 @@ static struct snd_soc_ops asoc_simple_card_ops = {
>  	.hw_params = asoc_simple_card_hw_params,
>  };
>  
> +static struct snd_soc_jack simple_card_hp_jack;
> +static struct snd_soc_jack_pin simple_card_hp_jack_pins[] = {
> +	{
> +		.pin = "Headphones",
> +		.mask = SND_JACK_HEADPHONE,
> +	},
> +};
> +static struct snd_soc_jack_gpio simple_card_hp_jack_gpio = {
> +	.name = "Headphone detection",
> +	.report = SND_JACK_HEADPHONE,
> +	.debounce_time = 150,
I think some board needs "invert" trigger level, i.e need to add "invert" property in dt ?
Pinky board do need it.
> +};
> +
> +static struct snd_soc_jack simple_card_mic_jack;
> +static struct snd_soc_jack_pin simple_card_mic_jack_pins[] = {
> +	{
> +		.pin = "Mic Jack",
> +		.mask = SND_JACK_MICROPHONE,
> +	},
> +};
> +static struct snd_soc_jack_gpio simple_card_mic_jack_gpio = {
> +	.name = "Mic detection",
> +	.report = SND_JACK_MICROPHONE,
> +	.debounce_time = 150,
> +};
> +
>  static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
>  				       struct asoc_simple_dai *set)
>  {
> @@ -109,6 +140,28 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (gpio_is_valid(priv->gpio_hp_det)) {
> +		snd_soc_jack_new(codec->codec, "Headphones", SND_JACK_HEADPHONE,
> +				 &simple_card_hp_jack);
> +		snd_soc_jack_add_pins(&simple_card_hp_jack,
> +				      ARRAY_SIZE(simple_card_hp_jack_pins),
> +				      simple_card_hp_jack_pins);
> +
> +		simple_card_hp_jack_gpio.gpio = priv->gpio_hp_det;
> +		snd_soc_jack_add_gpios(&simple_card_hp_jack, 1,
> +				       &simple_card_hp_jack_gpio);
> +	}
> +
> +	if (gpio_is_valid(priv->gpio_mic_det)) {
> +		snd_soc_jack_new(codec->codec, "Mic Jack", SND_JACK_MICROPHONE,
> +				 &simple_card_mic_jack);
> +		snd_soc_jack_add_pins(&simple_card_mic_jack,
> +				      ARRAY_SIZE(simple_card_mic_jack_pins),
> +				      simple_card_mic_jack_pins);
> +		simple_card_mic_jack_gpio.gpio = priv->gpio_mic_det;
> +		snd_soc_jack_add_gpios(&simple_card_mic_jack, 1,
> +				       &simple_card_mic_jack_gpio);
> +	}
>  	return 0;
>  }
>  
> @@ -383,6 +436,16 @@ static int asoc_simple_card_parse_of(struct device_node *node,
>  			return ret;
>  	}
>  
> +	priv->gpio_hp_det = of_get_named_gpio(node,
> +				"simple-audio-card,hp-det-gpio", 0);
> +	if (priv->gpio_hp_det == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +
> +	priv->gpio_mic_det = of_get_named_gpio(node,
> +				"simple-audio-card,mic-det-gpio", 0);
> +	if (priv->gpio_mic_det == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +
>  	if (!priv->snd_card.name)
>  		priv->snd_card.name = priv->snd_card.dai_link->name;
>  
> @@ -502,6 +565,16 @@ err:
>  
>  static int asoc_simple_card_remove(struct platform_device *pdev)
>  {
> +	struct snd_soc_card *card = platform_get_drvdata(pdev);
> +	struct simple_card_data *priv = snd_soc_card_get_drvdata(card);
> +
> +	if (gpio_is_valid(priv->gpio_hp_det))
> +		snd_soc_jack_free_gpios(&simple_card_hp_jack, 1,
> +					&simple_card_hp_jack_gpio);
> +	if (gpio_is_valid(priv->gpio_mic_det))
> +		snd_soc_jack_free_gpios(&simple_card_mic_jack, 1,
> +					&simple_card_mic_jack_gpio);
> +
>  	return asoc_simple_card_unref(pdev);
>  }
>

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index c2e9841..72d94b7 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -17,6 +17,10 @@  Optional properties:
 					  source.
 - simple-audio-card,mclk-fs             : Multiplication factor between stream rate and codec
   					  mclk.
+- simple-audio-card,hp_det_gpio		: Reference to GPIO that signals when
+					  headphones are attached.
+- simple-audio-card,mic_det_gpio	: Reference to GPIO that signals when
+					  a microphone is attached.
 
 Optional subnodes:
 
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 709ce67..fcb431f 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -10,10 +10,13 @@ 
  */
 #include <linux/clk.h>
 #include <linux/device.h>
+#include <linux/gpio.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
+#include <sound/jack.h>
 #include <sound/simple_card.h>
 #include <sound/soc-dai.h>
 #include <sound/soc.h>
@@ -25,6 +28,8 @@  struct simple_card_data {
 		struct asoc_simple_dai codec_dai;
 	} *dai_props;
 	unsigned int mclk_fs;
+	int gpio_hp_det;
+	int gpio_mic_det;
 	struct snd_soc_dai_link dai_link[];	/* dynamically allocated */
 };
 
@@ -54,6 +59,32 @@  static struct snd_soc_ops asoc_simple_card_ops = {
 	.hw_params = asoc_simple_card_hw_params,
 };
 
+static struct snd_soc_jack simple_card_hp_jack;
+static struct snd_soc_jack_pin simple_card_hp_jack_pins[] = {
+	{
+		.pin = "Headphones",
+		.mask = SND_JACK_HEADPHONE,
+	},
+};
+static struct snd_soc_jack_gpio simple_card_hp_jack_gpio = {
+	.name = "Headphone detection",
+	.report = SND_JACK_HEADPHONE,
+	.debounce_time = 150,
+};
+
+static struct snd_soc_jack simple_card_mic_jack;
+static struct snd_soc_jack_pin simple_card_mic_jack_pins[] = {
+	{
+		.pin = "Mic Jack",
+		.mask = SND_JACK_MICROPHONE,
+	},
+};
+static struct snd_soc_jack_gpio simple_card_mic_jack_gpio = {
+	.name = "Mic detection",
+	.report = SND_JACK_MICROPHONE,
+	.debounce_time = 150,
+};
+
 static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 				       struct asoc_simple_dai *set)
 {
@@ -109,6 +140,28 @@  static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 	if (ret < 0)
 		return ret;
 
+	if (gpio_is_valid(priv->gpio_hp_det)) {
+		snd_soc_jack_new(codec->codec, "Headphones", SND_JACK_HEADPHONE,
+				 &simple_card_hp_jack);
+		snd_soc_jack_add_pins(&simple_card_hp_jack,
+				      ARRAY_SIZE(simple_card_hp_jack_pins),
+				      simple_card_hp_jack_pins);
+
+		simple_card_hp_jack_gpio.gpio = priv->gpio_hp_det;
+		snd_soc_jack_add_gpios(&simple_card_hp_jack, 1,
+				       &simple_card_hp_jack_gpio);
+	}
+
+	if (gpio_is_valid(priv->gpio_mic_det)) {
+		snd_soc_jack_new(codec->codec, "Mic Jack", SND_JACK_MICROPHONE,
+				 &simple_card_mic_jack);
+		snd_soc_jack_add_pins(&simple_card_mic_jack,
+				      ARRAY_SIZE(simple_card_mic_jack_pins),
+				      simple_card_mic_jack_pins);
+		simple_card_mic_jack_gpio.gpio = priv->gpio_mic_det;
+		snd_soc_jack_add_gpios(&simple_card_mic_jack, 1,
+				       &simple_card_mic_jack_gpio);
+	}
 	return 0;
 }
 
@@ -383,6 +436,16 @@  static int asoc_simple_card_parse_of(struct device_node *node,
 			return ret;
 	}
 
+	priv->gpio_hp_det = of_get_named_gpio(node,
+				"simple-audio-card,hp-det-gpio", 0);
+	if (priv->gpio_hp_det == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	priv->gpio_mic_det = of_get_named_gpio(node,
+				"simple-audio-card,mic-det-gpio", 0);
+	if (priv->gpio_mic_det == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	if (!priv->snd_card.name)
 		priv->snd_card.name = priv->snd_card.dai_link->name;
 
@@ -502,6 +565,16 @@  err:
 
 static int asoc_simple_card_remove(struct platform_device *pdev)
 {
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct simple_card_data *priv = snd_soc_card_get_drvdata(card);
+
+	if (gpio_is_valid(priv->gpio_hp_det))
+		snd_soc_jack_free_gpios(&simple_card_hp_jack, 1,
+					&simple_card_hp_jack_gpio);
+	if (gpio_is_valid(priv->gpio_mic_det))
+		snd_soc_jack_free_gpios(&simple_card_mic_jack, 1,
+					&simple_card_mic_jack_gpio);
+
 	return asoc_simple_card_unref(pdev);
 }