[PATCH V3 0/4] dmaengine: xilinx_dma: MCDMA descriptor and metadata handling improvements

[PATCH V3 0/4] dmaengine: xilinx_dma: MCDMA descriptor and metadata handling improvements

[Srinivas Neeli]

This series improves the Xilinx AXI DMA and MCDMA driver's descriptor
handling and metadata reporting. It fixes direction-specific descriptor
field usage, ensures completion is based on the hardware completion bit
(important with interrupt coalescing), and extends metadata handling to
expose status and sideband fields alongside APP fields.

The axienet driver is updated to derive RX frame length from the standard
dmaengine residue mechanism rather than descriptor APP fields, making it
work on designs where the AXI4-Stream status/control interface is not
present.

Changes in V3:
 - Patch 1: Renamed subject, added static_assert for descriptor size,
   refactored residue calculation for clarity.
 - Patch 2: Added Fixes tag, expanded commit message explaining interrupt
   coalescing scenario, simplified completion check logic.
 - Patch 3: New patch - axienet now uses result->residue for RX length
   instead of APP metadata, removing dependency on status/control stream.
 - Patch 4: Complete rewrite - metadata pointer now starts at status field
   (index 0) exposing status/sideband to clients; uses EOF descriptor;
   removed 'chan' field from descriptor struct.
 - Dropped V2 patches 4/5 (dt-bindings) and 5/5 (xferred_bytes) as the
   approach changed to use standard residue mechanism.

Changes in V2:
 - Rebased on the AXI DMA binding YAML conversion.
 - Added xlnx,include-stscntrl-strm in the YAML binding.
 - Clarified cover letter to reflect metadata behavior with and without
   APP fields.
https://lore.kernel.org/all/20260309033444.3472359-1-abin.joseph@amd.com/

Srinivas Neeli (3):
  dmaengine: xilinx_dma: Fix MCDMA descriptor fields based on DMA
    direction
  dmaengine: xilinx_dma: Move descriptors to done list based on
    completion bit
  net: xilinx: axienet: Derive RX frame length from DMA residue

Suraj Gupta (1):
  dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA

 drivers/dma/xilinx/xilinx_dma.c               | 85 ++++++++++++++++---
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 14 ++-
 2 files changed, 76 insertions(+), 23 deletions(-)

--
2.25.1

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

[Srinivas Neeli]

The MCDMA BD format differs between memory-to-device (MM2S) and
device-to-memory (S2MM) directions, but the driver was using generic
'status' and 'sideband_status' fields for both. This led to incorrect
residue calculations when the hardware updates direction-specific fields.

Refactor the descriptor structure to use unions with direction-specific
field mappings, and update the residue calculation logic to select the
correct status field based on DMA direction.

This matches the hardware descriptor layout and fixes incorrect
residue reporting.

Fixes: 6ccd692bfb7f ("dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support")
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
 - Renamed subject from "for MM2S vs S2MM" to "based on DMA direction".
 - Reworded commit message for clarity.
 - Added XILINX_MCDMA_BD_HW_SIZE macro and static_assert to verify
   descriptor size at compile time.
 - Refactored residue calculation to separate addition and subtraction
   operations for better readability.

Changes in V2:
 - No change.
---
 drivers/dma/xilinx/xilinx_dma.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 98b41b8f8915..ff5b29a808e9 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -223,6 +223,7 @@
 #define XILINX_MCDMA_IRQ_ERR_MASK		BIT(7)
 #define XILINX_MCDMA_BD_EOP			BIT(30)
 #define XILINX_MCDMA_BD_SOP			BIT(31)
