[PATCH] dmaengine: milbeaut-hdmac: Fix deadlock with the channel IRQ handler

[PATCH] dmaengine: milbeaut-hdmac: Fix deadlock with the channel IRQ handler

[Ginger Li]

milbeaut_hdmac_interrupt() is a hardirq handler and takes mc->vc.lock with a
plain spin_lock() for the whole time it runs.  interrupts are disabled locally
in that context, so the handler itself is fine.

milbeaut_hdmac_chan_config(), milbeaut_hdmac_chan_pause() and
milbeaut_hdmac_chan_resume() are called from process context and take the same
lock with a plain spin_lock() as well, so interrupts stay enabled while the
lock is held.

If a channel interrupt is delivered on the CPU that is inside one of those
critical sections, milbeaut_hdmac_interrupt() spins on a lock that the
interrupted code is holding and can never release, and the CPU hangs.

Take mc->vc.lock with spin_lock_irqsave() in those three functions, as
milbeaut_hdmac_issue_pending() and the other process context users of this
lock already do.

Fixes: 6c3214e698e4 ("dmaengine: milbeaut-hdmac: Add HDMAC driver for Milbeaut platforms")
Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
---
 drivers/dma/milbeaut-hdmac.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/milbeaut-hdmac.c b/drivers/dma/milbeaut-hdmac.c
--- a/drivers/dma/milbeaut-hdmac.c
+++ b/drivers/dma/milbeaut-hdmac.c
@@ -214,10 +214,11 @@ milbeaut_hdmac_chan_config(struct dma_chan *chan, stru
 {
 	struct virt_dma_chan *vc = to_virt_chan(chan);
 	struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
+	unsigned long flags;
 
-	spin_lock(&mc->vc.lock);
+	spin_lock_irqsave(&mc->vc.lock, flags);
 	mc->cfg = *cfg;
-	spin_unlock(&mc->vc.lock);
+	spin_unlock_irqrestore(&mc->vc.lock, flags);
 
 	return 0;
 }
@@ -226,13 +227,14 @@ static int milbeaut_hdmac_chan_pause(struct dma_chan *
 {
 	struct virt_dma_chan *vc = to_virt_chan(chan);
 	struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
+	unsigned long flags;
 	u32 val;
 
-	spin_lock(&mc->vc.lock);
+	spin_lock_irqsave(&mc->vc.lock, flags);
 	val = readl_relaxed(mc->reg_ch_base + MLB_HDMAC_DMACA);
 	val |= MLB_HDMAC_PB;
 	writel_relaxed(val, mc->reg_ch_base + MLB_HDMAC_DMACA);
-	spin_unlock(&mc->vc.lock);
+	spin_unlock_irqrestore(&mc->vc.lock, flags);
 
 	return 0;
 }
@@ -241,13 +243,14 @@ static int milbeaut_hdmac_chan_resume(struct dma_chan 
 {
 	struct virt_dma_chan *vc = to_virt_chan(chan);
 	struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
+	unsigned long flags;
 	u32 val;
 
-	spin_lock(&mc->vc.lock);
+	spin_lock_irqsave(&mc->vc.lock, flags);
 	val = readl_relaxed(mc->reg_ch_base + MLB_HDMAC_DMACA);
 	val &= ~MLB_HDMAC_PB;
 	writel_relaxed(val, mc->reg_ch_base + MLB_HDMAC_DMACA);
-	spin_unlock(&mc->vc.lock);
+	spin_unlock_irqrestore(&mc->vc.lock, flags);
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH] dmaengine: milbeaut-hdmac: Fix deadlock with the channel IRQ handler

[Frank Li]

On Wed, Sep 23, 2026 at 05:44:26PM +0800, Ginger Li wrote:
> [You don't often get email from ginger.jzllee@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> milbeaut_hdmac_interrupt() is a hardirq handler and takes mc->vc.lock with a
> plain spin_lock() for the whole time it runs.  interrupts are disabled locally
> in that context, so the handler itself is fine.
>
> milbeaut_hdmac_chan_config(), milbeaut_hdmac_chan_pause() and
> milbeaut_hdmac_chan_resume() are called from process context and take the same
> lock with a plain spin_lock() as well, so interrupts stay enabled while the
> lock is held.
>
> If a channel interrupt is delivered on the CPU that is inside one of those
> critical sections, milbeaut_hdmac_interrupt() spins on a lock that the
> interrupted code is holding and can never release, and the CPU hangs.
>
> Take mc->vc.lock with spin_lock_irqsave() in those three functions, as
> milbeaut_hdmac_issue_pending() and the other process context users of this
> lock already do.
>
> Fixes: 6c3214e698e4 ("dmaengine: milbeaut-hdmac: Add HDMAC driver for Milbeaut platforms")
> Signed-off-by: Ginger Li <ginger.jzllee@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/milbeaut-hdmac.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/milbeaut-hdmac.c b/drivers/dma/milbeaut-hdmac.c
> --- a/drivers/dma/milbeaut-hdmac.c
> +++ b/drivers/dma/milbeaut-hdmac.c
> @@ -214,10 +214,11 @@ milbeaut_hdmac_chan_config(struct dma_chan *chan, stru
>  {
>         struct virt_dma_chan *vc = to_virt_chan(chan);
>         struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
> +       unsigned long flags;
>
> -       spin_lock(&mc->vc.lock);
> +       spin_lock_irqsave(&mc->vc.lock, flags);
>         mc->cfg = *cfg;
> -       spin_unlock(&mc->vc.lock);
> +       spin_unlock_irqrestore(&mc->vc.lock, flags);
>
>         return 0;
>  }
> @@ -226,13 +227,14 @@ static int milbeaut_hdmac_chan_pause(struct dma_chan *
>  {
>         struct virt_dma_chan *vc = to_virt_chan(chan);
>         struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
> +       unsigned long flags;
>         u32 val;
>
> -       spin_lock(&mc->vc.lock);
> +       spin_lock_irqsave(&mc->vc.lock, flags);
>         val = readl_relaxed(mc->reg_ch_base + MLB_HDMAC_DMACA);
>         val |= MLB_HDMAC_PB;
>         writel_relaxed(val, mc->reg_ch_base + MLB_HDMAC_DMACA);
> -       spin_unlock(&mc->vc.lock);
> +       spin_unlock_irqrestore(&mc->vc.lock, flags);
>
>         return 0;
>  }
> @@ -241,13 +243,14 @@ static int milbeaut_hdmac_chan_resume(struct dma_chan
>  {
>         struct virt_dma_chan *vc = to_virt_chan(chan);
>         struct milbeaut_hdmac_chan *mc = to_milbeaut_hdmac_chan(vc);
> +       unsigned long flags;
>         u32 val;
>
> -       spin_lock(&mc->vc.lock);
> +       spin_lock_irqsave(&mc->vc.lock, flags);
>         val = readl_relaxed(mc->reg_ch_base + MLB_HDMAC_DMACA);
>         val &= ~MLB_HDMAC_PB;
>         writel_relaxed(val, mc->reg_ch_base + MLB_HDMAC_DMACA);
> -       spin_unlock(&mc->vc.lock);
> +       spin_unlock_irqrestore(&mc->vc.lock, flags);
>
>         return 0;
>  }
> --
> 2.43.0
>