[PATCH 0/5] ASoC: Intel/rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[PATCH 0/5] ASoC: Intel/rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Hans de Goede]

The HP Elitepad 1000 G2 tablet has 2 headset jacks:

1. on the dock which uses the output of the codecs built-in HP-amp +
the standard IN2 input which is always used with the headset-jack.

2. on the tablet itself, this uses the line-out of the codec + an external
HP-amp, which gets enabled by the ALC5642 codec's GPIO1 pin; and IN1 for
the headset-mic.

The codec's GPIO1 is also its only IRQ output pin, so this means that
the codec's IRQ cannot be used on this tablet. Instead the jack-detect
is connected directly to GPIOs on the main SoC. The dock has a helper
chip which also detects if a headset-mic is present or not, so there
are 2 GPIOs for the jack-detect status of the dock. The tablet jack
uses a single GPIO which indicates if a jack is present or not.

Differentiating between between headphones vs a headset on the tablet jack
is done by using the usual mic-bias over-current-detection mechanism.

Regards,

Hans


Hans de Goede (5):
  ASoC: rt5640: Move rt5640_disable_jack_detect() up in the rt5640.c
    file
  ASoC: rt5640: Delay requesting IRQ until the machine-drv calls
    set_jack
  ASoC: rt5640: Add optional hp_det_gpio parameter to
    rt5640_detect_headset()
  ASoC: rt5640: Add rt5640_set_ovcd_params() helper
  ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2
    jack-detect

 sound/soc/codecs/rt5640.c             | 133 ++++++++++++-----------
 sound/soc/codecs/rt5640.h             |   6 ++
 sound/soc/intel/boards/bytcr_rt5640.c | 146 +++++++++++++++++++++++++-
 3 files changed, 225 insertions(+), 60 deletions(-)

-- 
2.31.1

[PATCH 1/5] ASoC: rt5640: Move rt5640_disable_jack_detect() up in the rt5640.c file

[Hans de Goede]

Move rt5640_disable_jack_detect() to above rt5640_enable_jack_detect().
This is a preparation patch for reworking how the IRQ gets requested.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5640.c | 46 +++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 9523f4b5c800..a425e6b1687d 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2362,6 +2362,29 @@ static void rt5640_cancel_work(void *data)
 	cancel_delayed_work_sync(&rt5640->bp_work);
 }
 
+static void rt5640_disable_jack_detect(struct snd_soc_component *component)
+{
+	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
+
+	/*
+	 * soc_remove_component() force-disables jack and thus rt5640->jack
+	 * could be NULL at the time of driver's module unloading.
+	 */
+	if (!rt5640->jack)
+		return;
+
+	disable_irq(rt5640->irq);
+	rt5640_cancel_work(rt5640);
+
+	if (rt5640->jack->status & SND_JACK_MICROPHONE) {
+		rt5640_disable_micbias1_ovcd_irq(component);
+		rt5640_disable_micbias1_for_ovcd(component);
+		snd_soc_jack_report(rt5640->jack, 0, SND_JACK_BTN_0);
+	}
+
+	rt5640->jack = NULL;
+}
+
 static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 				      struct snd_soc_jack *jack)
 {
@@ -2428,29 +2451,6 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 	queue_work(system_long_wq, &rt5640->jack_work);
 }
 
-static void rt5640_disable_jack_detect(struct snd_soc_component *component)
-{
-	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
-
-	/*
-	 * soc_remove_component() force-disables jack and thus rt5640->jack
-	 * could be NULL at the time of driver's module unloading.
-	 */
-	if (!rt5640->jack)
-		return;
-
-	disable_irq(rt5640->irq);
-	rt5640_cancel_work(rt5640);
-
-	if (rt5640->jack->status & SND_JACK_MICROPHONE) {
-		rt5640_disable_micbias1_ovcd_irq(component);
-		rt5640_disable_micbias1_for_ovcd(component);
-		snd_soc_jack_report(rt5640->jack, 0, SND_JACK_BTN_0);
-	}
-
-	rt5640->jack = NULL;
-}
-
 static int rt5640_set_jack(struct snd_soc_component *component,
 			   struct snd_soc_jack *jack, void *data)
 {
-- 
2.31.1

Re: [PATCH 1/5] ASoC: rt5640: Move rt5640_disable_jack_detect() up in the rt5640.c file

[Mark Brown]

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

On Sun, Aug 15, 2021 at 05:49:31PM +0200, Hans de Goede wrote:
> Move rt5640_disable_jack_detect() to above rt5640_enable_jack_detect().
> This is a preparation patch for reworking how the IRQ gets requested.

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/5] ASoC: rt5640: Delay requesting IRQ until the machine-drv calls set_jack

