Re: [PATCH v3 24/34] dmaengine: txx9dmac: route callbacks via channel BH

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized per-channel BH work in shared controller IRQ mode causes DMA transfers to stall and triggers warnings.
- [High] Deferring descriptor scanning to a background work while immediately re-enabling the shared IRQ causes an interrupt storm.
--

--- Patch [24]: [PATCH v3 24/34] dmaengine: txx9dmac: route callbacks via channel BH ---
commit c425e27c746c8c326d7eb6597032d4dcfe3fa1da
Author: Allen Pais <[email protected]>
Subject: dmaengine: txx9dmac: route callbacks via channel BH

This commit replaces tasklets with per-channel BH work helpers for handling
descriptor completions in the txx9dmac driver. It updates the probe, remove,
and interrupt handlers to schedule the channel BH work.

> diff --git a/drivers/dma/txx9dmac.c b/drivers/dma/txx9dmac.c
> index 05622b68a9363..0547f645b51c8 100644
> --- a/drivers/dma/txx9dmac.c
> +++ b/drivers/dma/txx9dmac.c

[ ... ]

> @@ -659,7 +659,7 @@ static void txx9dmac_tasklet(struct tasklet_struct *t)
>  			spin_lock(&dc->lock);
>  			if (csr & (TXX9_DMA_CSR_ABCHC | TXX9_DMA_CSR_NCHNC |
>  				   TXX9_DMA_CSR_NTRNFC))
> -				txx9dmac_scan_descriptors(dc);
> +				dmaengine_schedule_bh(&dc->chan);
>  			spin_unlock(&dc->lock);
>  		}
>  	}

[Severity: High]
Does deferring the descriptor scanning to a background work while immediately
re-enabling the shared IRQ cause an interrupt storm?

Looking at the rest of txx9dmac_tasklet(), it calls enable_irq(irq)
synchronously at the end. Since the hardware interrupt condition is cleared
by the deferred workqueue (which runs after softirqs), will re-enabling
the IRQ here instantly trigger the interrupt again, leading to an infinite
loop that starves the workqueue?

[ ... ]

> @@ -1113,7 +1113,7 @@ static int __init txx9dmac_chan_probe(struct platform_device *pdev)
>  		irq = platform_get_irq(pdev, 0);
>  		if (irq < 0)
>  			return irq;
> -		tasklet_setup(&dc->tasklet, txx9dmac_chan_tasklet);
> +		dmaengine_init_bh(&dc->chan, txx9dmac_chan_tasklet);
>  		dc->irq = irq;
>  		err = devm_request_irq(&pdev->dev, dc->irq,
>  			txx9dmac_chan_interrupt, 0, dev_name(&pdev->dev), dc);

[Severity: High]
Is the per-channel BH work left uninitialized in shared controller IRQ mode?

If dc->ddev->irq >= 0 in txx9dmac_chan_probe(), the code falls into the else
block and dmaengine_init_bh() is skipped. When an interrupt occurs, the shared
tasklet (txx9dmac_tasklet) will unconditionally call dmaengine_schedule_bh()
for the channel. 

Will this trigger a WARN_ON(!chan->bh_work_initialized) and return false,
permanently stalling transfers for shared IRQ setups?

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