[RFC] ASoC: da7219: Prevent hung task errors in interrupt handler

From: Guenter Roeck
Date: Thu Feb 02 2023 - 19:09:14 EST


Commit 969357ec94e6 ("ASoC: da7219: Fix pole orientation detection on
OMTP headsets when playing music") tried to improve pole orientation
on certain headsets. Part of the change was to add a long sleep in
the beginning of the interrupt handler, followed by enabling the
ground switch

Unfortunately, this results in hung tasks in the threaded interrupt
handler.

INFO: task irq/105-da7219-:2556 blocked for more than 122 seconds.
Not tainted 5.10.159-20945-g4390861bfc33 #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:irq/105-da7219- state:D stack: 0 pid: 2556 ppid: 2 flags:0x00004080
Call Trace:
__schedule+0x3b0/0xddb
schedule+0x44/0xa8
schedule_timeout+0xae/0x241
? run_local_timers+0x4e/0x4e
msleep+0x2c/0x38
da7219_aad_irq_thread+0x66/0x2b0
irq_thread_fn+0x22/0x4d
irq_thread+0x131/0x1cb
? irq_forced_thread_fn+0x5f/0x5f
? irq_thread_fn+0x4d/0x4d
kthread+0x142/0x153
? synchronize_irq+0xe0/0xe0
? kthread_blkcg+0x31/0x31
ret_from_fork+0x1f/0x30

Solve the problem by enabling the ground switch immediately and only
after an insertion has been detected. Delay pole orientation detection
until after the chip reports that detection is complete plus an
additional time depending on the chip configuration. Do this by
implementing ground switch detection in a delayed worker.

Fixes: 969357ec94e6 ("ASoC: da7219: Fix pole orientation detection on OMTP headsets when playing music")
Cc: David Rau <david.rau.zg@xxxxxxxxxxx>
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
sound/soc/codecs/da7219-aad.c | 156 ++++++++++++++++++++++------------
sound/soc/codecs/da7219-aad.h | 3 +
2 files changed, 105 insertions(+), 54 deletions(-)

diff --git a/sound/soc/codecs/da7219-aad.c b/sound/soc/codecs/da7219-aad.c
index c55b033d89da..47685c996bda 100644
--- a/sound/soc/codecs/da7219-aad.c
+++ b/sound/soc/codecs/da7219-aad.c
@@ -8,6 +8,7 @@
*/

#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/clk.h>
#include <linux/i2c.h>
@@ -339,6 +340,82 @@ static void da7219_aad_hptest_work(struct work_struct *work)
SND_JACK_HEADSET | SND_JACK_LINEOUT);
}

+static void da7219_aad_handle_removal(struct da7219_aad_priv *da7219_aad)
+{
+ struct snd_soc_component *component = da7219_aad->component;
+ struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
+ struct da7219_priv *da7219 = snd_soc_component_get_drvdata(component);
+
+ da7219_aad->jack_inserted = false;
+
+ /* Cancel any pending work */
+ cancel_work_sync(&da7219_aad->btn_det_work);
+ cancel_work_sync(&da7219_aad->hptest_work);
+
+ /* Un-drive headphones/lineout */
+ snd_soc_component_update_bits(component, DA7219_HP_R_CTRL,
+ DA7219_HP_R_AMP_OE_MASK, 0);
+ snd_soc_component_update_bits(component, DA7219_HP_L_CTRL,
+ DA7219_HP_L_AMP_OE_MASK, 0);
+
+ /* Ensure button detection disabled */
+ snd_soc_component_update_bits(component, DA7219_ACCDET_CONFIG_1,
+ DA7219_BUTTON_CONFIG_MASK, 0);
+
+ da7219->micbias_on_event = false;
+
+ /* Disable mic bias */
+ snd_soc_dapm_disable_pin(dapm, "Mic Bias");
+ snd_soc_dapm_sync(dapm);
+
+ /* Disable ground switch */
+ snd_soc_component_update_bits(component, 0xFB, 0x01, 0x00);
+
+ snd_soc_jack_report(da7219_aad->jack, 0, DA7219_AAD_REPORT_ALL_MASK);
+}
+
+static void da7219_aad_insertion_work(struct work_struct *work)
+{
+ struct da7219_aad_priv *da7219_aad =
+ container_of(work, struct da7219_aad_priv, hptest_work);
+ struct snd_soc_component *component = da7219_aad->component;
+ u8 statusa;
+
+ mutex_lock(&da7219_aad->insertion_mutex);
+
+ if (!da7219_aad->jack_inserted)
+ goto unlock;
+
+ /* Read status register for jack insertion & type status */
+ statusa = snd_soc_component_read(component, DA7219_ACCDET_STATUS_A);
+ if (!(statusa & DA7219_JACK_INSERTION_STS_MASK)) {
+ da7219_aad_handle_removal(da7219_aad);
+ goto unlock;
+ }
+
+ /*
+ * If 4-pole, then enable button detection, else perform
+ * HP impedance test to determine output type to report.
+ *
+ * We schedule work here as the tasks themselves can
+ * take time to complete, and in particular for hptest
+ * we want to be able to check if the jack was removed
+ * during the procedure as this will invalidate the
+ * result. By doing this as work, the IRQ thread can
+ * handle a removal, and we can check at the end of
+ * hptest if we have a valid result or not.
+ */
+ if (statusa & DA7219_JACK_TYPE_STS_MASK) {
+ schedule_work(&da7219_aad->btn_det_work);
+ snd_soc_jack_report(da7219_aad->jack, SND_JACK_HEADSET,
+ SND_JACK_HEADSET | SND_JACK_LINEOUT);
+ } else {
+ schedule_work(&da7219_aad->hptest_work);
+ }
+
+unlock:
+ mutex_unlock(&da7219_aad->insertion_mutex);
+}

