* [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal
@ 2026-10-30 17:48 Tristan Madani
2026-10-30 17:48 ` [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:48 UTC (permalink / raw)
To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai
Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
Tristan Madani
Several ASoC codec drivers schedule delayed works for jack detection
and microphone polling but do not cancel them on component removal,
leaving handlers that can run against freed memory.
This series adds proper teardown ordering in four codec drivers:
rt5668, rt5659, twl6040, and wm8994.
Changes since v4:
- Patch 1/4 (rt5668): correct Fixes tag from nonexistent
e13da14e8543 to d59fb2856223 ("ASoC: rt5668: add rt5668B codec
driver"). No code changes.
- Patches 2-4/4: no changes.
Changes since v3:
- rt5668 (1/4): track IRQ acquisition with irq_acquired flag to
avoid calling disable_irq() when devm_request_threaded_irq()
failed. Re-enable IRQ in rt5668_probe() when a sound card
rebinds after rt5668_remove() disabled it, preventing permanent
IRQ silencing across unbind/rebind cycles.
- Patches 2-4/4: no code changes.
Changes since v2:
- rt5668 (1/4): also cancel jd_check_work (re-arms itself every
500 jiffies while jack inserted). Disable IRQ before cancelling
to prevent the handler from scheduling new work in the window
between cancel and devm IRQ release.
- rt5659 (2/4): no code change from v2.
- twl6040 (3/4): swap ordering -- free_irq() first, then cancel
residual work. Fixes the race where the handler fires between
cancel and free_irq and re-arms the work.
- wm8994 (4/4): no code change from v2.
Changes since v1:
- Unified series per Mark Brown request.
- rt5659: guard cancel with jack_detect_work_inited flag since
INIT_DELAYED_WORK is conditional on jd_src, per Mark Brown.
- wm8994: cancel mic_work before and after jackdet_bootstrap for
cyclic re-arming through DAPM power-up chain.
Tristan Madani (4):
ASoC: rt5668: cancel delayed works on component removal
ASoC: rt5659: cancel jack_detect_work on component removal
ASoC: twl6040: cancel headset jack work on component removal
ASoC: wm8994: drain delayed works on remove and probe error
sound/soc/codecs/rt5659.c | 20 +++++++++++++++++++-
sound/soc/codecs/rt5659.h | 3 +++
sound/soc/codecs/rt5668.c | 20 +++++++++++++++++++-
sound/soc/codecs/twl6040.c | 1 +
sound/soc/codecs/wm8994.c | 14 ++++++++++++++
5 files changed, 56 insertions(+), 2 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal
2026-10-30 17:48 [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
@ 2026-10-30 17:48 ` Tristan Madani
2026-10-30 18:04 ` sashiko-bot
2026-10-30 17:48 ` [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:48 UTC (permalink / raw)
To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai
Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
Tristan Madani
rt5668_remove() does not disable the IRQ or cancel the pending
jack_detect_work and jd_check_work delayed works. If the component is
unbound while work is pending, the handlers access freed memory.
Track IRQ acquisition in rt5668_i2c_probe() via an irq_acquired flag.
In rt5668_remove(), disable the IRQ (if acquired) and cancel both
delayed works before resetting the codec. Re-enable the IRQ in
rt5668_probe() when a sound card rebinds to prevent permanent IRQ
silencing across card unbind/rebind cycles.
Fixes: d59fb2856223 ("ASoC: rt5668: add rt5668B codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
sound/soc/codecs/rt5668.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
index 4154bad276101..ab63a43b20b89 100644
--- a/sound/soc/codecs/rt5668.c
+++ b/sound/soc/codecs/rt5668.c
@@ -62,6 +62,9 @@ struct rt5668_priv {
int pll_out;
int jack_type;
+
+ bool irq_acquired;
+ bool irq_disabled;
};
static const struct reg_default rt5668_reg[] = {
@@ -2216,12 +2219,26 @@ static int rt5668_probe(struct snd_soc_component *component)
rt5668->component = component;
+ if (rt5668->irq_disabled) {
+ enable_irq(to_i2c_client(component->dev)->irq);
+ rt5668->irq_disabled = false;
+ }
+
return 0;
}
static void rt5668_remove(struct snd_soc_component *component)
{
struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
+ struct i2c_client *i2c = to_i2c_client(component->dev);
+
+ if (rt5668->irq_acquired) {
+ disable_irq(i2c->irq);
+ rt5668->irq_disabled = true;
+ }
+
+ cancel_delayed_work_sync(&rt5668->jack_detect_work);
+ cancel_delayed_work_sync(&rt5668->jd_check_work);
rt5668_reset(rt5668->regmap);
}
@@ -2533,7 +2550,8 @@ static int rt5668_i2c_probe(struct i2c_client *i2c)
| IRQF_ONESHOT, "rt5668", rt5668);
if (ret)
dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
-
+ else
+ rt5668->irq_acquired = true;
}
return devm_snd_soc_register_component(&i2c->dev, &soc_component_dev_rt5668,
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
2026-10-30 17:48 [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
2026-10-30 17:48 ` [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 17:48 ` Tristan Madani
2026-10-30 18:02 ` sashiko-bot
2026-10-30 17:48 ` [PATCH v5 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
2026-10-30 17:48 ` [PATCH v5 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani
3 siblings, 1 reply; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:48 UTC (permalink / raw)
To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai
Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
Tristan Madani
rt5659_remove() resets the codec via regmap_write() but does not cancel
the pending jack_detect_work. Since the IRQ is registered with
devm_request_threaded_irq(), it remains active until after the component
remove callback returns. The IRQ handler (rt5659_irq) schedules
jack_detect_work, so a late interrupt can queue work that runs against a
reset codec.
Track successful IRQ acquisition with a flag so that disable_irq()
only quiesces a line this driver actually owns. Guard the
cancel_delayed_work_sync() call with a flag that tracks whether the
work was initialized, since INIT_DELAYED_WORK is only called for
specific jd_src configurations (JD3, JD_HDA_HEADER) and cancelling
an uninitialized work is undefined behavior.
On component remove, disable the IRQ to synchronize with any running
handler and prevent new enqueues, then drain the residual
jack_detect_work synchronously before resetting the hardware. On the
next component probe (card rebind), re-enable the IRQ to restore jack
detection.
Fixes: d3cb2de2479b ("ASoC: rt5659: add rt5659 codec driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
sound/soc/codecs/rt5659.c | 20 +++++++++++++++++++-
sound/soc/codecs/rt5659.h | 3 +++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index 3097ee6d4e89f..cdf499fb36d67 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3667,6 +3667,11 @@ static int rt5659_probe(struct snd_soc_component *component)
rt5659->component = component;
+ if (rt5659->irq_disabled) {
+ enable_irq(to_i2c_client(component->dev)->irq);
+ rt5659->irq_disabled = false;
+ }
+
switch (rt5659->pdata.jd_src) {
case RT5659_JD_HDA_HEADER:
break;
@@ -3684,6 +3689,15 @@ static int rt5659_probe(struct snd_soc_component *component)
static void rt5659_remove(struct snd_soc_component *component)
{
struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
+ struct i2c_client *i2c = to_i2c_client(component->dev);
+
+ if (rt5659->irq_acquired) {
+ disable_irq(i2c->irq);
+ rt5659->irq_disabled = true;
+ }
+
+ if (rt5659->jack_detect_work_inited)
+ cancel_delayed_work_sync(&rt5659->jack_detect_work);
regmap_write(rt5659->regmap, RT5659_RESET, 0);
}
@@ -4271,6 +4285,7 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
regmap_write(rt5659->regmap, RT5659_IRQ_CTRL_2, 0x0040);
INIT_DELAYED_WORK(&rt5659->jack_detect_work,
rt5659_jack_detect_work);
+ rt5659->jack_detect_work_inited = true;
break;
case RT5659_JD_HDA_HEADER:
regmap_write(rt5659->regmap, RT5659_GPIO_CTRL_3, 0x8000);
@@ -4280,6 +4295,7 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
regmap_write(rt5659->regmap, RT5659_IRQ_CTRL_1, 0x0040);
INIT_DELAYED_WORK(&rt5659->jack_detect_work,
rt5659_jack_detect_intel_hd_header);
+ rt5659->jack_detect_work_inited = true;
rt5659_intel_hd_header_probe_setup(rt5659);
break;
default:
@@ -4290,7 +4306,9 @@ static int rt5659_i2c_probe(struct i2c_client *i2c)
ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
rt5659_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
| IRQF_ONESHOT, "rt5659", rt5659);
- if (ret)
+ if (!ret)
+ rt5659->irq_acquired = true;
+ else
dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
/* Enable IRQ output for GPIO1 pin any way */
diff --git a/sound/soc/codecs/rt5659.h b/sound/soc/codecs/rt5659.h
index b49fd8baf4e7e..e10b3844501b9 100644
--- a/sound/soc/codecs/rt5659.h
+++ b/sound/soc/codecs/rt5659.h
@@ -1813,6 +1813,9 @@ struct rt5659_priv {
int jack_type;
bool hda_hp_plugged;
bool hda_mic_plugged;
+ bool irq_acquired;
+ bool irq_disabled;
+ bool jack_detect_work_inited;
};
int rt5659_set_jack_detect(struct snd_soc_component *component,
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 3/4] ASoC: twl6040: cancel headset jack work on component removal
2026-10-30 17:48 [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
2026-10-30 17:48 ` [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
2026-10-30 17:48 ` [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 17:48 ` Tristan Madani
2026-10-30 17:48 ` [PATCH v5 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani
3 siblings, 0 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:48 UTC (permalink / raw)
To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai
Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
Tristan Madani
The twl6040 codec driver initializes a delayed work for headset jack
detection (hs_jack.work) but the component remove callback only frees
the plug IRQ without cancelling the pending work.
Free the IRQ first so that the threaded handler cannot schedule new
work, then cancel any residual queued work synchronously. This
ordering prevents the race where the handler fires between a cancel
and the free_irq, re-arming the work after it was drained.
Fixes: a2d2362edf9f ("ASoC: twl6040: Add jack support for headset and handset")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
sound/soc/codecs/twl6040.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c
index 650836f9615fb..826bde1a9ba46 100644
--- a/sound/soc/codecs/twl6040.c
+++ b/sound/soc/codecs/twl6040.c
@@ -1136,6 +1136,7 @@ static void twl6040_remove(struct snd_soc_component *component)
struct twl6040_data *priv = snd_soc_component_get_drvdata(component);
free_irq(priv->plug_irq, component);
+ cancel_delayed_work_sync(&priv->hs_jack.work);
}
static const struct snd_soc_component_driver soc_component_dev_twl6040 = {
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 4/4] ASoC: wm8994: drain delayed works on remove and probe error
2026-10-30 17:48 [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
` (2 preceding siblings ...)
2026-10-30 17:48 ` [PATCH v5 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
@ 2026-10-30 17:48 ` Tristan Madani
3 siblings, 0 replies; 7+ messages in thread
From: Tristan Madani @ 2026-10-30 17:48 UTC (permalink / raw)
To: Mark Brown, Oder Chiou, Liam Girdwood, Jaroslav Kysela,
Takashi Iwai
Cc: Bard Liao, Misael Lopez Cruz, patches, linux-sound, stable,
Tristan Madani
wm8994_component_probe() schedules several delayed works to handle jack
detection and microphone status polling. If the component is removed or
probe fails after these works are scheduled, they can execute after
resources are freed.
Cancel all pending delayed works in the error and removal paths. Guard
cancel_delayed_work_sync(&wm8994->mic_work) with a type check because
INIT_DELAYED_WORK for mic_work only runs in the WM8994 and WM1811 code
paths.
Cancel mic_work before and after jackdet_bootstrap because the two can
re-arm each other: wm1811_mic_work() can re-arm jackdet_bootstrap
through the DAPM power-up chain (force-enabling MICBIAS2 triggers
clk_sys_event which queues jackdet_bootstrap), and
wm1811_jackdet_bootstrap() detects a jack and ultimately queues
mic_work. The second mic_work cancel catches any re-arm from a
jackdet_bootstrap handler that was in flight during the first cancel.
Fixes: 99af79dff5a6 ("ASoC: wm8994: Ensure we get a notification on startup for jackdet")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
sound/soc/codecs/wm8994.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 8bf58a6a8af7c..b9e0df83b9bfe 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4551,6 +4551,13 @@ static int wm8994_component_probe(struct snd_soc_component *component)
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_FIFOS_ERR, component);
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_SHUT, component);
wm8994_free_irq(wm8994->wm8994, WM8994_IRQ_TEMP_WARN, component);
+ if (control->type == WM8994 || control->type == WM1811)
+ cancel_delayed_work_sync(&wm8994->mic_work);
+ cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+ if (control->type == WM8994 || control->type == WM1811)
+ cancel_delayed_work_sync(&wm8994->mic_work);
+ cancel_delayed_work_sync(&wm8994->mic_complete_work);
+ cancel_delayed_work_sync(&wm8994->open_circuit_work);
return ret;
}
@@ -4592,6 +4599,13 @@ static void wm8994_component_remove(struct snd_soc_component *component)
free_irq(wm8994->micdet_irq, wm8994);
break;
}
+ if (control->type == WM8994 || control->type == WM1811)
+ cancel_delayed_work_sync(&wm8994->mic_work);
+ cancel_delayed_work_sync(&wm8994->jackdet_bootstrap);
+ if (control->type == WM8994 || control->type == WM1811)
+ cancel_delayed_work_sync(&wm8994->mic_work);
+ cancel_delayed_work_sync(&wm8994->mic_complete_work);
+ cancel_delayed_work_sync(&wm8994->open_circuit_work);
release_firmware(wm8994->mbc);
release_firmware(wm8994->mbc_vss);
release_firmware(wm8994->enh_eq);
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work on component removal
2026-10-30 17:48 ` [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
@ 2026-10-30 18:02 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 18:02 UTC (permalink / raw)
To: Tristan Madani; +Cc: linux-sound
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5659: uncancelled jack_detect_work on I2C teardown without card
- [High] ASoC: rt5659: broken jack detect and dangling hs_jack in rt5659_probe()
--
commit ed495f70ecb1520022a86353ac80bee32a4816ff
Author: Tristan Madani <tristmd@gmail.com>
ASoC: rt5659: cancel jack_detect_work on component removal
This commit disables the codec IRQ and cancels any pending jack_detect_work
in rt5659_remove() before resetting the codec, and re-enables the IRQ in
rt5659_probe() when a card rebinds.
> diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
> index af8051fa69f89..68f7d5673e190 100644
> --- a/sound/soc/codecs/rt5659.c
> +++ b/sound/soc/codecs/rt5659.c
> @@ -3667,6 +3667,11 @@ static int rt5659_probe(struct snd_soc_component *component)
>
> rt5659->component = component;
>
> + if (rt5659->irq_disabled) {
> + enable_irq(to_i2c_client(component->dev)->irq);
> + rt5659->irq_disabled = false;
> + }
[Severity: High]
As reported by Sashiko on v4 [1], when a sound card unbinds and rebinds, can
re-enabling the IRQ in rt5659_probe() restore jack detection or is it unsafe
while rt5659->hs_jack still holds a pointer from the previous card?
At the end of rt5659_remove(), regmap_write(rt5659->regmap, RT5659_RESET, 0)
resets all codec hardware registers to power-on defaults, clearing the GPIO1
IRQ pin mux (RT5659_GP1_PIN_IRQ in RT5659_GPIO_CTRL_1) and all jack-detection
and IRQ control registers (such as RT5659_EJD_CTRL_1, RT5659_RC_CLK_CTRL,
RT5659_GPIO_CTRL_3, RT5659_JD_CTRL_1, RT5659_IRQ_CTRL_1, and
RT5659_IRQ_CTRL_2) that were only programmed once in rt5659_i2c_probe().
Because RT5659_RESET is marked volatile in rt5659_volatile_register(), the
REGCACHE_RBTREE regmap cache is not marked dirty or resynced on re-probe,
leaving the codec unable to generate jack-detection interrupts after rebind.
In addition, rt5659_remove() does not clear rt5659->hs_jack or
rt5659->component. When a card rebinds, snd_soc_component_probe() invokes
rt5659_probe() and calls enable_irq() before the machine driver's DAI-link
init callback registers a new snd_soc_jack. If an interrupt fires before a
new jack is set (or if the new card does not call rt5659_set_jack_detect()),
rt5659_jack_detect_work() or rt5659_jack_detect_intel_hd_header() will pass
the dangling rt5659->hs_jack pointer from the old card to
snd_soc_jack_report().
> +
> switch (rt5659->pdata.jd_src) {
> case RT5659_JD_HDA_HEADER:
> break;
> @@ -3684,6 +3689,15 @@ static int rt5659_probe(struct snd_soc_component *component)
> static void rt5659_remove(struct snd_soc_component *component)
> {
> struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
> + struct i2c_client *i2c = to_i2c_client(component->dev);
> +
> + if (rt5659->irq_acquired) {
> + disable_irq(i2c->irq);
> + rt5659->irq_disabled = true;
> + }
> +
> + if (rt5659->jack_detect_work_inited)
> + cancel_delayed_work_sync(&rt5659->jack_detect_work);
>
> regmap_write(rt5659->regmap, RT5659_RESET, 0);
> }
[Severity: High]
As reported by Sashiko on v4 [2], what happens if the I2C device is unbound
while no ASoC sound card is bound to the component, or if
devm_snd_soc_register_component() fails in rt5659_i2c_probe()?
In rt5659_i2c_probe(), when rt5659->pdata.jd_src is RT5659_JD3 or
RT5659_JD_HDA_HEADER, rt5659->jack_detect_work is initialized and hardware
interrupts are enabled via devm_request_threaded_irq() and
RT5659_GP1_PIN_IRQ before devm_snd_soc_register_component() is called. Once
devm_request_threaded_irq() succeeds, any interrupt on i2c->irq invokes
rt5659_irq() and queues rt5659->jack_detect_work onto
system_power_efficient_wq with a 250 ms delay.
Because disable_irq() and cancel_delayed_work_sync() are only placed in
rt5659_remove() (the snd_soc_component_driver.remove callback), and the ASoC
core in snd_soc_del_component_unlocked() -> snd_soc_component_remove() only
invokes component->driver->remove() when a card has probed the component,
unbinding the I2C device without a bound card (or failing in
devm_snd_soc_register_component()) never calls rt5659_remove(). Devres then
frees the IRQ, regmap, and struct rt5659_priv while rt5659->jack_detect_work
can still be pending, leading to a use-after-free when the worker runs.
Similarly, rt5659_i2c_shutdown() writes RT5659_RESET without disabling the
IRQ or cancelling rt5659->jack_detect_work. Should the work cancellation and
IRQ teardown also be tied to the I2C device lifecycle (for example via
devm_delayed_work_autocancel() or a devm action) and handled in
rt5659_i2c_shutdown()?
[1] https://lore.kernel.org/r/20261030170416.3561618-3-tristan@talencesecurity.com
[2] https://lore.kernel.org/r/20261030170416.3561618-3-tristan@talencesecurity.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030174837.1839402-1-tristan@talencesecurity.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal
2026-10-30 17:48 ` [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
@ 2026-10-30 18:04 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 18:04 UTC (permalink / raw)
To: Tristan Madani; +Cc: linux-sound
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: rt5668: uncancelled delayed work on I2C unbind in rt5668_remove()
- [High] ASoC: rt5668: stale jack and regcache state across rebind in rt5668_remove()
--
commit 17edcee2252c0f564a19636e40414b1293748590
Author: Tristan Madani <tristmd@gmail.com>
ASoC: rt5668: cancel delayed works on component removal
This commit disables the codec IRQ and cancels the jack_detect_work and
jd_check_work delayed works in rt5668_remove(), and re-enables the IRQ in
rt5668_probe() when a sound card rebinds.
> diff --git a/sound/soc/codecs/rt5668.c b/sound/soc/codecs/rt5668.c
> index cd6245d6b91af..29d70802a046a 100644
> --- a/sound/soc/codecs/rt5668.c
> +++ b/sound/soc/codecs/rt5668.c
[ ... ]
> @@ -2216,12 +2219,26 @@ static int rt5668_probe(struct snd_soc_component *component)
>
> rt5668->component = component;
>
> + if (rt5668->irq_disabled) {
> + enable_irq(to_i2c_client(component->dev)->irq);
> + rt5668->irq_disabled = false;
> + }
> +
> return 0;
> }
>
> static void rt5668_remove(struct snd_soc_component *component)
> {
> struct rt5668_priv *rt5668 = snd_soc_component_get_drvdata(component);
> + struct i2c_client *i2c = to_i2c_client(component->dev);
> +
> + if (rt5668->irq_acquired) {
> + disable_irq(i2c->irq);
> + rt5668->irq_disabled = true;
> + }
> +
> + cancel_delayed_work_sync(&rt5668->jack_detect_work);
> + cancel_delayed_work_sync(&rt5668->jd_check_work);
[Severity: High]
As reported by Sashiko on v2 [1], what happens if the I2C device is unbound
when no ASoC sound card is bound to the component, or if
devm_snd_soc_register_component() fails in rt5668_i2c_probe()?
In rt5668_i2c_probe(), struct rt5668_priv is allocated via devm_kzalloc(),
rt5668->jack_detect_work and rt5668->jd_check_work are initialized, and
i2c->irq is immediately requested and enabled via devm_request_threaded_irq()
before any sound card binds:
sound/soc/codecs/rt5668.c:rt5668_i2c_probe() {
...
INIT_DELAYED_WORK(&rt5668->jack_detect_work,
rt5668_jack_detect_handler);
INIT_DELAYED_WORK(&rt5668->jd_check_work,
rt5668_jd_check_handler);
mutex_init(&rt5668->calibrate_mutex);
if (i2c->irq) {
ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
rt5668_irq, IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
| IRQF_ONESHOT, "rt5668", rt5668);
if (ret)
dev_err(&i2c->dev, "Failed to request IRQ: %d\n", ret);
else
rt5668->irq_acquired = true;
}
return devm_snd_soc_register_component(&i2c->dev, &soc_component_dev_rt5668,
rt5668_dai, ARRAY_SIZE(rt5668_dai));
}
If i2c->irq fires before a sound card is bound or instantiated, rt5668_irq()
queues rt5668->jack_detect_work, and rt5668_jack_detect_handler() continuously
re-arms itself every 15 ms:
sound/soc/codecs/rt5668.c:rt5668_jack_detect_handler() {
...
if (!rt5668->component ||
!snd_soc_card_is_instantiated(rt5668->component->card)) {
/* card not yet ready, try later */
mod_delayed_work(system_power_efficient_wq,
&rt5668->jack_detect_work, msecs_to_jiffies(15));
return;
}
...
}
However, rt5668_remove() is the snd_soc_component_driver.remove callback, and
the ASoC core only invokes it when a card was bound to the component:
devm_component_release()
-> snd_soc_unregister_component_by_driver()
-> snd_soc_del_component_unlocked()
-> if (card) snd_soc_unbind_card(card)
-> soc_cleanup_card_resources()
-> snd_soc_dapm_shutdown() / snd_soc_component_remove()
Because rt5668_i2c_driver has no .remove callback and registers no devm action
to disable the IRQ and cancel the delayed works (and rt5668_i2c_shutdown()
also resets the codec without stopping them), won't unbinding the I2C device
without an instantiated card cause devres to free struct rt5668_priv while
rt5668->jack_detect_work remains pending and self-rearming?
>
> rt5668_reset(rt5668->regmap);
> }
[Severity: High]
As reported by Sashiko on v4 [2], since rt5668_probe() now calls enable_irq()
to support sound card unbind/rebind cycles, does rt5668_remove() leave the
regmap cache and jack state out of sync with the hardware after calling
rt5668_reset()?
In rt5668_remove(), rt5668_reset(rt5668->regmap) writes 0 to the volatile
RT5668_RESET register, resetting all codec registers in hardware to power-on
defaults without resetting or dirtying the REGCACHE_MAPLE regmap cache and
without clearing rt5668->hs_jack, rt5668->jack_type, or rt5668->component.
When a sound card subsequently binds again, can this cause the following
issues?
1. If the previous card's struct snd_soc_jack was freed when that card unbound
without clearing rt5668->hs_jack, rt5668->hs_jack is left dangling. When
rt5668_probe() immediately calls enable_irq(), if an interrupt triggers
rt5668_jack_detect_handler() before a new jack is registered (or if the new
card does not call snd_soc_component_set_jack()), snd_soc_jack_report() is
called with the dangling rt5668->hs_jack pointer.
2. When the newly bound card calls rt5668_set_jack_detect(), the
snd_soc_component_update_bits() and regmap_update_bits() calls on cached
non-volatile registers (RT5668_CBJ_CTRL_3, RT5668_SAR_IL_CMD_1,
RT5668_GPIO_CTRL_1, RT5668_RC_CLK_CTRL, RT5668_PWR_ANLG_2, and
RT5668_IRQ_CTRL_2) see those bits already set in the stale regmap cache
from the first bind and skip writing them to the reset hardware, leaving
the physical IRQ output pin and jack detection disabled in the codec.
3. If a headset was plugged in when the card unbound (rt5668->jack_type != 0),
rt5668_jack_detect_handler() on rebind sees rt5668->jack_type != 0 and
skips initial headset detection via rt5668_headset_detect():
sound/soc/codecs/rt5668.c:rt5668_jack_detect_handler() {
...
if (rt5668->jack_type == 0) {
/* jack was out, report jack type */
rt5668->jack_type =
rt5668_headset_detect(rt5668->component, 1);
} else {
/* jack is already in, report button event */
rt5668->jack_type = SND_JACK_HEADSET;
btn_type = rt5668_button_detect(rt5668->component);
...
}
[1] https://lore.kernel.org/r/sashiko-outbox-190014@kernel.org
[2] https://lore.kernel.org/r/20261030170416.3561618-2-tristan@talencesecurity.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030174837.1839402-1-tristan@talencesecurity.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-30 18:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-30 17:48 [PATCH v5 0/4] ASoC: cancel pending delayed works on codec removal Tristan Madani
2026-10-30 17:48 ` [PATCH v5 1/4] ASoC: rt5668: cancel delayed works on component removal Tristan Madani
2026-10-30 18:04 ` sashiko-bot
2026-10-30 17:48 ` [PATCH v5 2/4] ASoC: rt5659: cancel jack_detect_work " Tristan Madani
2026-10-30 18:02 ` sashiko-bot
2026-10-30 17:48 ` [PATCH v5 3/4] ASoC: twl6040: cancel headset jack work " Tristan Madani
2026-10-30 17:48 ` [PATCH v5 4/4] ASoC: wm8994: drain delayed works on remove and probe error Tristan Madani
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.