[PATCH] ASoC: fsl_xcvr: free IRQ before canceling reset work

[PATCH] ASoC: fsl_xcvr: free IRQ before canceling reset work

[Myeonghun Pak]

irq0_isr() schedules work_rst on a preamble error, and reset_rx_work()
touches regmap.  remove() cancels that work while the IRQ is still
registered, so the handler can queue it again during teardown.  The IRQ
is also requested before INIT_WORK() and spin_lock_init().  Probe
failure does not call remove(), so freeing the IRQ leaves queued work
running, and an earlier interrupt schedules uninitialized work.

Initialize the lock and devm_work_autocancel() before the IRQ.  Failed
probe then frees the IRQ and cancels the work.  Unbind frees the IRQ
before cancel_work_sync() and pm_runtime_disable().

Fixes: 1e5d0f106164 ("ASoC: fsl_xcvr: reset RX dpath after wrong preamble")
Cc: stable@vger.kernel.org # 6.13+
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>
---
 sound/soc/fsl/fsl_xcvr.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index 9828272..29929df 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -3,6 +3,7 @@
 
 #include <linux/bitrev.h>
 #include <linux/clk.h>
+#include <linux/devm-helpers.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -57,6 +58,7 @@ struct fsl_xcvr {
 	struct snd_aes_iec958 tx_iec958;
 	u8 cap_ds[FSL_XCVR_CAPDS_SIZE];
 	struct work_struct work_rst;
+	int irq;
 	spinlock_t lock; /* Protect hw_reset and trigger */
 	struct snd_pcm_hw_constraint_list spdif_constr_rates;
 	u32 spdif_constr_rates_list[SPDIF_NUM_RATES];
@@ -1617,7 +1619,7 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
 	struct fsl_xcvr *xcvr;
 	struct resource *rx_res, *tx_res;
 	void __iomem *regs;
-	int ret, irq;
+	int ret;
 
 	xcvr = devm_kzalloc(dev, sizeof(*xcvr), GFP_KERNEL);
 	if (!xcvr)
@@ -1705,12 +1707,22 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(xcvr->reset),
 				     "failed to get XCVR reset control\n");
 
+	/*
+	 * irq0_isr() schedules work_rst. Prepare the work and its lock
+	 * before the IRQ, and register the cancel action first so a failed
+	 * probe frees the IRQ and then cancels any queued work.
+	 */
+	spin_lock_init(&xcvr->lock);
+	ret = devm_work_autocancel(dev, &xcvr->work_rst, reset_rx_work);
+	if (ret)
+		return ret;
+
 	/* get IRQs */
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	xcvr->irq = platform_get_irq(pdev, 0);
+	if (xcvr->irq < 0)
+		return xcvr->irq;
 
-	ret = devm_request_irq(dev, irq, irq0_isr, 0, pdev->name, xcvr);
+	ret = devm_request_irq(dev, xcvr->irq, irq0_isr, 0, pdev->name, xcvr);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to claim IRQ0\n");
 
@@ -1751,8 +1763,6 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
 			fsl_xcvr_comp.name);
 	}
 
-	INIT_WORK(&xcvr->work_rst, reset_rx_work);
-	spin_lock_init(&xcvr->lock);
 	return ret;
 }
 
@@ -1760,6 +1770,8 @@ static void fsl_xcvr_remove(struct platform_device *pdev)
 {
 	struct fsl_xcvr *xcvr = dev_get_drvdata(&pdev->dev);
 
+	/* Free the IRQ first so irq0_isr() cannot requeue work_rst. */
+	devm_free_irq(&pdev->dev, xcvr->irq, xcvr);
 	cancel_work_sync(&xcvr->work_rst);
 	pm_runtime_disable(&pdev->dev);
 }

Re: [PATCH] ASoC: fsl_xcvr: free IRQ before canceling reset work

[Shengjiu Wang]

