Re: [PATCH] dma: altera-msgdma: Replace memcpy with io32write in msgdma_copy_one

[Frank Li]

On Mon, May 18, 2026 at 11:47:20PM -0700, tze.yee.ng@xxxxxxxxxx wrote:
> From: Adrian Ng Ho Yin <adrianhoyin.ng@xxxxxxxxxx>
>
> The descriptor FIFO requires that all words of a descriptor are written
> in order, with the control word written last to flush it into the DMA
> engine. Using memcpy() is unsafe since it does not guarantee ordering of
> MMIO writes across all architectures.
>
> Replace memcpy() with an explicit iowrite32() loop for each 32-bit word
> (except the control word). The control word is still written separately,
> with write barriers, to ensure it is always the final word pushed into
> the FIFO.
>
> This makes the programming of descriptors fully deterministic and
> portable across different architectures.
>
> Signed-off-by: Adrian Ng Ho Yin <adrianhoyin.ng@xxxxxxxxxx>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@xxxxxxxxxx>
> ---
>  drivers/dma/altera-msgdma.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c
> index b46999c81df0..5816973d2c70 100644
> --- a/drivers/dma/altera-msgdma.c
> +++ b/drivers/dma/altera-msgdma.c
> @@ -495,6 +495,9 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
>  			    struct msgdma_sw_desc *desc)
>  {
>  	void __iomem *hw_desc = mdev->desc;
> +	const u32 *src = (const u32 *)&desc->hw_desc;
> +	unsigned int i, nwords = offsetof(struct msgdma_extended_desc, control) /
> +				 sizeof(u32);
>
>  	/*
>  	 * Check if the DESC FIFO it not full. If its full, we need to wait
> @@ -505,16 +508,16 @@ static void msgdma_copy_one(struct msgdma_device *mdev,
>  		mdelay(1);
>
>  	/*
> -	 * The descriptor needs to get copied into the descriptor FIFO
> -	 * of the DMA controller. The descriptor will get flushed to the
> -	 * FIFO, once the last word (control word) is written. Since we
> -	 * are not 100% sure that memcpy() writes all word in the "correct"
> -	 * order (address from low to high) on all architectures, we make
> -	 * sure this control word is written last by single coding it and
> -	 * adding some write-barriers here.
> +	 * The descriptor must be written into the descriptor FIFO of the DMA
> +	 * controller. The FIFO is flushed and the descriptor becomes valid once
> +	 * the last word (the control word) is written. To guarantee the ordering
> +	 * of MMIO writes across all architectures, we write each 32-bit word
> +	 * individually using iowrite32(), and handle the control word separately
> +	 * at the end. This ensures the control word is always written last and
> +	 * prevents memcpy() or the compiler from reordering accesses.
>  	 */
> -	memcpy((void __force *)hw_desc, &desc->hw_desc,
> -	       sizeof(desc->hw_desc) - sizeof(u32));
> +	for (i = 0; i < nwords; i++)
> +		iowrite32(src[i], hw_desc + i * sizeof(u32));

why not use memcpy_toio()?

Frank
>
>  	/* Write control word last to flush this descriptor into the FIFO */
>  	mdev->idle = false;
> --
> 2.43.7
>