Re: [PATCH v2 3/9] dmaengine: dw-axi-dmac: Use bus width capability helpers

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

Pre-existing issues:
- [High] The driver requests a shared IRQ before fully initializing the channel structures and enabling device clocks, risking a system crash and NULL pointer dereference if an interrupt fires early.
--

--- Patch [3]: [PATCH v2 3/9] dmaengine: dw-axi-dmac: Use bus width capability helpers ---
commit 716fd3c99096094837bfe18d429e50e20434ac33
Author: Nuno Sá <[email protected]>
Subject: dmaengine: dw-axi-dmac: Use bus width capability helpers

This commit updates the driver to advertise supported bus widths through
the dma_set_src_bus_widths and dma_set_dst_bus_widths helpers. It moves
away from assigning legacy u32 masks directly.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index eebed2474210d..0fc4aa537346d 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c

[ ... ]

> @@ -1565,8 +1565,15 @@ static int dw_probe(struct platform_device *pdev)
>  
>  	/* DMA capabilities */
>  	dw->dma.max_burst = hdata->axi_rw_burst_len;

[Severity: High]
This is a pre-existing issue in dw_probe(), but does the current
initialization sequence risk a bus fault or NULL pointer dereference?

Earlier in dw_probe(), the driver requests a shared IRQ before fully
initializing the channel structures and enabling device clocks:

dw_probe() {
    ...
    ret = axi_req_irqs(pdev, chip);
    ...
    for (i = 0; i < hdata->nr_channels; i++) {
        struct axi_dma_chan *chan = &dw->chan[i];
        chan->chan_regs = chip->regs + ...
        ...
    }
    ...
    ret = axi_dma_resume(chip);
}

If the IRQ is shared and fires immediately after the request,
dw_axi_dma_interrupt() is invoked while the hardware clocks are still
disabled. It accesses hardware registers via axi_dma_irq_disable() which
might cause a synchronous external abort.

If the system survives the unclocked access, dw_axi_dma_interrupt()
iterates over the channels and calls axi_chan_irq_read(), dereferencing
the uninitialized chan->chan_regs pointer:

dw_axi_dma_interrupt() {
    ...
    axi_dma_irq_disable(chip);
    for (i = 0; i < dw->hdata->nr_channels; i++) {
        chan = &dw->chan[i];
        status = axi_chan_irq_read(chan);
        ...
}

Would it be safer to request the IRQ only after the channel structures
are fully initialized and the clocks are enabled?

> -	dw->dma.src_addr_widths = AXI_DMA_BUSWIDTHS;
> -	dw->dma.dst_addr_widths = AXI_DMA_BUSWIDTHS;
> +	ret = dma_set_src_bus_widths(&dw->dma, axi_dma_buswidths,
> +				     ARRAY_SIZE(axi_dma_buswidths));
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_set_dst_bus_widths(&dw->dma, axi_dma_buswidths,
> +				     ARRAY_SIZE(axi_dma_buswidths));
> +	if (ret)
> +		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-dmaengine-support-wider-dma-masks-v2-0-1f7b798d035f@analog.com?part=3