[Hans de Goede]

Delay requesting the IRQ until the machine-drv calls set_jack.

The main reason for this is that the codec's IRQ is unused on some boards,
in which case we really should not call request_irq at all.

On some boards there is an IRQ listed at index 0 for the codec, but
this is not connected to the codec, but rather is directly connected
to the jack's jack-detect pin. These special setups will be handled
by the machine-driver, but the machine driver can only request the IRQ
if it is not first requested by the codec driver. Moving the request_irq
to the set_jack callback (which will not get called in this case) avoids
the codec-driver clobbering the IRQ.

Moving the request_irq also removes the need to disable the IRQ immediately
after requesting it, avoiding a small race (this could also have been fixed
by using the new IRQF_NO_AUTOEN flag when requesting the IRQ).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5640.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index a425e6b1687d..d32e9d69231c 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2373,7 +2373,7 @@ static void rt5640_disable_jack_detect(struct snd_soc_component *component)
 	if (!rt5640->jack)
 		return;
 
-	disable_irq(rt5640->irq);
+	free_irq(rt5640->irq, rt5640);
 	rt5640_cancel_work(rt5640);
 
 	if (rt5640->jack->status & SND_JACK_MICROPHONE) {
@@ -2389,6 +2389,7 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 				      struct snd_soc_jack *jack)
 {
 	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
+	int ret;
 
 	/* Select JD-source */
 	snd_soc_component_update_bits(component, RT5640_JD_CTRL,
@@ -2446,7 +2447,17 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 		rt5640_enable_micbias1_ovcd_irq(component);
 	}
 
-	enable_irq(rt5640->irq);
+	ret = request_irq(rt5640->irq, rt5640_irq,
+			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+			  "rt5640", rt5640);
+	if (ret) {
+		dev_warn(component->dev, "Failed to reguest IRQ %d: %d\n", rt5640->irq, ret);
+		rt5640->irq = -ENXIO;
+		/* Undo above settings */
+		rt5640_disable_jack_detect(component);
+		return;
+	}
+
 	/* sync initial jack state */
 	queue_work(system_long_wq, &rt5640->jack_work);
 }
@@ -2836,18 +2847,6 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
 	if (ret)
 		return ret;
 
-	ret = devm_request_irq(&i2c->dev, rt5640->irq, rt5640_irq,
-			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
-			       | IRQF_ONESHOT, "rt5640", rt5640);
-	if (ret == 0) {
-		/* Gets re-enabled by rt5640_set_jack() */
-		disable_irq(rt5640->irq);
-	} else {
-		dev_warn(&i2c->dev, "Failed to reguest IRQ %d: %d\n",
-			 rt5640->irq, ret);
-		rt5640->irq = -ENXIO;
-	}
-
 	return devm_snd_soc_register_component(&i2c->dev,
 				      &soc_component_dev_rt5640,
 				      rt5640_dai, ARRAY_SIZE(rt5640_dai));
-- 
2.31.1

[PATCH 3/5] ASoC: rt5640: Add optional hp_det_gpio parameter to rt5640_detect_headset()

[Hans de Goede]

Some devices don't use the builtin jack-detect but can still benefit
from the mic-bias-current over-current-detection headphones vs
headset detection done by rt5640_detect_headset().

In this case the jack-inserted check done by rt5640_detect_headset()
needs to be done through a GPIO rather then by using the codec's
builtin jack-detect. Add an optional hp_det_gpio parameter and export
rt5640_detect_headset() for use on machines where jack-detect is
handled outside of the codec.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5640.c | 14 ++++++++++----
 sound/soc/codecs/rt5640.h |  2 ++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index d32e9d69231c..04820af03ae8 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2241,7 +2241,7 @@ static void rt5640_button_press_work(struct work_struct *work)
 	schedule_delayed_work(&rt5640->bp_work, msecs_to_jiffies(BP_POLL_TIME));
 }
 
