[PATCH 1/5] ASoC: rt5640: Change jack_work to a delayed_work

[PATCH 1/5] ASoC: rt5640: Change jack_work to a delayed_work

[Hans de Goede]

Change jack_work from a struct work_struct to a struct delayed_work, this
is a preparation patch for adding support for boards where an external
GPIO is used for jack-detect, rather then one of the JD pins of the codec.

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

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index d01fe73ab9c8..36c00ad17182 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2297,7 +2297,7 @@ EXPORT_SYMBOL_GPL(rt5640_detect_headset);
 static void rt5640_jack_work(struct work_struct *work)
 {
 	struct rt5640_priv *rt5640 =
-		container_of(work, struct rt5640_priv, jack_work);
+		container_of(work, struct rt5640_priv, jack_work.work);
 	struct snd_soc_component *component = rt5640->component;
 	int status;
 
@@ -2348,7 +2348,7 @@ static void rt5640_jack_work(struct work_struct *work)
 		 * disabled the OVCD IRQ, the IRQ pin will stay high and as
 		 * we react to edges, we miss the unplug event -> recheck.
 		 */
-		queue_work(system_long_wq, &rt5640->jack_work);
+		queue_delayed_work(system_long_wq, &rt5640->jack_work, 0);
 	}
 }
 
@@ -2357,7 +2357,7 @@ static irqreturn_t rt5640_irq(int irq, void *data)
 	struct rt5640_priv *rt5640 = data;
 
 	if (rt5640->jack)
-		queue_work(system_long_wq, &rt5640->jack_work);
+		queue_delayed_work(system_long_wq, &rt5640->jack_work, 0);
 
 	return IRQ_HANDLED;
 }
@@ -2366,7 +2366,7 @@ static void rt5640_cancel_work(void *data)
 {
 	struct rt5640_priv *rt5640 = data;
 
-	cancel_work_sync(&rt5640->jack_work);
+	cancel_delayed_work_sync(&rt5640->jack_work);
 	cancel_delayed_work_sync(&rt5640->bp_work);
 }
 
@@ -2475,7 +2475,7 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 	}
 
 	/* sync initial jack state */
