[PATCH] ASoC: codecs: wcd937x: use devm_free_irq() for watchdog IRQs

[PATCH] ASoC: codecs: wcd937x: use devm_free_irq() for watchdog IRQs

[Runyu Xiao]

The watchdog IRQs are requested with devm_request_threaded_irq() from the
codec component probe, but the remove callback frees them with free_irq().
The devres entries remain in place. Component teardown later tries to free
the same IRQs again.

The IRQs must be released before the SoundWire components are unbound. Use
devm_free_irq() to remove each IRQ and its devres entry at that point. Fail
component probe if an IRQ request fails, and unwind only IRQs that were
successfully requested.

Fixes: 9be3ec196da4 ("ASoC: codecs: wcd937x: add wcd937x codec driver")
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 sound/soc/codecs/wcd937x.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 0dd05604f5b88..ec902c81d5a4e 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -2540,19 +2540,19 @@ static int wcd937x_soc_codec_probe(struct snd_soc_component *component)
 					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
 					"HPHR PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request HPHR watchdog interrupt (%d)\n", ret);
+		goto err_free_clsh;
 
 	ret = devm_request_threaded_irq(dev, wcd937x->hphl_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
 					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
 					"HPHL PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request HPHL watchdog interrupt (%d)\n", ret);
+		goto err_free_hphr_irq;
 
 	ret = devm_request_threaded_irq(dev, wcd937x->aux_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
 					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
 					"AUX PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request Aux watchdog interrupt (%d)\n", ret);
+		goto err_free_hphl_irq;
 
 	/* Disable watchdog interrupt for HPH and AUX */
 	disable_irq_nosync(wcd937x->hphr_pdm_wd_int);
@@ -2564,24 +2564,34 @@ static int wcd937x_soc_codec_probe(struct snd_soc_component *component)
 						ARRAY_SIZE(wcd9375_dapm_widgets));
 		if (ret < 0) {
 			dev_err(component->dev, "Failed to add snd_ctls\n");
-			wcd_clsh_ctrl_free(wcd937x->clsh_info);
-			return ret;
+			goto err_free_irqs;
 		}
 
 		ret = snd_soc_dapm_add_routes(dapm, wcd9375_audio_map,
 					      ARRAY_SIZE(wcd9375_audio_map));
 		if (ret < 0) {
 			dev_err(component->dev, "Failed to add routes\n");
-			wcd_clsh_ctrl_free(wcd937x->clsh_info);
-			return ret;
+			goto err_free_irqs;
 		}
 	}
 
 	ret = wcd937x_mbhc_init(component);
-	if (ret)
+	if (ret) {
 		dev_err(component->dev, "mbhc initialization failed\n");
+		goto err_free_irqs;
+	}
 
 	return ret;
+
+err_free_irqs:
+	devm_free_irq(dev, wcd937x->aux_pdm_wd_int, wcd937x);
+err_free_hphl_irq:
+	devm_free_irq(dev, wcd937x->hphl_pdm_wd_int, wcd937x);
+err_free_hphr_irq:
+	devm_free_irq(dev, wcd937x->hphr_pdm_wd_int, wcd937x);
+err_free_clsh:
+	wcd_clsh_ctrl_free(wcd937x->clsh_info);
+	return ret;
 }
 
 static void wcd937x_soc_codec_remove(struct snd_soc_component *component)
@@ -2589,9 +2599,9 @@ static void wcd937x_soc_codec_remove(struct snd_soc_component *component)
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
 
 	wcd937x_mbhc_deinit(component);
-	free_irq(wcd937x->aux_pdm_wd_int, wcd937x);
-	free_irq(wcd937x->hphl_pdm_wd_int, wcd937x);
-	free_irq(wcd937x->hphr_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->aux_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->hphl_pdm_wd_int, wcd937x);
+	devm_free_irq(component->dev, wcd937x->hphr_pdm_wd_int, wcd937x);
 
 	wcd_clsh_ctrl_free(wcd937x->clsh_info);
 }
-- 
2.34.1

Re: [PATCH] ASoC: codecs: wcd937x: use devm_free_irq() for watchdog IRQs

[Mark Brown]

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

On Tue, Sep 29, 2026 at 05:42:23PM +0800, Runyu Xiao wrote:
> The watchdog IRQs are requested with devm_request_threaded_irq() from the
> codec component probe, but the remove callback frees them with free_irq().
> The devres entries remain in place. Component teardown later tries to free
> the same IRQs again.

