Re: [PATCH V6 1/4] dmaengine: xilinx_dma: Fix MCDMA descriptor fields based on DMA direction

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

Pre-existing issues:
- [High] xilinx_dma_tx_status ignores the requested cookie when reporting the residue.
--

--- Patch [1]: [PATCH V6 1/4] dmaengine: xilinx_dma: Fix MCDMA descriptor fields based on DMA direction ---
commit 5b7fc319de466b2688413e1d7e8a139d9c3d5d82
Author: Srinivas Neeli <[email protected]>

dmaengine: xilinx_dma: Fix MCDMA descriptor fields based on DMA direction

The MCDMA BD format differs between memory-to-device (MM2S) and
device-to-memory (S2MM) directions. This commit refactors the descriptor
structure to use unions with direction-specific field mappings and updates
the residue calculation logic to select the correct status field.

> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 404235c173538..965fb69a2a3ad 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
[ ... ]
> @@ -1013,9 +1023,11 @@ static u32 xilinx_dma_get_residue(struct xilinx_dma_chan *chan,
>  					   struct xilinx_aximcdma_tx_segment,
>  					   node);
>  			aximcdma_hw = &aximcdma_seg->hw;
> -			residue +=
> -				(aximcdma_hw->control & chan->xdev->max_buffer_len) -
> -				(aximcdma_hw->status & chan->xdev->max_buffer_len);
> +			residue += aximcdma_hw->control & chan->xdev->max_buffer_len;
> +			if (chan->direction == DMA_DEV_TO_MEM)
> +				residue -= aximcdma_hw->s2mm_status & chan->xdev->max_buffer_len;
> +			else
> +				residue -= aximcdma_hw->mm2s_status & chan->xdev->max_buffer_len;
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does xilinx_dma_tx_status()
ignore the requested cookie when reporting the residue?

Looking at xilinx_dma_tx_status() in drivers/dma/xilinx/xilinx_dma.c, it seems
to unconditionally compute and return the residue for the last entry in the
active list:

	desc = list_last_entry(&chan->active_list,
			       struct xilinx_dma_tx_descriptor, node);
	/*
	 * VDMA and simple mode do not support residue reporting, so the
	 * residue field will always be 0.
	 */
	if (chan->has_sg && chan->xdev->dma_config->dmatype != XDMA_TYPE_VDMA)
		residue = xilinx_dma_get_residue(chan, desc);

This means it will always report the residue of the most recently submitted
transaction, rather than the transaction the caller asked about via the cookie.

If a DMA client (e.g. a serial driver polling residue for partial Rx data)
queues multiple non-cyclic scatter-gather DMA transactions on a Xilinx DMA
channel, issues them, and then calls dmaengine_tx_status() to query the
residue of an earlier transaction, it would get the remaining byte count
of the last queued descriptor instead.

Should this iterate through the active and pending lists to look up the
descriptor associated with the requested cookie?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1