[PATCH v2] ASoC: wm8962: Stop IRQ from requeuing mic_work on remove

[PATCH v2] ASoC: wm8962: Stop IRQ from requeuing mic_work on remove

[Myeonghun Pak]

wm8962_remove() cancels mic_work, but the IRQ that queues it stays
registered for the I2C device.  Devres frees the IRQ only after that
cancel, and card unbind never unbinds the I2C client.  A later MICD
or MICSCD event queues the work again.  The work calls
snd_soc_component_read() and snd_soc_jack_report().

FLL, FIFO, and thermal handling do not use the jack, so leave the IRQ
registered.  Queue mic_work only while the jack pointer is set.  On
remove, clear that reference, synchronize_irq(), then cancel the
work.

This issue was identified during our ongoing static-analysis research
while reviewing kernel code.

Fixes: 0512615db6db ("ASoC: wm8962: Convert interrupt handler to direct regmap usage")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Changes in v2:
- Do not disable the IRQ from wm8962_i2c_remove().  Card unbind calls
  wm8962_remove() while the I2C client stays bound, so that disable
  never runs there (Charles Keepax).
- Keep the devm IRQ on the I2C device.  FLL, FIFO, and thermal
  handling do not use the jack.  Clear the jack, synchronize_irq(),
  then cancel mic_work so the handler cannot queue it again.

Found by inspection; I do not have the hardware, so this is not runtime
tested.

 sound/soc/codecs/wm8962.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 8a9598161b35..d97289461e02 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -3115,7 +3115,7 @@ static void wm8962_mic_work(struct work_struct *work)
 
 	wm8962->mic_status = status;
 
