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

AMD General

Hi,

> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Wednesday, August 19, 2026 11:17 PM
> To: Neeli, Srinivas <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]
> Subject: 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?
>

The issue pointed out is pre-existing and not introduced by this patch. I will address all pre-existing issues in a separate patch series.

Thanks
Srinivas Neeli

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