/*
* IRQ
@@ -348,23 +425,21 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
{
struct da7219_aad_priv *da7219_aad = data;
struct snd_soc_component *component = da7219_aad->component;
- struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
struct da7219_priv *da7219 = snd_soc_component_get_drvdata(component);
u8 events[DA7219_AAD_IRQ_REG_MAX];
- u8 statusa, srm_st;
+ u8 statusa;
int i, report = 0, mask = 0;

- srm_st = snd_soc_component_read(component, DA7219_PLL_SRM_STS) & DA7219_PLL_SRM_STS_MCLK;
- msleep(da7219_aad->gnd_switch_delay * ((srm_st == 0x0) ? 2 : 1) - 4);
- /* Enable ground switch */
- snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
+ mutex_lock(&da7219_aad->insertion_mutex);

/* Read current IRQ events */
regmap_bulk_read(da7219->regmap, DA7219_ACCDET_IRQ_EVENT_A,
events, DA7219_AAD_IRQ_REG_MAX);

- if (!events[DA7219_AAD_IRQ_REG_A] && !events[DA7219_AAD_IRQ_REG_B])
+ if (!events[DA7219_AAD_IRQ_REG_A] && !events[DA7219_AAD_IRQ_REG_B]) {
+ mutex_unlock(&da7219_aad->insertion_mutex);
return IRQ_NONE;
+ }

/* Read status register for jack insertion & type status */
statusa = snd_soc_component_read(component, DA7219_ACCDET_STATUS_A);
@@ -378,36 +453,29 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
statusa);

