mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support
@ 2026-01-20 13:33 Claudiu
  2026-01-20 13:33 ` [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists Claudiu
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds tx_status and pause/resume support for the rz-dmac driver.
Along with it were added fixes and improvements identified while working
on the above mentioned enhancements.

Previous versions were addressed by Biju. The previous versions were
posted here:

v4: https://lore.kernel.org/all/20240628151728.84470-1-biju.das.jz@bp.renesas.com/
v3: https://lore.kernel.org/all/20230412152445.117439-1-biju.das.jz@bp.renesas.com/
v2: https://lore.kernel.org/all/20230405140842.201883-1-biju.das.jz@bp.renesas.com/
v1: https://lore.kernel.org/all/20230324094957.115071-1-biju.das.jz@bp.renesas.com/

Changes in v8:
- rebased on top of https://lore.kernel.org/all/20260105114445.878262-1-cosmin-gabriel.tanislav.xa@renesas.com/
- populated engine->residue_granularity in patch 7/8
- report proper residue in case the channel is paused in patch 8/8

Changes in v7:
- adjusted the pause/resume support
- collected tags

Changes in v6:
- added patches:
-- dmaengine: sh: rz-dmac: Drop read of CHCTRL register
-- dmaengine: sh: rz-dmac: Drop goto instruction and label
- use vc lock in IRQ handler only for the error path
- fixed typos
- adjusted patch
  "dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks"

Changes in v5:
- added patches
-- dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
-- dmaengine: sh: rz-dmac: Protect the driver specific lists
-- dmaengine: sh: rz-dmac: Move all CHCTRL updates under spinlock
-- dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
-- dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
-- dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
- for pause/resume used the DMA controller support to pause/resume
  transfers compared with previous versions
- adjusted patches:
-- dmaengine: sh: rz-dmac: Add device_tx_status() callback

Thank you,
Claudiu

Biju Das (2):
  dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
  dmaengine: sh: rz-dmac: Add device_tx_status() callback

Claudiu Beznea (6):
  dmaengine: sh: rz-dmac: Protect the driver specific lists
  dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock
  dmaengine: sh: rz-dmac: Drop read of CHCTRL register
  dmaengine: sh: rz-dmac: Drop goto instruction and label
  dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
  dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

 drivers/dma/sh/rz-dmac.c | 289 ++++++++++++++++++++++++++++++++-------
 1 file changed, 239 insertions(+), 50 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:17   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock Claudiu
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea, stable

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The driver lists (ld_free, ld_queue) are used in
rz_dmac_free_chan_resources(), rz_dmac_terminate_all(),
rz_dmac_issue_pending(), and rz_dmac_irq_handler_thread(), all under
the virtual channel lock. Take the same lock in rz_dmac_prep_slave_sg()
and rz_dmac_prep_dma_memcpy() as well to avoid concurrency issues, since
these functions also check whether the lists are empty and update or
remove list entries.

Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
Cc: stable@vger.kernel.org
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- none

Changes in v6:
- none

Changes in v5:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 57 ++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 3dde4b006bcc..36f5fc80a17a 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -10,6 +10,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/interrupt.h>
@@ -453,6 +454,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
 		if (!desc)
 			break;
 
+		/* No need to lock. This is called only for the 1st client. */
 		list_add_tail(&desc->node, &channel->ld_free);
 		channel->descs_allocated++;
 	}
@@ -508,18 +510,21 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 	dev_dbg(dmac->dev, "%s channel: %d src=0x%pad dst=0x%pad len=%zu\n",
 		__func__, channel->index, &src, &dest, len);
 
-	if (list_empty(&channel->ld_free))
-		return NULL;
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		if (list_empty(&channel->ld_free))
+			return NULL;
+
+		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
 
-	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
+		desc->type = RZ_DMAC_DESC_MEMCPY;
+		desc->src = src;
+		desc->dest = dest;
+		desc->len = len;
+		desc->direction = DMA_MEM_TO_MEM;
 
-	desc->type = RZ_DMAC_DESC_MEMCPY;
-	desc->src = src;
-	desc->dest = dest;
-	desc->len = len;
-	desc->direction = DMA_MEM_TO_MEM;
+		list_move_tail(channel->ld_free.next, &channel->ld_queue);
+	}
 
-	list_move_tail(channel->ld_free.next, &channel->ld_queue);
 	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
 }
 
@@ -535,27 +540,29 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 	int dma_length = 0;
 	int i = 0;
 
-	if (list_empty(&channel->ld_free))
-		return NULL;
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		if (list_empty(&channel->ld_free))
+			return NULL;
 
-	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
+		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
 
-	for_each_sg(sgl, sg, sg_len, i) {
-		dma_length += sg_dma_len(sg);
-	}
+		for_each_sg(sgl, sg, sg_len, i)
+			dma_length += sg_dma_len(sg);
 
-	desc->type = RZ_DMAC_DESC_SLAVE_SG;
-	desc->sg = sgl;
-	desc->sgcount = sg_len;
-	desc->len = dma_length;
-	desc->direction = direction;
+		desc->type = RZ_DMAC_DESC_SLAVE_SG;
+		desc->sg = sgl;
+		desc->sgcount = sg_len;
+		desc->len = dma_length;
+		desc->direction = direction;
 
-	if (direction == DMA_DEV_TO_MEM)
-		desc->src = channel->src_per_address;
-	else
-		desc->dest = channel->dst_per_address;
+		if (direction == DMA_DEV_TO_MEM)
+			desc->src = channel->src_per_address;
+		else
+			desc->dest = channel->dst_per_address;
+
+		list_move_tail(channel->ld_free.next, &channel->ld_queue);
+	}
 
-	list_move_tail(channel->ld_free.next, &channel->ld_queue);
 	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
  2026-01-20 13:33 ` [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:19   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register Claudiu
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea, stable

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Both rz_dmac_disable_hw() and rz_dmac_irq_handle_channel() update the
CHCTRL register. To avoid concurrency issues when configuring
functionalities exposed by this registers, take the virtual channel lock.
All other CHCTRL updates were already protected by the same lock.

Previously, rz_dmac_disable_hw() disabled and re-enabled local IRQs, before
accessing CHCTRL registers but this does not ensure race-free access.
Remove the local IRQ disable/enable code as well.

Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
Cc: stable@vger.kernel.org
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- collected tags

Changes in v6:
- update patch title and description
- in rz_dmac_irq_handle_channel() lock only around the
  updates for the error path and continued using the vc lock
  as this is the error path and the channel will anyway be
  stopped; this avoids updating the code with another lock
  as it was suggested in the review process of v5 and the code
  remain simpler for a fix, w/o any impact on performance

Changes in v5:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 36f5fc80a17a..c0f1e77996bd 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -304,13 +304,10 @@ static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
 {
 	struct dma_chan *chan = &channel->vc.chan;
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	unsigned long flags;
 
 	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
 
-	local_irq_save(flags);
 	rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
-	local_irq_restore(flags);
 }
 
 static void rz_dmac_set_dmars_register(struct rz_dmac *dmac, int nr, u32 dmars)
@@ -574,8 +571,8 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
 	unsigned int i;
 	LIST_HEAD(head);
 
-	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+	rz_dmac_disable_hw(channel);
 	for (i = 0; i < DMAC_NR_LMDESC; i++)
 		lmdesc[i].header = 0;
 
@@ -706,7 +703,9 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 	if (chstat & CHSTAT_ER) {
 		dev_err(dmac->dev, "DMAC err CHSTAT_%d = %08X\n",
 			channel->index, chstat);
-		rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
+
+		scoped_guard(spinlock_irqsave, &channel->vc.lock)
+			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
 		goto done;
 	}
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
  2026-01-20 13:33 ` [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists Claudiu
  2026-01-20 13:33 ` [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:20   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label Claudiu
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The CHCTRL register has 11 bits that can be updated by software. The
documentation for all these bits states the following:
- A read operation results in 0 being read
- Writing zero does not affect the operation

All bits in the CHCTRL register accessible by software are set and clear
bits.

The documentation for the CLREND bit of CHCTRL states:
Setting this bit to 1 can clear the END bit of the CHSTAT_n/nS register.
Also, the DMA transfer end interrupt is cleared. An attempt to read this
bit results in 0 being read.
1: Clears the END bit.
0: Does not affect the operation.

Since writing zero to any bit in this register does not affect controller
operation and reads always return zero, there is no need to perform
read-modify-write accesses to set the CLREND bit. Drop the read of the
CHCTRL register.

Also, since setting the CLREND bit does not interact with other
functionalities exposed through this register and only clears the END
interrupt, there is no need to lock around this operation. Add a comment
to document this.

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- collected tags

Changes in v6:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index c0f1e77996bd..bb9ca19cf784 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -697,7 +697,7 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 {
 	struct dma_chan *chan = &channel->vc.chan;
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	u32 chstat, chctrl;
+	u32 chstat;
 
 	chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
 	if (chstat & CHSTAT_ER) {
@@ -709,8 +709,11 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 		goto done;
 	}
 
-	chctrl = rz_dmac_ch_readl(channel, CHCTRL, 1);
-	rz_dmac_ch_writel(channel, chctrl | CHCTRL_CLREND, CHCTRL, 1);
+	/*
+	 * No need to lock. This just clears the END interrupt. Writing
+	 * zeros to CHCTRL is just ignored by HW.
+	 */
+	rz_dmac_ch_writel(channel, CHCTRL_CLREND, CHCTRL, 1);
 done:
 	return;
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (2 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:23   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call Claudiu
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

There is no need to jump to the done label just to return.
Return immediately.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- none

Changes in v6:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index bb9ca19cf784..cc540b35dc29 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -706,7 +706,7 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 
 		scoped_guard(spinlock_irqsave, &channel->vc.lock)
 			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
-		goto done;
+		return;
 	}
 
 	/*
@@ -714,8 +714,6 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
 	 * zeros to CHCTRL is just ignored by HW.
 	 */
 	rz_dmac_ch_writel(channel, CHCTRL_CLREND, CHCTRL, 1);
-done:
-	return;
 }
 
 static irqreturn_t rz_dmac_irq_handler(int irq, void *dev_id)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (3 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:23   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc() Claudiu
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

rz_dmac_enable_hw() calls local_irq_save()/local_irq_restore(), but
this is not needed because the callers of rz_dmac_enable_hw() already
protect the critical section using
spin_lock_irqsave()/spin_lock_irqrestore().

Remove the local_irq_save()/local_irq_restore() calls.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- none

Changes in v6:
- none

Changes in v5:
- none, this patch is new

 drivers/dma/sh/rz-dmac.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index cc540b35dc29..1d2b50d6273b 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -273,15 +273,12 @@ static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)
 {
 	struct dma_chan *chan = &channel->vc.chan;
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	unsigned long flags;
 	u32 nxla;
 	u32 chctrl;
 	u32 chstat;
 
 	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
 
-	local_irq_save(flags);
-
 	rz_dmac_lmdesc_recycle(channel);
 
 	nxla = channel->lmdesc.base_dma +
@@ -296,8 +293,6 @@ static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)
 		rz_dmac_ch_writel(channel, CHCTRL_SWRST, CHCTRL, 1);
 		rz_dmac_ch_writel(channel, chctrl, CHCTRL, 1);
 	}
-
-	local_irq_restore(flags);
 }
 
 static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (4 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:26   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback Claudiu
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Biju Das <biju.das.jz@bp.renesas.com>

Add rz_dmac_invalidate_lmdesc() so that the same code can be shared
between rz_dmac_terminate_all() and rz_dmac_free_chan_resources().

Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
[claudiu.beznea: adjusted the commit description; defined the lmdesc
 inside the for block to have more compact code]
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- none

Changes in v7:
- none

Changes in v6:
- none

Changes in v5:
- adjusted the commit description
- defined the lmdesc inside the for block

 drivers/dma/sh/rz-dmac.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 1d2b50d6273b..4602f8b7408a 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -256,6 +256,13 @@ static void rz_lmdesc_setup(struct rz_dmac_chan *channel,
  * Descriptors preparation
  */
 
+static void rz_dmac_invalidate_lmdesc(struct rz_dmac_chan *channel)
+{
+	for (struct rz_lmdesc *lmdesc = channel->lmdesc.base;
+	     lmdesc < channel->lmdesc.base + DMAC_NR_LMDESC; lmdesc++)
+		lmdesc->header = 0;
+}
+
 static void rz_dmac_lmdesc_recycle(struct rz_dmac_chan *channel)
 {
 	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
@@ -461,15 +468,12 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	struct rz_dmac_desc *desc, *_desc;
 	unsigned long flags;
-	unsigned int i;
 
 	spin_lock_irqsave(&channel->vc.lock, flags);
 
-	for (i = 0; i < DMAC_NR_LMDESC; i++)
-		lmdesc[i].header = 0;
+	rz_dmac_invalidate_lmdesc(channel);
 
 	rz_dmac_disable_hw(channel);
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
@@ -561,15 +565,12 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 static int rz_dmac_terminate_all(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
-	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	unsigned long flags;
-	unsigned int i;
 	LIST_HEAD(head);
 
 	spin_lock_irqsave(&channel->vc.lock, flags);
 	rz_dmac_disable_hw(channel);
-	for (i = 0; i < DMAC_NR_LMDESC; i++)
-		lmdesc[i].header = 0;
+	rz_dmac_invalidate_lmdesc(channel);
 
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
 	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (5 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc() Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:43   ` Frank Li
  2026-01-20 13:33 ` [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks Claudiu
  2026-01-29  9:44 ` [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu Beznea
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Biju Das <biju.das.jz@bp.renesas.com>

Add support for device_tx_status() callback as it is needed for
RZ/G2L SCIFA driver.

Based on a patch in the BSP similar to rcar-dmac by
Long Luu <long.luu.ur@renesas.com>.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
[claudiu.beznea:
 - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
   pointer to advance
 - use 'lmdesc->nxla != crla' comparison instead of
   '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
 - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
   to verify if the full lmdesc list was checked
 - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
 - re-arranged comments so they span fewer lines and are wrapped to ~80
   characters
 - use u32 for the residue value and the functions returning it
 - use u32 for the variables storing register values
 - fixed typos]
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- populated engine->residue_granularity

Changes in v7:
- none

Changes in v6:
- s/byte/bytes in comment from rz_dmac_chan_get_residue()

Changes in v5:
- post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
  pointer to advance
- use 'lmdesc->nxla != crla' comparison instead of
  '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
- in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
  to verify if the full lmdesc list was checked
- drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
- re-arranged comments so they span fewer lines and are wrapped to ~80
  characters
- use u32 for the residue value and the functions returning it
- use u32 for the variables storing register values
- fixed typos

 drivers/dma/sh/rz-dmac.c | 145 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 144 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 4602f8b7408a..27c963083e29 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -125,10 +125,12 @@ struct rz_dmac {
  * Registers
  */
 
+#define CRTB				0x0020
 #define CHSTAT				0x0024
 #define CHCTRL				0x0028
 #define CHCFG				0x002c
 #define NXLA				0x0038
+#define CRLA				0x003c
 
 #define DCTRL				0x0000
 
@@ -684,6 +686,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
 	rz_dmac_set_dma_req_no(dmac, channel->index, dmac->info->default_dma_req_no);
 }
 
+static struct rz_lmdesc *
+rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct rz_lmdesc *lmdesc)
+{
+	struct rz_lmdesc *next = ++lmdesc;
+
+	if (next >= base + DMAC_NR_LMDESC)
+		next = base;
+
+	return next;
+}
+
+static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
+{
+	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
+	struct dma_chan *chan = &channel->vc.chan;
+	struct rz_dmac *dmac = to_rz_dmac(chan->device);
+	u32 residue = 0, crla, i = 0;
+
+	crla = rz_dmac_ch_readl(channel, CRLA, 1);
+	while (lmdesc->nxla != crla) {
+		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
+		if (++i >= DMAC_NR_LMDESC)
+			return 0;
+	}
+
+	/* Calculate residue from next lmdesc to end of virtual desc */
+	while (lmdesc->chcfg & CHCFG_DEM) {
+		residue += lmdesc->tb;
+		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
+	}
+
+	dev_dbg(dmac->dev, "%s: VD residue is %u\n", __func__, residue);
+
+	return residue;
+}
+
+static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
+				    dma_cookie_t cookie)
+{
+	struct rz_dmac_desc *current_desc, *desc;
+	enum dma_status status;
+	u32 crla, crtb, i;
+
+	/* Get current processing virtual descriptor */
+	current_desc = list_first_entry(&channel->ld_active,
+					struct rz_dmac_desc, node);
+	if (!current_desc)
+		return 0;
+
+	/*
+	 * If the cookie corresponds to a descriptor that has been completed
+	 * there is no residue. The same check has already been performed by the
+	 * caller but without holding the channel lock, so the descriptor could
+	 * now be complete.
+	 */
+	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
+	if (status == DMA_COMPLETE)
+		return 0;
+
+	/*
+	 * If the cookie doesn't correspond to the currently processing virtual
+	 * descriptor then the descriptor hasn't been processed yet, and the
+	 * residue is equal to the full descriptor size. Also, a client driver
+	 * is possible to call this function before rz_dmac_irq_handler_thread()
+	 * runs. In this case, the running descriptor will be the next
+	 * descriptor, and will appear in the done list. So, if the argument
+	 * cookie matches the done list's cookie, we can assume the residue is
+	 * zero.
+	 */
+	if (cookie != current_desc->vd.tx.cookie) {
+		list_for_each_entry(desc, &channel->ld_free, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return 0;
+		}
+
+		list_for_each_entry(desc, &channel->ld_queue, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return desc->len;
+		}
+
+		list_for_each_entry(desc, &channel->ld_active, node) {
+			if (cookie == desc->vd.tx.cookie)
+				return desc->len;
+		}
+
+		/*
+		 * No descriptor found for the cookie, there's thus no residue.
+		 * This shouldn't happen if the calling driver passes a correct
+		 * cookie value.
+		 */
+		WARN(1, "No descriptor for cookie!");
+		return 0;
+	}
+
+	/*
+	 * We need to read two registers. Make sure the hardware does not move
+	 * to next lmdesc while reading the current lmdesc. Trying it 3 times
+	 * should be enough: initial read, retry, retry for the paranoid.
+	 */
+	for (i = 0; i < 3; i++) {
+		crla = rz_dmac_ch_readl(channel, CRLA, 1);
+		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
+		/* Still the same? */
+		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
+			break;
+	}
+
+	WARN_ONCE(i >= 3, "residue might not be continuous!");
+
+	/*
+	 * Calculate number of bytes transferred in processing virtual descriptor.
+	 * One virtual descriptor can have many lmdesc.
+	 */
+	return crtb + rz_dmac_calculate_residue_bytes_in_vd(channel);
+}
+
+static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
+					 dma_cookie_t cookie,
+					 struct dma_tx_state *txstate)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	enum dma_status status;
+	u32 residue;
+
+	status = dma_cookie_status(chan, cookie, txstate);
+	if (status == DMA_COMPLETE || !txstate)
+		return status;
+
+	scoped_guard(spinlock_irqsave, &channel->vc.lock)
+		residue = rz_dmac_chan_get_residue(channel, cookie);
+
+	/* if there's no residue, the cookie is complete */
+	if (!residue)
+		return DMA_COMPLETE;
+
+	dma_set_residue(txstate, residue);
+
+	return status;
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -1007,6 +1149,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	engine = &dmac->engine;
 	dma_cap_set(DMA_SLAVE, engine->cap_mask);
 	dma_cap_set(DMA_MEMCPY, engine->cap_mask);
+	engine->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
 	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_0_7_COMMON_BASE + DCTRL);
 	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_8_15_COMMON_BASE + DCTRL);
 
@@ -1014,7 +1157,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
 
 	engine->device_alloc_chan_resources = rz_dmac_alloc_chan_resources;
 	engine->device_free_chan_resources = rz_dmac_free_chan_resources;
-	engine->device_tx_status = dma_cookie_status;
+	engine->device_tx_status = rz_dmac_tx_status;
 	engine->device_prep_slave_sg = rz_dmac_prep_slave_sg;
 	engine->device_prep_dma_memcpy = rz_dmac_prep_dma_memcpy;
 	engine->device_config = rz_dmac_config;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (6 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback Claudiu
@ 2026-01-20 13:33 ` Claudiu
  2026-02-25 16:49   ` Frank Li
  2026-01-29  9:44 ` [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu Beznea
  8 siblings, 1 reply; 21+ messages in thread
From: Claudiu @ 2026-01-20 13:33 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: claudiu.beznea, dmaengine, linux-kernel, Claudiu Beznea

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Add support for device_{pause, resume}() callbacks. These are required by
the RZ/G2L SCIFA driver.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v8:
- reported residue for paused channels as well

Changes in v7:
- use guard() instead of scoped_guard()
- in rz_dmac_device_pause() checked the channel is enabled
  before suspending it to avoid read poll timeouts
- added a comment in rz_dmac_device_resume()

Changes in v6:
- set CHCTRL_SETSUS for pause and CHCTRL_CLRSUS for resume
- dropped read-modify-update approach for CHCTRL updates as the
  HW returns zero when reading CHCTRL
- moved the read_poll_timeout_atomic() under spin lock to
  ensure avoid any races b/w pause and resume functionalities

Changes in v5:
- used suspend capability of the controller to pause/resume
  the transfers

 drivers/dma/sh/rz-dmac.c | 49 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 27c963083e29..caa3335bf95d 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -141,10 +141,12 @@ struct rz_dmac {
 #define CHANNEL_8_15_COMMON_BASE	0x0700
 
 #define CHSTAT_ER			BIT(4)
+#define CHSTAT_SUS			BIT(3)
 #define CHSTAT_EN			BIT(0)
 
 #define CHCTRL_CLRINTMSK		BIT(17)
 #define CHCTRL_CLRSUS			BIT(9)
+#define CHCTRL_SETSUS			BIT(8)
 #define CHCTRL_CLRTC			BIT(6)
 #define CHCTRL_CLREND			BIT(5)
 #define CHCTRL_CLRRQ			BIT(4)
@@ -814,11 +816,18 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
 	if (status == DMA_COMPLETE || !txstate)
 		return status;
 
-	scoped_guard(spinlock_irqsave, &channel->vc.lock)
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		u32 val;
+
 		residue = rz_dmac_chan_get_residue(channel, cookie);
 
-	/* if there's no residue, the cookie is complete */
-	if (!residue)
+		val = rz_dmac_ch_readl(channel, CHSTAT, 1);
+		if (val & CHSTAT_SUS)
+			status = DMA_PAUSED;
+	}
+
+	/* if there's no residue and no paused, the cookie is complete */
+	if (!residue && status != DMA_PAUSED)
 		return DMA_COMPLETE;
 
 	dma_set_residue(txstate, residue);
@@ -826,6 +835,38 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
 	return status;
 }
 
+static int rz_dmac_device_pause(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	u32 val;
+
+	guard(spinlock_irqsave)(&channel->vc.lock);
+
+	val = rz_dmac_ch_readl(channel, CHSTAT, 1);
+	if (!(val & CHSTAT_EN))
+		return 0;
+
+	rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
+	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
+					(val & CHSTAT_SUS), 1, 1024,
+					false, channel, CHSTAT, 1);
+}
+
+static int rz_dmac_device_resume(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	u32 val;
+
+	guard(spinlock_irqsave)(&channel->vc.lock);
+
+	/* Do not check CHSTAT_SUS but rely on HW capabilities. */
+
+	rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
+	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
+					!(val & CHSTAT_SUS), 1, 1024,
+					false, channel, CHSTAT, 1);
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -1164,6 +1205,8 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	engine->device_terminate_all = rz_dmac_terminate_all;
 	engine->device_issue_pending = rz_dmac_issue_pending;
 	engine->device_synchronize = rz_dmac_device_synchronize;
+	engine->device_pause = rz_dmac_device_pause;
+	engine->device_resume = rz_dmac_device_resume;
 
 	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
 	dma_set_max_seg_size(engine->dev, U32_MAX);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support
  2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
                   ` (7 preceding siblings ...)
  2026-01-20 13:33 ` [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks Claudiu
@ 2026-01-29  9:44 ` Claudiu Beznea
  8 siblings, 0 replies; 21+ messages in thread
From: Claudiu Beznea @ 2026-01-29  9:44 UTC (permalink / raw)
  To: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj
  Cc: dmaengine, linux-kernel, Claudiu Beznea

Hi,

Gentle ping on this series.

Thank you,
Claudiu

On 1/20/26 15:33, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Hi,
> 
> Series adds tx_status and pause/resume support for the rz-dmac driver.
> Along with it were added fixes and improvements identified while working
> on the above mentioned enhancements.
> 
> Previous versions were addressed by Biju. The previous versions were
> posted here:
> 
> v4: https://lore.kernel.org/all/20240628151728.84470-1-biju.das.jz@bp.renesas.com/
> v3: https://lore.kernel.org/all/20230412152445.117439-1-biju.das.jz@bp.renesas.com/
> v2: https://lore.kernel.org/all/20230405140842.201883-1-biju.das.jz@bp.renesas.com/
> v1: https://lore.kernel.org/all/20230324094957.115071-1-biju.das.jz@bp.renesas.com/
> 
> Changes in v8:
> - rebased on top of https://lore.kernel.org/all/20260105114445.878262-1-cosmin-gabriel.tanislav.xa@renesas.com/
> - populated engine->residue_granularity in patch 7/8
> - report proper residue in case the channel is paused in patch 8/8
> 
> Changes in v7:
> - adjusted the pause/resume support
> - collected tags
> 
> Changes in v6:
> - added patches:
> -- dmaengine: sh: rz-dmac: Drop read of CHCTRL register
> -- dmaengine: sh: rz-dmac: Drop goto instruction and label
> - use vc lock in IRQ handler only for the error path
> - fixed typos
> - adjusted patch
>    "dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks"
> 
> Changes in v5:
> - added patches
> -- dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
> -- dmaengine: sh: rz-dmac: Protect the driver specific lists
> -- dmaengine: sh: rz-dmac: Move all CHCTRL updates under spinlock
> -- dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
> -- dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
> -- dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
> - for pause/resume used the DMA controller support to pause/resume
>    transfers compared with previous versions
> - adjusted patches:
> -- dmaengine: sh: rz-dmac: Add device_tx_status() callback
> 
> Thank you,
> Claudiu
> 
> Biju Das (2):
>    dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
>    dmaengine: sh: rz-dmac: Add device_tx_status() callback
> 
> Claudiu Beznea (6):
>    dmaengine: sh: rz-dmac: Protect the driver specific lists
>    dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock
>    dmaengine: sh: rz-dmac: Drop read of CHCTRL register
>    dmaengine: sh: rz-dmac: Drop goto instruction and label
>    dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
>    dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
> 
>   drivers/dma/sh/rz-dmac.c | 289 ++++++++++++++++++++++++++++++++-------
>   1 file changed, 239 insertions(+), 50 deletions(-)
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists
  2026-01-20 13:33 ` [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists Claudiu
@ 2026-02-25 16:17   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:17 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea, stable

On Tue, Jan 20, 2026 at 03:33:23PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The driver lists (ld_free, ld_queue) are used in
> rz_dmac_free_chan_resources(), rz_dmac_terminate_all(),
> rz_dmac_issue_pending(), and rz_dmac_irq_handler_thread(), all under
> the virtual channel lock. Take the same lock in rz_dmac_prep_slave_sg()
> and rz_dmac_prep_dma_memcpy() as well to avoid concurrency issues, since
> these functions also check whether the lists are empty and update or
> remove list entries.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 57 ++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 3dde4b006bcc..36f5fc80a17a 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  #include <linux/interrupt.h>
> @@ -453,6 +454,7 @@ static int rz_dmac_alloc_chan_resources(struct dma_chan *chan)
>  		if (!desc)
>  			break;
>
> +		/* No need to lock. This is called only for the 1st client. */
>  		list_add_tail(&desc->node, &channel->ld_free);
>  		channel->descs_allocated++;
>  	}
> @@ -508,18 +510,21 @@ rz_dmac_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>  	dev_dbg(dmac->dev, "%s channel: %d src=0x%pad dst=0x%pad len=%zu\n",
>  		__func__, channel->index, &src, &dest, len);
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
> +
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc->type = RZ_DMAC_DESC_MEMCPY;
> +		desc->src = src;
> +		desc->dest = dest;
> +		desc->len = len;
> +		desc->direction = DMA_MEM_TO_MEM;
>
> -	desc->type = RZ_DMAC_DESC_MEMCPY;
> -	desc->src = src;
> -	desc->dest = dest;
> -	desc->len = len;
> -	desc->direction = DMA_MEM_TO_MEM;
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> @@ -535,27 +540,29 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  	int dma_length = 0;
>  	int i = 0;
>
> -	if (list_empty(&channel->ld_free))
> -		return NULL;
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		if (list_empty(&channel->ld_free))
> +			return NULL;
>
> -	desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
> +		desc = list_first_entry(&channel->ld_free, struct rz_dmac_desc, node);
>
> -	for_each_sg(sgl, sg, sg_len, i) {
> -		dma_length += sg_dma_len(sg);
> -	}
> +		for_each_sg(sgl, sg, sg_len, i)
> +			dma_length += sg_dma_len(sg);
>
> -	desc->type = RZ_DMAC_DESC_SLAVE_SG;
> -	desc->sg = sgl;
> -	desc->sgcount = sg_len;
> -	desc->len = dma_length;
> -	desc->direction = direction;
> +		desc->type = RZ_DMAC_DESC_SLAVE_SG;
> +		desc->sg = sgl;
> +		desc->sgcount = sg_len;
> +		desc->len = dma_length;
> +		desc->direction = direction;
>
> -	if (direction == DMA_DEV_TO_MEM)
> -		desc->src = channel->src_per_address;
> -	else
> -		desc->dest = channel->dst_per_address;
> +		if (direction == DMA_DEV_TO_MEM)
> +			desc->src = channel->src_per_address;
> +		else
> +			desc->dest = channel->dst_per_address;
> +
> +		list_move_tail(channel->ld_free.next, &channel->ld_queue);
> +	}
>
> -	list_move_tail(channel->ld_free.next, &channel->ld_queue);
>  	return vchan_tx_prep(&channel->vc, &desc->vd, flags);
>  }
>
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock
  2026-01-20 13:33 ` [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock Claudiu
@ 2026-02-25 16:19   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:19 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea, stable

On Tue, Jan 20, 2026 at 03:33:24PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Both rz_dmac_disable_hw() and rz_dmac_irq_handle_channel() update the
> CHCTRL register. To avoid concurrency issues when configuring
> functionalities exposed by this registers, take the virtual channel lock.
> All other CHCTRL updates were already protected by the same lock.
>
> Previously, rz_dmac_disable_hw() disabled and re-enabled local IRQs, before
> accessing CHCTRL registers but this does not ensure race-free access.
> Remove the local IRQ disable/enable code as well.
>
> Fixes: 5000d37042a6 ("dmaengine: sh: Add DMAC driver for RZ/G2L SoC")
> Cc: stable@vger.kernel.org
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - collected tags
>
> Changes in v6:
> - update patch title and description
> - in rz_dmac_irq_handle_channel() lock only around the
>   updates for the error path and continued using the vc lock
>   as this is the error path and the channel will anyway be
>   stopped; this avoids updating the code with another lock
>   as it was suggested in the review process of v5 and the code
>   remain simpler for a fix, w/o any impact on performance
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 36f5fc80a17a..c0f1e77996bd 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -304,13 +304,10 @@ static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
>  {
>  	struct dma_chan *chan = &channel->vc.chan;
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	unsigned long flags;
>
>  	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
>
> -	local_irq_save(flags);
>  	rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> -	local_irq_restore(flags);
>  }
>
>  static void rz_dmac_set_dmars_register(struct rz_dmac *dmac, int nr, u32 dmars)
> @@ -574,8 +571,8 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
>  	unsigned int i;
>  	LIST_HEAD(head);
>
> -	rz_dmac_disable_hw(channel);
>  	spin_lock_irqsave(&channel->vc.lock, flags);
> +	rz_dmac_disable_hw(channel);
>  	for (i = 0; i < DMAC_NR_LMDESC; i++)
>  		lmdesc[i].header = 0;
>
> @@ -706,7 +703,9 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  	if (chstat & CHSTAT_ER) {
>  		dev_err(dmac->dev, "DMAC err CHSTAT_%d = %08X\n",
>  			channel->index, chstat);
> -		rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> +
> +		scoped_guard(spinlock_irqsave, &channel->vc.lock)
> +			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
>  		goto done;
>  	}
>
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register
  2026-01-20 13:33 ` [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register Claudiu
@ 2026-02-25 16:20   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:20 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:25PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The CHCTRL register has 11 bits that can be updated by software. The
> documentation for all these bits states the following:
> - A read operation results in 0 being read
> - Writing zero does not affect the operation
>
> All bits in the CHCTRL register accessible by software are set and clear
> bits.
>
> The documentation for the CLREND bit of CHCTRL states:
> Setting this bit to 1 can clear the END bit of the CHSTAT_n/nS register.
> Also, the DMA transfer end interrupt is cleared. An attempt to read this
> bit results in 0 being read.
> 1: Clears the END bit.
> 0: Does not affect the operation.
>
> Since writing zero to any bit in this register does not affect controller
> operation and reads always return zero, there is no need to perform
> read-modify-write accesses to set the CLREND bit. Drop the read of the
> CHCTRL register.
>
> Also, since setting the CLREND bit does not interact with other
> functionalities exposed through this register and only clears the END
> interrupt, there is no need to lock around this operation. Add a comment
> to document this.
>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - collected tags
>
> Changes in v6:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index c0f1e77996bd..bb9ca19cf784 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -697,7 +697,7 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  {
>  	struct dma_chan *chan = &channel->vc.chan;
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	u32 chstat, chctrl;
> +	u32 chstat;
>
>  	chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
>  	if (chstat & CHSTAT_ER) {
> @@ -709,8 +709,11 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  		goto done;
>  	}
>
> -	chctrl = rz_dmac_ch_readl(channel, CHCTRL, 1);
> -	rz_dmac_ch_writel(channel, chctrl | CHCTRL_CLREND, CHCTRL, 1);
> +	/*
> +	 * No need to lock. This just clears the END interrupt. Writing
> +	 * zeros to CHCTRL is just ignored by HW.
> +	 */
> +	rz_dmac_ch_writel(channel, CHCTRL_CLREND, CHCTRL, 1);
>  done:
>  	return;
>  }
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label
  2026-01-20 13:33 ` [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label Claudiu
@ 2026-02-25 16:23   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:23 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:26PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> There is no need to jump to the done label just to return.
> Return immediately.

Nit: There is no need to jump to the done label, so return immediately.

I think vnod can change it when apply.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index bb9ca19cf784..cc540b35dc29 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -706,7 +706,7 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>
>  		scoped_guard(spinlock_irqsave, &channel->vc.lock)
>  			rz_dmac_ch_writel(channel, CHCTRL_DEFAULT, CHCTRL, 1);
> -		goto done;
> +		return;
>  	}
>
>  	/*
> @@ -714,8 +714,6 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  	 * zeros to CHCTRL is just ignored by HW.
>  	 */
>  	rz_dmac_ch_writel(channel, CHCTRL_CLREND, CHCTRL, 1);
> -done:
> -	return;
>  }
>
>  static irqreturn_t rz_dmac_irq_handler(int irq, void *dev_id)
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call
  2026-01-20 13:33 ` [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call Claudiu
@ 2026-02-25 16:23   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:23 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:27PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> rz_dmac_enable_hw() calls local_irq_save()/local_irq_restore(), but
> this is not needed because the callers of rz_dmac_enable_hw() already
> protect the critical section using
> spin_lock_irqsave()/spin_lock_irqrestore().
>
> Remove the local_irq_save()/local_irq_restore() calls.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index cc540b35dc29..1d2b50d6273b 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -273,15 +273,12 @@ static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)
>  {
>  	struct dma_chan *chan = &channel->vc.chan;
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	unsigned long flags;
>  	u32 nxla;
>  	u32 chctrl;
>  	u32 chstat;
>
>  	dev_dbg(dmac->dev, "%s channel %d\n", __func__, channel->index);
>
> -	local_irq_save(flags);
> -
>  	rz_dmac_lmdesc_recycle(channel);
>
>  	nxla = channel->lmdesc.base_dma +
> @@ -296,8 +293,6 @@ static void rz_dmac_enable_hw(struct rz_dmac_chan *channel)
>  		rz_dmac_ch_writel(channel, CHCTRL_SWRST, CHCTRL, 1);
>  		rz_dmac_ch_writel(channel, chctrl, CHCTRL, 1);
>  	}
> -
> -	local_irq_restore(flags);
>  }
>
>  static void rz_dmac_disable_hw(struct rz_dmac_chan *channel)
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()
  2026-01-20 13:33 ` [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc() Claudiu
@ 2026-02-25 16:26   ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:26 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:28PM +0200, Claudiu wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> Add rz_dmac_invalidate_lmdesc() so that the same code can be shared
> between rz_dmac_terminate_all() and rz_dmac_free_chan_resources().
>
> Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> [claudiu.beznea: adjusted the commit description; defined the lmdesc
>  inside the for block to have more compact code]
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Changes in v8:
> - none
>
> Changes in v7:
> - none
>
> Changes in v6:
> - none
>
> Changes in v5:
> - adjusted the commit description
> - defined the lmdesc inside the for block
>
>  drivers/dma/sh/rz-dmac.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 1d2b50d6273b..4602f8b7408a 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -256,6 +256,13 @@ static void rz_lmdesc_setup(struct rz_dmac_chan *channel,
>   * Descriptors preparation
>   */
>
> +static void rz_dmac_invalidate_lmdesc(struct rz_dmac_chan *channel)
> +{
> +	for (struct rz_lmdesc *lmdesc = channel->lmdesc.base;
> +	     lmdesc < channel->lmdesc.base + DMAC_NR_LMDESC; lmdesc++)
> +		lmdesc->header = 0;
> +}
> +
>  static void rz_dmac_lmdesc_recycle(struct rz_dmac_chan *channel)
>  {
>  	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> @@ -461,15 +468,12 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
>  {
>  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
>  	struct rz_dmac_desc *desc, *_desc;
>  	unsigned long flags;
> -	unsigned int i;
>
>  	spin_lock_irqsave(&channel->vc.lock, flags);
>
> -	for (i = 0; i < DMAC_NR_LMDESC; i++)
> -		lmdesc[i].header = 0;
> +	rz_dmac_invalidate_lmdesc(channel);
>
>  	rz_dmac_disable_hw(channel);
>  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
> @@ -561,15 +565,12 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  static int rz_dmac_terminate_all(struct dma_chan *chan)
>  {
>  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> -	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
>  	unsigned long flags;
> -	unsigned int i;
>  	LIST_HEAD(head);
>
>  	spin_lock_irqsave(&channel->vc.lock, flags);
>  	rz_dmac_disable_hw(channel);
> -	for (i = 0; i < DMAC_NR_LMDESC; i++)
> -		lmdesc[i].header = 0;
> +	rz_dmac_invalidate_lmdesc(channel);
>
>  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
>  	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback
  2026-01-20 13:33 ` [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback Claudiu
@ 2026-02-25 16:43   ` Frank Li
  2026-03-02 13:49     ` Claudiu Beznea
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:43 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:29PM +0200, Claudiu wrote:
> From: Biju Das <biju.das.jz@bp.renesas.com>
>
> Add support for device_tx_status() callback as it is needed for
> RZ/G2L SCIFA driver.
>
> Based on a patch in the BSP similar to rcar-dmac by
> Long Luu <long.luu.ur@renesas.com>.

If you want to give credit to Long Luu, any public link?

>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> [claudiu.beznea:
>  - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>    pointer to advance
>  - use 'lmdesc->nxla != crla' comparison instead of
>    '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
>  - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>    to verify if the full lmdesc list was checked
>  - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
>  - re-arranged comments so they span fewer lines and are wrapped to ~80
>    characters
>  - use u32 for the residue value and the functions returning it
>  - use u32 for the variables storing register values
>  - fixed typos]

Suppose needn't this section

> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v8:
> - populated engine->residue_granularity
>
> Changes in v7:
> - none
>
> Changes in v6:
> - s/byte/bytes in comment from rz_dmac_chan_get_residue()
>
> Changes in v5:
> - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>   pointer to advance
> - use 'lmdesc->nxla != crla' comparison instead of
>   '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
> - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>   to verify if the full lmdesc list was checked
> - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
> - re-arranged comments so they span fewer lines and are wrapped to ~80
>   characters
> - use u32 for the residue value and the functions returning it
> - use u32 for the variables storing register values
> - fixed typos
>
>  drivers/dma/sh/rz-dmac.c | 145 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 4602f8b7408a..27c963083e29 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -125,10 +125,12 @@ struct rz_dmac {
>   * Registers
>   */
>
> +#define CRTB				0x0020
>  #define CHSTAT				0x0024
>  #define CHCTRL				0x0028
>  #define CHCFG				0x002c
>  #define NXLA				0x0038
> +#define CRLA				0x003c
>
>  #define DCTRL				0x0000
>
> @@ -684,6 +686,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
>  	rz_dmac_set_dma_req_no(dmac, channel->index, dmac->info->default_dma_req_no);
>  }
>
> +static struct rz_lmdesc *
> +rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct rz_lmdesc *lmdesc)
> +{
> +	struct rz_lmdesc *next = ++lmdesc;
> +
> +	if (next >= base + DMAC_NR_LMDESC)
> +		next = base;
> +
> +	return next;
> +}
> +
> +static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
> +{
> +	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> +	struct dma_chan *chan = &channel->vc.chan;
> +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> +	u32 residue = 0, crla, i = 0;
> +
> +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
> +	while (lmdesc->nxla != crla) {
> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
> +		if (++i >= DMAC_NR_LMDESC)
> +			return 0;
> +	}
> +
> +	/* Calculate residue from next lmdesc to end of virtual desc */
> +	while (lmdesc->chcfg & CHCFG_DEM) {
> +		residue += lmdesc->tb;
> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
> +	}

can use one loop

for (int i=0; i<DMAC_NR_LMDESC; i++) {
	if (lmdesc->nxla == crla)
		residue = 0; 	//reset to 0;

	if (lmdesc->chcfg & CHCFG_DEM)
		residue += lmdesc->tb;

	lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
}

return residue;

> +
> +	dev_dbg(dmac->dev, "%s: VD residue is %u\n", __func__, residue);
> +
> +	return residue;
> +}
> +
> +static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
> +				    dma_cookie_t cookie)
> +{
> +	struct rz_dmac_desc *current_desc, *desc;
> +	enum dma_status status;
> +	u32 crla, crtb, i;
> +
> +	/* Get current processing virtual descriptor */
> +	current_desc = list_first_entry(&channel->ld_active,
> +					struct rz_dmac_desc, node);
> +	if (!current_desc)
> +		return 0;
> +
> +	/*
> +	 * If the cookie corresponds to a descriptor that has been completed
> +	 * there is no residue. The same check has already been performed by the
> +	 * caller but without holding the channel lock, so the descriptor could
> +	 * now be complete.
> +	 */
> +	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
> +	if (status == DMA_COMPLETE)
> +		return 0;
> +
> +	/*
> +	 * If the cookie doesn't correspond to the currently processing virtual
> +	 * descriptor then the descriptor hasn't been processed yet, and the
> +	 * residue is equal to the full descriptor size. Also, a client driver
> +	 * is possible to call this function before rz_dmac_irq_handler_thread()
> +	 * runs. In this case, the running descriptor will be the next
> +	 * descriptor, and will appear in the done list. So, if the argument
> +	 * cookie matches the done list's cookie, we can assume the residue is
> +	 * zero.
> +	 */
> +	if (cookie != current_desc->vd.tx.cookie) {
> +		list_for_each_entry(desc, &channel->ld_free, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return 0;
> +		}
> +
> +		list_for_each_entry(desc, &channel->ld_queue, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return desc->len;
> +		}
> +
> +		list_for_each_entry(desc, &channel->ld_active, node) {
> +			if (cookie == desc->vd.tx.cookie)
> +				return desc->len;
> +		}
> +
> +		/*
> +		 * No descriptor found for the cookie, there's thus no residue.
> +		 * This shouldn't happen if the calling driver passes a correct
> +		 * cookie value.
> +		 */
> +		WARN(1, "No descriptor for cookie!");
> +		return 0;
> +	}
> +
> +	/*
> +	 * We need to read two registers. Make sure the hardware does not move
> +	 * to next lmdesc while reading the current lmdesc. Trying it 3 times
> +	 * should be enough: initial read, retry, retry for the paranoid.
> +	 */
> +	for (i = 0; i < 3; i++) {
> +		crla = rz_dmac_ch_readl(channel, CRLA, 1);
> +		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
> +		/* Still the same? */
> +		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
> +			break;
> +	}
> +
> +	WARN_ONCE(i >= 3, "residue might not be continuous!");
> +
> +	/*
> +	 * Calculate number of bytes transferred in processing virtual descriptor.
> +	 * One virtual descriptor can have many lmdesc.
> +	 */
> +	return crtb + rz_dmac_calculate_residue_bytes_in_vd(channel);

you don't use varible 'ctra' here, so retry 3 become useless. suppose
rz_dmac_calculate_residue_bytes_in_vd(channel, ctra)

and avoid rz_dmac_ch_readl(channel, CRLA, 1) in
rz_dmac_calculate_residue_bytes_in_vd() to keep ctra and ctrb reflect the
correct hardware state.

Frank

> +}
> +
> +static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
> +					 dma_cookie_t cookie,
> +					 struct dma_tx_state *txstate)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	enum dma_status status;
> +	u32 residue;
> +
> +	status = dma_cookie_status(chan, cookie, txstate);
> +	if (status == DMA_COMPLETE || !txstate)
> +		return status;
> +
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock)
> +		residue = rz_dmac_chan_get_residue(channel, cookie);
> +
> +	/* if there's no residue, the cookie is complete */
> +	if (!residue)
> +		return DMA_COMPLETE;
> +
> +	dma_set_residue(txstate, residue);
> +
> +	return status;
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -1007,6 +1149,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	engine = &dmac->engine;
>  	dma_cap_set(DMA_SLAVE, engine->cap_mask);
>  	dma_cap_set(DMA_MEMCPY, engine->cap_mask);
> +	engine->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
>  	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_0_7_COMMON_BASE + DCTRL);
>  	rz_dmac_writel(dmac, DCTRL_DEFAULT, CHANNEL_8_15_COMMON_BASE + DCTRL);
>
> @@ -1014,7 +1157,7 @@ static int rz_dmac_probe(struct platform_device *pdev)
>
>  	engine->device_alloc_chan_resources = rz_dmac_alloc_chan_resources;
>  	engine->device_free_chan_resources = rz_dmac_free_chan_resources;
> -	engine->device_tx_status = dma_cookie_status;
> +	engine->device_tx_status = rz_dmac_tx_status;
>  	engine->device_prep_slave_sg = rz_dmac_prep_slave_sg;
>  	engine->device_prep_dma_memcpy = rz_dmac_prep_dma_memcpy;
>  	engine->device_config = rz_dmac_config;
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
  2026-01-20 13:33 ` [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks Claudiu
@ 2026-02-25 16:49   ` Frank Li
  2026-03-02 13:50     ` Claudiu Beznea
  0 siblings, 1 reply; 21+ messages in thread
From: Frank Li @ 2026-02-25 16:49 UTC (permalink / raw)
  To: Claudiu
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Tue, Jan 20, 2026 at 03:33:30PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Add support for device_{pause, resume}() callbacks. These are required by
> the RZ/G2L SCIFA driver.

"These are required by the RZ/G2L SCIFA driver", is not good enough. Can
you descript why RZ/G2L SCIFA require it?

Frank

>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>
> Changes in v8:
> - reported residue for paused channels as well
>
> Changes in v7:
> - use guard() instead of scoped_guard()
> - in rz_dmac_device_pause() checked the channel is enabled
>   before suspending it to avoid read poll timeouts
> - added a comment in rz_dmac_device_resume()
>
> Changes in v6:
> - set CHCTRL_SETSUS for pause and CHCTRL_CLRSUS for resume
> - dropped read-modify-update approach for CHCTRL updates as the
>   HW returns zero when reading CHCTRL
> - moved the read_poll_timeout_atomic() under spin lock to
>   ensure avoid any races b/w pause and resume functionalities
>
> Changes in v5:
> - used suspend capability of the controller to pause/resume
>   the transfers
>
>  drivers/dma/sh/rz-dmac.c | 49 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 27c963083e29..caa3335bf95d 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -141,10 +141,12 @@ struct rz_dmac {
>  #define CHANNEL_8_15_COMMON_BASE	0x0700
>
>  #define CHSTAT_ER			BIT(4)
> +#define CHSTAT_SUS			BIT(3)
>  #define CHSTAT_EN			BIT(0)
>
>  #define CHCTRL_CLRINTMSK		BIT(17)
>  #define CHCTRL_CLRSUS			BIT(9)
> +#define CHCTRL_SETSUS			BIT(8)
>  #define CHCTRL_CLRTC			BIT(6)
>  #define CHCTRL_CLREND			BIT(5)
>  #define CHCTRL_CLRRQ			BIT(4)
> @@ -814,11 +816,18 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
>  	if (status == DMA_COMPLETE || !txstate)
>  		return status;
>
> -	scoped_guard(spinlock_irqsave, &channel->vc.lock)
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		u32 val;
> +
>  		residue = rz_dmac_chan_get_residue(channel, cookie);
>
> -	/* if there's no residue, the cookie is complete */
> -	if (!residue)
> +		val = rz_dmac_ch_readl(channel, CHSTAT, 1);
> +		if (val & CHSTAT_SUS)
> +			status = DMA_PAUSED;
> +	}
> +
> +	/* if there's no residue and no paused, the cookie is complete */
> +	if (!residue && status != DMA_PAUSED)
>  		return DMA_COMPLETE;
>
>  	dma_set_residue(txstate, residue);
> @@ -826,6 +835,38 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
>  	return status;
>  }
>
> +static int rz_dmac_device_pause(struct dma_chan *chan)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	u32 val;
> +
> +	guard(spinlock_irqsave)(&channel->vc.lock);
> +
> +	val = rz_dmac_ch_readl(channel, CHSTAT, 1);
> +	if (!(val & CHSTAT_EN))
> +		return 0;
> +
> +	rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
> +	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					(val & CHSTAT_SUS), 1, 1024,
> +					false, channel, CHSTAT, 1);
> +}
> +
> +static int rz_dmac_device_resume(struct dma_chan *chan)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	u32 val;
> +
> +	guard(spinlock_irqsave)(&channel->vc.lock);
> +
> +	/* Do not check CHSTAT_SUS but rely on HW capabilities. */
> +
> +	rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> +	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					!(val & CHSTAT_SUS), 1, 1024,
> +					false, channel, CHSTAT, 1);
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -1164,6 +1205,8 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	engine->device_terminate_all = rz_dmac_terminate_all;
>  	engine->device_issue_pending = rz_dmac_issue_pending;
>  	engine->device_synchronize = rz_dmac_device_synchronize;
> +	engine->device_pause = rz_dmac_device_pause;
> +	engine->device_resume = rz_dmac_device_resume;
>
>  	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
>  	dma_set_max_seg_size(engine->dev, U32_MAX);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback
  2026-02-25 16:43   ` Frank Li