-static int rt5640_detect_headset(struct snd_soc_component *component)
+int rt5640_detect_headset(struct snd_soc_component *component, struct gpio_desc *hp_det_gpio)
 {
 	int i, headset_count = 0, headphone_count = 0;
 
@@ -2259,8 +2259,13 @@ static int rt5640_detect_headset(struct snd_soc_component *component)
 		msleep(JACK_SETTLE_TIME);
 
 		/* Check the jack is still connected before checking ovcd */
-		if (!rt5640_jack_inserted(component))
-			return 0;
+		if (hp_det_gpio) {
+			if (gpiod_get_value_cansleep(hp_det_gpio))
+				return 0;
+		} else {
+			if (!rt5640_jack_inserted(component))
+				return 0;
+		}
 
 		if (rt5640_micbias1_ovcd(component)) {
 			/*
@@ -2285,6 +2290,7 @@ static int rt5640_detect_headset(struct snd_soc_component *component)
 	dev_err(component->dev, "Error detecting headset vs headphones, bad contact?, assuming headphones\n");
 	return SND_JACK_HEADPHONE;
 }
+EXPORT_SYMBOL_GPL(rt5640_detect_headset);
 
 static void rt5640_jack_work(struct work_struct *work)
 {
@@ -2309,7 +2315,7 @@ static void rt5640_jack_work(struct work_struct *work)
 		/* Jack inserted */
 		WARN_ON(rt5640->ovcd_irq_enabled);
 		rt5640_enable_micbias1_for_ovcd(component);
-		status = rt5640_detect_headset(component);
+		status = rt5640_detect_headset(component, NULL);
 		if (status == SND_JACK_HEADSET) {
 			/* Enable ovcd IRQ for button press detect. */
 			rt5640_enable_micbias1_ovcd_irq(component);
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 4fd47f2b936b..4d19997dd684 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -10,6 +10,7 @@
 #define _RT5640_H
 
 #include <linux/clk.h>
+#include <linux/gpio/consumer.h>
 #include <linux/workqueue.h>
 #include <dt-bindings/sound/rt5640.h>
 
@@ -2156,5 +2157,6 @@ int rt5640_dmic_enable(struct snd_soc_component *component,
 		       bool dmic1_data_pin, bool dmic2_data_pin);
 int rt5640_sel_asrc_clk_src(struct snd_soc_component *component,
 		unsigned int filter_mask, unsigned int clk_src);
+int rt5640_detect_headset(struct snd_soc_component *component, struct gpio_desc *hp_det_gpio);
 
 #endif
-- 
2.31.1

[PATCH 4/5] ASoC: rt5640: Add rt5640_set_ovcd_params() helper

[Hans de Goede]

Some devices don't use the builtin jack-detect but can still benefit
from the mic-bias-current over-current-detection to differentiate
between headphones vs a headset.

Move the ovcd init code from rt5640_enable_jack_detect() into a new
rt5640_set_ovcd_params() helper and export this helper as well
as a couple of related ovcd functions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5640.c | 50 +++++++++++++++++++++++----------------
 sound/soc/codecs/rt5640.h |  4 ++++
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 04820af03ae8..cd1db5caabad 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2093,7 +2093,7 @@ int rt5640_sel_asrc_clk_src(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(rt5640_sel_asrc_clk_src);
 
-static void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component)
+void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
@@ -2105,8 +2105,9 @@ static void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component)
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
+EXPORT_SYMBOL_GPL(rt5640_enable_micbias1_for_ovcd);
 
-static void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component)
+void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 
@@ -2117,6 +2118,7 @@ static void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component
 	snd_soc_dapm_sync_unlocked(dapm);
 	snd_soc_dapm_mutex_unlock(dapm);
 }
+EXPORT_SYMBOL_GPL(rt5640_disable_micbias1_for_ovcd);
 
 static void rt5640_enable_micbias1_ovcd_irq(struct snd_soc_component *component)
 {
@@ -2368,6 +2370,31 @@ static void rt5640_cancel_work(void *data)
 	cancel_delayed_work_sync(&rt5640->bp_work);
 }
 
