Re: [PATCH 2/9] dmaengine: dma-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] dma_dev->copy_align is incorrectly populated with a linear byte wi=
dth rather than a logarithmic one, causing undefined behavior for bus width=
s >=3D 32 bytes.
--

--- Patch [2]: [PATCH 2/9] dmaengine: dma-axi-dmac: Use bus width capabilit=
y helpers ---
commit 99138308c2fc808f6bee3d2ac55aef702ecbd98a
Author: Nuno S=C3=A1 <[email protected]>

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 =3D axi_dmac_terminate_all;
>  	dma_dev->device_synchronize =3D axi_dmac_synchronize;
>  	dma_dev->dev =3D &pdev->dev;
> -	dma_dev->src_addr_widths =3D BIT(dmac->chan.src_width);
> -	dma_dev->dst_addr_widths =3D BIT(dmac->chan.dest_width);
> +	ret =3D dma_set_src_bus_width(dma_dev, dmac->chan.src_width);
> +	if (ret)
> +		return ret;
> +
> +	ret =3D 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 widt=
h:

drivers/dma/dma-axi-dmac.c:axi_dmac_probe() {
    ...
    dma_dev->copy_align =3D (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 =3D BIT(dmac->chan.direction);
>  	dma_dev->residue_granularity =3D DMA_RESIDUE_GRANULARITY_DESCRIPTOR;
>  	dma_dev->max_sg_burst =3D 31; /* 31 SGs maximum in one burst */

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730-dmaengine-=
[email protected]?part=3D2