@ 2026-03-02 13:49     ` Claudiu Beznea
  2026-03-04 16:37       ` Frank Li
  0 siblings, 1 reply; 21+ messages in thread
From: Claudiu Beznea @ 2026-03-02 13:49 UTC (permalink / raw)
  To: Frank Li
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

Hi, Frank,

On 2/25/26 18:43, Frank Li wrote:
> On Tue, Jan 20, 2026 at 03:33:29PM +0200, Claudiu wrote:
>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>
>> Add support for device_tx_status() callback as it is needed for
>> RZ/G2L SCIFA driver.
>>
>> Based on a patch in the BSP similar to rcar-dmac by
>> Long Luu <long.luu.ur@renesas.com>.
> 
> If you want to give credit to Long Luu, any public link?

No public link as far as I'm aware. Anyway, I'll add his SoB + Co-developed-by.

> 
>>
>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>> [claudiu.beznea:
>>   - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>>     pointer to advance
>>   - use 'lmdesc->nxla != crla' comparison instead of
>>     '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
>>   - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>>     to verify if the full lmdesc list was checked
>>   - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
>>   - re-arranged comments so they span fewer lines and are wrapped to ~80
>>     characters
>>   - use u32 for the residue value and the functions returning it
>>   - use u32 for the variables storing register values
>>   - fixed typos]
> 
> Suppose needn't this section