+void rt5640_set_ovcd_params(struct snd_soc_component *component)
+{
+	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
+
+	snd_soc_component_write(component, RT5640_PR_BASE + RT5640_BIAS_CUR4,
+		0xa800 | rt5640->ovcd_sf);
+
+	snd_soc_component_update_bits(component, RT5640_MICBIAS,
+		RT5640_MIC1_OVTH_MASK | RT5640_MIC1_OVCD_MASK,
+		rt5640->ovcd_th | RT5640_MIC1_OVCD_EN);
+
+	/*
+	 * The over-current-detect is only reliable in detecting the absence
+	 * of over-current, when the mic-contact in the jack is short-circuited,
+	 * the hardware periodically retries if it can apply the bias-current
+	 * leading to the ovcd status flip-flopping 1-0-1 with it being 0 about
+	 * 10% of the time, as we poll the ovcd status bit we might hit that
+	 * 10%, so we enable sticky mode and when checking OVCD we clear the
+	 * status, msleep() a bit and then check to get a reliable reading.
+	 */
+	snd_soc_component_update_bits(component, RT5640_IRQ_CTRL2,
+		RT5640_MB1_OC_STKY_MASK, RT5640_MB1_OC_STKY_EN);
+}
+EXPORT_SYMBOL_GPL(rt5640_set_ovcd_params);
+
 static void rt5640_disable_jack_detect(struct snd_soc_component *component)
 {
 	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
@@ -2415,24 +2442,7 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 	/* Enabling jd2 in general control 2 */
 	snd_soc_component_write(component, RT5640_DUMMY2, 0x4001);
 
-	snd_soc_component_write(component, RT5640_PR_BASE + RT5640_BIAS_CUR4,
-		0xa800 | rt5640->ovcd_sf);
-
-	snd_soc_component_update_bits(component, RT5640_MICBIAS,
-		RT5640_MIC1_OVTH_MASK | RT5640_MIC1_OVCD_MASK,
-		rt5640->ovcd_th | RT5640_MIC1_OVCD_EN);
-
-	/*
-	 * The over-current-detect is only reliable in detecting the absence
-	 * of over-current, when the mic-contact in the jack is short-circuited,
-	 * the hardware periodically retries if it can apply the bias-current
-	 * leading to the ovcd status flip-flopping 1-0-1 with it being 0 about
-	 * 10% of the time, as we poll the ovcd status bit we might hit that
-	 * 10%, so we enable sticky mode and when checking OVCD we clear the
-	 * status, msleep() a bit and then check to get a reliable reading.
-	 */
-	snd_soc_component_update_bits(component, RT5640_IRQ_CTRL2,
-		RT5640_MB1_OC_STKY_MASK, RT5640_MB1_OC_STKY_EN);
+	rt5640_set_ovcd_params(component);
 
 	/*
 	 * All IRQs get or-ed together, so we need the jack IRQ to report 0
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 4d19997dd684..2c28f83e338a 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2157,6 +2157,10 @@ int rt5640_dmic_enable(struct snd_soc_component *component,
 		       bool dmic1_data_pin, bool dmic2_data_pin);
 int rt5640_sel_asrc_clk_src(struct snd_soc_component *component,
 		unsigned int filter_mask, unsigned int clk_src);
+
+void rt5640_set_ovcd_params(struct snd_soc_component *component);
+void rt5640_enable_micbias1_for_ovcd(struct snd_soc_component *component);
+void rt5640_disable_micbias1_for_ovcd(struct snd_soc_component *component);
 int rt5640_detect_headset(struct snd_soc_component *component, struct gpio_desc *hp_det_gpio);
 
 #endif
-- 
2.31.1

[PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Hans de Goede]

The HP Elitepad 1000 G2 tablet has 2 headset jacks:

1. on the dock which uses the output of the codecs built-in HP-amp +
the standard IN2 input which is always used with the headset-jack.

2. on the tablet itself, this uses the line-out of the codec + an external
HP-amp, which gets enabled by the ALC5642 codec's GPIO1 pin; and IN1 for
the headset-mic.

The codec's GPIO1 is also its only IRQ output pin, so this means that
the codec's IRQ cannot be used on this tablet. Instead the jack-detect
is connected directly to GPIOs on the main SoC. The dock has a helper
chip which also detects if a headset-mic is present or not, so there
are 2 GPIOs for the jack-detect status of the dock. The tablet jack
uses a single GPIO which indicates if a jack is present or not.

Differentiating between between headphones vs a headset on the tablet jack
is done by using the usual mic-bias over-current-detection mechanism.

Add support for this unique setup, this support gets enabled on this
tablet through a new BYT_RT5640_JD_HP_ELITEP_1000G2 quirk.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213415
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 146 +++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index fecccff76caf..369642c07a5d 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -18,6 +18,8 @@
 #include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/dmi.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
 #include <linux/input.h>
 #include <linux/slab.h>
 #include <sound/pcm.h>
@@ -76,6 +78,7 @@ enum {
 #define BYT_RT5640_LINEOUT		BIT(25)
 #define BYT_RT5640_LINEOUT_AS_HP2	BIT(26)
 #define BYT_RT5640_HSMIC2_ON_IN1	BIT(27)
+#define BYT_RT5640_JD_HP_ELITEP_1000G2	BIT(28)
 
 #define BYTCR_INPUT_DEFAULTS				\
 	(BYT_RT5640_IN3_MAP |				\
@@ -89,6 +92,8 @@ enum {
 
 struct byt_rt5640_private {
 	struct snd_soc_jack jack;
+	struct snd_soc_jack jack2;
+	struct gpio_desc *hsmic_detect;
 	struct clk *mclk;
 	struct device *codec_dev;
 };
@@ -141,6 +146,8 @@ static void log_quirks(struct device *dev)
 	}
 	if (byt_rt5640_quirk & BYT_RT5640_JD_NOT_INV)
 		dev_info(dev, "quirk JD_NOT_INV enabled\n");
+	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2)
+		dev_info(dev, "quirk JD_HP_ELITEPAD_1000G2 enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_MONO_SPEAKER)
 		dev_info(dev, "quirk MONO_SPEAKER enabled\n");
 	if (byt_rt5640_quirk & BYT_RT5640_NO_SPEAKERS)
@@ -438,6 +445,76 @@ static struct snd_soc_jack_pin rt5640_pins[] = {
 	},
 };
 
+static struct snd_soc_jack_pin rt5640_pins2[] = {
+	{
+		/* The 2nd headset jack uses lineout with an external HP-amp */
+		.pin	= "Line Out",
+		.mask	= SND_JACK_HEADPHONE,
+	},
+	{
+		/* BYT_RT5640_HSMIC2_ON_IN1 uses byt_rt5640_intmic_in1_map */
+		.pin	= "Internal Mic",
+		.mask	= SND_JACK_MICROPHONE,
+	},
+};
+
+struct snd_soc_jack_gpio rt5640_jack_gpio = {
+	.name = "hp-detect",
+	.report = SND_JACK_HEADSET,
+	.invert = true,
+	.debounce_time = 200,
+};
+
+struct snd_soc_jack_gpio rt5640_jack2_gpio = {
+	.name = "hp2-detect",
+	.report = SND_JACK_HEADSET,
+	.invert = true,
+	.debounce_time = 200,
+};
+
+static const struct acpi_gpio_params acpi_gpio0 = { 0, 0, false };
+static const struct acpi_gpio_params acpi_gpio1 = { 1, 0, false };
+static const struct acpi_gpio_params acpi_gpio2 = { 2, 0, false };
+
+static const struct acpi_gpio_mapping byt_rt5640_hp_elitepad_1000g2_gpios[] = {
+	{ "hp-detect-gpios", &acpi_gpio0, 1, },
+	{ "headset-mic-detect-gpios", &acpi_gpio1, 1, },
+	{ "hp2-detect-gpios", &acpi_gpio2, 1, },
+	{ },
+};
+
+int byt_rt5640_hp_elitepad_1000g2_jack1_check(void *data)
+{
+	struct byt_rt5640_private *priv = data;
+	int jack_status, mic_status;
+
+	jack_status = gpiod_get_value_cansleep(rt5640_jack_gpio.desc);
+	if (jack_status)
+		return 0;
+
+	mic_status = gpiod_get_value_cansleep(priv->hsmic_detect);
+	if (mic_status)
+		return SND_JACK_HEADPHONE;
+	else
+		return SND_JACK_HEADSET;
+}
+
+int byt_rt5640_hp_elitepad_1000g2_jack2_check(void *data)
+{
+	struct snd_soc_component *component = data;
+	int jack_status, report;
+
+	jack_status = gpiod_get_value_cansleep(rt5640_jack2_gpio.desc);
+	if (jack_status)
+		return 0;
+
+	rt5640_enable_micbias1_for_ovcd(component);
+	report = rt5640_detect_headset(component, rt5640_jack2_gpio.desc);
+	rt5640_disable_micbias1_for_ovcd(component);
+
+	return report;
+}
+
 static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream,
 					struct snd_pcm_hw_params *params)
 {
@@ -649,7 +726,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 					BYT_RT5640_MCLK_EN |
 					BYT_RT5640_LINEOUT |
 					BYT_RT5640_LINEOUT_AS_HP2 |
-					BYT_RT5640_HSMIC2_ON_IN1),
+					BYT_RT5640_HSMIC2_ON_IN1 |
+					BYT_RT5640_JD_HP_ELITEP_1000G2),
 	},
 	{	/* HP Pavilion x2 10-k0XX, 10-n0XX */
 		.matches = {
@@ -1172,9 +1250,60 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 		snd_soc_component_set_jack(component, &priv->jack, NULL);
 	}
 
+	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
+		ret = snd_soc_card_jack_new(card, "Headset",
+					    SND_JACK_HEADSET,
+					    &priv->jack, rt5640_pins,
+					    ARRAY_SIZE(rt5640_pins));
+		if (ret)
+			return ret;
+
+		ret = snd_soc_card_jack_new(card, "Headset 2",
+					    SND_JACK_HEADSET,
+					    &priv->jack2, rt5640_pins2,
+					    ARRAY_SIZE(rt5640_pins2));
+		if (ret)
+			return ret;
+
+		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
+					  byt_rt5640_hp_elitepad_1000g2_gpios);
+
+		rt5640_jack_gpio.data = priv;
+		rt5640_jack_gpio.gpiod_dev = priv->codec_dev;
+		rt5640_jack_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack1_check;
+		ret = snd_soc_jack_add_gpios(&priv->jack, 1, &rt5640_jack_gpio);
+		if (ret) {
+			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
+			return ret;
+		}
+
+		rt5640_set_ovcd_params(component);
+		rt5640_jack2_gpio.data = component;
+		rt5640_jack2_gpio.gpiod_dev = priv->codec_dev;
+		rt5640_jack2_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack2_check;
+		ret = snd_soc_jack_add_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
+		if (ret) {
+			snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
+			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
+static void byt_rt5640_exit(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_soc_card *card = runtime->card;
+	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
+
+	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
+		snd_soc_jack_free_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
+		snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
+		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
+	}
+}
+
 static int byt_rt5640_codec_fixup(struct snd_soc_pcm_runtime *rtd,
 			    struct snd_pcm_hw_params *params)
 {
@@ -1287,6 +1416,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = {
 		.dpcm_playback = 1,
 		.dpcm_capture = 1,
 		.init = byt_rt5640_init,
+		.exit = byt_rt5640_exit,
 		.ops = &byt_rt5640_be_ssp2_ops,
 		SND_SOC_DAILINK_REG(ssp2_port, ssp2_codec, platform),
 	},
@@ -1490,6 +1620,20 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 		return -EPROBE_DEFER;
 	priv->codec_dev = get_device(codec_dev);
 
+	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
+		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
+					  byt_rt5640_hp_elitepad_1000g2_gpios);
+		priv->hsmic_detect = devm_fwnode_gpiod_get(&pdev->dev, codec_dev->fwnode,
+							   "headset-mic-detect", GPIOD_IN,
+							   "headset-mic-detect");
+		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
+		if (IS_ERR(priv->hsmic_detect)) {
+			ret_val = PTR_ERR(priv->hsmic_detect);
+			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
+			goto err_device;
+		}
+	}
+
 	/* Must be called before register_card, also see declaration comment. */
 	ret_val = byt_rt5640_add_codec_device_props(codec_dev, priv);
 	if (ret_val)
