Re: [PATCH v2 3/6] dmaengine: arm-dma350: Sync the slave command list before starting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dma: arm-dma350: API misuse passing cmds_bus instead of cmds_dma to DMA API
--

commit 2fca36adfda1fb6f57b81f4afd90229dd12fdb46
Author: Jelly Jia <Jelly.Jia@cixtech.com>

dmaengine: arm-dma350: Sync the slave command list before starting

This commit fixes a stalled transfer issue on non-coherent devices by
synchronizing the slave command list before starting the channel. It adds a
sync step to ensure written command words are visible to the hardware and
SWIOTLB bounce buffers are refreshed.

> diff --git a/drivers/dma/arm-dma350.c b/drivers/dma/arm-dma350.c
> index bbcf4071c3e75..e3fbfcce66e8e 100644
> --- a/drivers/dma/arm-dma350.c
> +++ b/drivers/dma/arm-dma350.c
> @@ -306,6 +306,21 @@ static int d350_alloc_cmds(struct dma_chan *dchan, struct d350_desc *desc,
>  	return 0;
>  }
>  
> +/*
> + * The command words are written after the command list has been mapped, so
> + * make them visible to the device before the descriptor is submitted. This
> + * also refreshes the bounce buffer when the mapping is bounced through the
> + * SWIOTLB. It is a no-op for a DMA coherent device.
> + */
> +static void d350_sync_cmds(struct device *dev, struct d350_desc *desc)
> +{
> +	if (!desc->cmds)
> +		return;
> +
> +	dma_sync_single_for_device(dev, desc->cmds_bus, desc->cmds_size,
> +				   DMA_TO_DEVICE);

[Severity: Low]
Is desc->cmds_bus the correct address to pass to the DMA API here?

The DMA API requires the dma_addr_t handle returned by dma_map_single()
for all synchronization operations. It looks like the d350_desc struct
explicitly defines desc->cmds_dma for this purpose, while desc->cmds_bus is
designated as the hardware-visible address.

While both fields might currently hold identical values, passing the hardware
address to the DMA API could violate the API semantics. If bus address
translation or IOMMU offsets are introduced in the future where the hardware
view diverges from the DMA API token, could this result in SWIOTLB sync
failures or memory corruption?

Should this use desc->cmds_dma instead?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009051846.1115962-1-Jelly.Jia@cixtech.com?part=3