On Wed, Sep 23, 2026 at 1:40 PM Myeonghun Pak <mhun512@gmail.com> wrote:
>
> irq0_isr() schedules work_rst on a preamble error, and reset_rx_work()
> touches regmap.  remove() cancels that work while the IRQ is still
> registered, so the handler can queue it again during teardown.  The IRQ
> is also requested before INIT_WORK() and spin_lock_init().  Probe
> failure does not call remove(), so freeing the IRQ leaves queued work
> running, and an earlier interrupt schedules uninitialized work.
>
> Initialize the lock and devm_work_autocancel() before the IRQ.  Failed
> probe then frees the IRQ and cancels the work.  Unbind frees the IRQ
> before cancel_work_sync() and pm_runtime_disable().
>
> Fixes: 1e5d0f106164 ("ASoC: fsl_xcvr: reset RX dpath after wrong preamble")
> Cc: stable@vger.kernel.org # 6.13+
> 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>

Reviewed-by: Shengjiu Wang <shengjiu.wang@gmail.com>

Best regards
Shengjiu Wang
> ---
>  sound/soc/fsl/fsl_xcvr.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
> index 9828272..29929df 100644
> --- a/sound/soc/fsl/fsl_xcvr.c
> +++ b/sound/soc/fsl/fsl_xcvr.c
> @@ -3,6 +3,7 @@
>
>  #include <linux/bitrev.h>
>  #include <linux/clk.h>
> +#include <linux/devm-helpers.h>
>  #include <linux/firmware.h>
>  #include <linux/interrupt.h>
>  #include <linux/module.h>
> @@ -57,6 +58,7 @@ struct fsl_xcvr {
>         struct snd_aes_iec958 tx_iec958;
>         u8 cap_ds[FSL_XCVR_CAPDS_SIZE];
>         struct work_struct work_rst;
> +       int irq;
>         spinlock_t lock; /* Protect hw_reset and trigger */
>         struct snd_pcm_hw_constraint_list spdif_constr_rates;
>         u32 spdif_constr_rates_list[SPDIF_NUM_RATES];
> @@ -1617,7 +1619,7 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
>         struct fsl_xcvr *xcvr;
>         struct resource *rx_res, *tx_res;
>         void __iomem *regs;
> -       int ret, irq;
> +       int ret;
>
>         xcvr = devm_kzalloc(dev, sizeof(*xcvr), GFP_KERNEL);
>         if (!xcvr)
> @@ -1705,12 +1707,22 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
>                 return dev_err_probe(dev, PTR_ERR(xcvr->reset),
>                                      "failed to get XCVR reset control\n");
>
> +       /*
> +        * irq0_isr() schedules work_rst. Prepare the work and its lock
> +        * before the IRQ, and register the cancel action first so a failed
> +        * probe frees the IRQ and then cancels any queued work.
> +        */
> +       spin_lock_init(&xcvr->lock);
> +       ret = devm_work_autocancel(dev, &xcvr->work_rst, reset_rx_work);
> +       if (ret)
> +               return ret;
> +
>         /* get IRQs */
> -       irq = platform_get_irq(pdev, 0);
> -       if (irq < 0)
> -               return irq;
> +       xcvr->irq = platform_get_irq(pdev, 0);
> +       if (xcvr->irq < 0)
> +               return xcvr->irq;
>
> -       ret = devm_request_irq(dev, irq, irq0_isr, 0, pdev->name, xcvr);
> +       ret = devm_request_irq(dev, xcvr->irq, irq0_isr, 0, pdev->name, xcvr);
>         if (ret)
>                 return dev_err_probe(dev, ret, "failed to claim IRQ0\n");
>
> @@ -1751,8 +1763,6 @@ static int fsl_xcvr_probe(struct platform_device *pdev)
>                         fsl_xcvr_comp.name);
>         }
>
> -       INIT_WORK(&xcvr->work_rst, reset_rx_work);
> -       spin_lock_init(&xcvr->lock);
>         return ret;
>  }
>
> @@ -1760,6 +1770,8 @@ static void fsl_xcvr_remove(struct platform_device *pdev)
>  {
>         struct fsl_xcvr *xcvr = dev_get_drvdata(&pdev->dev);
>
> +       /* Free the IRQ first so irq0_isr() cannot requeue work_rst. */
> +       devm_free_irq(&pdev->dev, xcvr->irq, xcvr);
>         cancel_work_sync(&xcvr->work_rst);
>         pm_runtime_disable(&pdev->dev);
>  }