-- 
2.31.1

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Pierre-Louis Bossart]

Hi Hans,
I am a bit confused by the use of acpi_dev_add_driver_gpios(). You call
it from the dailink .init function, and you call
acpi_dev_remove_driver_gpios() from the .exit function.

> @@ -1172,9 +1250,60 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
>  		snd_soc_component_set_jack(component, &priv->jack, NULL);
>  	}
>  
> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
> +		ret = snd_soc_card_jack_new(card, "Headset",
> +					    SND_JACK_HEADSET,
> +					    &priv->jack, rt5640_pins,
> +					    ARRAY_SIZE(rt5640_pins));
> +		if (ret)
> +			return ret;
> +
> +		ret = snd_soc_card_jack_new(card, "Headset 2",
> +					    SND_JACK_HEADSET,
> +					    &priv->jack2, rt5640_pins2,
> +					    ARRAY_SIZE(rt5640_pins2));
> +		if (ret)
> +			return ret;
> +
> +		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
> +					  byt_rt5640_hp_elitepad_1000g2_gpios);
> +
> +		rt5640_jack_gpio.data = priv;
> +		rt5640_jack_gpio.gpiod_dev = priv->codec_dev;
> +		rt5640_jack_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack1_check;
> +		ret = snd_soc_jack_add_gpios(&priv->jack, 1, &rt5640_jack_gpio);
> +		if (ret) {
> +			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
> +			return ret;
> +		}
> +
> +		rt5640_set_ovcd_params(component);
> +		rt5640_jack2_gpio.data = component;
> +		rt5640_jack2_gpio.gpiod_dev = priv->codec_dev;
> +		rt5640_jack2_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack2_check;
> +		ret = snd_soc_jack_add_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
> +		if (ret) {
> +			snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
> +			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
> +			return ret;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> +static void byt_rt5640_exit(struct snd_soc_pcm_runtime *runtime)
> +{
> +	struct snd_soc_card *card = runtime->card;
> +	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
> +
> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
> +		snd_soc_jack_free_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
> +		snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
> +		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
> +	}
> +}

