[PATCH] ASoC: mchp-spdiftx: Fix a deadlock with the interrupt handler

[PATCH] ASoC: mchp-spdiftx: Fix a deadlock with the interrupt handler

[Ginger Li]

mchp_spdiftx_interrupt() is a hardirq handler and takes ctrl->lock to
serialize the user data and channel status updates, while
mchp_spdiftx_trigger() takes the same lock with spin_lock() from process
context.

Interrupts stay enabled while the lock is held in mchp_spdiftx_trigger(), so
if an SPDIFTX interrupt is delivered on the CPU that is inside that critical
section, the hardirq handler spins on a lock that the interrupted code is
holding and can never release.

Take ctrl->lock with spin_lock_irqsave() in mchp_spdiftx_trigger(), like the
other process context users of this lock in the driver already do.

Fixes: 06ca24e98e6b ("ASoC: mchp-spdiftx: add driver for S/PDIF TX Controller")
Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
---
 sound/soc/atmel/mchp-spdiftx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c
--- a/sound/soc/atmel/mchp-spdiftx.c
+++ b/sound/soc/atmel/mchp-spdiftx.c
@@ -306,10 +306,11 @@ static int mchp_spdiftx_trigger(struct snd_pcm_substre
 {
 	struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai);
 	struct mchp_spdiftx_mixer_control *ctrl = &dev->control;
+	unsigned long flags;
 	int ret;
 
 	/* do not start/stop while channel status or user data is updated */
-	spin_lock(&ctrl->lock);
+	spin_lock_irqsave(&ctrl->lock, flags);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_START:
@@ -335,7 +336,7 @@ static int mchp_spdiftx_trigger(struct snd_pcm_substre
 	default:
 		ret = -EINVAL;
 	}
-	spin_unlock(&ctrl->lock);
+	spin_unlock_irqrestore(&ctrl->lock, flags);
 	if (ret)
 		dev_err(dev->dev, "unable to start/stop TX: %d\n", ret);
 
-- 
2.43.0

Re: [PATCH] ASoC: mchp-spdiftx: Fix a deadlock with the interrupt handler

[Mark Brown]

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

On Wed, Sep 23, 2026 at 04:44:01PM +0800, Ginger Li wrote:
> mchp_spdiftx_interrupt() is a hardirq handler and takes ctrl->lock to
> serialize the user data and channel status updates, while
> mchp_spdiftx_trigger() takes the same lock with spin_lock() from process
> context.

> Interrupts stay enabled while the lock is held in mchp_spdiftx_trigger(), so
> if an SPDIFTX interrupt is delivered on the CPU that is inside that critical
> section, the hardirq handler spins on a lock that the interrupted code is
> holding and can never release.

Are you sure, interrupts should be disabled during trigger...

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