Just followed the process. I can drop it.

> 
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>
>> Changes in v8:
>> - populated engine->residue_granularity
>>
>> Changes in v7:
>> - none
>>
>> Changes in v6:
>> - s/byte/bytes in comment from rz_dmac_chan_get_residue()
>>
>> Changes in v5:
>> - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
>>    pointer to advance
>> - use 'lmdesc->nxla != crla' comparison instead of
>>    '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
>> - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
>>    to verify if the full lmdesc list was checked
>> - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
>> - re-arranged comments so they span fewer lines and are wrapped to ~80
>>    characters
>> - use u32 for the residue value and the functions returning it
>> - use u32 for the variables storing register values
>> - fixed typos
>>
>>   drivers/dma/sh/rz-dmac.c | 145 ++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
>> index 4602f8b7408a..27c963083e29 100644
>> --- a/drivers/dma/sh/rz-dmac.c
>> +++ b/drivers/dma/sh/rz-dmac.c
>> @@ -125,10 +125,12 @@ struct rz_dmac {
>>    * Registers
>>    */
>>
>> +#define CRTB				0x0020
>>   #define CHSTAT				0x0024
>>   #define CHCTRL				0x0028
>>   #define CHCFG				0x002c
>>   #define NXLA				0x0038
>> +#define CRLA				0x003c
>>
>>   #define DCTRL				0x0000
>>
>> @@ -684,6 +686,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
>>   	rz_dmac_set_dma_req_no(dmac, channel->index, dmac->info->default_dma_req_no);
>>   }
>>
>> +static struct rz_lmdesc *
>> +rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct rz_lmdesc *lmdesc)
>> +{
>> +	struct rz_lmdesc *next = ++lmdesc;
>> +
>> +	if (next >= base + DMAC_NR_LMDESC)
>> +		next = base;
>> +
>> +	return next;
>> +}
>> +
>> +static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
>> +{
>> +	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
>> +	struct dma_chan *chan = &channel->vc.chan;
>> +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
>> +	u32 residue = 0, crla, i = 0;
>> +
>> +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
>> +	while (lmdesc->nxla != crla) {
>> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
>> +		if (++i >= DMAC_NR_LMDESC)
>> +			return 0;
>> +	}
>> +
>> +	/* Calculate residue from next lmdesc to end of virtual desc */
>> +	while (lmdesc->chcfg & CHCFG_DEM) {
>> +		residue += lmdesc->tb;
>> +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
>> +	}
> 
> can use one loop
> 
> for (int i=0; i<DMAC_NR_LMDESC; i++) {
> 	if (lmdesc->nxla == crla)
> 		residue = 0; 	//reset to 0;
> 
> 	if (lmdesc->chcfg & CHCFG_DEM)
> 		residue += lmdesc->tb;
> 
> 	lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);

I'm not sure this will work as the descriptors list is cyclic and resetting the 
residue to zero when lmdesc->nxla == crla and then start acumulating from there 
will not work if there are descriptors enqueued for the current transfers at the 
end and the beginning of the list, e.g:

descriptors list:

| d3 | d5 | d6 | ... | d0 | d1 | d2 |

^				    ^
start			 	   end
(index 0)			(index DMAC_NR_LMDESC-1)

> }
> 
> return residue;
> 
>> +
>> +	dev_dbg(dmac->dev, "%s: VD residue is %u\n", __func__, residue);
>> +
>> +	return residue;
>> +}
>> +
>> +static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
>> +				    dma_cookie_t cookie)
>> +{
>> +	struct rz_dmac_desc *current_desc, *desc;
>> +	enum dma_status status;
>> +	u32 crla, crtb, i;
>> +
>> +	/* Get current processing virtual descriptor */
>> +	current_desc = list_first_entry(&channel->ld_active,
>> +					struct rz_dmac_desc, node);
>> +	if (!current_desc)
>> +		return 0;
>> +
>> +	/*
>> +	 * If the cookie corresponds to a descriptor that has been completed
>> +	 * there is no residue. The same check has already been performed by the
>> +	 * caller but without holding the channel lock, so the descriptor could
>> +	 * now be complete.
>> +	 */
>> +	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
>> +	if (status == DMA_COMPLETE)
>> +		return 0;
>> +
>> +	/*
>> +	 * If the cookie doesn't correspond to the currently processing virtual
>> +	 * descriptor then the descriptor hasn't been processed yet, and the
>> +	 * residue is equal to the full descriptor size. Also, a client driver
>> +	 * is possible to call this function before rz_dmac_irq_handler_thread()
>> +	 * runs. In this case, the running descriptor will be the next
>> +	 * descriptor, and will appear in the done list. So, if the argument
>> +	 * cookie matches the done list's cookie, we can assume the residue is
>> +	 * zero.
>> +	 */
>> +	if (cookie != current_desc->vd.tx.cookie) {
>> +		list_for_each_entry(desc, &channel->ld_free, node) {
>> +			if (cookie == desc->vd.tx.cookie)
>> +				return 0;
>> +		}
>> +
>> +		list_for_each_entry(desc, &channel->ld_queue, node) {
>> +			if (cookie == desc->vd.tx.cookie)
>> +				return desc->len;
>> +		}
>> +
>> +		list_for_each_entry(desc, &channel->ld_active, node) {
>> +			if (cookie == desc->vd.tx.cookie)
>> +				return desc->len;
>> +		}
>> +
>> +		/*
>> +		 * No descriptor found for the cookie, there's thus no residue.
>> +		 * This shouldn't happen if the calling driver passes a correct
>> +		 * cookie value.
>> +		 */
>> +		WARN(1, "No descriptor for cookie!");
>> +		return 0;
>> +	}
>> +
>> +	/*
>> +	 * We need to read two registers. Make sure the hardware does not move
>> +	 * to next lmdesc while reading the current lmdesc. Trying it 3 times
>> +	 * should be enough: initial read, retry, retry for the paranoid.
>> +	 */
>> +	for (i = 0; i < 3; i++) {
>> +		crla = rz_dmac_ch_readl(channel, CRLA, 1);
>> +		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
>> +		/* Still the same? */
>> +		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
>> +			break;
>> +	}
>> +
>> +	WARN_ONCE(i >= 3, "residue might not be continuous!");
>> +
>> +	/*
>> +	 * Calculate number of bytes transferred in processing virtual descriptor.
>> +	 * One virtual descriptor can have many lmdesc.
>> +	 */
>> +	return crtb + rz_dmac_calculate_residue_bytes_in_vd(channel);
> 
> you don't use varible 'ctra' here, so retry 3 become useless. suppose
> rz_dmac_calculate_residue_bytes_in_vd(channel, ctra)
> 
> and avoid rz_dmac_ch_readl(channel, CRLA, 1) in
> rz_dmac_calculate_residue_bytes_in_vd() to keep ctra and ctrb reflect the
> correct hardware state.

Good point, I'll update it.

Thank you for your review,
Claudiu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
  2026-02-25 16:49   ` Frank Li
@ 2026-03-02 13:50     ` Claudiu Beznea
  0 siblings, 0 replies; 21+ messages in thread
From: Claudiu Beznea @ 2026-03-02 13:50 UTC (permalink / raw)
  To: Frank Li
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

Hi, Frank,

On 2/25/26 18:49, Frank Li wrote:
> On Tue, Jan 20, 2026 at 03:33:30PM +0200, Claudiu wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Add support for device_{pause, resume}() callbacks. These are required by
>> the RZ/G2L SCIFA driver.
> 
> "These are required by the RZ/G2L SCIFA driver", is not good enough. Can
> you descript why RZ/G2L SCIFA require it?

Sure, I'll update it.

Thank you,
Claudiu

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback
  2026-03-02 13:49     ` Claudiu Beznea
@ 2026-03-04 16:37       ` Frank Li
  0 siblings, 0 replies; 21+ messages in thread
From: Frank Li @ 2026-03-04 16:37 UTC (permalink / raw)
  To: Claudiu Beznea
  Cc: vkoul, geert+renesas, biju.das.jz, fabrizio.castro.jz,
	prabhakar.mahadev-lad.rj, dmaengine, linux-kernel,
	Claudiu Beznea

On Mon, Mar 02, 2026 at 03:49:54PM +0200, Claudiu Beznea wrote:
> Hi, Frank,
>
> On 2/25/26 18:43, Frank Li wrote:
> > On Tue, Jan 20, 2026 at 03:33:29PM +0200, Claudiu wrote:
> > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > Add support for device_tx_status() callback as it is needed for
> > > RZ/G2L SCIFA driver.
> > >
> > > Based on a patch in the BSP similar to rcar-dmac by
> > > Long Luu <long.luu.ur@renesas.com>.
> >
> > If you want to give credit to Long Luu, any public link?
>
> No public link as far as I'm aware. Anyway, I'll add his SoB + Co-developed-by.
>
> >
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > [claudiu.beznea:
> > >   - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
> > >     pointer to advance
> > >   - use 'lmdesc->nxla != crla' comparison instead of
> > >     '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
> > >   - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
> > >     to verify if the full lmdesc list was checked
> > >   - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
> > >   - re-arranged comments so they span fewer lines and are wrapped to ~80
> > >     characters
> > >   - use u32 for the residue value and the functions returning it
> > >   - use u32 for the variables storing register values
> > >   - fixed typos]
> >
> > Suppose needn't this section
>
> Just followed the process. I can drop it.
>
> >
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > ---
> > >
> > > Changes in v8:
> > > - populated engine->residue_granularity
> > >
> > > Changes in v7:
> > > - none
> > >
> > > Changes in v6:
> > > - s/byte/bytes in comment from rz_dmac_chan_get_residue()
> > >
> > > Changes in v5:
> > > - post-increment lmdesc in rz_dmac_get_next_lmdesc() to allow the next
> > >    pointer to advance
> > > - use 'lmdesc->nxla != crla' comparison instead of
> > >    '!(lmdesc->nxla == crla)' in rz_dmac_calculate_residue_bytes_in_vd()
> > > - in rz_dmac_calculate_residue_bytes_in_vd() use '++i >= DMAC_NR_LMDESC'
> > >    to verify if the full lmdesc list was checked
> > > - drop rz_dmac_calculate_total_bytes_in_vd() and use desc->len instead
> > > - re-arranged comments so they span fewer lines and are wrapped to ~80
> > >    characters
> > > - use u32 for the residue value and the functions returning it
> > > - use u32 for the variables storing register values
> > > - fixed typos
> > >
> > >   drivers/dma/sh/rz-dmac.c | 145 ++++++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 144 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > > index 4602f8b7408a..27c963083e29 100644
> > > --- a/drivers/dma/sh/rz-dmac.c
> > > +++ b/drivers/dma/sh/rz-dmac.c
> > > @@ -125,10 +125,12 @@ struct rz_dmac {
> > >    * Registers
> > >    */
> > >
> > > +#define CRTB				0x0020
> > >   #define CHSTAT				0x0024
> > >   #define CHCTRL				0x0028
> > >   #define CHCFG				0x002c
> > >   #define NXLA				0x0038
> > > +#define CRLA				0x003c
> > >
> > >   #define DCTRL				0x0000
> > >
> > > @@ -684,6 +686,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
> > >   	rz_dmac_set_dma_req_no(dmac, channel->index, dmac->info->default_dma_req_no);
> > >   }
> > >
> > > +static struct rz_lmdesc *
> > > +rz_dmac_get_next_lmdesc(struct rz_lmdesc *base, struct rz_lmdesc *lmdesc)
> > > +{
> > > +	struct rz_lmdesc *next = ++lmdesc;
> > > +
> > > +	if (next >= base + DMAC_NR_LMDESC)
> > > +		next = base;
> > > +
> > > +	return next;
> > > +}
> > > +
> > > +static u32 rz_dmac_calculate_residue_bytes_in_vd(struct rz_dmac_chan *channel)
> > > +{
> > > +	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
> > > +	struct dma_chan *chan = &channel->vc.chan;
> > > +	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > > +	u32 residue = 0, crla, i = 0;
> > > +
> > > +	crla = rz_dmac_ch_readl(channel, CRLA, 1);
> > > +	while (lmdesc->nxla != crla) {
> > > +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
> > > +		if (++i >= DMAC_NR_LMDESC)
> > > +			return 0;
> > > +	}
> > > +
> > > +	/* Calculate residue from next lmdesc to end of virtual desc */
> > > +	while (lmdesc->chcfg & CHCFG_DEM) {
> > > +		residue += lmdesc->tb;
> > > +		lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
> > > +	}
> >
> > can use one loop
> >
> > for (int i=0; i<DMAC_NR_LMDESC; i++) {
> > 	if (lmdesc->nxla == crla)
> > 		residue = 0; 	//reset to 0;
> >
> > 	if (lmdesc->chcfg & CHCFG_DEM)
> > 		residue += lmdesc->tb;
> >
> > 	lmdesc = rz_dmac_get_next_lmdesc(channel->lmdesc.base, lmdesc);
>
> I'm not sure this will work as the descriptors list is cyclic and resetting
> the residue to zero when lmdesc->nxla == crla and then start acumulating
> from there will not work if there are descriptors enqueued for the current
> transfers at the end and the beginning of the list, e.g:
>
> descriptors list:
>
> | d3 | d5 | d6 | ... | d0 | d1 | d2 |
>
> ^				    ^
> start			 	   end
> (index 0)			(index DMAC_NR_LMDESC-1)

Okay, you are right.

Frank
>
> > }
> >
> > return residue;
> >
> > > +
> > > +	dev_dbg(dmac->dev, "%s: VD residue is %u\n", __func__, residue);
> > > +
> > > +	return residue;
> > > +}
> > > +
> > > +static u32 rz_dmac_chan_get_residue(struct rz_dmac_chan *channel,
> > > +				    dma_cookie_t cookie)
> > > +{
> > > +	struct rz_dmac_desc *current_desc, *desc;
> > > +	enum dma_status status;
> > > +	u32 crla, crtb, i;
> > > +
> > > +	/* Get current processing virtual descriptor */
> > > +	current_desc = list_first_entry(&channel->ld_active,
> > > +					struct rz_dmac_desc, node);
> > > +	if (!current_desc)
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * If the cookie corresponds to a descriptor that has been completed
> > > +	 * there is no residue. The same check has already been performed by the
> > > +	 * caller but without holding the channel lock, so the descriptor could
> > > +	 * now be complete.
> > > +	 */
> > > +	status = dma_cookie_status(&channel->vc.chan, cookie, NULL);
> > > +	if (status == DMA_COMPLETE)
> > > +		return 0;
> > > +
> > > +	/*
> > > +	 * If the cookie doesn't correspond to the currently processing virtual
> > > +	 * descriptor then the descriptor hasn't been processed yet, and the
> > > +	 * residue is equal to the full descriptor size. Also, a client driver
> > > +	 * is possible to call this function before rz_dmac_irq_handler_thread()
> > > +	 * runs. In this case, the running descriptor will be the next
> > > +	 * descriptor, and will appear in the done list. So, if the argument
> > > +	 * cookie matches the done list's cookie, we can assume the residue is
> > > +	 * zero.
> > > +	 */
> > > +	if (cookie != current_desc->vd.tx.cookie) {
> > > +		list_for_each_entry(desc, &channel->ld_free, node) {
> > > +			if (cookie == desc->vd.tx.cookie)
> > > +				return 0;
> > > +		}
> > > +
> > > +		list_for_each_entry(desc, &channel->ld_queue, node) {
> > > +			if (cookie == desc->vd.tx.cookie)
> > > +				return desc->len;
> > > +		}
> > > +
> > > +		list_for_each_entry(desc, &channel->ld_active, node) {
> > > +			if (cookie == desc->vd.tx.cookie)
> > > +				return desc->len;
> > > +		}
> > > +
> > > +		/*
> > > +		 * No descriptor found for the cookie, there's thus no residue.
> > > +		 * This shouldn't happen if the calling driver passes a correct
> > > +		 * cookie value.
> > > +		 */
> > > +		WARN(1, "No descriptor for cookie!");
> > > +		return 0;
> > > +	}
> > > +
> > > +	/*
> > > +	 * We need to read two registers. Make sure the hardware does not move
> > > +	 * to next lmdesc while reading the current lmdesc. Trying it 3 times
> > > +	 * should be enough: initial read, retry, retry for the paranoid.
> > > +	 */
> > > +	for (i = 0; i < 3; i++) {
> > > +		crla = rz_dmac_ch_readl(channel, CRLA, 1);
> > > +		crtb = rz_dmac_ch_readl(channel, CRTB, 1);
> > > +		/* Still the same? */
> > > +		if (crla == rz_dmac_ch_readl(channel, CRLA, 1))
> > > +			break;
> > > +	}
> > > +
> > > +	WARN_ONCE(i >= 3, "residue might not be continuous!");
> > > +
> > > +	/*
> > > +	 * Calculate number of bytes transferred in processing virtual descriptor.
> > > +	 * One virtual descriptor can have many lmdesc.
> > > +	 */
> > > +	return crtb + rz_dmac_calculate_residue_bytes_in_vd(channel);
> >
> > you don't use varible 'ctra' here, so retry 3 become useless. suppose
> > rz_dmac_calculate_residue_bytes_in_vd(channel, ctra)
> >
> > and avoid rz_dmac_ch_readl(channel, CRLA, 1) in
> > rz_dmac_calculate_residue_bytes_in_vd() to keep ctra and ctrb reflect the
> > correct hardware state.
>
> Good point, I'll update it.
>
> Thank you for your review,
> Claudiu

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-03-04 16:38 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-20 13:33 [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu
2026-01-20 13:33 ` [PATCH v8 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists Claudiu
2026-02-25 16:17   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock Claudiu
2026-02-25 16:19   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register Claudiu
2026-02-25 16:20   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label Claudiu
2026-02-25 16:23   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call Claudiu
2026-02-25 16:23   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc() Claudiu
2026-02-25 16:26   ` Frank Li
2026-01-20 13:33 ` [PATCH v8 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback Claudiu
2026-02-25 16:43   ` Frank Li
2026-03-02 13:49     ` Claudiu Beznea
2026-03-04 16:37       ` Frank Li
2026-01-20 13:33 ` [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks Claudiu
2026-02-25 16:49   ` Frank Li
2026-03-02 13:50     ` Claudiu Beznea
2026-01-29  9:44 ` [PATCH v8 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support Claudiu Beznea

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®