[PATCH v4 0/8] Miscellaneous xdma driver enhancements

[PATCH v4 0/8] Miscellaneous xdma driver enhancements

[Jan Kuliga]

Hi,

This patchset introduces a couple of xdma driver enhancements. The most
important change is the introduction of interleaved DMA transfers
feature, which is a big deal, as it allows DMAEngine clients to express
DMA transfers in an arbitrary way. This is extremely useful in FPGA
environments, where in one FPGA system there may be a need to do DMA both
to/from FIFO at a fixed address and to/from a (non)contiguous RAM.

It is a another reroll of my previous patch series [1], but it is heavily
modified one as it is based on Miquel's patchset [2]. We agreed on doing
it that way, as both our patchsets touched the very same piece of code.
The discussion took place under [2] thread.

I tested it with XDMA v4.1 (Rev.20) IP core, with both sg and
interleaved DMA transfers.

Jan

Changes since v1:
[PATCH 1/5]: 
Complete a terminated descriptor with dma_cookie_complete()
Don't reinitialize temporary list head in xdma_terminate_all() 
[PATCH 4/5]:
Fix incorrect text wrapping

Changes since v2:
[PATCH 1/5]:
DO NOT schedule callback from within xdma_terminate_all()

Changes since v3:
Base patchset on Miquel's [2] series
Reorganize commits` structure
Introduce interleaved DMA transfers feature
Implement transfer error reporting

[1]:
https://lore.kernel.org/dmaengine/20231124192524.134989-1-jankul@alatek.krakow.pl/T/#t

[2]:
https://lore.kernel.org/dmaengine/20231130111315.729430-1-miquel.raynal@bootlin.com/T/#t

---
Jan Kuliga (8):
  dmaengine: xilinx: xdma: Get rid of unused code
  dmaengine: xilinx: xdma: Add necessary macro definitions
  dmaengine: xilinx: xdma: Ease dma_pool alignment requirements
  dmaengine: xilinx: xdma: Rework xdma_terminate_all()
  dmaengine: xilinx: xdma: Add error checking in xdma_channel_isr()
  dmaengine: xilinx: xdma: Add transfer error reporting
  dmaengine: xilinx: xdma: Prepare the introduction of interleaved DMA
    transfers
  dmaengine: xilinx: xdma: Introduce interleaved DMA transfers

 drivers/dma/xilinx/xdma-regs.h |  30 ++--
 drivers/dma/xilinx/xdma.c      | 285 ++++++++++++++++++++++-----------
 2 files changed, 210 insertions(+), 105 deletions(-)

-- 
2.34.1

[PATCH v4 1/8] dmaengine: xilinx: xdma: Get rid of unused code

[Jan Kuliga]

Get rid of duplicated macro definitions, as these macros are defined
earlier in the file. Also, get rid of unused member
of 'struct xdma_desc'.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma-regs.h | 12 ------------
 drivers/dma/xilinx/xdma.c      |  2 --
 2 files changed, 14 deletions(-)

diff --git a/drivers/dma/xilinx/xdma-regs.h b/drivers/dma/xilinx/xdma-regs.h
index e641a5083e14..0b17a931f583 100644
--- a/drivers/dma/xilinx/xdma-regs.h
+++ b/drivers/dma/xilinx/xdma-regs.h
@@ -134,18 +134,6 @@ struct xdma_hw_desc {
 #define XDMA_SGDMA_DESC_ADJ	0x4088
 #define XDMA_SGDMA_DESC_CREDIT	0x408c

-/* bits of the SG DMA control register */
-#define XDMA_CTRL_RUN_STOP			BIT(0)
-#define XDMA_CTRL_IE_DESC_STOPPED		BIT(1)
-#define XDMA_CTRL_IE_DESC_COMPLETED		BIT(2)
-#define XDMA_CTRL_IE_DESC_ALIGN_MISMATCH	BIT(3)
-#define XDMA_CTRL_IE_MAGIC_STOPPED		BIT(4)
-#define XDMA_CTRL_IE_IDLE_STOPPED		BIT(6)
-#define XDMA_CTRL_IE_READ_ERROR			GENMASK(13, 9)
-#define XDMA_CTRL_IE_DESC_ERROR			GENMASK(23, 19)
-#define XDMA_CTRL_NON_INCR_ADDR			BIT(25)
-#define XDMA_CTRL_POLL_MODE_WB			BIT(26)
-
 /*
  * interrupt registers
  */
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 290bb5d2d1e2..ddb9e7d07461 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -78,7 +78,6 @@ struct xdma_chan {
  * @vdesc: Virtual DMA descriptor
  * @chan: DMA channel pointer
  * @dir: Transferring direction of the request
- * @dev_addr: Physical address on DMA device side
  * @desc_blocks: Hardware descriptor blocks
  * @dblk_num: Number of hardware descriptor blocks
  * @desc_num: Number of hardware descriptors
@@ -91,7 +90,6 @@ struct xdma_desc {
 	struct virt_dma_desc		vdesc;
 	struct xdma_chan		*chan;
 	enum dma_transfer_direction	dir;
-	u64				dev_addr;
 	struct xdma_desc_block		*desc_blocks;
 	u32				dblk_num;
 	u32				desc_num;
--
2.34.1

[PATCH v4 2/8] dmaengine: xilinx: xdma: Add necessary macro definitions

[Jan Kuliga]

Complete lacking bits describing the status/control register values.
Add macros describing the status/control registers.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma-regs.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/dma/xilinx/xdma-regs.h b/drivers/dma/xilinx/xdma-regs.h
index 0b17a931f583..b12dd60629f6 100644
--- a/drivers/dma/xilinx/xdma-regs.h
+++ b/drivers/dma/xilinx/xdma-regs.h
@@ -76,6 +76,7 @@ struct xdma_hw_desc {
 #define XDMA_CHAN_CONTROL_W1S		0x8
 #define XDMA_CHAN_CONTROL_W1C		0xc
 #define XDMA_CHAN_STATUS		0x40
+#define XDMA_CHAN_STATUS_RC		0x44
 #define XDMA_CHAN_COMPLETED_DESC	0x48
 #define XDMA_CHAN_ALIGNMENTS		0x4c
 #define XDMA_CHAN_INTR_ENABLE		0x90
@@ -101,6 +102,7 @@ struct xdma_hw_desc {
 #define CHAN_CTRL_IE_MAGIC_STOPPED		BIT(4)
 #define CHAN_CTRL_IE_IDLE_STOPPED		BIT(6)
 #define CHAN_CTRL_IE_READ_ERROR			GENMASK(13, 9)
+#define CHAN_CTRL_IE_WRITE_ERROR		GENMASK(18, 14)
 #define CHAN_CTRL_IE_DESC_ERROR			GENMASK(23, 19)
 #define CHAN_CTRL_NON_INCR_ADDR			BIT(25)
 #define CHAN_CTRL_POLL_MODE_WB			BIT(26)
@@ -111,8 +113,17 @@ struct xdma_hw_desc {
 			 CHAN_CTRL_IE_DESC_ALIGN_MISMATCH |		\
 			 CHAN_CTRL_IE_MAGIC_STOPPED |			\
 			 CHAN_CTRL_IE_READ_ERROR |			\
+			 CHAN_CTRL_IE_WRITE_ERROR |			\
 			 CHAN_CTRL_IE_DESC_ERROR)

+#define XDMA_CHAN_STATUS_MASK CHAN_CTRL_START
+
+#define XDMA_CHAN_ERROR_MASK (CHAN_CTRL_IE_DESC_ALIGN_MISMATCH |	\
+				CHAN_CTRL_IE_MAGIC_STOPPED |		\
+				CHAN_CTRL_IE_READ_ERROR |		\
+				CHAN_CTRL_IE_WRITE_ERROR |		\
+				CHAN_CTRL_IE_DESC_ERROR)
+
 /* bits of the channel interrupt enable mask */
 #define CHAN_IM_DESC_ERROR			BIT(19)
 #define CHAN_IM_READ_ERROR			BIT(9)
--
2.34.1

[PATCH v4 3/8] dmaengine: xilinx: xdma: Ease dma_pool alignment requirements

[Jan Kuliga]

According to the XDMA datasheet (PG195), the address of any descriptor
must be 32 byte aligned. The datasheet also states that a contiguous
block of descriptors must not cross a 4k address boundary. Therefore,
it is possible to ease the pressure put on the dma_pool allocator
just by requiring sufficient alignment and boundary values. Add proper
macro definition and change the values passed into the
dma_pool_create().

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma-regs.h | 7 ++++---
 drivers/dma/xilinx/xdma.c      | 6 +++---
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/xilinx/xdma-regs.h b/drivers/dma/xilinx/xdma-regs.h
index b12dd60629f6..2a224e3da672 100644
--- a/drivers/dma/xilinx/xdma-regs.h
+++ b/drivers/dma/xilinx/xdma-regs.h
@@ -64,9 +64,10 @@ struct xdma_hw_desc {
 	__le64		next_desc;
 };

-#define XDMA_DESC_SIZE		sizeof(struct xdma_hw_desc)
-#define XDMA_DESC_BLOCK_SIZE	(XDMA_DESC_SIZE * XDMA_DESC_ADJACENT)
-#define XDMA_DESC_BLOCK_ALIGN	4096
+#define XDMA_DESC_SIZE			sizeof(struct xdma_hw_desc)
+#define XDMA_DESC_BLOCK_SIZE		(XDMA_DESC_SIZE * XDMA_DESC_ADJACENT)
+#define XDMA_DESC_BLOCK_ALIGN		32
+#define XDMA_DESC_BLOCK_BOUNDARY	4096

 /*
  * Channel registers
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index ddb9e7d07461..1bce48e5d86c 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -741,9 +741,9 @@ static int xdma_alloc_chan_resources(struct dma_chan *chan)
 		return -EINVAL;
 	}

-	xdma_chan->desc_pool = dma_pool_create(dma_chan_name(chan),
-					       dev, XDMA_DESC_BLOCK_SIZE,
-					       XDMA_DESC_BLOCK_ALIGN, 0);
+	xdma_chan->desc_pool = dma_pool_create(dma_chan_name(chan), dev,
+				XDMA_DESC_BLOCK_SIZE, XDMA_DESC_BLOCK_ALIGN,
+						XDMA_DESC_BLOCK_BOUNDARY);
 	if (!xdma_chan->desc_pool) {
 		xdma_err(xdev, "unable to allocate descriptor pool");
 		return -ENOMEM;
--
2.34.1

[PATCH v4 4/8] dmaengine: xilinx: xdma: Rework xdma_terminate_all()

[Jan Kuliga]

Simplify xdma_xfer_stop(). Stop the dma engine and clear its status
register unconditionally - just do what its name states. This change
also allows to call it without grabbing a lock, which minimizes
the total time spent with a spinlock held.

Delete the currently processed vd.node from the vc.desc_issued list
prior to passing it to vchan_terminate_vdesc(). In case there's more
than one descriptor pending on vc.desc_issued list, calling
vchan_terminate_desc() results in losing the link between
vc.desc_issued list head and the second descriptor on the list. Doing so
results in resources leakege, as vchan_dma_desc_free_list() won't be
able to properly free memory resources attached to descriptors,
resulting in dma_pool_destroy() failure.

Don't call vchan_dma_desc_free_list() from within xdma_terminate_all().
Move all terminated descriptors to the vc.desc_terminated list instead.
This allows to postpone freeing memory resources associated with
descriptors until the call to vchan_synchronize(), which is called from
xdma_synchronize() callback. This is the right way to do it -
xdma_terminate_all() should return as soon as possible, while freeing
resources (that may be time consuming in case of large number of
descriptors) can be done safely later.

Fixes: 290bb5d2d1e2
("dmaengine: xilinx: xdma: Add terminate_all/synchronize callbacks")

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 1bce48e5d86c..521ba2a653b6 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -379,20 +379,20 @@ static int xdma_xfer_start(struct xdma_chan *xchan)
  */
 static int xdma_xfer_stop(struct xdma_chan *xchan)
 {
-	struct virt_dma_desc *vd = vchan_next_desc(&xchan->vchan);
-	struct xdma_device *xdev = xchan->xdev_hdl;
 	int ret;
-
-	if (!vd || !xchan->busy)
-		return -EINVAL;
+	u32 val;
+	struct xdma_device *xdev = xchan->xdev_hdl;

 	/* clear run stop bit to prevent any further auto-triggering */
 	ret = regmap_write(xdev->rmap, xchan->base + XDMA_CHAN_CONTROL_W1C,
-			   CHAN_CTRL_RUN_STOP);
+							CHAN_CTRL_RUN_STOP);
 	if (ret)
 		return ret;

-	xchan->busy = false;
+	/* Clear the channel status register */
+	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &val);
+	if (ret)
+		return ret;

 	return 0;
 }
@@ -505,25 +505,25 @@ static void xdma_issue_pending(struct dma_chan *chan)
 static int xdma_terminate_all(struct dma_chan *chan)
 {
 	struct xdma_chan *xdma_chan = to_xdma_chan(chan);
-	struct xdma_desc *desc = NULL;
 	struct virt_dma_desc *vd;
 	unsigned long flags;
 	LIST_HEAD(head);

-	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
 	xdma_xfer_stop(xdma_chan);

+	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
+
+	xdma_chan->busy = false;
 	vd = vchan_next_desc(&xdma_chan->vchan);
-	if (vd)
-		desc = to_xdma_desc(vd);
-	if (desc) {
-		dma_cookie_complete(&desc->vdesc.tx);
-		vchan_terminate_vdesc(&desc->vdesc);
+	if (vd) {
+		list_del(&vd->node);
+		dma_cookie_complete(&vd->tx);
+		vchan_terminate_vdesc(vd);
 	}
-
 	vchan_get_all_descriptors(&xdma_chan->vchan, &head);
+	list_splice_tail(&head, &xdma_chan->vchan.desc_terminated);
+
 	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
-	vchan_dma_desc_free_list(&xdma_chan->vchan, &head);

 	return 0;
 }
--
2.34.1

[PATCH v4 5/8] dmaengine: xilinx: xdma: Add error checking in xdma_channel_isr()

[Jan Kuliga]

Check and clear the status register value before proceeding any
further in xdma_channel_isr(). It is necessary to do it since the
interrupt may occur on any error condition enabled at the start of a
transfer.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 521ba2a653b6..d1bc36133a45 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -812,21 +812,25 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 	desc = to_xdma_desc(vd);
 	xdev = xchan->xdev_hdl;

+	/* Clear-on-read the status register */
+	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &st);
+	if (ret)
+		goto out;
+
+	st &= XDMA_CHAN_STATUS_MASK;
+	if ((st & XDMA_CHAN_ERROR_MASK) ||
+		!(st & (CHAN_CTRL_IE_DESC_COMPLETED | CHAN_CTRL_IE_DESC_STOPPED))) {
+		xdma_err(xdev, "channel error, status register value: 0x%x", st);
+		goto out;
+	}
+
 	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_COMPLETED_DESC,
-			  &complete_desc_num);
+							&complete_desc_num);
 	if (ret)
 		goto out;

 	if (desc->cyclic) {
 		desc->completed_desc_num = complete_desc_num;
-
-		ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS,
-				  &st);
-		if (ret)
-			goto out;
-
-		regmap_write(xdev->rmap, xchan->base + XDMA_CHAN_STATUS, st);
-
 		vchan_cyclic_callback(vd);
 	} else {
 		xchan->busy = false;
--
2.34.1

[PATCH v4 6/8] dmaengine: xilinx: xdma: Add transfer error reporting

[Jan Kuliga]

Extend the capability of transfer status reporting. Introduce error flag,
which allows to report error in case of a interrupt-reported error
condition.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index d1bc36133a45..dbde6905acc7 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -85,6 +85,7 @@ struct xdma_chan {
  * @cyclic: Cyclic transfer vs. scatter-gather
  * @periods: Number of periods in the cyclic transfer
  * @period_size: Size of a period in bytes in cyclic transfers
+ * @error: tx error flag
  */
 struct xdma_desc {
 	struct virt_dma_desc		vdesc;
@@ -97,6 +98,7 @@ struct xdma_desc {
 	bool				cyclic;
 	u32				periods;
 	u32				period_size;
+	bool				error;
 };

 #define XDMA_DEV_STATUS_REG_DMA		BIT(0)
@@ -274,6 +276,7 @@ xdma_alloc_desc(struct xdma_chan *chan, u32 desc_num, bool cyclic)
 	sw_desc->chan = chan;
 	sw_desc->desc_num = desc_num;
 	sw_desc->cyclic = cyclic;
+	sw_desc->error = false;
 	dblk_num = DIV_ROUND_UP(desc_num, XDMA_DESC_ADJACENT);
 	sw_desc->desc_blocks = kcalloc(dblk_num, sizeof(*sw_desc->desc_blocks),
 				       GFP_NOWAIT);
@@ -770,20 +773,20 @@ static enum dma_status xdma_tx_status(struct dma_chan *chan, dma_cookie_t cookie
 	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);

 	vd = vchan_find_desc(&xdma_chan->vchan, cookie);
-	if (vd)
-		desc = to_xdma_desc(vd);
-	if (!desc || !desc->cyclic) {
-		spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
-		return ret;
-	}
-
-	period_idx = desc->completed_desc_num % desc->periods;
-	residue = (desc->periods - period_idx) * desc->period_size;
+	if (!vd)
+		goto out;

+	desc = to_xdma_desc(vd);
+	if (desc->error) {
+		ret = DMA_ERROR;
+	} else if (desc->cyclic) {
+		period_idx = desc->completed_desc_num % desc->periods;
+		residue = (desc->periods - period_idx) * desc->period_size;
+		dma_set_residue(state, residue);
+	}
+out:
 	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);

-	dma_set_residue(state, residue);
-
 	return ret;
}

@@ -808,6 +811,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 	vd = vchan_next_desc(&xchan->vchan);
 	if (!vd)
 		goto out;
+	desc = to_xdma_desc(vd);

 	desc = to_xdma_desc(vd);
 	xdev = xchan->xdev_hdl;
@@ -820,6 +824,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 	st &= XDMA_CHAN_STATUS_MASK;
 	if ((st & XDMA_CHAN_ERROR_MASK) ||
 		!(st & (CHAN_CTRL_IE_DESC_COMPLETED | CHAN_CTRL_IE_DESC_STOPPED))) {
+		desc->error = true;
 		xdma_err(xdev, "channel error, status register value: 0x%x", st);
 		goto out;
 	}
--
2.34.1

[PATCH v4 7/8] dmaengine: xilinx: xdma: Prepare the introduction of interleaved DMA transfers

[Jan Kuliga]

Make generic code generic. As descriptor-filling logic stays the same
regardless of a dmaengine's type of transfer, it is possible to write
the descriptor-filling function in a generic way, so that it can be used
for every single type of transfer preparation callback.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 101 +++++++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 44 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index dbde6905acc7..c8ac047249bc 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -542,6 +542,43 @@ static void xdma_synchronize(struct dma_chan *chan)
 	vchan_synchronize(&xdma_chan->vchan);
 }

+/**
+ * xdma_fill_descs - Fill hardware descriptors with contiguous memory block addresses
+ * @sw_desc - tx descriptor state container
+ * @src_addr - Value for a ->src_addr field of a first descriptor
+ * @dst_addr - Value for a ->dst_addr field of a first descriptor
+ * @size - Total size of a contiguous memory block
+ * @filled_descs_num - Number of filled hardware descriptors for corresponding sw_desc
+ */
+static inline u32 xdma_fill_descs(struct xdma_desc *sw_desc, u64 src_addr,
+				u64 dst_addr, u32 size, u32 filled_descs_num)
+{
+	u32 left = size, len, desc_num = filled_descs_num;
+	struct xdma_desc_block *dblk;
+	struct xdma_hw_desc *desc;
+
+	dblk = sw_desc->desc_blocks + (desc_num / XDMA_DESC_ADJACENT);
+	desc = dblk->virt_addr;
+	desc += desc_num & XDMA_DESC_ADJACENT_MASK;
+	do {
+		len = min_t(u32, left, XDMA_DESC_BLEN_MAX);
+		/* set hardware descriptor */
+		desc->bytes = cpu_to_le32(len);
+		desc->src_addr = cpu_to_le64(src_addr);
+		desc->dst_addr = cpu_to_le64(dst_addr);
+		if (!(++desc_num & XDMA_DESC_ADJACENT_MASK))
+			desc = (++dblk)->virt_addr;
+		else
+			desc++;
+
+		src_addr += len;
+		dst_addr += len;
+		left -= len;
+	} while (left);
+
+	return desc_num - filled_descs_num;
+}
+
/**
  * xdma_prep_device_sg - prepare a descriptor for a DMA transaction
  * @chan: DMA channel pointer
@@ -558,13 +595,10 @@ xdma_prep_device_sg(struct dma_chan *chan, struct scatterlist *sgl,
 {
 	struct xdma_chan *xdma_chan = to_xdma_chan(chan);
 	struct dma_async_tx_descriptor *tx_desc;
-	u32 desc_num = 0, i, len, rest;
-	struct xdma_desc_block *dblk;
-	struct xdma_hw_desc *desc;
 	struct xdma_desc *sw_desc;
-	u64 dev_addr, *src, *dst;
+	u32 desc_num = 0, i;
+	u64 addr, dev_addr, *src, *dst;
 	struct scatterlist *sg;
-	u64 addr;

 	for_each_sg(sgl, sg, sg_len, i)
 		desc_num += DIV_ROUND_UP(sg_dma_len(sg), XDMA_DESC_BLEN_MAX);
@@ -584,32 +618,11 @@ xdma_prep_device_sg(struct dma_chan *chan, struct scatterlist *sgl,
 		dst = &addr;
 	}

-	dblk = sw_desc->desc_blocks;
-	desc = dblk->virt_addr;
-	desc_num = 1;
+	desc_num = 0;
 	for_each_sg(sgl, sg, sg_len, i) {
 		addr = sg_dma_address(sg);
-		rest = sg_dma_len(sg);
-
-		do {
-			len = min_t(u32, rest, XDMA_DESC_BLEN_MAX);
-			/* set hardware descriptor */
-			desc->bytes = cpu_to_le32(len);
-			desc->src_addr = cpu_to_le64(*src);
-			desc->dst_addr = cpu_to_le64(*dst);
-
-			if (!(desc_num & XDMA_DESC_ADJACENT_MASK)) {
-				dblk++;
-				desc = dblk->virt_addr;
-			} else {
-				desc++;
-			}
-
-			desc_num++;
-			dev_addr += len;
-			addr += len;
-			rest -= len;
-		} while (rest);
+		desc_num += xdma_fill_descs(sw_desc, *src, *dst, sg_dma_len(sg), desc_num);
+		dev_addr += sg_dma_len(sg);
 	}

 	tx_desc = vchan_tx_prep(&xdma_chan->vchan, &sw_desc->vdesc, flags);
@@ -643,9 +656,9 @@ xdma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t address,
 	struct xdma_device *xdev = xdma_chan->xdev_hdl;
 	unsigned int periods = size / period_size;
 	struct dma_async_tx_descriptor *tx_desc;
-	struct xdma_desc_block *dblk;
-	struct xdma_hw_desc *desc;
 	struct xdma_desc *sw_desc;
+	u64 addr, dev_addr, *src, *dst;
+	u32 desc_num;
 	unsigned int i;

 	/*
@@ -670,21 +683,21 @@ xdma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t address,
 	sw_desc->period_size = period_size;
 	sw_desc->dir = dir;

-	dblk = sw_desc->desc_blocks;
-	desc = dblk->virt_addr;
+	addr = address;
+	if (dir == DMA_MEM_TO_DEV) {
+		dev_addr = xdma_chan->cfg.dst_addr;
+		src = &addr;
+		dst = &dev_addr;
+	} else {
+		dev_addr = xdma_chan->cfg.src_addr;
+		src = &dev_addr;
+		dst = &addr;
+	}

-	/* fill hardware descriptor */
+	desc_num = 0;
 	for (i = 0; i < periods; i++) {
-		desc->bytes = cpu_to_le32(period_size);
-		if (dir == DMA_MEM_TO_DEV) {
-			desc->src_addr = cpu_to_le64(address + i * period_size);
-			desc->dst_addr = cpu_to_le64(xdma_chan->cfg.dst_addr);
-		} else {
-			desc->src_addr = cpu_to_le64(xdma_chan->cfg.src_addr);
-			desc->dst_addr = cpu_to_le64(address + i * period_size);
-		}
-
-		desc++;
+		desc_num += xdma_fill_descs(sw_desc, *src, *dst, period_size, desc_num);
+		addr += i * period_size;
 	}

	tx_desc = vchan_tx_prep(&xdma_chan->vchan, &sw_desc->vdesc, flags);
--
2.34.1

[PATCH v4 8/8] dmaengine: xilinx: xdma: Introduce interleaved DMA transfers

[Jan Kuliga]

Interleaved DMA functionality allows dmaengine clients' to express
DMA transfers in an arbitrary way. This is extremely useful in FPGA
environments, where a greater transfer flexibility is needed. For
instance, in one FPGA design there may be need to do DMA to/from a FIFO
(at a fixed address) and also to do DMA to/from a (non)contiguous RAM
memory.

Introduce separate tx preparation callback and add tx-flags handling
logic. Their behavior is based on the description of interleaved DMA
transfers in both source code and the DMAEngine's documentation.

Since XDMA is a fully-fledged scatter-gather dma engine, the logic of
xdma_prep_interleaved_dma() is fairly simple and similar to the other
tx preparation callbacks. The whole tx-flags handling logic resides in
xdma_channel_isr(). Transfer of a single frame from a interleaved DMA
transfer template is pretty similar to the single sg transaction.
Therefore, the transaction of the whole interleaved DMA transfer
template is basically a cyclic dma transaction with finite cycles/periods
(equal to the frame of count) of a single sg transfers.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 103 ++++++++++++++++++++++++++++++++++----
 1 file changed, 94 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index c8ac047249bc..c7d4def4124b 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -83,8 +83,10 @@ struct xdma_chan {
  * @desc_num: Number of hardware descriptors
  * @completed_desc_num: Completed hardware descriptors
  * @cyclic: Cyclic transfer vs. scatter-gather
+ * @interleaved_dma: Interleaved DMA transfer
  * @periods: Number of periods in the cyclic transfer
  * @period_size: Size of a period in bytes in cyclic transfers
+ * @frames_left: Number of frames left in interleaved DMA transfer
  * @error: tx error flag
  */
 struct xdma_desc {
@@ -96,8 +98,10 @@ struct xdma_desc {
 	u32				desc_num;
 	u32				completed_desc_num;
 	bool				cyclic;
+	bool				interleaved_dma;
 	u32				periods;
 	u32				period_size;
+	u32				frames_left;
 	bool				error;
 };

@@ -607,6 +611,7 @@ xdma_prep_device_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	if (!sw_desc)
 		return NULL;
 	sw_desc->dir = dir;
+	sw_desc->cyclic = sw_desc->interleaved_dma = false;

 	if (dir == DMA_MEM_TO_DEV) {
 		dev_addr = xdma_chan->cfg.dst_addr;
@@ -682,6 +687,7 @@ xdma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t address,
 	sw_desc->periods = periods;
 	sw_desc->period_size = period_size;
 	sw_desc->dir = dir;
+	sw_desc->interleaved_dma = false;

 	addr = address;
 	if (dir == DMA_MEM_TO_DEV) {
@@ -712,6 +718,54 @@ xdma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t address,
 	return NULL;
}

+/**
+ * xdma_prep_interleaved_dma - Prepare virtual descriptor for interleaved DMA transfers
+ * @chan: DMA channel
+ * @xt: DMA transfer template
+ * @flags: tx flags
+ */
+struct dma_async_tx_descriptor *
+xdma_prep_interleaved_dma(struct dma_chan *chan,
+			struct dma_interleaved_template *xt,
+			unsigned long flags)
+{
+	int i;
+	u32 desc_num = 0, period_size = 0;
+	struct dma_async_tx_descriptor *tx_desc;
+	struct xdma_chan *xchan = to_xdma_chan(chan);
+	struct xdma_desc *sw_desc;
+	u64 src_addr, dst_addr;
+
+	for (i = 0; i < xt->frame_size; ++i)
+		desc_num += DIV_ROUND_UP(xt->sgl[i].size, XDMA_DESC_BLEN_MAX);
+
+	sw_desc = xdma_alloc_desc(xchan, desc_num, false);
+	if (!sw_desc)
+		return NULL;
+	sw_desc->dir = xt->dir;
+	sw_desc->interleaved_dma = true;
+	sw_desc->cyclic = flags & DMA_PREP_REPEAT;
+	sw_desc->frames_left = sw_desc->periods = xt->numf;
+
+	desc_num = 0;
+	src_addr = xt->src_start;
+	dst_addr = xt->dst_start;
+	for (i = 0; i < xt->frame_size; ++i) {
+		desc_num += xdma_fill_descs(sw_desc, src_addr, dst_addr, xt->sgl[i].size, desc_num);
+		src_addr += dmaengine_get_src_icg(xt, &xt->sgl[i]) + xt->src_inc ? xt->sgl[i].size : 0;
+		dst_addr += dmaengine_get_dst_icg(xt, &xt->sgl[i]) + xt->dst_inc ? xt->sgl[i].size : 0;
+		period_size += xt->sgl[i].size;
+	}
+	sw_desc->period_size = period_size;
+
+	tx_desc = vchan_tx_prep(&xchan->vchan, &sw_desc->vdesc, flags);
+	if (tx_desc)
+		return tx_desc;
+
+	xdma_free_desc(&sw_desc->vdesc);
+	return NULL;
+}
+
 /**
  * xdma_device_config - Configure the DMA channel
  * @chan: DMA channel
@@ -812,11 +866,12 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
{
 	struct xdma_chan *xchan = dev_id;
 	u32 complete_desc_num = 0;
-	struct xdma_device *xdev;
-	struct virt_dma_desc *vd;
+	struct xdma_device *xdev = xchan->xdev_hdl;
+	struct virt_dma_desc *vd, *next_vd;
 	struct xdma_desc *desc;
 	int ret;
 	u32 st;
+	bool repeat_tx;

 	spin_lock(&xchan->vchan.lock);

@@ -826,9 +881,6 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 		goto out;
 	desc = to_xdma_desc(vd);

-	desc = to_xdma_desc(vd);
-	xdev = xchan->xdev_hdl;
-
 	/* Clear-on-read the status register */
 	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &st);
 	if (ret)
@@ -847,10 +899,36 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 	if (ret)
 		goto out;

-	if (desc->cyclic) {
-		desc->completed_desc_num = complete_desc_num;
-		vchan_cyclic_callback(vd);
-	} else {
+	desc = to_xdma_desc(vd);
+	if (desc->interleaved_dma) {
+		xchan->busy = false;
+		desc->completed_desc_num += complete_desc_num;
+		if (complete_desc_num == XDMA_DESC_BLOCK_NUM * XDMA_DESC_ADJACENT) {
+			xdma_xfer_start(xchan);
+			goto out;
+		}
+
+		/* last desc of any frame */
+		desc->frames_left--;
+		if (desc->frames_left)
+			goto out;
+
+		/* last desc of the last frame  */
+		repeat_tx = vd->tx.flags & DMA_PREP_REPEAT;
+		next_vd = list_first_entry_or_null(&vd->node, struct virt_dma_desc, node);
+		if (next_vd)
+			repeat_tx = repeat_tx && !(next_vd->tx.flags & DMA_PREP_LOAD_EOT);
+		if (repeat_tx) {
+			desc->frames_left = desc->periods;
+			desc->completed_desc_num = 0;
+			vchan_cyclic_callback(vd);
+		} else {
+			list_del(&vd->node);
+			vchan_cookie_complete(vd);
+		}
+		/* start (or continue) the tx of a first desc on the vc.desc_issued list, if any */
+		xdma_xfer_start(xchan);
+	} else if (!desc->cyclic) {
 		xchan->busy = false;
 		desc->completed_desc_num += complete_desc_num;

@@ -867,6 +945,9 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)

 		/* transfer the rest of data */
 		xdma_xfer_start(xchan);
+	} else {
+		desc->completed_desc_num = complete_desc_num;
+		vchan_cyclic_callback(vd);
 	}

 out:
@@ -1165,6 +1246,9 @@ static int xdma_probe(struct platform_device *pdev)
 	dma_cap_set(DMA_SLAVE, xdev->dma_dev.cap_mask);
 	dma_cap_set(DMA_PRIVATE, xdev->dma_dev.cap_mask);
 	dma_cap_set(DMA_CYCLIC, xdev->dma_dev.cap_mask);
+	dma_cap_set(DMA_INTERLEAVE, xdev->dma_dev.cap_mask);
+	dma_cap_set(DMA_REPEAT, xdev->dma_dev.cap_mask);
+	dma_cap_set(DMA_LOAD_EOT, xdev->dma_dev.cap_mask);

 	xdev->dma_dev.dev = &pdev->dev;
 	xdev->dma_dev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
@@ -1180,6 +1264,7 @@ static int xdma_probe(struct platform_device *pdev)
 	xdev->dma_dev.filter.mapcnt = pdata->device_map_cnt;
 	xdev->dma_dev.filter.fn = xdma_filter_fn;
 	xdev->dma_dev.device_prep_dma_cyclic = xdma_prep_dma_cyclic;
+	xdev->dma_dev.device_prep_interleaved_dma = xdma_prep_interleaved_dma;

	ret = dma_async_device_register(&xdev->dma_dev);
 	if (ret) {
--
2.34.1

Re: [PATCH v4 6/8] dmaengine: xilinx: xdma: Add transfer error reporting

[Lizhi Hou]

On 12/8/23 05:49, Jan Kuliga wrote:
> Extend the capability of transfer status reporting. Introduce error flag,
> which allows to report error in case of a interrupt-reported error
> condition.
>
> Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
> ---
>   drivers/dma/xilinx/xdma.c | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> index d1bc36133a45..dbde6905acc7 100644
> --- a/drivers/dma/xilinx/xdma.c
> +++ b/drivers/dma/xilinx/xdma.c
> @@ -85,6 +85,7 @@ struct xdma_chan {
>    * @cyclic: Cyclic transfer vs. scatter-gather
>    * @periods: Number of periods in the cyclic transfer
>    * @period_size: Size of a period in bytes in cyclic transfers
> + * @error: tx error flag
>    */
>   struct xdma_desc {
>   	struct virt_dma_desc		vdesc;
> @@ -97,6 +98,7 @@ struct xdma_desc {
>   	bool				cyclic;
>   	u32				periods;
>   	u32				period_size;
> +	bool				error;
>   };
>
>   #define XDMA_DEV_STATUS_REG_DMA		BIT(0)
> @@ -274,6 +276,7 @@ xdma_alloc_desc(struct xdma_chan *chan, u32 desc_num, bool cyclic)
>   	sw_desc->chan = chan;
>   	sw_desc->desc_num = desc_num;
>   	sw_desc->cyclic = cyclic;
> +	sw_desc->error = false;
>   	dblk_num = DIV_ROUND_UP(desc_num, XDMA_DESC_ADJACENT);
>   	sw_desc->desc_blocks = kcalloc(dblk_num, sizeof(*sw_desc->desc_blocks),
>   				       GFP_NOWAIT);
> @@ -770,20 +773,20 @@ static enum dma_status xdma_tx_status(struct dma_chan *chan, dma_cookie_t cookie
>   	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
>
>   	vd = vchan_find_desc(&xdma_chan->vchan, cookie);
> -	if (vd)
> -		desc = to_xdma_desc(vd);
> -	if (!desc || !desc->cyclic) {
> -		spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
> -		return ret;
> -	}
> -
> -	period_idx = desc->completed_desc_num % desc->periods;
> -	residue = (desc->periods - period_idx) * desc->period_size;
> +	if (!vd)
> +		goto out;
>
> +	desc = to_xdma_desc(vd);
> +	if (desc->error) {
> +		ret = DMA_ERROR;
> +	} else if (desc->cyclic) {
> +		period_idx = desc->completed_desc_num % desc->periods;
> +		residue = (desc->periods - period_idx) * desc->period_size;
> +		dma_set_residue(state, residue);
> +	}
> +out:
>   	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
>
> -	dma_set_residue(state, residue);
> -
>   	return ret;
> }
>
> @@ -808,6 +811,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
>   	vd = vchan_next_desc(&xchan->vchan);
>   	if (!vd)
>   		goto out;
> +	desc = to_xdma_desc(vd);
Duplicated line.
>
>   	desc = to_xdma_desc(vd);
>   	xdev = xchan->xdev_hdl;
> @@ -820,6 +824,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
>   	st &= XDMA_CHAN_STATUS_MASK;
>   	if ((st & XDMA_CHAN_ERROR_MASK) ||
>   		!(st & (CHAN_CTRL_IE_DESC_COMPLETED | CHAN_CTRL_IE_DESC_STOPPED))) {
> +		desc->error = true;
>   		xdma_err(xdev, "channel error, status register value: 0x%x", st);
>   		goto out;
>   	}
> --
> 2.34.1
>

[FIXED PATCH v4 6/8] dmaengine: xilinx: xdma: Add transfer error reporting

[Jan Kuliga]

Extend the capability of transfer status reporting. Introduce error flag,
which allows to report error in case of a interrupt-reported error
condition.

Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
---
 drivers/dma/xilinx/xdma.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index d1bc36133a45..a7cd378b7e9a 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -85,6 +85,7 @@ struct xdma_chan {
  * @cyclic: Cyclic transfer vs. scatter-gather
  * @periods: Number of periods in the cyclic transfer
  * @period_size: Size of a period in bytes in cyclic transfers
+ * @error: tx error flag
  */
 struct xdma_desc {
 	struct virt_dma_desc		vdesc;
@@ -97,6 +98,7 @@ struct xdma_desc {
 	bool				cyclic;
 	u32				periods;
 	u32				period_size;
+	bool				error;
 };

 #define XDMA_DEV_STATUS_REG_DMA		BIT(0)
@@ -274,6 +276,7 @@ xdma_alloc_desc(struct xdma_chan *chan, u32 desc_num, bool cyclic)
 	sw_desc->chan = chan;
 	sw_desc->desc_num = desc_num;
 	sw_desc->cyclic = cyclic;
+	sw_desc->error = false;
 	dblk_num = DIV_ROUND_UP(desc_num, XDMA_DESC_ADJACENT);
 	sw_desc->desc_blocks = kcalloc(dblk_num, sizeof(*sw_desc->desc_blocks),
 				       GFP_NOWAIT);
@@ -770,20 +773,20 @@ static enum dma_status xdma_tx_status(struct dma_chan *chan, dma_cookie_t cookie
 	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);

 	vd = vchan_find_desc(&xdma_chan->vchan, cookie);
-	if (vd)
-		desc = to_xdma_desc(vd);
-	if (!desc || !desc->cyclic) {
-		spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
-		return ret;
-	}
-
-	period_idx = desc->completed_desc_num % desc->periods;
-	residue = (desc->periods - period_idx) * desc->period_size;
+	if (!vd)
+		goto out;

+	desc = to_xdma_desc(vd);
+	if (desc->error) {
+		ret = DMA_ERROR;
+	} else if (desc->cyclic) {
+		period_idx = desc->completed_desc_num % desc->periods;
+		residue = (desc->periods - period_idx) * desc->period_size;
+		dma_set_residue(state, residue);
+	}
+out:
 	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);

-	dma_set_residue(state, residue);
-
 	return ret;
 }

@@ -820,6 +823,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
 	st &= XDMA_CHAN_STATUS_MASK;
 	if ((st & XDMA_CHAN_ERROR_MASK) ||
 		!(st & (CHAN_CTRL_IE_DESC_COMPLETED | CHAN_CTRL_IE_DESC_STOPPED))) {
+		desc->error = true;
 		xdma_err(xdev, "channel error, status register value: 0x%x", st);
 		goto out;
 	}
--
2.34.1

Re: [PATCH v4 4/8] dmaengine: xilinx: xdma: Rework xdma_terminate_all()

[Vinod Koul]

On 08-12-23, 14:49, Jan Kuliga wrote:
> Simplify xdma_xfer_stop(). Stop the dma engine and clear its status
> register unconditionally - just do what its name states. This change
> also allows to call it without grabbing a lock, which minimizes
> the total time spent with a spinlock held.
> 
> Delete the currently processed vd.node from the vc.desc_issued list
> prior to passing it to vchan_terminate_vdesc(). In case there's more
> than one descriptor pending on vc.desc_issued list, calling
> vchan_terminate_desc() results in losing the link between
> vc.desc_issued list head and the second descriptor on the list. Doing so
> results in resources leakege, as vchan_dma_desc_free_list() won't be
> able to properly free memory resources attached to descriptors,
> resulting in dma_pool_destroy() failure.
> 
> Don't call vchan_dma_desc_free_list() from within xdma_terminate_all().
> Move all terminated descriptors to the vc.desc_terminated list instead.
> This allows to postpone freeing memory resources associated with
> descriptors until the call to vchan_synchronize(), which is called from
> xdma_synchronize() callback. This is the right way to do it -
> xdma_terminate_all() should return as soon as possible, while freeing
> resources (that may be time consuming in case of large number of
> descriptors) can be done safely later.
> 
> Fixes: 290bb5d2d1e2
> ("dmaengine: xilinx: xdma: Add terminate_all/synchronize callbacks")
> 
> Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
> ---
>  drivers/dma/xilinx/xdma.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> index 1bce48e5d86c..521ba2a653b6 100644
> --- a/drivers/dma/xilinx/xdma.c
> +++ b/drivers/dma/xilinx/xdma.c
> @@ -379,20 +379,20 @@ static int xdma_xfer_start(struct xdma_chan *xchan)
>   */
>  static int xdma_xfer_stop(struct xdma_chan *xchan)
>  {
> -	struct virt_dma_desc *vd = vchan_next_desc(&xchan->vchan);
> -	struct xdma_device *xdev = xchan->xdev_hdl;
>  	int ret;
> -
> -	if (!vd || !xchan->busy)
> -		return -EINVAL;
> +	u32 val;
> +	struct xdma_device *xdev = xchan->xdev_hdl;
> 
>  	/* clear run stop bit to prevent any further auto-triggering */
>  	ret = regmap_write(xdev->rmap, xchan->base + XDMA_CHAN_CONTROL_W1C,
> -			   CHAN_CTRL_RUN_STOP);
> +							CHAN_CTRL_RUN_STOP);

Why this change, checkpatch would tell you this is not expected
alignment (run with strict)

>  	if (ret)
>  		return ret;
> 
> -	xchan->busy = false;
> +	/* Clear the channel status register */
> +	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &val);
> +	if (ret)
> +		return ret;
> 
>  	return 0;
>  }
> @@ -505,25 +505,25 @@ static void xdma_issue_pending(struct dma_chan *chan)
>  static int xdma_terminate_all(struct dma_chan *chan)
>  {
>  	struct xdma_chan *xdma_chan = to_xdma_chan(chan);
> -	struct xdma_desc *desc = NULL;
>  	struct virt_dma_desc *vd;
>  	unsigned long flags;
>  	LIST_HEAD(head);
> 
> -	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
>  	xdma_xfer_stop(xdma_chan);
> 
> +	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
> +
> +	xdma_chan->busy = false;
>  	vd = vchan_next_desc(&xdma_chan->vchan);
> -	if (vd)
> -		desc = to_xdma_desc(vd);
> -	if (desc) {
> -		dma_cookie_complete(&desc->vdesc.tx);
> -		vchan_terminate_vdesc(&desc->vdesc);
> +	if (vd) {
> +		list_del(&vd->node);
> +		dma_cookie_complete(&vd->tx);
> +		vchan_terminate_vdesc(vd);
>  	}
> -
>  	vchan_get_all_descriptors(&xdma_chan->vchan, &head);
> +	list_splice_tail(&head, &xdma_chan->vchan.desc_terminated);
> +
>  	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
> -	vchan_dma_desc_free_list(&xdma_chan->vchan, &head);
> 
>  	return 0;
>  }
> --
> 2.34.1

-- 
~Vinod

Re: [FIXED PATCH v4 6/8] dmaengine: xilinx: xdma: Add transfer error reporting

[Vinod Koul]

On 08-12-23, 23:08, Jan Kuliga wrote:
> Extend the capability of transfer status reporting. Introduce error flag,
> which allows to report error in case of a interrupt-reported error
> condition.

Pls post the updated series, noting changes from last rev

> 
> Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
> ---
>  drivers/dma/xilinx/xdma.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> index d1bc36133a45..a7cd378b7e9a 100644
> --- a/drivers/dma/xilinx/xdma.c
> +++ b/drivers/dma/xilinx/xdma.c
> @@ -85,6 +85,7 @@ struct xdma_chan {
>   * @cyclic: Cyclic transfer vs. scatter-gather
>   * @periods: Number of periods in the cyclic transfer
>   * @period_size: Size of a period in bytes in cyclic transfers
> + * @error: tx error flag
>   */
>  struct xdma_desc {
>  	struct virt_dma_desc		vdesc;
> @@ -97,6 +98,7 @@ struct xdma_desc {
>  	bool				cyclic;
>  	u32				periods;
>  	u32				period_size;
> +	bool				error;
>  };
> 
>  #define XDMA_DEV_STATUS_REG_DMA		BIT(0)
> @@ -274,6 +276,7 @@ xdma_alloc_desc(struct xdma_chan *chan, u32 desc_num, bool cyclic)
>  	sw_desc->chan = chan;
>  	sw_desc->desc_num = desc_num;
>  	sw_desc->cyclic = cyclic;
> +	sw_desc->error = false;
>  	dblk_num = DIV_ROUND_UP(desc_num, XDMA_DESC_ADJACENT);
>  	sw_desc->desc_blocks = kcalloc(dblk_num, sizeof(*sw_desc->desc_blocks),
>  				       GFP_NOWAIT);
> @@ -770,20 +773,20 @@ static enum dma_status xdma_tx_status(struct dma_chan *chan, dma_cookie_t cookie
>  	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
> 
>  	vd = vchan_find_desc(&xdma_chan->vchan, cookie);
> -	if (vd)
> -		desc = to_xdma_desc(vd);
> -	if (!desc || !desc->cyclic) {
> -		spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
> -		return ret;
> -	}
> -
> -	period_idx = desc->completed_desc_num % desc->periods;
> -	residue = (desc->periods - period_idx) * desc->period_size;
> +	if (!vd)
> +		goto out;
> 
> +	desc = to_xdma_desc(vd);
> +	if (desc->error) {
> +		ret = DMA_ERROR;
> +	} else if (desc->cyclic) {
> +		period_idx = desc->completed_desc_num % desc->periods;
> +		residue = (desc->periods - period_idx) * desc->period_size;
> +		dma_set_residue(state, residue);
> +	}
> +out:
>  	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
> 
> -	dma_set_residue(state, residue);
> -
>  	return ret;
>  }
> 
> @@ -820,6 +823,7 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
>  	st &= XDMA_CHAN_STATUS_MASK;
>  	if ((st & XDMA_CHAN_ERROR_MASK) ||
>  		!(st & (CHAN_CTRL_IE_DESC_COMPLETED | CHAN_CTRL_IE_DESC_STOPPED))) {
> +		desc->error = true;
>  		xdma_err(xdev, "channel error, status register value: 0x%x", st);
>  		goto out;
>  	}
> --
> 2.34.1

-- 
~Vinod

Re: [PATCH v4 4/8] dmaengine: xilinx: xdma: Rework xdma_terminate_all()

[Jan Kuliga]

Hi Vinod,

Thanks for reviewing my patchset.

On 11.12.2023 11:54, Vinod Koul wrote:
> On 08-12-23, 14:49, Jan Kuliga wrote:
>> Simplify xdma_xfer_stop(). Stop the dma engine and clear its status
>> register unconditionally - just do what its name states. This change
>> also allows to call it without grabbing a lock, which minimizes
>> the total time spent with a spinlock held.
>>
>> Delete the currently processed vd.node from the vc.desc_issued list
>> prior to passing it to vchan_terminate_vdesc(). In case there's more
>> than one descriptor pending on vc.desc_issued list, calling
>> vchan_terminate_desc() results in losing the link between
>> vc.desc_issued list head and the second descriptor on the list. Doing so
>> results in resources leakege, as vchan_dma_desc_free_list() won't be
>> able to properly free memory resources attached to descriptors,
>> resulting in dma_pool_destroy() failure.
>>
>> Don't call vchan_dma_desc_free_list() from within xdma_terminate_all().
>> Move all terminated descriptors to the vc.desc_terminated list instead.
>> This allows to postpone freeing memory resources associated with
>> descriptors until the call to vchan_synchronize(), which is called from
>> xdma_synchronize() callback. This is the right way to do it -
>> xdma_terminate_all() should return as soon as possible, while freeing
>> resources (that may be time consuming in case of large number of
>> descriptors) can be done safely later.
>>
>> Fixes: 290bb5d2d1e2
>> ("dmaengine: xilinx: xdma: Add terminate_all/synchronize callbacks")
>>
>> Signed-off-by: Jan Kuliga <jankul@alatek.krakow.pl>
>> ---
>>  drivers/dma/xilinx/xdma.c | 32 ++++++++++++++++----------------
>>  1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
>> index 1bce48e5d86c..521ba2a653b6 100644
>> --- a/drivers/dma/xilinx/xdma.c
>> +++ b/drivers/dma/xilinx/xdma.c
>> @@ -379,20 +379,20 @@ static int xdma_xfer_start(struct xdma_chan *xchan)
>>   */
>>  static int xdma_xfer_stop(struct xdma_chan *xchan)
>>  {
>> -	struct virt_dma_desc *vd = vchan_next_desc(&xchan->vchan);
>> -	struct xdma_device *xdev = xchan->xdev_hdl;
>>  	int ret;
>> -
>> -	if (!vd || !xchan->busy)
>> -		return -EINVAL;
>> +	u32 val;
>> +	struct xdma_device *xdev = xchan->xdev_hdl;
>>
>>  	/* clear run stop bit to prevent any further auto-triggering */
>>  	ret = regmap_write(xdev->rmap, xchan->base + XDMA_CHAN_CONTROL_W1C,
>> -			   CHAN_CTRL_RUN_STOP);
>> +							CHAN_CTRL_RUN_STOP);
> 
> Why this change, checkpatch would tell you this is not expected
> alignment (run with strict)
Actually, it does not. I've run it like this:
$LINUX_DIR/scripts/checkpatch.pl --strict -g <commit-id>

and it produced no output related to this line. Anyway, I've already prepared v5 patchset, that conforms to your hint:
Message-Id: 20231218113904.9071-1-jankul@alatek.krakow.pl

> 
>>  	i.f (ret)
>>  		return ret;
>>
>> -	xchan->busy = false;
>> +	/* Clear the channel status register */
>> +	ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &val);
>> +	if (ret)
>> +		return ret;
>>
>>  	return 0;
>>  }
>> @@ -505,25 +505,25 @@ static void xdma_issue_pending(struct dma_chan *chan)
>>  static int xdma_terminate_all(struct dma_chan *chan)
>>  {
>>  	struct xdma_chan *xdma_chan = to_xdma_chan(chan);
>> -	struct xdma_desc *desc = NULL;
>>  	struct virt_dma_desc *vd;
>>  	unsigned long flags;
>>  	LIST_HEAD(head);
>>
>> -	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
>>  	xdma_xfer_stop(xdma_chan);
>>
>> +	spin_lock_irqsave(&xdma_chan->vchan.lock, flags);
>> +
>> +	xdma_chan->busy = false;
>>  	vd = vchan_next_desc(&xdma_chan->vchan);
>> -	if (vd)
>> -		desc = to_xdma_desc(vd);
>> -	if (desc) {
>> -		dma_cookie_complete(&desc->vdesc.tx);
>> -		vchan_terminate_vdesc(&desc->vdesc);
>> +	if (vd) {
>> +		list_del(&vd->node);
>> +		dma_cookie_complete(&vd->tx);
>> +		vchan_terminate_vdesc(vd);
>>  	}
>> -
>>  	vchan_get_all_descriptors(&xdma_chan->vchan, &head);
>> +	list_splice_tail(&head, &xdma_chan->vchan.desc_terminated);
>> +
>>  	spin_unlock_irqrestore(&xdma_chan->vchan.lock, flags);
>> -	vchan_dma_desc_free_list(&xdma_chan->vchan, &head);
>>
>>  	return 0;
>>  }
>> --
>> 2.34.1
> 

Thanks,
Jan