[PATCH 1/5] ASoC: rt5640: Change jack_work to a delayed_work
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;
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,
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.
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
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,
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; } }
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;
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/ea3de8e47414371fdeeae819c6...
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
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/ea3de8e47414371fdeeae819c6...
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
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/ea3de8e47414371fdeeae819c6...
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
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
participants (3)
-
Hans de Goede
-
Mark Brown
-
Stephan Gerhold