Surely the obvious thing here is that we shouldn't be using devm for the
interrupts in the first place, and/or should be moving to the driver
probe and not having to manually free them in the first place?  This
isn't the right fix.

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

[PATCH v2] ASoC: codecs: wcd937x: request watchdog IRQs without devm

[Runyu Xiao]

The watchdog IRQs are requested from the codec component probe and released
from its remove callback. Use the same non-devm lifetime for both
operations.

Use request_threaded_irq() and keep free_irq() in the component remove
callback. Return errors from each request and unwind only IRQs that were
successfully requested, including failures later in component probe.

Fixes: 9be3ec196da4 ("ASoC: codecs: wcd937x: add wcd937x codec driver")
Assisted-by: LLM
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
v2:
- Use request_threaded_irq() and free_irq() as suggested by Mark Brown.
- Keep error unwinding limited to IRQs that were successfully requested.

 sound/soc/codecs/wcd937x.c | 44 +++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 0dd05604f5b88..7ffbb445f8ac4 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -2536,23 +2536,23 @@ static int wcd937x_soc_codec_probe(struct snd_soc_component *component)
 						      WCD937X_IRQ_AUX_PDM_WD_INT);
 
 	/* Request for watchdog interrupt */
-	ret = devm_request_threaded_irq(dev, wcd937x->hphr_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
-					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
-					"HPHR PDM WDOG INT", wcd937x);
+	ret = request_threaded_irq(wcd937x->hphr_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
+				   IRQF_ONESHOT | IRQF_TRIGGER_RISING,
+				   "HPHR PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request HPHR watchdog interrupt (%d)\n", ret);
+		goto err_free_clsh;
 
-	ret = devm_request_threaded_irq(dev, wcd937x->hphl_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
-					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
-					"HPHL PDM WDOG INT", wcd937x);
+	ret = request_threaded_irq(wcd937x->hphl_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
+				   IRQF_ONESHOT | IRQF_TRIGGER_RISING,
+				   "HPHL PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request HPHL watchdog interrupt (%d)\n", ret);
+		goto err_free_hphr_irq;
 
-	ret = devm_request_threaded_irq(dev, wcd937x->aux_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
-					IRQF_ONESHOT | IRQF_TRIGGER_RISING,
-					"AUX PDM WDOG INT", wcd937x);
+	ret = request_threaded_irq(wcd937x->aux_pdm_wd_int, NULL, wcd937x_wd_handle_irq,
+				   IRQF_ONESHOT | IRQF_TRIGGER_RISING,
+				   "AUX PDM WDOG INT", wcd937x);
 	if (ret)
-		dev_err(dev, "Failed to request Aux watchdog interrupt (%d)\n", ret);
+		goto err_free_hphl_irq;
 
 	/* Disable watchdog interrupt for HPH and AUX */
 	disable_irq_nosync(wcd937x->hphr_pdm_wd_int);
@@ -2564,23 +2564,33 @@ static int wcd937x_soc_codec_probe(struct snd_soc_component *component)
 						ARRAY_SIZE(wcd9375_dapm_widgets));
 		if (ret < 0) {
 			dev_err(component->dev, "Failed to add snd_ctls\n");
-			wcd_clsh_ctrl_free(wcd937x->clsh_info);
-			return ret;
+			goto err_free_irqs;
 		}
 
 		ret = snd_soc_dapm_add_routes(dapm, wcd9375_audio_map,
 					      ARRAY_SIZE(wcd9375_audio_map));
 		if (ret < 0) {
 			dev_err(component->dev, "Failed to add routes\n");
-			wcd_clsh_ctrl_free(wcd937x->clsh_info);
-			return ret;
+			goto err_free_irqs;
 		}
 	}
 
 	ret = wcd937x_mbhc_init(component);
-	if (ret)
+	if (ret) {
 		dev_err(component->dev, "mbhc initialization failed\n");
+		goto err_free_irqs;
+	}
+
+	return ret;
 
+err_free_irqs:
+	free_irq(wcd937x->aux_pdm_wd_int, wcd937x);
+err_free_hphl_irq:
+	free_irq(wcd937x->hphl_pdm_wd_int, wcd937x);
+err_free_hphr_irq:
+	free_irq(wcd937x->hphr_pdm_wd_int, wcd937x);
+err_free_clsh:
+	wcd_clsh_ctrl_free(wcd937x->clsh_info);
 	return ret;
 }
 
-- 
2.34.1