[PATCH v6 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support

[PATCH v6 0/8] dmaengine: sh: rz-dmac: Add tx_status and pause/resume support

[Claudiu]

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 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 | 281 ++++++++++++++++++++++++++++++++-------
 1 file changed, 231 insertions(+), 50 deletions(-)

-- 
2.43.0

[PATCH v6 1/8] dmaengine: sh: rz-dmac: Protect the driver specific lists

[Claudiu]

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 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 9e5f088355e2..c8e3d9f77b8a 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>
@@ -448,6 +449,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++;
 	}
@@ -503,18 +505,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);
 }
 
@@ -530,27 +535,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

[PATCH v6 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock

[Claudiu]

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
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

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 c8e3d9f77b8a..818d1ef6f0bf 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -298,13 +298,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)
@@ -569,8 +566,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;
 
@@ -707,7 +704,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

[PATCH v6 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register

[Claudiu]

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.

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

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 818d1ef6f0bf..43a772e4478c 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -698,7 +698,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) {
@@ -710,8 +710,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

[PATCH v6 4/8] dmaengine: sh: rz-dmac: Drop goto instruction and label

[Claudiu]

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 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 43a772e4478c..a2e16b52efe8 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -707,7 +707,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;
 	}
 
 	/*
@@ -715,8 +715,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

[PATCH v6 5/8] dmaengine: sh: rz-dmac: Drop unnecessary local_irq_save() call

[Claudiu]

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 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 a2e16b52efe8..72ec42fedac6 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -267,15 +267,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 +
@@ -290,8 +287,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

[PATCH v6 6/8] dmaengine: sh: rz-dmac: Add rz_dmac_invalidate_lmdesc()

[Claudiu]

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 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 72ec42fedac6..45c45053e9df 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -250,6 +250,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;
@@ -456,15 +463,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);
@@ -556,15 +560,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

[PATCH v6 7/8] dmaengine: sh: rz-dmac: Add device_tx_status() callback

[Claudiu]

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 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 | 144 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 45c45053e9df..44f0f72cbcf1 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -119,10 +119,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
 
@@ -685,6 +687,146 @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
 	}
 }
 
+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
@@ -1016,7 +1158,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

[PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Claudiu]

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 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 | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 44f0f72cbcf1..377bdd5c9425 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -135,10 +135,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)
@@ -827,6 +829,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;
+	int ret;
+
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
+		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
+					       (val & CHSTAT_SUS), 1, 1024,
+					       false, channel, CHSTAT, 1);
+	}
+
+	return ret;
+}
+
+static int rz_dmac_device_resume(struct dma_chan *chan)
+{
+	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	u32 val;
+	int ret;
+
+	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
+		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
+		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
+					       !(val & CHSTAT_SUS), 1, 1024,
+					       false, channel, CHSTAT, 1);
+	}
+
+	return ret;
+}
+
 /*
  * -----------------------------------------------------------------------------
  * IRQ handling
@@ -1165,6 +1199,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

RE: [PATCH v6 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: 23 December 2025 13:50
> Subject: [PATCH v6 2/8] dmaengine: sh: rz-dmac: Move CHCTRL updates under spinlock
> 
> 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
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju

> ---
> 
> 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 c8e3d9f77b8a..818d1ef6f0bf
> 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -298,13 +298,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) @@ -569,8 +566,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;
> 
> @@ -707,7 +704,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

RE: [PATCH v6 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: 23 December 2025 13:50
> Subject: [PATCH v6 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register
> 
> 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.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>

Cheers,
Biju
> ---
> 
> 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 818d1ef6f0bf..43a772e4478c
> 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -698,7 +698,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) {
> @@ -710,8 +710,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

RE: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@tuxon.dev>
> Sent: 23 December 2025 13:50
> Subject: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
> 
> 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 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 | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 44f0f72cbcf1..377bdd5c9425
> 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -135,10 +135,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)
> @@ -827,6 +829,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;
> +	int ret;
> +
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {

> +		rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);


Probably first you need to check CHSTAT_EN first before setting CHCTRL_SETSUS??

As per the hardware manual

"
Suspends the current DMA transfer. Setting this bit to 1 when 1 is set in EN of the
CHSTAT_n/nS register can suspend the current DMA transfer."


> +		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					       (val & CHSTAT_SUS), 1, 1024,
> +					       false, channel, CHSTAT, 1);
> +	}
> +
> +	return ret;
> +}
> +
> +static int rz_dmac_device_resume(struct dma_chan *chan) {
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	u32 val;
> +	int ret;
> +
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {


> +		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);


Similarly, first you need to check  CHSTAT_SUS bit first and then clear suspend state.


Clears the suspend status. Setting this bit to 1 when 1 is set in SUS of the
CHSTAT_n/nS register can clear the suspend status.

Cheers,
Biju


> +		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					       !(val & CHSTAT_SUS), 1, 1024,
> +					       false, channel, CHSTAT, 1);
> +	}
> +
> +	return ret;
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -1165,6 +1199,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

Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Claudiu Beznea]

Hi, Biju,

On 12/23/25 16:43, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu <claudiu.beznea@tuxon.dev>
>> Sent: 23 December 2025 13:50
>> Subject: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
>>
>> 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 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 | 36 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 44f0f72cbcf1..377bdd5c9425
>> 100644
>> --- a/drivers/dma/sh/rz-dmac.c
>> +++ b/drivers/dma/sh/rz-dmac.c
>> @@ -135,10 +135,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)
>> @@ -827,6 +829,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;
>> +	int ret;
>> +
>> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> 
>> +		rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
> 
> 
> Probably first you need to check CHSTAT_EN first before setting CHCTRL_SETSUS??
> 
> As per the hardware manual
> 
> "
> Suspends the current DMA transfer. Setting this bit to 1 when 1 is set in EN of the
> CHSTAT_n/nS register can suspend the current DMA transfer."

OK, I'll update it as follows:

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

This avoids timeouts reported by read_poll_timeout_atomic() when pause 
is set for a disabled channel.

> 
> 
>> +		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
>> +					       (val & CHSTAT_SUS), 1, 1024,
>> +					       false, channel, CHSTAT, 1);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int rz_dmac_device_resume(struct dma_chan *chan) {
>> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
>> +	u32 val;
>> +	int ret;
>> +
>> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> 
> 
>> +		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> 
> 
> Similarly, first you need to check  CHSTAT_SUS bit first and then clear suspend state.
> 
> 
> Clears the suspend status. Setting this bit to 1 when 1 is set in SUS of the
> CHSTAT_n/nS register can clear the suspend status.

I'll update this one as follows, to keep the code simple:

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

With this:

1/ in case the channel is not suspended and the CHCTRL_CLRSUS is set, 
the read_poll_timeout_atomic() will not timeout, as the default value of 
the CHSTAT_SUS is zero.

2/ in case the channel is suspended and the CLRSUS is set, it is 
behaving as expected but without an extra check of the CHSTAT_SUS bit 
before setting CHCTRL_CLRSUS.

Thank you,
Claudiu

RE: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Sent: 07 January 2026 13:18
> Subject: Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
> 
> Hi, Biju,
> 
> On 12/23/25 16:43, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Claudiu <claudiu.beznea@tuxon.dev>
> >> Sent: 23 December 2025 13:50
> >> Subject: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add
> >> device_{pause,resume}() callbacks
> >>
> >> 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 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 | 36 ++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 36 insertions(+)
> >>
> >> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> >> index 44f0f72cbcf1..377bdd5c9425
> >> 100644
> >> --- a/drivers/dma/sh/rz-dmac.c
> >> +++ b/drivers/dma/sh/rz-dmac.c
> >> @@ -135,10 +135,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)
> >> @@ -827,6 +829,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;
> >> +	int ret;
> >> +
> >> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> >
> >> +		rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
> >
> >
> > Probably first you need to check CHSTAT_EN first before setting CHCTRL_SETSUS??
> >
> > As per the hardware manual
> >
> > "
> > Suspends the current DMA transfer. Setting this bit to 1 when 1 is set
> > in EN of the CHSTAT_n/nS register can suspend the current DMA transfer."
> 
> OK, I'll update it as follows:
> 
> 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);
> }

OK.

> 
> This avoids timeouts reported by read_poll_timeout_atomic() when pause is set for a disabled channel.
> 
> >
> >
> >> +		ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> >> +					       (val & CHSTAT_SUS), 1, 1024,
> >> +					       false, channel, CHSTAT, 1);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int rz_dmac_device_resume(struct dma_chan *chan) {
> >> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> >> +	u32 val;
> >> +	int ret;
> >> +
> >> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> >
> >
> >> +		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> >
> >
> > Similarly, first you need to check  CHSTAT_SUS bit first and then clear suspend state.
> >
> >
> > Clears the suspend status. Setting this bit to 1 when 1 is set in SUS
> > of the CHSTAT_n/nS register can clear the suspend status.
> 
> I'll update this one as follows, to keep the code simple:
> 
> 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);
> }
> 
> With this:
> 
> 1/ in case the channel is not suspended and the CHCTRL_CLRSUS is set, the read_poll_timeout_atomic()
> will not timeout, as the default value of the CHSTAT_SUS is zero.

Just a question as we are not following the hardware manual.

At hardware level does it have any implications? 

Eg: we set this write only register without the device being suspended 
    

The next suspend operation, immediately clears the suspend operation
    
    Or 

does it work normally.

Cheers,
Biju

Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Claudiu Beznea]

On 1/7/26 15:48, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> Sent: 07 January 2026 13:18
>> Subject: Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
>>
>> Hi, Biju,
>>
>> On 12/23/25 16:43, Biju Das wrote:
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: Claudiu <claudiu.beznea@tuxon.dev>
>>>> Sent: 23 December 2025 13:50
>>>> Subject: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add
>>>> device_{pause,resume}() callbacks
>>>>
>>>> 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>
>>>> ---
>>>>

[...]

>>>> +
>>>> +static int rz_dmac_device_resume(struct dma_chan *chan) {
>>>> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
>>>> +	u32 val;
>>>> +	int ret;
>>>> +
>>>> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
>>>
>>>
>>>> +		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
>>>
>>>
>>> Similarly, first you need to check  CHSTAT_SUS bit first and then clear suspend state.
>>>
>>>
>>> Clears the suspend status. Setting this bit to 1 when 1 is set in SUS
>>> of the CHSTAT_n/nS register can clear the suspend status.
>>
>> I'll update this one as follows, to keep the code simple:
>>
>> 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);
>> }
>>
>> With this:
>>
>> 1/ in case the channel is not suspended and the CHCTRL_CLRSUS is set, the read_poll_timeout_atomic()
>> will not timeout, as the default value of the CHSTAT_SUS is zero.
> 
> Just a question as we are not following the hardware manual.
> 
> At hardware level does it have any implications?

The documentation of CHCTRL_CLRSUS states:

Setting this bit to 1 *when 1 is set in SUS of the CHSTAT_n/nS register* 
can clear the suspend status.

So, it takes effect only when CHSTAT.SUS=1

> 
> Eg: we set this write only register without the device being suspended
>      
> 
> The next suspend operation, immediately clears the suspend operation

The next suspend operation will switch the DMA channel to suspend if the 
channel is enabled. Nothing should take place if the device is not enabled.

>      
>      Or
> 
> does it work normally.

Yes, it works normally.

I set the CLRSUS bit in a loop for a DMA channel where the audio was 
playing, with the following command:

while :; do devmem2 0x118200a8 w 0x200 > /dev/null; done

There were no issues with the audio stream, as expected.

If I set the SETSUS bit for the same audio channel after the CLRSUS was 
set in a loop, the audio is stopped as expected.

Thank you,
Claudiu