Re: [PATCH v5 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA

["Pandey, Radhey Shyam" <radheys@amd.com>]

On 7/17/2026 2:38 PM, Srinivas Neeli wrote:
> From: Suraj Gupta <suraj.gupta2@amd.com>
> 
> xilinx_dma_get_metadata_ptr() returns the AXI DMA APP words from the SOP
> descriptor in both directions. This is wrong for RX, where the hardware
> writes the APP words into the EOF descriptor. It also leaves AXI MCDMA
> without metadata support.
> 
> Return the metadata from the SOP descriptor for TX and from the EOF
> descriptor for RX, matching where the hardware reads and writes the
> fields. For AXI DMA, expose the APP words (20 bytes). For AXI MCDMA,
> expose the control sideband, status, and APP fields (28 bytes). On TX
> the control sideband holds TID and TUSER configuration for the outgoing
> stream. On RX the sideband status holds the received TID, TDEST and TUSER
> from the incoming stream. The field layout differs between MM2S and S2MM,
> and the wider payload lets a consumer distinguish the two controllers.
> No in-tree consumer is affected.
> 
> Read xlnx,axistream-connected for AXI MCDMA. Attach metadata_ops in
> xilinx_mcdma_prep_slave_sg() when an AXI4-Stream interface is present,
> so MCDMA clients use the metadata API the same way as AXI DMA clients.
> 
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---

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

> Changes in V5:
>   - Take the metadata pointer from the SOP descriptor for TX and the EOF
>     descriptor for RX, matching where the hardware reads and writes the
>     fields (TX previously used the EOF descriptor).
>   - AXI DMA now exposes only the APP words (20 bytes) in both directions,
>     instead of the status word followed by APP (24 bytes).
>   - AXI MCDMA exposes the control sideband, status and APP fields
>     (28 bytes), with the sideband position differing between MM2S and S2MM.
>   - Reworked the kernel-doc index table and commit message accordingly.
> 
> Changes in V4:
>   - Restructured xilinx_dma_get_metadata_ptr(): AXIDMA is now the
>     fall-through path instead of a separate branch guarded by
>     WARN_ON_ONCE()/ERR_PTR().
>   - Rewrote the kernel-doc as an index table covering AXI DMA, MCDMA S2MM
>     and MCDMA MM2S, and documented that the pointer and payload length are
>     the same for both MCDMA directions.
>   - Added an inline comment explaining the union aliasing.
>   - Condensed the commit message.
> 
> Changes in V3:
>   - Renamed subject to include "AXI DMA and MCDMA" (was "AXI MCDMA" only).
>   - Complete rewrite of commit message and implementation.
>   - Metadata pointer now returns status field at index 0 instead of APP
>     fields, exposing status and sideband information to clients.
>   - Changed from list_first_entry to list_last_entry to return the EOF
>     descriptor where hardware writes status and APP fields.
>   - Added explicit handling for both AXIDMA and MCDMA types with proper
>     payload length calculation.
>   - Added WARN_ON_ONCE for unsupported DMA types.
>   - Removed the 'chan' field from struct xilinx_dma_tx_descriptor (was
>     added in V2) as it's no longer needed; channel is obtained from
>     tx->chan instead.
>   - Dropped V2 patches 4/5 (dt-bindings xlnx,include-stscntrl-strm) and
>     5/5 (xferred_bytes support) as the approach changed to use residue.
> 
> Changes in V2:
>   - Added support for MCDMA metadata handling alongside AXIDMA.
>   - Added 'chan' field to struct xilinx_dma_tx_descriptor.
> ---
>   drivers/dma/xilinx/xilinx_dma.c | 48 +++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 1b5b00f08c5f..6bf509d33e7c 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -651,17 +651,51 @@ static inline void xilinx_aximcdma_buf(struct xilinx_dma_chan *chan,
>    * @tx: async transaction descriptor
>    * @payload_len: metadata payload length
>    * @max_len: metadata max length
> - * Return: The app field pointer.
> + *
> + * The metadata lives in the SOP descriptor for TX and the EOF descriptor for RX.
> + * Field order depends on dmatype and direction:
> + *
> + *   AXI DMA:         [0..] app
> + *   AXI MCDMA (TX):  [0] ctrl_sideband, [1] status, [2..] app
> + *   AXI MCDMA (RX):  [0] status, [1] sideband, [2..] app
> + *
> + * Return: Pointer to the first metadata word.
>    */
>   static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
>   					 size_t *payload_len, size_t *max_len)
>   {
>   	struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
> +	struct xilinx_dma_chan *chan = to_xilinx_chan(tx->chan);
> +
> +	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
> +		struct xilinx_aximcdma_tx_segment *seg;
> +
> +		if (chan->direction == DMA_DEV_TO_MEM) {
> +			seg = list_last_entry(&desc->segments,
> +					      struct xilinx_aximcdma_tx_segment, node);
> +			*max_len = *payload_len = sizeof(seg->hw.s2mm_status) +
> +						  sizeof(seg->hw.s2mm_sideband_status) +
> +						  sizeof(seg->hw.app);
> +			return &seg->hw.s2mm_status;
> +		}
> +		seg = list_first_entry(&desc->segments,
> +				       struct xilinx_aximcdma_tx_segment, node);
> +		*max_len = *payload_len = sizeof(seg->hw.mm2s_ctrl_sideband) +
> +					  sizeof(seg->hw.mm2s_status) +
> +					  sizeof(seg->hw.app);
> +		return &seg->hw.mm2s_ctrl_sideband;
> +	}
> +
>   	struct xilinx_axidma_tx_segment *seg;
>   
> -	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> -	seg = list_first_entry(&desc->segments,
> -			       struct xilinx_axidma_tx_segment, node);
> +	if (chan->direction == DMA_DEV_TO_MEM)
> +		seg = list_last_entry(&desc->segments,
> +				      struct xilinx_axidma_tx_segment, node);
> +	else
> +		seg = list_first_entry(&desc->segments,
> +				       struct xilinx_axidma_tx_segment, node);
> +
> +	*max_len = *payload_len = sizeof(seg->hw.app);
>   	return seg->hw.app;
>   }
>   
> @@ -2639,6 +2673,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>   		segment->hw.control |= XILINX_MCDMA_BD_EOP;
>   	}
>   
> +	if (chan->xdev->has_axistream_connected)
> +		desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
> +
>   	return &desc->async_tx;
>   
>   error:
> @@ -3287,7 +3324,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
>   
>   	dma_set_max_seg_size(xdev->dev, xdev->max_buffer_len);
>   
> -	if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> +	if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA ||
> +	    xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
>   		xdev->has_axistream_connected =
>   			of_property_read_bool(node, "xlnx,axistream-connected");
>   	}