-	snd_soc_jack_report(wm8962->jack, status,
+	snd_soc_jack_report(READ_ONCE(wm8962->jack), status,
 			    SND_JACK_MICROPHONE | SND_JACK_BTN_0);
 
 	snd_soc_component_update_bits(component, WM8962_MICINT_SOURCE_POL,
@@ -3203,9 +3203,14 @@ static irqreturn_t wm8962_irq(int irq, void *data)
 
 		pm_wakeup_event(dev, 300);
 
-		queue_delayed_work(system_power_efficient_wq,
-				   &wm8962->mic_work,
-				   msecs_to_jiffies(250));
+		/*
+		 * mic_work reports this jack.  Removal clears it before
+		 * synchronize_irq(), so do not queue once it is gone.
+		 */
+		if (READ_ONCE(wm8962->jack))
+			queue_delayed_work(system_power_efficient_wq,
+					   &wm8962->mic_work,
+					   msecs_to_jiffies(250));
 	}
 
 	pm_runtime_put(dev);
@@ -3232,7 +3237,7 @@ int wm8962_mic_detect(struct snd_soc_component *component, struct snd_soc_jack *
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
 	int irq_mask, enable;
 
-	wm8962->jack = jack;
+	WRITE_ONCE(wm8962->jack, jack);
 	if (jack) {
 		irq_mask = 0;
 		enable = WM8962_MICDET_ENA;
@@ -3247,7 +3252,7 @@ int wm8962_mic_detect(struct snd_soc_component *component, struct snd_soc_jack *
 			    WM8962_MICDET_ENA, enable);
 
 	/* Send an initial empty report */
-	snd_soc_jack_report(wm8962->jack, 0,
+	snd_soc_jack_report(jack, 0,
 			    SND_JACK_MICROPHONE | SND_JACK_BTN_0);
 
 	snd_soc_dapm_mutex_lock(dapm);
@@ -3594,6 +3599,15 @@ static void wm8962_remove(struct snd_soc_component *component)
 {
 	struct wm8962_priv *wm8962 = snd_soc_component_get_drvdata(component);
 
+	/*
+	 * The IRQ stays registered for the I2C device, including across
+	 * card unbind.  It is the only producer of mic_work.  Clear the
+	 * jack first so a new handler does not queue, wait out a handler
+	 * that already loaded the old pointer, then drain the work.
+	 */
+	WRITE_ONCE(wm8962->jack, NULL);
+	if (wm8962->irq)
+		synchronize_irq(wm8962->irq);
 	cancel_delayed_work_sync(&wm8962->mic_work);
 
 	wm8962_free_gpio(component);
base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
-- 
2.53.0

Re: [PATCH v2] ASoC: wm8962: Stop IRQ from requeuing mic_work on remove

[Charles Keepax]

On Tue, Sep 22, 2026 at 11:06:41PM -0400, Myeonghun Pak wrote:
> --- a/sound/soc/codecs/wm8962.c
> +++ b/sound/soc/codecs/wm8962.c
> @@ -3115,7 +3115,7 @@ static void wm8962_mic_work(struct work_struct *work)
>  
>  	wm8962->mic_status = status;
>  
> -	snd_soc_jack_report(wm8962->jack, status,
> +	snd_soc_jack_report(READ_ONCE(wm8962->jack), status,
>  			    SND_JACK_MICROPHONE | SND_JACK_BTN_0);
>  
>  	snd_soc_component_update_bits(component, WM8962_MICINT_SOURCE_POL,
> @@ -3203,9 +3203,14 @@ static irqreturn_t wm8962_irq(int irq, void *data)
>  
>  		pm_wakeup_event(dev, 300);
>  
> -		queue_delayed_work(system_power_efficient_wq,
> -				   &wm8962->mic_work,
> -				   msecs_to_jiffies(250));
> +		/*
> +		 * mic_work reports this jack.  Removal clears it before
> +		 * synchronize_irq(), so do not queue once it is gone.
> +		 */
> +		if (READ_ONCE(wm8962->jack))

This feels a bit like its duplicating the mask check above, I
think it would be nicer to ensure that works as intended than
adding a second layer of filtering.

> +			queue_delayed_work(system_power_efficient_wq,
> +					   &wm8962->mic_work,
> +					   msecs_to_jiffies(250));
>  	}
>  
>  	pm_runtime_put(dev);
> @@ -3232,7 +3237,7 @@ int wm8962_mic_detect(struct snd_soc_component *component, struct snd_soc_jack *
>  	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
>  	int irq_mask, enable;
>  
> -	wm8962->jack = jack;
> +	WRITE_ONCE(wm8962->jack, jack);
>  	if (jack) {
>  		irq_mask = 0;
>  		enable = WM8962_MICDET_ENA;
> @@ -3247,7 +3252,7 @@ int wm8962_mic_detect(struct snd_soc_component *component, struct snd_soc_jack *
>  			    WM8962_MICDET_ENA, enable);
>  
>  	/* Send an initial empty report */
> -	snd_soc_jack_report(wm8962->jack, 0,
> +	snd_soc_jack_report(jack, 0,
>  			    SND_JACK_MICROPHONE | SND_JACK_BTN_0);
>  
>  	snd_soc_dapm_mutex_lock(dapm);
> @@ -3594,6 +3599,15 @@ static void wm8962_remove(struct snd_soc_component *component)
>  {
>  	struct wm8962_priv *wm8962 = snd_soc_component_get_drvdata(component);
>  
> +	/*
> +	 * The IRQ stays registered for the I2C device, including across
> +	 * card unbind.  It is the only producer of mic_work.  Clear the
> +	 * jack first so a new handler does not queue, wait out a handler
> +	 * that already loaded the old pointer, then drain the work.
> +	 */
> +	WRITE_ONCE(wm8962->jack, NULL);

So probably calling wm8962_mic_detect() here. Although
granted that probably needs some small fixups as there are a
couple races, probably also makes sense to move the mic_work
sync in there too.

> +	if (wm8962->irq)
> +		synchronize_irq(wm8962->irq);
>  	cancel_delayed_work_sync(&wm8962->mic_work);
>  
>  	wm8962_free_gpio(component);
> base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5
> -- 
> 2.53.0

Thanks,
Charles