so far so good, but you also add/remove gpios in the machine driver probe

> @@ -1490,6 +1620,20 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>  		return -EPROBE_DEFER;
>  	priv->codec_dev = get_device(codec_dev);
>  
> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
> +		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
> +					  byt_rt5640_hp_elitepad_1000g2_gpios);
> +		priv->hsmic_detect = devm_fwnode_gpiod_get(&pdev->dev, codec_dev->fwnode,
> +							   "headset-mic-detect", GPIOD_IN,
> +							   "headset-mic-detect");
> +		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
> +		if (IS_ERR(priv->hsmic_detect)) {
> +			ret_val = PTR_ERR(priv->hsmic_detect);
> +			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
> +			goto err_device;
> +		}
> +	}
Does this part need to be part of the machine driver probe, or could it
be moved in the dailink .init function? Is this because you wanted to
use devm_ function?

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Hans de Goede]

Hi Pierre-Louis,

On 8/16/21 3:31 PM, Pierre-Louis Bossart wrote:
> Hi Hans,
> I am a bit confused by the use of acpi_dev_add_driver_gpios().

I understand admittedly my solution there is a bit hacky.

> You call
> it from the dailink .init function, and you call
> acpi_dev_remove_driver_gpios() from the .exit function.
> 
>> @@ -1172,9 +1250,60 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
>>  		snd_soc_component_set_jack(component, &priv->jack, NULL);
>>  	}
>>  
>> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
>> +		ret = snd_soc_card_jack_new(card, "Headset",
>> +					    SND_JACK_HEADSET,
>> +					    &priv->jack, rt5640_pins,
>> +					    ARRAY_SIZE(rt5640_pins));
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = snd_soc_card_jack_new(card, "Headset 2",
>> +					    SND_JACK_HEADSET,
>> +					    &priv->jack2, rt5640_pins2,
>> +					    ARRAY_SIZE(rt5640_pins2));
>> +		if (ret)
>> +			return ret;
>> +
>> +		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
>> +					  byt_rt5640_hp_elitepad_1000g2_gpios);
>> +
>> +		rt5640_jack_gpio.data = priv;
>> +		rt5640_jack_gpio.gpiod_dev = priv->codec_dev;
>> +		rt5640_jack_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack1_check;
>> +		ret = snd_soc_jack_add_gpios(&priv->jack, 1, &rt5640_jack_gpio);
>> +		if (ret) {
>> +			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
>> +			return ret;
>> +		}
>> +
>> +		rt5640_set_ovcd_params(component);
>> +		rt5640_jack2_gpio.data = component;
>> +		rt5640_jack2_gpio.gpiod_dev = priv->codec_dev;
>> +		rt5640_jack2_gpio.jack_status_check = byt_rt5640_hp_elitepad_1000g2_jack2_check;
>> +		ret = snd_soc_jack_add_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
>> +		if (ret) {
>> +			snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
>> +			acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
>> +			return ret;
>> +		}
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> +static void byt_rt5640_exit(struct snd_soc_pcm_runtime *runtime)
>> +{
>> +	struct snd_soc_card *card = runtime->card;
>> +	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
>> +
>> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
>> +		snd_soc_jack_free_gpios(&priv->jack2, 1, &rt5640_jack2_gpio);
>> +		snd_soc_jack_free_gpios(&priv->jack, 1, &rt5640_jack_gpio);
>> +		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));
>> +	}
>> +}
> 
> so far so good, but you also add/remove gpios in the machine driver probe

