Re: [PATCH 2/9] dmaengine: dma-axi-dmac: Use bus width capability helpers

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] dma_dev->copy_align is incorrectly populated with a linear byte width rather than a logarithmic one, causing undefined behavior for bus widths >= 32 bytes.
--

--- Patch [2]: [PATCH 2/9] dmaengine: dma-axi-dmac: Use bus width capability helpers ---
commit 99138308c2fc808f6bee3d2ac55aef702ecbd98a
Author: Nuno Sá <nuno.sa@analog.com>

dmaengine: dma-axi-dmac: Use bus width capability helpers

This patch updates the driver to use the dma_set_src_bus_width() and
dma_set_dst_bus_width() helpers instead of open-coding the legacy BIT()
mask. This allows the driver to express bus widths of 32 bytes and above
while keeping the legacy u32 fields populated.

> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index d47ff27e1408f..26912330e3c83 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c

[ ... ]

> @@ -1262,8 +1262,13 @@ static int axi_dmac_probe(struct platform_device *pdev)
>  	dma_dev->device_terminate_all = axi_dmac_terminate_all;
>  	dma_dev->device_synchronize = axi_dmac_synchronize;
>  	dma_dev->dev = &pdev->dev;
> -	dma_dev->src_addr_widths = BIT(dmac->chan.src_width);
> -	dma_dev->dst_addr_widths = BIT(dmac->chan.dest_width);
> +	ret = dma_set_src_bus_width(dma_dev, dmac->chan.src_width);
> +	if (ret)
> +		return ret;
> +
> +	ret = dma_set_dst_bus_width(dma_dev, dmac->chan.dest_width);
> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but since this patch enables bus widths of 32
bytes and above, does it expose an undefined behavior in the generic DMA
framework?

Further down in axi_dmac_probe(), copy_align is assigned a linear byte width:

drivers/dma/dma-axi-dmac.c:axi_dmac_probe() {
    ...
    dma_dev->copy_align = (dmac->chan.address_align_mask + 1);
    ...
}

The copy_align field is expected to be a logarithmic value based on enum
dmaengine_alignment. When a bus width of 32 bytes or more is configured,
copy_align becomes 32 or greater.

Functions in the generic DMA framework use this value as a shift amount.
For example:

include/linux/dmaengine.h:dmaengine_check_align() {
    ...
    return !(((1 << align) - 1) & (off1 | off2 | len));
}

If align is 32 or greater, shifting a 32-bit integer by that amount causes
undefined behavior. Should copy_align be updated to use a logarithmic
value to avoid this issue now that larger bus widths are supported?

>  	dma_dev->directions = BIT(dmac->chan.direction);
>  	dma_dev->residue_granularity = DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
>  	dma_dev->max_sg_burst = 31; /* 31 SGs maximum in one burst */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-dmaengine-support-wider-dma-masks-v1-0-3732f1f9d9ca@analog.com?part=2