-	queue_work(system_long_wq, &rt5640->jack_work);
+	queue_delayed_work(system_long_wq, &rt5640->jack_work, 0);
 }
 
 static int rt5640_set_jack(struct snd_soc_component *component,
@@ -2856,7 +2856,7 @@ static int rt5640_i2c_probe(struct i2c_client *i2c,
 	rt5640->hp_mute = true;
 	rt5640->irq = i2c->irq;
 	INIT_DELAYED_WORK(&rt5640->bp_work, rt5640_button_press_work);
-	INIT_WORK(&rt5640->jack_work, rt5640_jack_work);
+	INIT_DELAYED_WORK(&rt5640->jack_work, rt5640_jack_work);
 
 	/* Make sure work is stopped on probe-error / remove */
 	ret = devm_add_action_or_reset(&i2c->dev, rt5640_cancel_work, rt5640);
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 2c28f83e338a..7ab930def8dd 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2145,7 +2145,7 @@ struct rt5640_priv {
 	int release_count;
 	int poll_count;
 	struct delayed_work bp_work;
-	struct work_struct jack_work;
+	struct delayed_work jack_work;
 	struct snd_soc_jack *jack;
 	unsigned int jd_src;
 	bool jd_inverted;
-- 
2.33.1

[PATCH 2/5] ASoC: rt5640: Allow snd_soc_component_set_jack() to override the codec IRQ

[Hans de Goede]

On some boards where the firmware/fwnode information is in essence
read-only (x86 + ACPI boards) the i2c_client for the codec may contain
the wrong IRQ or no IRQ at all.

Since we only request the IRQ once snd_soc_component_set_jack() gets
called, allow machine drivers to override the IRQ with the proper one
through the data parameter to snd_soc_component_set_jack().

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

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 36c00ad17182..5244b6f7de84 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2419,7 +2419,8 @@ static void rt5640_disable_jack_detect(struct snd_soc_component *component)
 }
 
 static void rt5640_enable_jack_detect(struct snd_soc_component *component,
-				      struct snd_soc_jack *jack)
+				      struct snd_soc_jack *jack,
+				      struct rt5640_set_jack_data *jack_data)
 {
 	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
 	int ret;
@@ -2463,6 +2464,9 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 		rt5640_enable_micbias1_ovcd_irq(component);
 	}
 
+	if (jack_data && jack_data->codec_irq_override)
+		rt5640->irq = jack_data->codec_irq_override;
+
 	ret = request_irq(rt5640->irq, rt5640_irq,
 			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 			  "rt5640", rt5640);
@@ -2482,7 +2486,7 @@ static int rt5640_set_jack(struct snd_soc_component *component,
 			   struct snd_soc_jack *jack, void *data)
 {
 	if (jack)
-		rt5640_enable_jack_detect(component, jack);
+		rt5640_enable_jack_detect(component, jack, data);
 	else
 		rt5640_disable_jack_detect(component);
 
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 7ab930def8dd..2f4da5a8ecb2 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2153,6 +2153,10 @@ struct rt5640_priv {
 	unsigned int ovcd_sf;
 };
 
+struct rt5640_set_jack_data {
+	int codec_irq_override;
+};
+
 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,
-- 
2.33.1

Re: [PATCH 2/5] ASoC: rt5640: Allow snd_soc_component_set_jack() to override the codec IRQ

[Mark Brown]

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

On Mon, Dec 27, 2021 at 04:33:41PM +0100, Hans de Goede wrote:
> On some boards where the firmware/fwnode information is in essence
> read-only (x86 + ACPI boards) the i2c_client for the codec may contain
> the wrong IRQ or no IRQ at all.

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

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

Re: [PATCH 2/5] ASoC: rt5640: Allow snd_soc_component_set_jack() to override the codec IRQ

[Hans de Goede]

Hi,

On 12/31/21 14:22, Mark Brown wrote:
> On Mon, Dec 27, 2021 at 04:33:41PM +0100, Hans de Goede wrote:
>> On some boards where the firmware/fwnode information is in essence
>> read-only (x86 + ACPI boards) the i2c_client for the codec may contain
>> the wrong IRQ or no IRQ at all.
> 
> This doesn't apply against current code, please check and resend.

Ok, I will send a v2 rebased on top of broonie/sound.git/for-next

Regards,

Hans

[PATCH 3/5] ASoC: rt5640: Add support for boards with an external jack-detect GPIO

[Hans de Goede]

Some boards have the codec IRQ hooked-up as normally, so the driver can
still do things like headset vs headphones and button-press detection,
but instead of using one of the JD pins of the codec, an external GPIO
is used to report the jack-presence switch status of the jack.

Add support for this.

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

diff --git a/sound/soc/codecs/rt5640.c b/sound/soc/codecs/rt5640.c
index 5244b6f7de84..47a21765c8c4 100644
--- a/sound/soc/codecs/rt5640.c
+++ b/sound/soc/codecs/rt5640.c
@@ -2159,7 +2159,11 @@ static bool rt5640_jack_inserted(struct snd_soc_component *component)
 	struct rt5640_priv *rt5640 = snd_soc_component_get_drvdata(component);
 	int val;
 
-	val = snd_soc_component_read(component, RT5640_INT_IRQ_ST);
+	if (rt5640->jd_gpio)
+		val = gpiod_get_value(rt5640->jd_gpio) ? RT5640_JD_STATUS : 0;
+	else
+		val = snd_soc_component_read(component, RT5640_INT_IRQ_ST);
+
 	dev_dbg(component->dev, "irq status %#04x\n", val);
 
 	if (rt5640->jd_inverted)
@@ -2362,6 +2366,16 @@ static irqreturn_t rt5640_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t rt5640_jd_gpio_irq(int irq, void *data)
+{
+	struct rt5640_priv *rt5640 = data;
+
+	queue_delayed_work(system_long_wq, &rt5640->jack_work,
+			   msecs_to_jiffies(JACK_SETTLE_TIME));
+
+	return IRQ_HANDLED;
+}
+
 static void rt5640_cancel_work(void *data)
 {
 	struct rt5640_priv *rt5640 = data;
@@ -2406,7 +2420,12 @@ static void rt5640_disable_jack_detect(struct snd_soc_component *component)
 	if (!rt5640->jack)
 		return;
 
-	free_irq(rt5640->irq, rt5640);
+	if (rt5640->jd_gpio_irq_requested)
+		free_irq(rt5640->jd_gpio_irq, rt5640);
+
+	if (rt5640->irq_requested)
+		free_irq(rt5640->irq, rt5640);
+
 	rt5640_cancel_work(rt5640);
 
 	if (rt5640->jack->status & SND_JACK_MICROPHONE) {
@@ -2415,6 +2434,9 @@ static void rt5640_disable_jack_detect(struct snd_soc_component *component)
 		snd_soc_jack_report(rt5640->jack, 0, SND_JACK_BTN_0);
 	}
 
+	rt5640->jd_gpio_irq_requested = false;
+	rt5640->irq_requested = false;
+	rt5640->jd_gpio = NULL;
 	rt5640->jack = NULL;
 }
 
@@ -2467,16 +2489,31 @@ static void rt5640_enable_jack_detect(struct snd_soc_component *component,
 	if (jack_data && jack_data->codec_irq_override)
 		rt5640->irq = jack_data->codec_irq_override;
 
+	if (jack_data && jack_data->jd_gpio) {
+		rt5640->jd_gpio = jack_data->jd_gpio;
+		rt5640->jd_gpio_irq = gpiod_to_irq(rt5640->jd_gpio);
+
+		ret = request_irq(rt5640->jd_gpio_irq, rt5640_jd_gpio_irq,
+				  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				  "rt5640-jd-gpio", rt5640);
+		if (ret) {
+			dev_warn(component->dev, "Failed to request jd GPIO IRQ %d: %d\n",
+				 rt5640->jd_gpio_irq, ret);
+			rt5640_disable_jack_detect(component);
+			return;
+		}
+		rt5640->jd_gpio_irq_requested = true;
+	}
+
 	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;
 	}
+	rt5640->irq_requested = true;
 
 	/* sync initial jack state */
 	queue_delayed_work(system_long_wq, &rt5640->jack_work, 0);
diff --git a/sound/soc/codecs/rt5640.h b/sound/soc/codecs/rt5640.h
index 2f4da5a8ecb2..9e49b9a0ccaa 100644
--- a/sound/soc/codecs/rt5640.h
+++ b/sound/soc/codecs/rt5640.h
@@ -2124,6 +2124,7 @@ struct rt5640_priv {
 
 	int ldo1_en; /* GPIO for LDO1_EN */
 	int irq;
+	int jd_gpio_irq;
 	int sysclk;
 	int sysclk_src;
 	int lrck[RT5640_AIFS];
@@ -2136,6 +2137,8 @@ struct rt5640_priv {
 
 	bool hp_mute;
 	bool asrc_en;
+	bool irq_requested;
+	bool jd_gpio_irq_requested;
 
 	/* Jack and button detect data */
 	bool ovcd_irq_enabled;
@@ -2147,6 +2150,7 @@ struct rt5640_priv {
 	struct delayed_work bp_work;
 	struct delayed_work jack_work;
 	struct snd_soc_jack *jack;
+	struct gpio_desc *jd_gpio;
 	unsigned int jd_src;
 	bool jd_inverted;
 	unsigned int ovcd_th;
@@ -2155,6 +2159,7 @@ struct rt5640_priv {
 
 struct rt5640_set_jack_data {
 	int codec_irq_override;
+	struct gpio_desc *jd_gpio;
 };
 
 int rt5640_dmic_enable(struct snd_soc_component *component,
-- 
2.33.1

[PATCH 4/5] ASoC: Intel: bytcr_rt5640: Support retrieving the codec IRQ from the AMCR0F28 ACPI dev

[Hans de Goede]

Some X86 tablets, which ship with Android as factory installed OS,
specify codec IRQs/GPIOS in a special Android AMCR0F28 ACPI device.

Add support for retrieving the codec IRQ from this ACPI device instead
of from the 10EC5640 device describing the codec itself and enable this
on Asus MemoPad 7 ME176C tablets.

This fixes jack-detect not working on these tablets.

Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 43 +++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index a0c5f0e9c22a..f37ab44ae957 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -79,6 +79,7 @@ enum {
 #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 BYT_RT5640_USE_AMCR0F28		BIT(29)
 
 #define BYTCR_INPUT_DEFAULTS				\
 	(BYT_RT5640_IN3_MAP |				\
@@ -93,6 +94,7 @@ enum {
 struct byt_rt5640_private {
 	struct snd_soc_jack jack;
 	struct snd_soc_jack jack2;
+	struct rt5640_set_jack_data jack_data;
 	struct gpio_desc *hsmic_detect;
 	struct clk *mclk;
 	struct device *codec_dev;
@@ -597,7 +599,8 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 					BYT_RT5640_OVCD_TH_2000UA |
 					BYT_RT5640_OVCD_SF_0P75 |
 					BYT_RT5640_SSP0_AIF1 |
-					BYT_RT5640_MCLK_EN),
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_USE_AMCR0F28),
 	},
 	{
 		.matches = {
@@ -1109,6 +1112,32 @@ static int byt_rt5640_add_codec_device_props(struct device *i2c_dev,
 	return ret;
 }
 
+/* Some Android devs specify IRQs/GPIOS in a special AMCR0F28 ACPI device */
+static int byt_rt5640_get_amcr0f28_settings(struct snd_soc_card *card)
+{
+	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
+	struct rt5640_set_jack_data *data = &priv->jack_data;
+	struct acpi_device *adev;
+	int ret = 0;
+
+	adev = acpi_dev_get_first_match_dev("AMCR0F28", "1", -1);
+	if (!adev) {
+		dev_err(card->dev, "error cannot find AMCR0F28 adev\n");
+		return -ENOENT;
+	}
+
+	data->codec_irq_override = acpi_dev_gpio_irq_get(adev, 0);
+	if (data->codec_irq_override < 0) {
+		ret = data->codec_irq_override;
+		dev_err(card->dev, "error %d getting codec IRQ\n", ret);
+		goto put_adev;
+	}
+
+put_adev:
+	acpi_dev_put(adev);
+	return ret;
+}
+
 static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
@@ -1244,7 +1273,14 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime)
 		}
 		snd_jack_set_key(priv->jack.jack, SND_JACK_BTN_0,
 				 KEY_PLAYPAUSE);
-		snd_soc_component_set_jack(component, &priv->jack, NULL);
+
+		if (byt_rt5640_quirk & BYT_RT5640_USE_AMCR0F28) {
+			ret = byt_rt5640_get_amcr0f28_settings(card);
+			if (ret)
+				return ret;
+		}
+
+		snd_soc_component_set_jack(component, &priv->jack, &priv->jack_data);
 	}
 
 	if (byt_rt5640_quirk & BYT_RT5640_JD_HP_ELITEP_1000G2) {
@@ -1448,7 +1484,8 @@ static int byt_rt5640_resume(struct snd_soc_card *card)
 	for_each_card_components(card, component) {
 		if (!strcmp(component->name, byt_rt5640_codec_name)) {
 			dev_dbg(component->dev, "re-enabling jack detect after resume\n");
-			snd_soc_component_set_jack(component, &priv->jack, NULL);
+			snd_soc_component_set_jack(component, &priv->jack,
+						   &priv->jack_data);
 			break;
 		}
 	}
-- 
2.33.1

[PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for external GPIO jack-detect

[Hans de Goede]

Some boards have the codec IRQ hooked-up as normally, so the driver can
still do things like headset vs headphones and button-press detection,
but instead of using one of the JD pins of the codec, an external GPIO
is used to report the jack-presence switch status of the jack.

Add support for boards which have this setup and which specify which
external GPIO to use in the special Android AMCR0F28 ACPI device.

And add a quirk for the Asus TF103C tablet which uses this setup.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/intel/boards/bytcr_rt5640.c | 43 +++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index f37ab44ae957..2ace32c03ec9 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -40,6 +40,8 @@ enum {
 	BYT_RT5640_NO_INTERNAL_MIC_MAP,
 };
 
+#define RT5640_JD_SRC_EXT_GPIO			0x0f
+
 enum {
 	BYT_RT5640_JD_SRC_GPIO1		= (RT5640_JD_SRC_GPIO1 << 4),
 	BYT_RT5640_JD_SRC_JD1_IN4P	= (RT5640_JD_SRC_JD1_IN4P << 4),
@@ -47,6 +49,7 @@ enum {
 	BYT_RT5640_JD_SRC_GPIO2		= (RT5640_JD_SRC_GPIO2 << 4),
 	BYT_RT5640_JD_SRC_GPIO3		= (RT5640_JD_SRC_GPIO3 << 4),
 	BYT_RT5640_JD_SRC_GPIO4		= (RT5640_JD_SRC_GPIO4 << 4),
+	BYT_RT5640_JD_SRC_EXT_GPIO	= (RT5640_JD_SRC_EXT_GPIO << 4)
 };
 
 enum {
@@ -627,6 +630,19 @@ static const struct dmi_system_id byt_rt5640_quirk_table[] = {
 					BYT_RT5640_SSP0_AIF2 |
 					BYT_RT5640_MCLK_EN),
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
+		},
+		.driver_data = (void *)(BYT_RT5640_IN1_MAP |
+					BYT_RT5640_JD_SRC_EXT_GPIO |
+					BYT_RT5640_OVCD_TH_2000UA |
+					BYT_RT5640_OVCD_SF_0P75 |
+					BYT_RT5640_SSP0_AIF1 |
+					BYT_RT5640_MCLK_EN |
+					BYT_RT5640_USE_AMCR0F28),
+	},
 	{	/* Chuwi Vi8 (CWI506) */
 		.matches = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Insyde"),
@@ -1083,9 +1099,11 @@ static int byt_rt5640_add_codec_device_props(struct device *i2c_dev,
 	}
 
 	if (BYT_RT5640_JDSRC(byt_rt5640_quirk)) {
-		props[cnt++] = PROPERTY_ENTRY_U32(
-				    "realtek,jack-detect-source",
-				    BYT_RT5640_JDSRC(byt_rt5640_quirk));
+		if (BYT_RT5640_JDSRC(byt_rt5640_quirk) != RT5640_JD_SRC_EXT_GPIO) {
+			props[cnt++] = PROPERTY_ENTRY_U32(
+					    "realtek,jack-detect-source",
+					    BYT_RT5640_JDSRC(byt_rt5640_quirk));
+		}
 
 		props[cnt++] = PROPERTY_ENTRY_U32(
 				    "realtek,over-current-threshold-microamp",
@@ -1113,6 +1131,13 @@ static int byt_rt5640_add_codec_device_props(struct device *i2c_dev,
 }
 
 /* Some Android devs specify IRQs/GPIOS in a special AMCR0F28 ACPI device */
+static const struct acpi_gpio_params amcr0f28_jd_gpio = { 1, 0, false };
+
+static const struct acpi_gpio_mapping amcr0f28_gpios[] = {
+	{ "rt5640-jd-gpios", &amcr0f28_jd_gpio, 1 },
+	{ }
+};
+
 static int byt_rt5640_get_amcr0f28_settings(struct snd_soc_card *card)
 {
 	struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card);
@@ -1133,6 +1158,18 @@ static int byt_rt5640_get_amcr0f28_settings(struct snd_soc_card *card)
 		goto put_adev;
 	}
 
+	if (BYT_RT5640_JDSRC(byt_rt5640_quirk) == RT5640_JD_SRC_EXT_GPIO) {
+		acpi_dev_add_driver_gpios(adev, amcr0f28_gpios);
+		data->jd_gpio = devm_fwnode_gpiod_get(card->dev, acpi_fwnode_handle(adev),
+						      "rt5640-jd", GPIOD_IN, "rt5640-jd");
+		acpi_dev_remove_driver_gpios(adev);
+
+		if (IS_ERR(data->jd_gpio)) {
+			ret = PTR_ERR(data->jd_gpio);
+			dev_err(card->dev, "error %d getting jd GPIO\n", ret);
+		}
+	}
+
 put_adev:
 	acpi_dev_put(adev);
 	return ret;
-- 
2.33.1

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for external GPIO jack-detect

[Stephan Gerhold]

On Mon, Dec 27, 2021 at 04:33:44PM +0100, Hans de Goede wrote:
> Some boards have the codec IRQ hooked-up as normally, so the driver can
> still do things like headset vs headphones and button-press detection,
> but instead of using one of the JD pins of the codec, an external GPIO
> is used to report the jack-presence switch status of the jack.
> 
> Add support for boards which have this setup and which specify which
> external GPIO to use in the special Android AMCR0F28 ACPI device.
> 
> And add a quirk for the Asus TF103C tablet which uses this setup.
> 

Can you clarify what exactly is the difference between the setup on
ME176C and the TF103C? I'm a bit confused why you're using the GpioInt
as GPIO for TF103C and as IRQ for ME176C. It's GPO2 pin 0x0004 for both
of them as far as I can tell.

If I remember correctly the vendor kernel from ASUS also used it as
simple GPIO on ME176C. I'm not sure if it actually belongs to the
RT5640, I just tried using it in a way that is compatible with your
headphone detection code. :)

Before I switched to your code I was actually using it as simple GPIO
similar to your changes here (this could only detect headphones though):
https://github.com/me176c-dev/linux-me176c/commit/ea3de8e47414371fdeeae819c686f737c02fac7d#diff-28a5a6c5e3db2a315d022023f3cda69ef0475ef036e22dd5ffa0fb4af31c9f81

In other words, my question is: Should we also use BYT_RT5640_JD_SRC_EXT_GPIO
for ME176C? Or can TF103C also use the same setup as ME176C?

Thanks,
Stephan

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for external GPIO jack-detect

[Hans de Goede]

Hi,

On 12/30/21 19:56, Stephan Gerhold wrote:
> On Mon, Dec 27, 2021 at 04:33:44PM +0100, Hans de Goede wrote:
>> Some boards have the codec IRQ hooked-up as normally, so the driver can
>> still do things like headset vs headphones and button-press detection,
>> but instead of using one of the JD pins of the codec, an external GPIO
>> is used to report the jack-presence switch status of the jack.
>>
>> Add support for boards which have this setup and which specify which
>> external GPIO to use in the special Android AMCR0F28 ACPI device.
>>
>> And add a quirk for the Asus TF103C tablet which uses this setup.
>>
> 
> Can you clarify what exactly is the difference between the setup on
> ME176C and the TF103C? I'm a bit confused why you're using the GpioInt
> as GPIO for TF103C and as IRQ for ME176C. It's GPO2 pin 0x0004 for both
> of them as far as I can tell.
> 
> If I remember correctly the vendor kernel from ASUS also used it as
> simple GPIO on ME176C. I'm not sure if it actually belongs to the
> RT5640, I just tried using it in a way that is compatible with your
> headphone detection code. :)
> 
> Before I switched to your code I was actually using it as simple GPIO
> similar to your changes here (this could only detect headphones though):
> https://github.com/me176c-dev/linux-me176c/commit/ea3de8e47414371fdeeae819c686f737c02fac7d#diff-28a5a6c5e3db2a315d022023f3cda69ef0475ef036e22dd5ffa0fb4af31c9f81
> 
> In other words, my question is: Should we also use BYT_RT5640_JD_SRC_EXT_GPIO
> for ME176C? Or can TF103C also use the same setup as ME176C?

The Bay Trail SoC GPO2 pin 4 is connected to the codecs GPIO1 pin,
which is best thought of as the codecs IRQ pin, because that is how
the rt5640 codec driver configures it:

        /* Selecting GPIO01 as an interrupt */
        snd_soc_component_update_bits(component, RT5640_GPIO_CTRL1,
                RT5640_GP1_PIN_MASK, RT5640_GP1_PIN_IRQ);

        /* Set GPIO1 output */
        snd_soc_component_update_bits(component, RT5640_GPIO_CTRL3,
                RT5640_GP1_PF_MASK, RT5640_GP1_PF_OUT);

This may seem to be directly connected to the jack-inserted switch
of the physical jack, but it is not, the IRQ just happens to go
low/high together with the jack state (the IRQ handler is sensitive
to both edges).

But when low (jack inserted), it can go high again even though the
jack is not removed *at all*. This happens on an overcurrent situation
on the mic bias current, which is how heapdhones vs headset are
detected (headphones short the mic contact triggering OVCD). So it
really is a totally different pin from the jack-inserted switch,
it just seems to be directly connected most of the time.

On the ME176C the actual physical jack-inserted switch is connected
to the JD2 aka IN4N pin of the codec and the codec driver checks
for jack-insertion like this:

	val = snd_soc_component_read(component, RT5640_INT_IRQ_ST);

        if (rt5640->jd_inverted)
                return !(val & RT5640_JD_STATUS);
        else
                return (val & RT5640_JD_STATUS);

And the physical jack-inserted switch is *also* connected to
the Bay Trail SoC GPO2 pin 27 (second GPIO resource in the AMCROf28
device), but we ignore that since before this patch-set the rt5640
codec code assumes the switch is always connected to one of the
codecs JD pins.


On the Asus TF103C, the codecs GPIO1 aka IRQ pin is connected
to the Bay Trail SoC's GPO2 pin 4 just like on the ME176C.

What is different is that the jack's physical insertion switch
is *only* connected to the Bay Trail SoC's GPO2 pin 27 and not
connected to any of the codecs JD1/JD2 pins *at all* so we need
to work around that.

I hope this helps explain.

and yes this means that we *could* also use
BYT_RT5640_JD_SRC_EXT_GPIO on the ME176C (and actually on quite
a few BYT devices) but it does not gain us anything,
it is just another method of getting the same information,
since the jack-inserted switch is connected to both the codec
and the Bay Trail SoC.

Regards,

Hans

Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for external GPIO jack-detect

[Stephan Gerhold]

On Thu, Dec 30, 2021 at 08:12:27PM +0100, Hans de Goede wrote:
> On 12/30/21 19:56, Stephan Gerhold wrote:
> > On Mon, Dec 27, 2021 at 04:33:44PM +0100, Hans de Goede wrote:
> >> Some boards have the codec IRQ hooked-up as normally, so the driver can
> >> still do things like headset vs headphones and button-press detection,
> >> but instead of using one of the JD pins of the codec, an external GPIO
> >> is used to report the jack-presence switch status of the jack.
> >>
> >> Add support for boards which have this setup and which specify which
> >> external GPIO to use in the special Android AMCR0F28 ACPI device.
> >>
> >> And add a quirk for the Asus TF103C tablet which uses this setup.
> >>
> > 
> > Can you clarify what exactly is the difference between the setup on
> > ME176C and the TF103C? I'm a bit confused why you're using the GpioInt
> > as GPIO for TF103C and as IRQ for ME176C. It's GPO2 pin 0x0004 for both
> > of them as far as I can tell.
> > 
> > If I remember correctly the vendor kernel from ASUS also used it as
> > simple GPIO on ME176C. I'm not sure if it actually belongs to the
> > RT5640, I just tried using it in a way that is compatible with your
> > headphone detection code. :)
> > 
> > Before I switched to your code I was actually using it as simple GPIO
> > similar to your changes here (this could only detect headphones though):
> > https://github.com/me176c-dev/linux-me176c/commit/ea3de8e47414371fdeeae819c686f737c02fac7d#diff-28a5a6c5e3db2a315d022023f3cda69ef0475ef036e22dd5ffa0fb4af31c9f81
> > 
> > In other words, my question is: Should we also use BYT_RT5640_JD_SRC_EXT_GPIO
> > for ME176C? Or can TF103C also use the same setup as ME176C?
> 
> The Bay Trail SoC GPO2 pin 4 is connected to the codecs GPIO1 pin,
> which is best thought of as the codecs IRQ pin, because that is how
> the rt5640 codec driver configures it:
> 
>         /* Selecting GPIO01 as an interrupt */
>         snd_soc_component_update_bits(component, RT5640_GPIO_CTRL1,
>                 RT5640_GP1_PIN_MASK, RT5640_GP1_PIN_IRQ);
> 
>         /* Set GPIO1 output */
>         snd_soc_component_update_bits(component, RT5640_GPIO_CTRL3,
>                 RT5640_GP1_PF_MASK, RT5640_GP1_PF_OUT);
> 
> This may seem to be directly connected to the jack-inserted switch
> of the physical jack, but it is not, the IRQ just happens to go
> low/high together with the jack state (the IRQ handler is sensitive
> to both edges).
> 
> But when low (jack inserted), it can go high again even though the
> jack is not removed *at all*. This happens on an overcurrent situation
> on the mic bias current, which is how heapdhones vs headset are
> detected (headphones short the mic contact triggering OVCD). So it
> really is a totally different pin from the jack-inserted switch,
> it just seems to be directly connected most of the time.
> 
> On the ME176C the actual physical jack-inserted switch is connected
> to the JD2 aka IN4N pin of the codec and the codec driver checks
> for jack-insertion like this:
> 
> 	val = snd_soc_component_read(component, RT5640_INT_IRQ_ST);
> 
>         if (rt5640->jd_inverted)
>                 return !(val & RT5640_JD_STATUS);
>         else
>                 return (val & RT5640_JD_STATUS);
> 
> And the physical jack-inserted switch is *also* connected to
> the Bay Trail SoC GPO2 pin 27 (second GPIO resource in the AMCROf28
> device), but we ignore that since before this patch-set the rt5640
> codec code assumes the switch is always connected to one of the
> codecs JD pins.
> 

Ahh, I think I mixed up the first and second GPIO resource in the
AMCR0F28 ACPI device. I thought you're using pin 4 as GPIO here but it's
actually pin 27. Looks I was also using pin 27 in my "driver" back then.

When quickly looking at the following line in your patch and my "driver"

static const struct acpi_gpio_params amcr0f28_jd_gpio = { 1, 0, false };

... I had a 50 percent chance if it refers to resource 1 or resource 0
and clearly I guessed wrong. :(

> 
> On the Asus TF103C, the codecs GPIO1 aka IRQ pin is connected
> to the Bay Trail SoC's GPO2 pin 4 just like on the ME176C.
> 
> What is different is that the jack's physical insertion switch
> is *only* connected to the Bay Trail SoC's GPO2 pin 27 and not
> connected to any of the codecs JD1/JD2 pins *at all* so we need
> to work around that.
> 
> I hope this helps explain.
> 

Yep it does, thanks for the detailed explanation!

Stephan

Re: [PATCH 1/5] ASoC: rt5640: Change jack_work to a delayed_work

[Mark Brown]

On Mon, 27 Dec 2021 16:33:40 +0100, Hans de Goede wrote:
> Change jack_work from a struct work_struct to a struct delayed_work, this
> is a preparation patch for adding support for boards where an external
> GPIO is used for jack-detect, rather then one of the JD pins of the codec.
> 
> 

Applied to

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

Thanks!

[1/5] ASoC: rt5640: Change jack_work to a delayed_work
      commit: a3b1aaf7aef9fa945810de3fd7c15b2e93ecdbfd
[2/5] ASoC: rt5640: Allow snd_soc_component_set_jack() to override the codec IRQ
      commit: b35a9ab4904973a68b4473c2985b8ac0b6d57089
[3/5] ASoC: rt5640: Add support for boards with an external jack-detect GPIO
      commit: 701d636a224a77a4371f57ca2d4322ab0401a866
[4/5] ASoC: Intel: bytcr_rt5640: Support retrieving the codec IRQ from the AMCR0F28 ACPI dev
      commit: 45ed0166c39f878162872babc88830d91426beb5
[5/5] ASoC: Intel: bytcr_rt5640: Add support for external GPIO jack-detect
      commit: 44125fd5315154c6b8326b5c27646af3b33ba25c

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