+#define XILINX_MCDMA_BD_HW_SIZE			64
 
 /**
  * struct xilinx_vdma_desc_hw - Hardware Descriptor
@@ -277,8 +278,10 @@ struct xilinx_axidma_desc_hw {
  * @buf_addr_msb: MSB of Buffer address @0x0C
  * @rsvd: Reserved field @0x10
  * @control: Control Information field @0x14
- * @status: Status field @0x18
- * @sideband_status: Status of sideband signals @0x1C
+ * @mm2s_ctrl_sideband: Sideband control info for mm2s @0x18
+ * @s2mm_status: Status field for s2mm @0x18
+ * @mm2s_status: Status field for mm2s @0x1C
+ * @s2mm_sideband_status: Sideband status for s2mm @0x1C
  * @app: APP Fields @0x20 - 0x30
  */
 struct xilinx_aximcdma_desc_hw {
@@ -288,10 +291,17 @@ struct xilinx_aximcdma_desc_hw {
 	u32 buf_addr_msb;
 	u32 rsvd;
 	u32 control;
-	u32 status;
-	u32 sideband_status;
+	union {
+		u32 mm2s_ctrl_sideband;
+		u32 s2mm_status;
+	};
+	union {
+		u32 mm2s_status;
+		u32 s2mm_sideband_status;
+	};
 	u32 app[XILINX_DMA_NUM_APP_WORDS];
 } __aligned(64);
+static_assert(sizeof(struct xilinx_aximcdma_desc_hw) == XILINX_MCDMA_BD_HW_SIZE);
 
 /**
  * struct xilinx_cdma_desc_hw - Hardware Descriptor
@@ -1015,9 +1025,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;
 		}
 	}
 
-- 
2.25.1

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

[Pandey, Radhey Shyam]

> The MCDMA BD format differs between memory-to-device (MM2S) and
> device-to-memory (S2MM) directions, but the driver was using generic
> 'status' and 'sideband_status' fields for both. This led to incorrect
> residue calculations when the hardware updates direction-specific fields.
> 
> Refactor the descriptor structure to use unions with direction-specific
> field mappings, and update the residue calculation logic to select the
> correct status field based on DMA direction.
> 
> This matches the hardware descriptor layout and fixes incorrect
> residue reporting.
> 
> Fixes: 6ccd692bfb7f ("dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support")
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>

Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thanks!
> ---
> Changes in V3:
>   - Renamed subject from "for MM2S vs S2MM" to "based on DMA direction".
>   - Reworded commit message for clarity.
>   - Added XILINX_MCDMA_BD_HW_SIZE macro and static_assert to verify
>     descriptor size at compile time.
>   - Refactored residue calculation to separate addition and subtraction
>     operations for better readability.
> 
> Changes in V2:
>   - No change.
> ---
>   drivers/dma/xilinx/xilinx_dma.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 98b41b8f8915..ff5b29a808e9 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -223,6 +223,7 @@
>   #define XILINX_MCDMA_IRQ_ERR_MASK		BIT(7)
>   #define XILINX_MCDMA_BD_EOP			BIT(30)
>   #define XILINX_MCDMA_BD_SOP			BIT(31)
> +#define XILINX_MCDMA_BD_HW_SIZE			64
>   
>   /**
>    * struct xilinx_vdma_desc_hw - Hardware Descriptor
> @@ -277,8 +278,10 @@ struct xilinx_axidma_desc_hw {
>    * @buf_addr_msb: MSB of Buffer address @0x0C
>    * @rsvd: Reserved field @0x10
>    * @control: Control Information field @0x14
> - * @status: Status field @0x18
> - * @sideband_status: Status of sideband signals @0x1C
> + * @mm2s_ctrl_sideband: Sideband control info for mm2s @0x18
> + * @s2mm_status: Status field for s2mm @0x18
> + * @mm2s_status: Status field for mm2s @0x1C
> + * @s2mm_sideband_status: Sideband status for s2mm @0x1C
>    * @app: APP Fields @0x20 - 0x30
>    */
>   struct xilinx_aximcdma_desc_hw {
> @@ -288,10 +291,17 @@ struct xilinx_aximcdma_desc_hw {
>   	u32 buf_addr_msb;
>   	u32 rsvd;
>   	u32 control;
> -	u32 status;
> -	u32 sideband_status;
> +	union {
> +		u32 mm2s_ctrl_sideband;
> +		u32 s2mm_status;
> +	};
> +	union {
> +		u32 mm2s_status;
> +		u32 s2mm_sideband_status;
> +	};
>   	u32 app[XILINX_DMA_NUM_APP_WORDS];
>   } __aligned(64);
> +static_assert(sizeof(struct xilinx_aximcdma_desc_hw) == XILINX_MCDMA_BD_HW_SIZE);
>   
>   /**
>    * struct xilinx_cdma_desc_hw - Hardware Descriptor
> @@ -1015,9 +1025,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;
>   		}
>   	}
>   

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

[Srinivas Neeli]

In AXI MCDMA scatter-gather mode, xilinx_dma_complete_descriptor() walks
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.

Fixes: 6ccd692bfb7f ("dmaengine: xilinx_dma: Add Xilinx AXI MCDMA Engine driver support")
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
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)
-- 
2.25.1

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@amd.com>
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@amd.com>
> ---
> 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)

[PATCH V3 3/4] net: xilinx: axienet: Derive RX frame length from DMA residue

[Srinivas Neeli]

The dmaengine RX path determined the received frame length by reading APP
word 4 of the DMA descriptor metadata, masking the lower 16 bits of
app_metadata[LEN_APP].

This relies on the optional AXI4-Stream status/control interface being
present in the design. The descriptor APP fields are only populated by the
hardware when that interface is enabled. On designs without it the APP
fields are not updated, so the length read back is invalid.

The AXI DMA engine already reports how many bytes it wrote into the buffer
through the standard dmaengine residue mechanism
(dmaengine_result.residue). The received frame length is therefore the
posted buffer length minus the residue, which is independent of the
status/control interface and correct across all designs, including
multi-descriptor frames where the residue is summed over the chain.

Use result->residue to compute the RX frame length and drop the descriptor
metadata lookup, which was only used for this purpose. The error path now
uses the standard dmaengine_result.result status instead of the metadata
pointer return value, and the now-unused LEN_APP macro is removed.

The transmit path is unaffected. It still passes APP metadata for checksum
offload and derives its length from the skb.

Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
---
Changes in V3:
 - New patch in this series.
 - This patch enables axienet to work on designs where the AXI4-Stream
   status/control interface is not present. By using the standard
   dmaengine residue mechanism, the driver no longer depends on APP
   fields being populated by hardware.
 - This approach replaces the V2 xferred_bytes mechanism (V2 patch 5/5),
   making the dt-bindings patch (V2 patch 4/5) for xlnx,include-stscntrl-strm
   also unnecessary. Both V2 patches are dropped in this series.
---
 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index fcf517069d16..67d1b8e91d68 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -53,7 +53,6 @@
 #define TX_BD_NUM_MAX			4096
 #define RX_BD_NUM_MAX			4096
 #define DMA_NUM_APP_WORDS		5
-#define LEN_APP				4
 #define RX_BUF_NUM_DEFAULT		128
 
 /* Must be shorter than length of ethtool_drvinfo.driver field to fit */
@@ -1159,29 +1158,26 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 static void axienet_dma_rx_cb(void *data, const struct dmaengine_result *result)
 {
 	struct skbuf_dma_descriptor *skbuf_dma;
-	size_t meta_len, meta_max_len, rx_len;
 	struct axienet_local *lp = data;
 	struct sk_buff *skb;
-	u32 *app_metadata;
+	size_t rx_len;
 	int i;
 
 	skbuf_dma = axienet_get_rx_desc(lp, lp->rx_ring_tail++);
 	skb = skbuf_dma->skb;
-	app_metadata = dmaengine_desc_get_metadata_ptr(skbuf_dma->desc, &meta_len,
-						       &meta_max_len);
 	dma_unmap_single(lp->dev, skbuf_dma->dma_address, lp->max_frm_size,
 			 DMA_FROM_DEVICE);
 
-	if (IS_ERR(app_metadata)) {
+	if (result->result != DMA_TRANS_NOERROR) {
 		if (net_ratelimit())
-			netdev_err(lp->ndev, "Failed to get RX metadata pointer\n");
+			netdev_err(lp->ndev, "RX DMA transfer failed\n");
 		dev_kfree_skb_any(skb);
 		lp->ndev->stats.rx_dropped++;
 		goto rx_submit;
 	}
 
-	/* TODO: Derive app word index programmatically */
-	rx_len = (app_metadata[LEN_APP] & 0xFFFF);
+	/* Actual length = posted buffer length - residue. */
+	rx_len = lp->max_frm_size - result->residue;
 	skb_put(skb, rx_len);
 	skb->protocol = eth_type_trans(skb, lp->ndev);
 	skb->ip_summed = CHECKSUM_NONE;
-- 
2.25.1

Re: [PATCH V3 3/4] net: xilinx: axienet: Derive RX frame length from DMA residue

[Pandey, Radhey Shyam]

rephrase to - derive RX frame length from residue in dmaengine path

> The dmaengine RX path determined the received frame length by reading APP
> word 4 of the DMA descriptor metadata, masking the lower 16 bits of
> app_metadata[LEN_APP].

Avoid above explanation and make commit description concise.>
> This relies on the optional AXI4-Stream status/control interface being
> present in the design. The descriptor APP fields are only populated by the
> hardware when that interface is enabled. On designs without it the APP
> fields are not updated, so the length read back is invalid.
> 
> The AXI DMA engine already reports how many bytes it wrote into the buffer
> through the standard dmaengine residue mechanism
> (dmaengine_result.residue). The received frame length is therefore the
> posted buffer length minus the residue, which is independent of the
> status/control interface and correct across all designs, including
> multi-descriptor frames where the residue is summed over the chain.
> 
> Use result->residue to compute the RX frame length and drop the descriptor
> metadata lookup, which was only used for this purpose. The error path now
> uses the standard dmaengine_result.result status instead of the metadata
> pointer return value, and the now-unused LEN_APP macro is removed.

now unused>
> The transmit path is unaffected. It still passes APP metadata for checksum
> offload and derives its length from the skb.
> 

Switching to dmaengine residue is better alternative but consider it as 
an enhancement. Drop the fixes tag. The non-dmaengine axienet RX path 
still derives frame length from APP field and it's a design assumption.

> Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---
> Changes in V3:
>   - New patch in this series.
>   - This patch enables axienet to work on designs where the AXI4-Stream
>     status/control interface is not present. By using the standard
>     dmaengine residue mechanism, the driver no longer depends on APP
>     fields being populated by hardware.
>   - This approach replaces the V2 xferred_bytes mechanism (V2 patch 5/5),
>     making the dt-bindings patch (V2 patch 4/5) for xlnx,include-stscntrl-strm
>     also unnecessary. Both V2 patches are dropped in this series.
> ---
>   drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index fcf517069d16..67d1b8e91d68 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -53,7 +53,6 @@
>   #define TX_BD_NUM_MAX			4096
>   #define RX_BD_NUM_MAX			4096
>   #define DMA_NUM_APP_WORDS		5
> -#define LEN_APP				4
>   #define RX_BUF_NUM_DEFAULT		128
>   
>   /* Must be shorter than length of ethtool_drvinfo.driver field to fit */
> @@ -1159,29 +1158,26 @@ axienet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>   static void axienet_dma_rx_cb(void *data, const struct dmaengine_result *result)
>   {
>   	struct skbuf_dma_descriptor *skbuf_dma;
> -	size_t meta_len, meta_max_len, rx_len;
>   	struct axienet_local *lp = data;
>   	struct sk_buff *skb;
> -	u32 *app_metadata;
> +	size_t rx_len;
>   	int i;
>   
>   	skbuf_dma = axienet_get_rx_desc(lp, lp->rx_ring_tail++);
>   	skb = skbuf_dma->skb;
> -	app_metadata = dmaengine_desc_get_metadata_ptr(skbuf_dma->desc, &meta_len,
> -						       &meta_max_len);
>   	dma_unmap_single(lp->dev, skbuf_dma->dma_address, lp->max_frm_size,
>   			 DMA_FROM_DEVICE);
>   
> -	if (IS_ERR(app_metadata)) {
> +	if (result->result != DMA_TRANS_NOERROR) {
>   		if (net_ratelimit())
> -			netdev_err(lp->ndev, "Failed to get RX metadata pointer\n");
> +			netdev_err(lp->ndev, "RX DMA transfer failed\n");
>   		dev_kfree_skb_any(skb);
>   		lp->ndev->stats.rx_dropped++;
>   		goto rx_submit;
>   	}
>   
> -	/* TODO: Derive app word index programmatically */
> -	rx_len = (app_metadata[LEN_APP] & 0xFFFF);
> +	/* Actual length = posted buffer length - residue. */
> +	rx_len = lp->max_frm_size - result->residue;
>   	skb_put(skb, rx_len);
>   	skb->protocol = eth_type_trans(skb, lp->ndev);
>   	skb->ip_summed = CHECKSUM_NONE;

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

[Srinivas Neeli]

From: Suraj Gupta <suraj.gupta2@amd.com>

xilinx_dma_get_metadata_ptr() exposed only the descriptor APP fields. Both
AXI DMA and AXI MCDMA descriptors carry a status word with the transfer
status, and AXI MCDMA additionally carries an AXI4-Stream sideband status
word holding TID, TDEST and TUSER that clients may need. Extend the
metadata handling to expose these.

The returned pointer now starts at the descriptor status word for both AXI
DMA and AXI MCDMA. As the descriptor words are contiguous, the client sees
the status at index 0, followed for AXI MCDMA by the sideband status
(TID/TDEST/TUSER) at index 1 and the APP fields, or for AXI DMA by the APP
fields directly. The payload length is derived from the field sizes.

This changes the get_metadata_ptr() contract for AXI DMA. The pointer now
starts at the status word of the last (EOF) descriptor instead of the APP
fields of the first, and the payload grows from 20 to 24 bytes. A client
reading app[0] now reads the status word. No in-tree consumer is affected,
as the axienet driver reads the RX frame length from result->residue
rather than the APP fields. Reading the EOF descriptor is also correct, as
the hardware writes the status and APP fields there.

The index 0 and 1 layout described above is for the AXI MCDMA receive
(S2MM) direction, which is where metadata is consumed. On the transmit
(MM2S) direction the same descriptor words hold different fields.

The probe logic is extended to read xlnx,axistream-connected for MCDMA, and
xilinx_mcdma_prep_slave_sg() attaches metadata_ops when an AXI Stream
interface is present, so MCDMA clients can use the metadata API in 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>
---
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, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 1b5b00f08c5f..f5c4e0ca2cc4 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -651,18 +651,48 @@ 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 returned pointer starts at the descriptor status word for both AXI DMA
+ * and AXI MCDMA. As the descriptor words are contiguous, the client sees the
+ * status at index 0, followed for AXI MCDMA by the sideband status
+ * (TID/TDEST/TUSER) at index 1 and the APP fields from index 2, or for AXI DMA
+ * by the APP fields from index 1. These fields are populated by the hardware on
+ * the End-Of-Frame descriptor, so the pointer is taken from there.
+ *
+ * Return: Pointer to the descriptor status field.
  */
 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_axidma_tx_segment *seg;
+	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 =
+			list_last_entry(&desc->segments,
+					struct xilinx_aximcdma_tx_segment, node);
+
+		/* [0] = status, [1] = sideband (TID/TDEST/TUSER), [2..] = app */
+		*max_len = *payload_len = sizeof(seg->hw.s2mm_status) +
+					 sizeof(seg->hw.s2mm_sideband_status) +
+					 sizeof(seg->hw.app);
+		return &seg->hw.s2mm_status;
+	}
+
+	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
+		struct xilinx_axidma_tx_segment *seg =
+			list_last_entry(&desc->segments,
+					struct xilinx_axidma_tx_segment, node);
+
+		/* [0] = status, [1..] = app */
+		*max_len = *payload_len = sizeof(seg->hw.status) +
+					 sizeof(seg->hw.app);
+		return &seg->hw.status;
+	}
 
-	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
-	seg = list_first_entry(&desc->segments,
-			       struct xilinx_axidma_tx_segment, node);
-	return seg->hw.app;
+	/* Only AXIDMA and MCDMA attach metadata_ops today. */
+	WARN_ON_ONCE(1);
+	return ERR_PTR(-EINVAL);
 }
 
 static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
@@ -2639,6 +2669,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 +3320,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");
 	}
-- 
2.25.1

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

[Pandey, Radhey Shyam]

> From: Suraj Gupta <suraj.gupta2@amd.com>
> 
> xilinx_dma_get_metadata_ptr() exposed only the descriptor APP fields. Both
> AXI DMA and AXI MCDMA descriptors carry a status word with the transfer
> status, and AXI MCDMA additionally carries an AXI4-Stream sideband status
> word holding TID, TDEST and TUSER that clients may need. Extend the
> metadata handling to expose these.
> 
> The returned pointer now starts at the descriptor status word for both AXI
> DMA and AXI MCDMA. As the descriptor words are contiguous, the client sees
> the status at index 0, followed for AXI MCDMA by the sideband status
> (TID/TDEST/TUSER) at index 1 and the APP fields, or for AXI DMA by the APP
> fields directly. The payload length is derived from the field sizes.
> 
> This changes the get_metadata_ptr() contract for AXI DMA. The pointer now
> starts at the status word of the last (EOF) descriptor instead of the APP
> fields of the first, and the payload grows from 20 to 24 bytes. A client
> reading app[0] now reads the status word. No in-tree consumer is affected,
> as the axienet driver reads the RX frame length from result->residue
> rather than the APP fields. Reading the EOF descriptor is also correct, as
> the hardware writes the status and APP fields there.
> 
> The index 0 and 1 layout described above is for the AXI MCDMA receive
> (S2MM) direction, which is where metadata is consumed. On the transmit
> (MM2S) direction the same descriptor words hold different fields.
> 
> The probe logic is extended to read xlnx,axistream-connected for MCDMA, and
> xilinx_mcdma_prep_slave_sg() attaches metadata_ops when an AXI Stream
> interface is present, so MCDMA clients can use the metadata API in the same
> way as AXI DMA clients.

Nit - make the commit description precise.>
> 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>
> ---
> 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, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 1b5b00f08c5f..f5c4e0ca2cc4 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -651,18 +651,48 @@ 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 returned pointer starts at the descriptor status word for both AXI DMA
> + * and AXI MCDMA. As the descriptor words are contiguous, the client sees the
> + * status at index 0, followed for AXI MCDMA by the sideband status
> + * (TID/TDEST/TUSER) at index 1 and the APP fields from index 2, or for AXI DMA
> + * by the APP fields from index 1. These fields are populated by the hardware on
> + * the End-Of-Frame descriptor, so the pointer is taken from there.
> + *
> + * Return: Pointer to the descriptor status field.
>    */
>   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_axidma_tx_segment *seg;
> +	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 =
> +			list_last_entry(&desc->segments,
> +					struct xilinx_aximcdma_tx_segment, node);
> +

This func is common for MCDMA s2mm and mm2s channel. Please make it 
generic to address both.

> +		/* [0] = status, [1] = sideband (TID/TDEST/TUSER), [2..] = app */
> +		*max_len = *payload_len = sizeof(seg->hw.s2mm_status) +
> +					 sizeof(seg->hw.s2mm_sideband_status) +
> +					 sizeof(seg->hw.app);
> +		return &seg->hw.s2mm_status;
> +	}
> +
> +	if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> +		struct xilinx_axidma_tx_segment *seg =
> +			list_last_entry(&desc->segments,
> +					struct xilinx_axidma_tx_segment, node);
> +
> +		/* [0] = status, [1..] = app */
> +		*max_len = *payload_len = sizeof(seg->hw.status) +
> +					 sizeof(seg->hw.app);
> +		return &seg->hw.status;
> +	}
>   
> -	*max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> -	seg = list_first_entry(&desc->segments,
> -			       struct xilinx_axidma_tx_segment, node);
> -	return seg->hw.app;
> +	/* Only AXIDMA and MCDMA attach metadata_ops today. */
> +	WARN_ON_ONCE(1);
> +	return ERR_PTR(-EINVAL);

We are registering metadata ops for MCDMA and AXI DMA so when is this 
error condition going to happen?

>   }
>   
>   static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
> @@ -2639,6 +2669,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 +3320,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");
>   	}