Re: [PATCH v3 14/34] dmaengine: nbpfaxi: switch callbacks to dmaengine BH

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Shared IRQs are registered before the device clock is enabled, risking a bus fault or system crash.
--

--- Patch [14]: [PATCH v3 14/34] dmaengine: nbpfaxi: switch callbacks to dmaengine BH ---
commit 0e57a5ca2e574d87910252599a7cb1273a78c7e5
Author: Allen Pais <allen.lkml@gmail.com>

dmaengine: nbpfaxi: switch callbacks to dmaengine BH
    
Replace the per-channel tasklet with the shared dmaengine BH helper.
The handler continues to run in softirq context while dmaengine owns
the common scheduling and teardown mechanism.

> diff --git a/drivers/dma/nbpfaxi.c b/drivers/dma/nbpfaxi.c
> index 05d7321629cc8..7dd5fc55228d2 100644
> --- a/drivers/dma/nbpfaxi.c
> +++ b/drivers/dma/nbpfaxi.c
[ ... ]
> @@ -1272,7 +1271,7 @@ static int nbpf_chan_probe(struct nbpf_device *nbpf, int n)
>  
>  	snprintf(chan->name, sizeof(chan->name), "nbpf %d", n);
>  
> -	tasklet_setup(&chan->tasklet, nbpf_chan_tasklet);
> +	dmaengine_init_bh(&chan->dma_chan, nbpf_chan_tasklet);
>  	ret = devm_request_irq(dma_dev->dev, chan->irq,
>  			nbpf_chan_irq, IRQF_SHARED,
>  			chan->name, chan);

[Severity: High]
This is a pre-existing issue, but shared IRQs are registered here in
nbpf_chan_probe() before the device clock is enabled.

If another device sharing the IRQ line triggers an interrupt before
clk_prepare_enable(nbpf->clk) is called later in nbpf_probe(), or if a
spurious interrupt occurs, the kernel will invoke nbpf_chan_irq().

The handler calls nbpf_status_get() which reads from device registers.
Accessing memory-mapped registers of a device while its clock is disabled
can cause a synchronous external abort (bus fault) and crash the system.

Furthermore, on the error path of nbpf_probe(), clk_disable_unprepare()
is called manually while the devm-managed IRQs remain active until devres
cleanup runs, creating a second window for the same crash.

Does this initialization sequence need to be restructured so that IRQs are
registered only after the clock is enabled, and disabled before the clock
is turned off?

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