Re: [PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Checking `ret <= 0` from `pm_runtime_get_if_active` unconditionally returns IRQ_NONE when Runtime PM is disabled, causing a deterministic interrupt storm.

Pre-existing issues:
- [High] The interrupt handler unconditionally returns IRQ_HANDLED even if no interrupt status bits were found, defeating spurious interrupt detection and causing CPU lockups.
--

--- Patch [2]: [PATCH v2 02/13] dmaengine: ste_dma40: Check runtime PM in IRQ ---
commit a32adbd5256a0eee9c50c9b0d29d46183f86ef06
Author: Linus Walleij <[email protected]>

dmaengine: ste_dma40: Check runtime PM in IRQ
    
d40_handle_interrupt() reads DMA40 interrupt registers unconditionally. A
spurious interrupt can arrive while the device is runtime suspended, after
dma40_runtime_suspend() has disabled the GCC clock.

Reported-by: [email protected]
Closes: https://lore.kernel.org/dmaengine/[email protected]/
Assisted-by: Codex:gpt-5-5
Signed-off-by: Linus Walleij <[email protected]>

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index facc1bcf42a03..dd449d4285efb 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c
> @@ -1674,6 +1674,11 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
>  	u32 *regs = base->regs_interrupt;
>  	struct d40_interrupt_lookup *il = base->gen_dmac.il;
>  	u32 il_size = base->gen_dmac.il_size;
> +	int ret;
> +
> +	ret = pm_runtime_get_if_active(base->dev);
> +	if (ret <= 0)
> +		return IRQ_NONE;

[Severity: High]
If the kernel is compiled with CONFIG_PM=n, pm_runtime_get_if_active() returns
-EINVAL. Doesn't checking ret <= 0 mean that valid interrupts will be
rejected with IRQ_NONE when Runtime PM is disabled?

This would leave the hardware interrupt line asserted and cause a
deterministic interrupt storm until the kernel's spurious interrupt detector
disables the IRQ line.

[ ... ]

> @@ -1722,6 +1727,8 @@ static irqreturn_t d40_handle_interrupt(int irq, void *data)
>  
>  	spin_unlock(&base->interrupt_lock);
>  
> +	pm_runtime_put_autosuspend(base->dev);
> +
>  	return IRQ_HANDLED;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but the interrupt handler
unconditionally returns IRQ_HANDLED even if no interrupt status bits were
found earlier in the function:

	/* No more set bits found? */
	if (chan == BITS_PER_LONG * il_size)
		break;

If a hardware glitch asserts the DMA interrupt line without setting any bits,
the loop breaks without clearing any interrupts. By unconditionally returning
IRQ_HANDLED here at the end of the function, does this prevent the kernel's
spurious interrupt detector from counting the unhandled interrupt and
gracefully disabling the faulty IRQ line?

This could cause the CPU to enter an infinite loop of servicing the
unacknowledged hardware interrupt.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2