Ack.

>> @@ -1490,6 +1620,20 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>>  		return -EPROBE_DEFER;
>>  	priv->codec_dev = get_device(codec_dev);
>>  
>> +	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
>> +		acpi_dev_add_driver_gpios(ACPI_COMPANION(priv->codec_dev),
>> +					  byt_rt5640_hp_elitepad_1000g2_gpios);

So this second add here (which runs first, so it really is the first add)

>> +		priv->hsmic_detect = devm_fwnode_gpiod_get(&pdev->dev, codec_dev->fwnode,
>> +							   "headset-mic-detect", GPIOD_IN,
>> +							   "headset-mic-detect");

Is to make sure this call can succeed.

>> +		acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev));

And this is to not have to add yet an other error-exit path which does this
to the probe() function. By simply always removing the lookup here after the
gpiod_get() we keep the error-exit paths as they were, rather then making
them more complicated.

But I guess things won't be so bad wrt err-exit-path complexity as for
them to really be a problem, so if you prefer I can also:

1. Remove the second acpi_dev_add_driver_gpios + acpi_dev_remove_driver_gpios
pair from the dai_link .init/.exit.
2. Remove the acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev) call here
moving it to snd_byt_rt5640_mc_remove()
3. Introduce a acpi_dev_remove_driver_gpios() remove in the error-exit paths
of snd_byt_rt5640_mc_probe where necessary.

