Re: [PATCH V3 2/4] dmaengine: xilinx_dma: Move descriptors to done list based on completion bit

[Pandey, Radhey Shyam]

> In AXI MCDMA scatter-gather mode, xilinx_dma_complete_descriptor() walks

avoid "scatter-gather" mode.

> the channel's active_list and unconditionally moves every entry to the
> done_list. The MCDMA IOC interrupt handler invokes this function on
> every interrupt-on-completion, but with interrupt coalescing
> (IRQThreshold > 1) an IOC interrupt may fire after only a subset of the
> queued descriptors have actually been processed by the hardware. As a
> result, descriptors whose completion bit is not yet set in the BD status
> were being reported as completed to client drivers.
>
> Add a check for the descriptor completion bit before moving entries from
> the active list to the done list, using the appropriate direction-
> specific status field (s2mm_status for DMA_DEV_TO_MEM, mm2s_status for
> DMA_MEM_TO_DEV).
>
> The MCDMA completion check is intentionally not guarded by chan->has_sg,
> unlike the AXIDMA branch above. AXI MCDMA only operates in scatter-gather
> mode (has_sg is always true), so the guard would always pass and is
> omitted. The completion bit is therefore checked unconditionally.

Not need to talk about has_sg check and compare it with AXIDMA but if 
you still prefer make it precise.

Once addressed , feel free to add.

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx>
Thanks!

Just a note that this change aligns with changes that were done in past 
for AXIDMA in commit 7bcdaa658102 dmaengine: xilinx_dma: Freeup active 
list based on descriptor completion bit

> Fixes: 6ccd692bfb7f ("dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support")
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xxxxxxx>
> ---
> Changes in V3:
>   - Added Fixes tag.
>   - Expanded commit message to explain the interrupt coalescing scenario
>     and why the has_sg guard is omitted for MCDMA.
>   - Changed local variable from 'bool completed' to 'u32 status' for
>     cleaner status field access.
>   - Simplified completion check logic.
>
> Changes in V2:
>   - No change.
> ---
>   drivers/dma/xilinx/xilinx_dma.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index ff5b29a808e9..1b5b00f08c5f 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -1784,6 +1784,17 @@ static void xilinx_dma_complete_descriptor(struct xilinx_dma_chan *chan)
>   					      struct xilinx_axidma_tx_segment, node);
>   			if (!(seg->hw.status & XILINX_DMA_BD_COMP_MASK) && chan->has_sg)
>   				break;
> +		} else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
> +			struct xilinx_aximcdma_tx_segment *seg;
> +			u32 status;
> +
> +			seg = list_last_entry(&desc->segments,
> +					      struct xilinx_aximcdma_tx_segment,
> +					      node);
> +			status = (chan->direction == DMA_DEV_TO_MEM) ?
> +				seg->hw.s2mm_status : seg->hw.mm2s_status;
> +			if (!(status & XILINX_DMA_BD_COMP_MASK))
> +				break;
>   		}
>   		if (chan->has_sg && chan->xdev->dma_config->dmatype !=
>   		    XDMA_TYPE_VDMA)