if (statusa & DA7219_JACK_INSERTION_STS_MASK) {
+ u8 srm_st;
+
+ srm_st = snd_soc_component_read(component, DA7219_PLL_SRM_STS) &
+ DA7219_PLL_SRM_STS_MCLK;
+
/* Jack Insertion */
if (events[DA7219_AAD_IRQ_REG_A] &
DA7219_E_JACK_INSERTED_MASK) {
report |= SND_JACK_MECHANICAL;
mask |= SND_JACK_MECHANICAL;
da7219_aad->jack_inserted = true;
+
+ /* Enable ground switch */
+ snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
}

/* Jack type detection */
if (events[DA7219_AAD_IRQ_REG_A] &
DA7219_E_JACK_DETECT_COMPLETE_MASK) {
- /*
- * If 4-pole, then enable button detection, else perform
- * HP impedance test to determine output type to report.
- *
- * We schedule work here as the tasks themselves can
- * take time to complete, and in particular for hptest
- * we want to be able to check if the jack was removed
- * during the procedure as this will invalidate the
- * result. By doing this as work, the IRQ thread can
- * handle a removal, and we can check at the end of
- * hptest if we have a valid result or not.
- */
- if (statusa & DA7219_JACK_TYPE_STS_MASK) {
- report |= SND_JACK_HEADSET;
- mask |= SND_JACK_HEADSET | SND_JACK_LINEOUT;
- schedule_work(&da7219_aad->btn_det_work);
- } else {
- schedule_work(&da7219_aad->hptest_work);
- }
+ int delay = da7219_aad->gnd_switch_delay *
+ ((srm_st == 0x0) ? 2 : 1) - 4;
+
+ schedule_delayed_work(&da7219_aad->insertion_work, delay);
}

/* Button support for 4-pole jack */
@@ -431,40 +499,16 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
}
}
}
+ snd_soc_jack_report(da7219_aad->jack, report, mask);
} else {
/* Jack removal */
if (events[DA7219_AAD_IRQ_REG_A] & DA7219_E_JACK_REMOVED_MASK) {
- report = 0;
- mask |= DA7219_AAD_REPORT_ALL_MASK;
- da7219_aad->jack_inserted = false;
-
- /* Cancel any pending work */
- cancel_work_sync(&da7219_aad->btn_det_work);
- cancel_work_sync(&da7219_aad->hptest_work);
-
- /* Un-drive headphones/lineout */
- snd_soc_component_update_bits(component, DA7219_HP_R_CTRL,
- DA7219_HP_R_AMP_OE_MASK, 0);
- snd_soc_component_update_bits(component, DA7219_HP_L_CTRL,
- DA7219_HP_L_AMP_OE_MASK, 0);
-
- /* Ensure button detection disabled */
- snd_soc_component_update_bits(component, DA7219_ACCDET_CONFIG_1,
- DA7219_BUTTON_CONFIG_MASK, 0);
-
- da7219->micbias_on_event = false;
-
- /* Disable mic bias */
- snd_soc_dapm_disable_pin(dapm, "Mic Bias");
- snd_soc_dapm_sync(dapm);
-
- /* Disable ground switch */
- snd_soc_component_update_bits(component, 0xFB, 0x01, 0x00);
+ cancel_delayed_work(&da7219_aad->insertion_work);
+ da7219_aad_handle_removal(da7219_aad);
}
}

- snd_soc_jack_report(da7219_aad->jack, report, mask);
-
+ mutex_unlock(&da7219_aad->insertion_mutex);
return IRQ_HANDLED;
}

@@ -938,6 +982,9 @@ int da7219_aad_init(struct snd_soc_component *component)
snd_soc_component_update_bits(component, DA7219_ACCDET_CONFIG_1,
DA7219_BUTTON_CONFIG_MASK, 0);

+ mutex_init(&da7219_aad->insertion_mutex);
+
+ INIT_DELAYED_WORK(&da7219_aad->insertion_work, da7219_aad_insertion_work);
INIT_WORK(&da7219_aad->btn_det_work, da7219_aad_btn_det_work);
INIT_WORK(&da7219_aad->hptest_work, da7219_aad_hptest_work);

@@ -973,6 +1020,7 @@ void da7219_aad_exit(struct snd_soc_component *component)

free_irq(da7219_aad->irq, da7219_aad);

+ cancel_delayed_work_sync(&da7219_aad->insertion_work);
cancel_work_sync(&da7219_aad->btn_det_work);
cancel_work_sync(&da7219_aad->hptest_work);
}
diff --git a/sound/soc/codecs/da7219-aad.h b/sound/soc/codecs/da7219-aad.h
index 21fdf53095cc..b1b7f8ba45bd 100644
--- a/sound/soc/codecs/da7219-aad.h
+++ b/sound/soc/codecs/da7219-aad.h
@@ -10,6 +10,7 @@
#ifndef __DA7219_AAD_H
#define __DA7219_AAD_H

+#include <linux/mutex.h>
#include <linux/timer.h>
#include <sound/soc.h>
#include <sound/jack.h>
@@ -194,6 +195,8 @@ struct da7219_aad_priv {

u8 btn_cfg;

+ struct mutex insertion_mutex;
+ struct delayed_work insertion_work;
struct work_struct btn_det_work;
struct work_struct hptest_work;

--
2.39.1