>> +		if (IS_ERR(priv->hsmic_detect)) {
>> +			ret_val = PTR_ERR(priv->hsmic_detect);
>> +			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
>> +			goto err_device;
>> +		}
>> +	}
> Does this part need to be part of the machine driver probe, or could it
> be moved in the dailink .init function?

The idea here is that the gpiod_get may fail with -EPROBE_DEFER and then I want
to fail as early as possible, so right in the probe function.

This is also why the error is logged with dev_err_probe() which does not
log anything for EPROBE_DEFER as retval.

> Is this because you wanted to use devm_ function?

No, I did consider adding the gpiod_get() for priv->hsmic_detect to the
dai_link .init function and then I would just use a normal get, combined
with an explicit _put in the dailink exit. I put this gpiod_get in
the platform_driver probe to handle EPROBE_DEFER early on, rather then
having it happen deep inside the devm_snd_soc_register_card() call-graph
(when it calls the dailink .init).

I would prefer to keep the gpiod_get inside snd_byt_rt5640_mc_probe for this
reason, but as mentioned I can removed the second acpi_dev_add_driver_gpios +
acpi_dev_remove_driver_gpios call pair from the dai_link .init/.exit.

Please let me know how you want to proceed with this.

Regards,

Hans

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Pierre-Louis Bossart]

> But I guess things won't be so bad wrt err-exit-path complexity as for
> them to really be a problem, so if you prefer I can also:
> 
> 1. Remove the second acpi_dev_add_driver_gpios + acpi_dev_remove_driver_gpios
> pair from the dai_link .init/.exit.
> 2. Remove the acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev) call here
> moving it to snd_byt_rt5640_mc_remove()
> 3. Introduce a acpi_dev_remove_driver_gpios() remove in the error-exit paths
> of snd_byt_rt5640_mc_probe where necessary.

that sounds good to me, it's probably better to do things once with a
bit of additional error handling.

>>> +		if (IS_ERR(priv->hsmic_detect)) {
>>> +			ret_val = PTR_ERR(priv->hsmic_detect);
>>> +			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
>>> +			goto err_device;
>>> +		}
>>> +	}
>> Does this part need to be part of the machine driver probe, or could it
>> be moved in the dailink .init function?
> 
> The idea here is that the gpiod_get may fail with -EPROBE_DEFER and then I want
> to fail as early as possible, so right in the probe function.
> 
> This is also why the error is logged with dev_err_probe() which does not
> log anything for EPROBE_DEFER as retval.
> 
>> Is this because you wanted to use devm_ function?
> 
> No, I did consider adding the gpiod_get() for priv->hsmic_detect to the
> dai_link .init function and then I would just use a normal get, combined
> with an explicit _put in the dailink exit. I put this gpiod_get in
> the platform_driver probe to handle EPROBE_DEFER early on, rather then
> having it happen deep inside the devm_snd_soc_register_card() call-graph
> (when it calls the dailink .init).
> 
> I would prefer to keep the gpiod_get inside snd_byt_rt5640_mc_probe for this
> reason, but as mentioned I can removed the second acpi_dev_add_driver_gpios +
> acpi_dev_remove_driver_gpios call pair from the dai_link .init/.exit.
> 
> Please let me know how you want to proceed with this.

ok with the suggestion above. Thanks Hans!

Re: [PATCH 0/5] ASoC: Intel/rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Mark Brown]

On Sun, 15 Aug 2021 17:49:30 +0200, Hans de Goede wrote:
> The HP Elitepad 1000 G2 tablet has 2 headset jacks:
> 
> 1. on the dock which uses the output of the codecs built-in HP-amp +
> the standard IN2 input which is always used with the headset-jack.
> 
> 2. on the tablet itself, this uses the line-out of the codec + an external
> HP-amp, which gets enabled by the ALC5642 codec's GPIO1 pin; and IN1 for
> the headset-mic.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] ASoC: rt5640: Move rt5640_disable_jack_detect() up in the rt5640.c file
      commit: 5caab9f48b96f6998fb23d38a7b57fca91ef1653
[2/5] ASoC: rt5640: Delay requesting IRQ until the machine-drv calls set_jack
      commit: 15d54840ecf6f00061d03180394a0a21ff8ffa48
[3/5] ASoC: rt5640: Add optional hp_det_gpio parameter to rt5640_detect_headset()
      commit: d21213b4503ea66777313e4345e116cc8a5366bf
[4/5] ASoC: rt5640: Add rt5640_set_ovcd_params() helper
      commit: e3f2a6603a982467601e0831d706786ed1ade833
[5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect
      commit: 9ba00856686ade106afee2884b5e8ac1e09d137a

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark