Re: [PATCH v3 22/34] dmaengine: tegra20-apb: use channel BH helpers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The channel interrupt is requested before critical channel structures (spinlock, waitqueue, and bottom-half work) are initialized.
--

--- Patch [22]: [PATCH v3 22/34] dmaengine: tegra20-apb: use channel BH helpers ---
commit 4e26e9a252e11cfe82fed93039f39258debd48f7
Author: Allen Pais <allen.lkml@gmail.com>

dmaengine: tegra20-apb: use channel BH helpers

Switch the per-channel tasklet to dmaengine_*_bh so completion handling
runs through the shared per-channel deferral mechanism.

> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 640b8a218c9ad..45995b82a1330 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c

[ ... ]

> @@ -1511,7 +1511,7 @@ static int tegra_dma_probe(struct platform_device *pdev)
>  		tdc->id = i;
>  		tdc->slave_id = TEGRA_APBDMA_SLAVE_ID_INVALID;
>  
> -		tasklet_setup(&tdc->tasklet, tegra_dma_tasklet);
> +		dmaengine_init_bh(&tdc->dma_chan, tegra_dma_tasklet);

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition here
during initialization.

In tegra_dma_probe(), the interrupt is requested and enabled earlier in the
channel initialization loop before these critical structures are set up:

drivers/dma/tegra20-apb-dma.c:tegra_dma_probe() {
    ...
		ret = devm_request_irq(&pdev->dev, irq, tegra_dma_isr, 0,
				       tdc->name, tdc);
    ...
}

Could an early interrupt fire before dmaengine_init_bh(), spin_lock_init(),
and init_waitqueue_head() are called?

If the bootloader leaves the DMA controller with a pending interrupt, or a
spurious interrupt occurs immediately after devm_request_irq() enables the
IRQ line, the ISR might execute before these initializations.

Would this result in acquiring an uninitialized spinlock, triggering a WARN
when scheduling the uninitialized bottom-half work, or causing a kernel panic
by dereferencing a NULL tdc->isr_handler?

>  		spin_lock_init(&tdc->lock);
>  		init_waitqueue_head(&tdc->wq);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786384168.git.allen.lkml@gmail.com?part=22