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

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

> 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;
>   		}
>   	}
>