[PATCH 0/4] loongson2-apb-cmc-dma: Fixes issues and pause/resume support

[PATCH 0/4] loongson2-apb-cmc-dma: Fixes issues and pause/resume support

[Binbin Zhou]

Hi all:

This series fixes three bugs in the Loongson 2 APB CMC DMA driver and
adds pause/resume support on top.

Thanks.
Binbin

Binbin Zhou (4):
  dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap
    NULL dereference
  dmaengine: loongson: loongson2-apb-cmc-dma: Fix residue for queued
    descriptors
  dmaengine: loongson: loongson2-apb-cmc-dma: Program CNDTR when
    advancing sg
  dmaengine: loongson: loongson2-apb-cmc-dma: Add pause/resume support

 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 134 +++++++++++++++++--
 1 file changed, 124 insertions(+), 10 deletions(-)


base-commit: aff5e6a77fc69a7c490246465940d18422d7c631
-- 
2.52.0

[PATCH 1/4] dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap NULL dereference

[Binbin Zhou]

loongson2_cmc_dma_desc_residue()'s cyclic wrap special case checks
lchan->desc->cyclic instead of desc->cyclic.

When tx_status() is called for a queued descriptor while no descriptor
is active, it reaches the "else if (vdesc)" branch with lchan->desc ==
NULL and passes the queued descriptor to this helper. The helper then
dereferences lchan->desc->cyclic and oopses.

Even when lchan->desc is non-NULL, keying the check on lchan->desc is
semantically wrong: the wrap decision belongs to the descriptor being
queried, not to whatever descriptor happens to be active.

Use desc->cyclic. This fixes the NULL dereference and makes the helper
self-contained.

Cc: stable@vger.kernel.org
Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index 2037120b23ca..f090fbe0b8a1 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -484,7 +484,19 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
 	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
 	residue = ndtr << width;
 
-	if (lchan->desc->cyclic && next_sg == 0)
+	/*
+	 * For a cyclic descriptor that has wrapped (next_sg == 0), the
+	 * current period only has the in-flight entry left; the entries
+	 * from sg[0] onwards belong to the next period. Adding them would
+	 * make residue exceed one period.
+	 *
+	 * Use desc->cyclic, not lchan->desc->cyclic: this helper is also
+	 * reachable with a descriptor that is not lchan->desc, and only
+	 * the active-descriptor branch passes lchan->desc today. Keying
+	 * the wrap decision on the descriptor being queried keeps the
+	 * result correct and avoids depending on lchan->desc being set.
+	 */
+	if (desc->cyclic && next_sg == 0)
 		return residue;
 
 	for (i = next_sg; i < desc->num_sgs; i++)
-- 
2.52.0

Re: [PATCH 1/4] dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap NULL dereference

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: loongson: wrong next_sg check in loongson2_cmc_dma_desc_residue()
--

commit d1cdc3886ec73f6a09aef30478b0b2741d57690c
Author: Binbin Zhou <zhoubinbin@loongson.cn>

dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap NULL dereference

This commit updates loongson2_cmc_dma_desc_residue() to check desc->cyclic
instead of lchan->desc->cyclic when next_sg == 0, and adds a comment
explaining the cyclic wrap behavior.

> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 2037120b23caa..f090fbe0b8a1e 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -484,7 +484,19 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
>  	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
>  	residue = ndtr << width;
>  
> -	if (lchan->desc->cyclic && next_sg == 0)
> +	/*
> +	 * For a cyclic descriptor that has wrapped (next_sg == 0), the
> +	 * current period only has the in-flight entry left; the entries
> +	 * from sg[0] onwards belong to the next period. Adding them would
> +	 * make residue exceed one period.
> +	 *
> +	 * Use desc->cyclic, not lchan->desc->cyclic: this helper is also
> +	 * reachable with a descriptor that is not lchan->desc, and only
> +	 * the active-descriptor branch passes lchan->desc today. Keying
> +	 * the wrap decision on the descriptor being queried keeps the
> +	 * result correct and avoids depending on lchan->desc being set.
> +	 */
> +	if (desc->cyclic && next_sg == 0)
>  		return residue;

[Severity: Medium]
Can an active cyclic descriptor ever reach loongson2_cmc_dma_desc_residue()
in drivers/dma/loongson/loongson2-apb-cmc-dma.c with next_sg == 0?

When starting a transfer in loongson2_cmc_dma_start_transfer(), lchan->next_sg
is set to 0 before configuring sg_req[0] and immediately incremented to 1
under lchan->vchan.lock:

    if (lchan->next_sg == lchan->desc->num_sgs)
        lchan->next_sg = 0;

    sg_req = &lchan->desc->sg_req[lchan->next_sg];
    ...
    lchan->next_sg++;

Similarly, on cyclic period completion in loongson2_cmc_dma_handle_chan_done(),
loongson2_cmc_dma_configure_next_sg() wraps lchan->next_sg from
lchan->desc->num_sgs to 0 to program sg_req[0], and
loongson2_cmc_dma_handle_chan_done() immediately increments lchan->next_sg to
1 under lchan->vchan.lock:

    if (lchan->desc->cyclic) {
        vchan_cyclic_callback(&lchan->desc->vdesc);
        /* LOONGSON2_CMCDMA_CCR_CIRC mode don't need update register */
        if (lchan->desc->num_sgs == 1)
            return;
        loongson2_cmc_dma_configure_next_sg(lchan);
        lchan->next_sg++;
    }

While the final period sg_req[num_sgs - 1] is in flight, lchan->next_sg equals
desc->num_sgs, so the loop

    for (i = next_sg; i < desc->num_sgs; i++)
        residue += desc->sg_req[i].len;

naturally executes zero iterations. Thus for an active descriptor,
lchan->next_sg is always in [1, desc->num_sgs] when
loongson2_cmc_dma_tx_status() queries it under lchan->vchan.lock.

In this patch, the only caller that passes next_sg == 0 to
loongson2_cmc_dma_desc_residue() is the else if (vdesc) branch in
loongson2_cmc_dma_tx_status() for an unstarted queued descriptor on
vchan.desc_issued:

    if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
        state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
                                                        lchan->next_sg);
    else if (vdesc)
        state->residue = loongson2_cmc_dma_desc_residue(lchan,
                                                        to_lmdma_desc(vdesc), 0);

With desc->cyclic && next_sg == 0, querying a queued cyclic descriptor (while
another descriptor is active) returns early before summing desc->sg_req[i].len,
reporting only the active transfer's ndtr << width.

Even after the later commit in the series ("dmaengine: loongson:
loongson2-apb-cmc-dma: Fix residue for queued descriptors") switches the
else if (vdesc) branch to loongson2_cmc_dma_desc_total_len(),
loongson2_cmc_dma_desc_residue() is only called for the active descriptor
lchan->desc with lchan->next_sg in [1, desc->num_sgs]. Doesn't that leave the
next_sg == 0 check as dead code and the comment describing cyclic wrap and
non-active descriptors inaccurate?

Also, regarding the commit message description of the NULL dereference when
lchan->desc == NULL: since vchan_find_desc() searches vchan.desc_issued, and
both loongson2_cmc_dma_issue_pending() and loongson2_cmc_dma_handle_chan_done()
immediately start the first descriptor from vchan.desc_issued under
lchan->vchan.lock, can vchan.desc_issued ever be non-empty while lchan->desc
is NULL? Should this check and its comment be removed altogether once queued
descriptors use loongson2_cmc_dma_desc_total_len()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792988985.git.zhoubinbin@loongson.cn?part=1

Re: [PATCH 1/4] dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap NULL dereference

[Binbin Zhou]

On 2026/10/26 17:17, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] dmaengine: loongson: wrong next_sg check in loongson2_cmc_dma_desc_residue()
> --
> 
> commit d1cdc3886ec73f6a09aef30478b0b2741d57690c
> Author: Binbin Zhou <zhoubinbin@loongson.cn>
> 
> dmaengine: loongson: loongson2-apb-cmc-dma: Fix cyclic residue wrap NULL dereference
> 
> This commit updates loongson2_cmc_dma_desc_residue() to check desc->cyclic
> instead of lchan->desc->cyclic when next_sg == 0, and adds a comment
> explaining the cyclic wrap behavior.
> 
>> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
>> index 2037120b23caa..f090fbe0b8a1e 100644
>> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
>> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
>> @@ -484,7 +484,19 @@ static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lcha
>>   	ndtr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CNDTR, lchan->id);
>>   	residue = ndtr << width;
>>   
>> -	if (lchan->desc->cyclic && next_sg == 0)
>> +	/*
>> +	 * For a cyclic descriptor that has wrapped (next_sg == 0), the
>> +	 * current period only has the in-flight entry left; the entries
>> +	 * from sg[0] onwards belong to the next period. Adding them would
>> +	 * make residue exceed one period.
>> +	 *
>> +	 * Use desc->cyclic, not lchan->desc->cyclic: this helper is also
>> +	 * reachable with a descriptor that is not lchan->desc, and only
>> +	 * the active-descriptor branch passes lchan->desc today. Keying
>> +	 * the wrap decision on the descriptor being queried keeps the
>> +	 * result correct and avoids depending on lchan->desc being set.
>> +	 */
>> +	if (desc->cyclic && next_sg == 0)
>>   		return residue;
> 
> [Severity: Medium]
> Can an active cyclic descriptor ever reach loongson2_cmc_dma_desc_residue()
> in drivers/dma/loongson/loongson2-apb-cmc-dma.c with next_sg == 0?
> 
> When starting a transfer in loongson2_cmc_dma_start_transfer(), lchan->next_sg
> is set to 0 before configuring sg_req[0] and immediately incremented to 1
> under lchan->vchan.lock:
> 
>      if (lchan->next_sg == lchan->desc->num_sgs)
>          lchan->next_sg = 0;
> 
>      sg_req = &lchan->desc->sg_req[lchan->next_sg];
>      ...
>      lchan->next_sg++;
> 
> Similarly, on cyclic period completion in loongson2_cmc_dma_handle_chan_done(),
> loongson2_cmc_dma_configure_next_sg() wraps lchan->next_sg from
> lchan->desc->num_sgs to 0 to program sg_req[0], and
> loongson2_cmc_dma_handle_chan_done() immediately increments lchan->next_sg to
> 1 under lchan->vchan.lock:
> 
>      if (lchan->desc->cyclic) {
>          vchan_cyclic_callback(&lchan->desc->vdesc);
>          /* LOONGSON2_CMCDMA_CCR_CIRC mode don't need update register */
>          if (lchan->desc->num_sgs == 1)
>              return;
>          loongson2_cmc_dma_configure_next_sg(lchan);
>          lchan->next_sg++;
>      }
> 
> While the final period sg_req[num_sgs - 1] is in flight, lchan->next_sg equals
> desc->num_sgs, so the loop
> 
>      for (i = next_sg; i < desc->num_sgs; i++)
>          residue += desc->sg_req[i].len;
> 
> naturally executes zero iterations. Thus for an active descriptor,
> lchan->next_sg is always in [1, desc->num_sgs] when
> loongson2_cmc_dma_tx_status() queries it under lchan->vchan.lock.
> 
> In this patch, the only caller that passes next_sg == 0 to
> loongson2_cmc_dma_desc_residue() is the else if (vdesc) branch in
> loongson2_cmc_dma_tx_status() for an unstarted queued descriptor on
> vchan.desc_issued:
> 
>      if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
>          state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
>                                                          lchan->next_sg);
>      else if (vdesc)
>          state->residue = loongson2_cmc_dma_desc_residue(lchan,
>                                                          to_lmdma_desc(vdesc), 0);
> 
> With desc->cyclic && next_sg == 0, querying a queued cyclic descriptor (while
> another descriptor is active) returns early before summing desc->sg_req[i].len,
> reporting only the active transfer's ndtr << width.
> 
> Even after the later commit in the series ("dmaengine: loongson:
> loongson2-apb-cmc-dma: Fix residue for queued descriptors") switches the
> else if (vdesc) branch to loongson2_cmc_dma_desc_total_len(),
> loongson2_cmc_dma_desc_residue() is only called for the active descriptor
> lchan->desc with lchan->next_sg in [1, desc->num_sgs]. Doesn't that leave the
> next_sg == 0 check as dead code and the comment describing cyclic wrap and
> non-active descriptors inaccurate?

Yes, an active cyclic descriptor next_sg is never 0.
loongson2_cmc_dma_start_transfer() sets next_sg to 0 and immediately
increments it to 1 before programming sg_req[0], and
loongson2_cmc_dma_configure_next_sg() does the same after wrapping. So
the next_sg == 0 branch is dead code for the active path, and the
existing loop already handles the last period correctly (next_sg ==
num_sgs means zero iterations).

I'm inclined to merge patch-1 and patch-2 because this section of code 
needs to be removed in patch-2, so there's no point in fixing it first 
and then deleting it.
> 
> Also, regarding the commit message description of the NULL dereference when
> lchan->desc == NULL: since vchan_find_desc() searches vchan.desc_issued, and
> both loongson2_cmc_dma_issue_pending() and loongson2_cmc_dma_handle_chan_done()
> immediately start the first descriptor from vchan.desc_issued under
> lchan->vchan.lock, can vchan.desc_issued ever be non-empty while lchan->desc
> is NULL? Should this check and its comment be removed altogether once queued
> descriptors use loongson2_cmc_dma_desc_total_len()?
> 

-- 
Thanks.
Binbin

[PATCH 2/4] dmaengine: loongson: loongson2-apb-cmc-dma: Fix residue for queued descriptors

[Binbin Zhou]

tx_status() computes residue for both the active and the queued
descriptors through loongson2_cmc_dma_desc_residue(), which reads the
hardware CNDTR register. For a descriptor that has not started yet that
register does not describe its transfer, so clients can observe a
residue that does not match the amount of data still to be moved.

Report residue from the descriptor length for queued transfers, and keep
the hardware-based calculation only for the active descriptor.

Cc: stable@vger.kernel.org
Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 23 ++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index f090fbe0b8a1..8b8d530d60c3 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -472,6 +472,17 @@ loongson2_cmc_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, si
 	return vchan_tx_prep(&lchan->vchan, &desc->vdesc, flags);
 }
 
+static size_t loongson2_cmc_dma_desc_total_len(struct loongson2_cmc_dma_desc *desc)
+{
+	size_t total = 0;
+	u32 i;
+
+	for (i = 0; i < desc->num_sgs; i++)
+		total += desc->sg_req[i].len;
+
+	return total;
+}
+
 static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lchan,
 					     struct loongson2_cmc_dma_desc *desc, u32 next_sg)
 {
@@ -518,12 +529,16 @@ static enum dma_status loongson2_cmc_dma_tx_status(struct dma_chan *chan, dma_co
 
 	scoped_guard(spinlock_irqsave, &lchan->vchan.lock) {
 		vdesc = vchan_find_desc(&lchan->vchan, cookie);
-		if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
+		if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie) {
+			/* Active descriptor: compute residue from hardware progress */
 			state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
 									lchan->next_sg);
-		else if (vdesc)
-			state->residue = loongson2_cmc_dma_desc_residue(lchan,
-									to_lmdma_desc(vdesc), 0);
+		} else if (vdesc) {
+			/* Queued descriptor not yet started: residue is the total length */
+			struct loongson2_cmc_dma_desc *ldesc = to_lmdma_desc(vdesc);
+
+			state->residue = loongson2_cmc_dma_desc_total_len(ldesc);
+		}
 	}
 
 	return status;
-- 
2.52.0

Re: [PATCH 2/4] dmaengine: loongson: loongson2-apb-cmc-dma: Fix residue for queued descriptors

[Frank Li]

On Mon, Oct 26, 2026 at 12:31:21PM +0800, Binbin Zhou wrote:
> tx_status() computes residue for both the active and the queued
> descriptors through loongson2_cmc_dma_desc_residue(), which reads the
> hardware CNDTR register. For a descriptor that has not started yet that
> register does not describe its transfer, so clients can observe a
> residue that does not match the amount of data still to be moved.

why this happen, suppose write CNDTR at loongson2_cmc_dma_start_transfer()
when issue pending.

Maybe there are raise condition, between send to issue list and start_transfer()

suppose loongson2_cmc_dma_tx_status() should not find vdesc by cookie id
if issue pending have not complete yet.

Maybe move loongson2_cmc_dma_start_transfer() into lchan->vchan.lock, you
will not see this problem.

Frank
>
> Report residue from the descriptor length for queued transfers, and keep
> the hardware-based calculation only for the active descriptor.
>
> Cc: stable@vger.kernel.org
> Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/dma/loongson/loongson2-apb-cmc-dma.c | 23 ++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index f090fbe0b8a1..8b8d530d60c3 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -472,6 +472,17 @@ loongson2_cmc_dma_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, si
>  	return vchan_tx_prep(&lchan->vchan, &desc->vdesc, flags);
>  }
>
> +static size_t loongson2_cmc_dma_desc_total_len(struct loongson2_cmc_dma_desc *desc)
> +{
> +	size_t total = 0;
> +	u32 i;
> +
> +	for (i = 0; i < desc->num_sgs; i++)
> +		total += desc->sg_req[i].len;
> +
> +	return total;
> +}
> +
>  static size_t loongson2_cmc_dma_desc_residue(struct loongson2_cmc_dma_chan *lchan,
>  					     struct loongson2_cmc_dma_desc *desc, u32 next_sg)
>  {
> @@ -518,12 +529,16 @@ static enum dma_status loongson2_cmc_dma_tx_status(struct dma_chan *chan, dma_co
>
>  	scoped_guard(spinlock_irqsave, &lchan->vchan.lock) {
>  		vdesc = vchan_find_desc(&lchan->vchan, cookie);
> -		if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie)
> +		if (lchan->desc && cookie == lchan->desc->vdesc.tx.cookie) {
> +			/* Active descriptor: compute residue from hardware progress */
>  			state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
>  									lchan->next_sg);
> -		else if (vdesc)
> -			state->residue = loongson2_cmc_dma_desc_residue(lchan,
> -									to_lmdma_desc(vdesc), 0);
> +		} else if (vdesc) {
> +			/* Queued descriptor not yet started: residue is the total length */
> +			struct loongson2_cmc_dma_desc *ldesc = to_lmdma_desc(vdesc);
> +
> +			state->residue = loongson2_cmc_dma_desc_total_len(ldesc);
> +		}
>  	}
>
>  	return status;
> --
> 2.52.0
>

[PATCH 3/4] dmaengine: loongson: loongson2-apb-cmc-dma: Program CNDTR when advancing sg

[Binbin Zhou]

When configure_next_sg() moves to the next scatterlist entry it updates
CMAR but leaves CNDTR at the value left over from the previous entry.
The controller therefore keeps using a stale transfer count, so the
hardware does not move the amount of data the descriptor describes.

Program CNDTR together with CMAR so software and hardware progress stay
in sync.

Cc: stable@vger.kernel.org
Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index 8b8d530d60c3..c654e0572902 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -266,6 +266,7 @@ static void loongson2_cmc_dma_configure_next_sg(struct loongson2_cmc_dma_chan *l
 
 	sg_req = &lchan->desc->sg_req[lchan->next_sg];
 	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CMAR, id, sg_req->chan_reg.cmar);
+	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CNDTR, id, sg_req->chan_reg.cndtr);
 
 	/* Start transition */
 	ccr |= LOONGSON2_CMCDMA_CCR_EN;
-- 
2.52.0

Re: [PATCH 3/4] dmaengine: loongson: loongson2-apb-cmc-dma: Program CNDTR when advancing sg

[Frank Li]

On Mon, Oct 26, 2026 at 12:31:22PM +0800, Binbin Zhou wrote:
> When configure_next_sg() moves to the next scatterlist entry it updates
> CMAR but leaves CNDTR at the value left over from the previous entry.
> The controller therefore keeps using a stale transfer count, so the
> hardware does not move the amount of data the descriptor describes.

How it works at 1c0028e725f1? not test ?

Frank

>
> Program CNDTR together with CMAR so software and hardware progress stay
> in sync.
>
> Cc: stable@vger.kernel.org
> Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> ---
>  drivers/dma/loongson/loongson2-apb-cmc-dma.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index 8b8d530d60c3..c654e0572902 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> @@ -266,6 +266,7 @@ static void loongson2_cmc_dma_configure_next_sg(struct loongson2_cmc_dma_chan *l
>
>  	sg_req = &lchan->desc->sg_req[lchan->next_sg];
>  	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CMAR, id, sg_req->chan_reg.cmar);
> +	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CNDTR, id, sg_req->chan_reg.cndtr);
>
>  	/* Start transition */
>  	ccr |= LOONGSON2_CMCDMA_CCR_EN;
> --
> 2.52.0
>

Re: [PATCH 3/4] dmaengine: loongson: loongson2-apb-cmc-dma: Program CNDTR when advancing sg

[Binbin Zhou]

Hi Frank:

On Thu, Oct 29, 2026 at 12:12 AM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Mon, Oct 26, 2026 at 12:31:22PM +0800, Binbin Zhou wrote:
> > When configure_next_sg() moves to the next scatterlist entry it updates
> > CMAR but leaves CNDTR at the value left over from the previous entry.
> > The controller therefore keeps using a stale transfer count, so the
> > hardware does not move the amount of data the descriptor describes.
>
> How it works at 1c0028e725f1? not test ?

Let me clarify how I found this and what I actually tested, since
there is a fair concern the patch may be redundant.

I hit this while debugging a CAN-FD driver. Reading through the
Loongson CMC driver I noticed that loongson2_cmc_dma_start_transfer()
programs both CMAR and CNDTR for an SG entry, but
loongson2_cmc_dma_configure_next_sg() programs only CMAR. The
asymmetry looked wrong, and I wanted configure_next_sg() to program
the same set of registers as start_transfer().

I instrumented the driver to read back CNDTR before and after
configure_next_sg(), and forced a non-uniform descriptor by
temporarily making period 0 shorter than the rest. With the CNDTR
write in configure_next_sg() reverted, the read-back stayed at the
period 0 length for every subsequent period, and the TC interval
matched that length — so the hardware was reusing the first period's
count. With the write restored, the read-back tracked each period's
actual length.

That test required a non-uniform descriptor that I built by hacking
prep_dma_cyclic(). As you know, cyclic mode itself is uniform by
construction — buf_len % period_len is checked, and every
sg_req[i].len is set to period_len. So in any real cyclic user today,
the hardware CNDTR preload mechanism already reloads the correct value
on each CCR.EN edge, and this patch changes nothing. I2S audio works
fine without it, and I could not have found the bug from userspace
alone.

So yes, this patch is arguably redundant for the current in-tree
users. I kept it because it makes configure_next_sg() consistent with
start_transfer() and removes a hidden "all SG entries are equal
length" assumption, also I'm happy to drop it.

>
> Frank
>
> >
> > Program CNDTR together with CMAR so software and hardware progress stay
> > in sync.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 1c0028e725f1 ("dmaengine: loongson: New driver for the Loongson Multi-Channel DMA controller")
> > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > ---
> >  drivers/dma/loongson/loongson2-apb-cmc-dma.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> > index 8b8d530d60c3..c654e0572902 100644
> > --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> > +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> > @@ -266,6 +266,7 @@ static void loongson2_cmc_dma_configure_next_sg(struct loongson2_cmc_dma_chan *l
> >
> >       sg_req = &lchan->desc->sg_req[lchan->next_sg];
> >       loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CMAR, id, sg_req->chan_reg.cmar);
> > +     loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CNDTR, id, sg_req->chan_reg.cndtr);
> >
> >       /* Start transition */
> >       ccr |= LOONGSON2_CMCDMA_CCR_EN;
> > --
> > 2.52.0
> >


-- 
Thanks.
Binbin

[PATCH 4/4] dmaengine: loongson: loongson2-apb-cmc-dma: Add pause/resume support

[Binbin Zhou]

Clients that run cyclic transfers sometimes need to stop the channel
temporarily and continue from the same position later. Without pause and
resume, such clients have to terminate and restart the transfer, which
loses the current position and can cause observable glitches or
duplicated data. In addition, advancing to the next scatterlist entry
while the client has requested a pause must not silently restart the
channel.

Provide pause/resume semantics that preserve the current hardware
position, and make scatterlist advancement honor the paused state so a
paused channel stays stopped until the client explicitly resumes it.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 drivers/dma/loongson/loongson2-apb-cmc-dma.c | 94 +++++++++++++++++++-
 1 file changed, 90 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
index c654e0572902..a4153ea85462 100644
--- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
@@ -93,6 +93,7 @@ struct loongson2_cmc_dma_chan {
 	int irq;
 	u32 next_sg;
 	struct loongson2_cmc_dma_chan_reg chan_reg;
+	bool paused;
 };
 
 struct loongson2_cmc_dma_dev {
@@ -184,6 +185,8 @@ static void loongson2_cmc_dma_stop(struct loongson2_cmc_dma_chan *lchan)
 	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, lchan->id, ccr);
 
 	loongson2_cmc_dma_irq_clear(lchan, LOONGSON2_CMCDMA_MASKI);
+
+	lchan->paused = false;
 }
 
 static int loongson2_cmc_dma_terminate_all(struct dma_chan *chan)
@@ -254,12 +257,13 @@ static void loongson2_cmc_dma_configure_next_sg(struct loongson2_cmc_dma_chan *l
 {
 	struct loongson2_cmc_dma_dev *lddev = lmdma_get_dev(lchan);
 	struct loongson2_cmc_dma_sg_req *sg_req;
+	bool keep_paused = lchan->paused;
 	u32 ccr, id = lchan->id;
 
 	if (lchan->next_sg == lchan->desc->num_sgs)
 		lchan->next_sg = 0;
 
-	/* Stop to update mem addr */
+	/* Stop to update mem addr (idempotent when already paused) */
 	ccr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CCR, id);
 	ccr &= ~LOONGSON2_CMCDMA_CCR_EN;
 	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, id, ccr);
@@ -268,9 +272,16 @@ static void loongson2_cmc_dma_configure_next_sg(struct loongson2_cmc_dma_chan *l
 	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CMAR, id, sg_req->chan_reg.cmar);
 	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CNDTR, id, sg_req->chan_reg.cndtr);
 
-	/* Start transition */
-	ccr |= LOONGSON2_CMCDMA_CCR_EN;
-	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, id, ccr);
+	/*
+	 * Only re-enable when the client has not paused the channel.
+	 * A paused cyclic channel still needs its CMAR/CNDTR advanced
+	 * so that resume() starts on the right scatterlist entry, but
+	 * the enable bit must stay clear until the client asks for it.
+	 */
+	if (!keep_paused) {
+		ccr |= LOONGSON2_CMCDMA_CCR_EN;
+		loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, id, ccr);
+	}
 }
 
 static void loongson2_cmc_dma_handle_chan_done(struct loongson2_cmc_dma_chan *lchan)
@@ -534,6 +545,13 @@ static enum dma_status loongson2_cmc_dma_tx_status(struct dma_chan *chan, dma_co
 			/* Active descriptor: compute residue from hardware progress */
 			state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
 									lchan->next_sg);
+			/*
+			 * A paused channel still has an active descriptor but is not making
+			 * progress. Report DMA_PAUSED so clients can distinguish it from a
+			 * running transfer.
+			 */
+			if (lchan->paused && status == DMA_IN_PROGRESS)
+				status = DMA_PAUSED;
 		} else if (vdesc) {
 			/* Queued descriptor not yet started: residue is the total length */
 			struct loongson2_cmc_dma_desc *ldesc = to_lmdma_desc(vdesc);
@@ -624,6 +642,72 @@ static int loongson2_cmc_dma_of_controller_register(struct loongson2_cmc_dma_dev
 	return of_dma_controller_register(dev->of_node, loongson2_cmc_dma_of_xlate, lddev);
 }
 
+/**
+ * loongson2_cmc_dma_pause() - Stop the channel without losing state
+ * @chan: DMA channel
+ *
+ * Clears CCR.EN so the controller stops issuing requests to the
+ * peripheral. CNDTR and CMAR keep their current values, so
+ * loongson2_cmc_dma_resume() can pick up exactly where the transfer
+ * left off. The channel remains in cyclic mode; only the enable bit is
+ * dropped, so a paused cyclic channel does not advance the write
+ * pointer until resumed.
+ *
+ * Return: 0 on success.
+ */
+static int loongson2_cmc_dma_pause(struct dma_chan *chan)
+{
+	struct loongson2_cmc_dma_chan *lchan = to_lmdma_chan(chan);
+	struct loongson2_cmc_dma_dev *lddev = lmdma_get_dev(lchan);
+	u32 ccr;
+
+	guard(spinlock_irqsave)(&lchan->vchan.lock);
+
+	if (lchan->paused || !lchan->desc)
+		return 0;
+
+	lchan->paused = true;
+
+	ccr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CCR, lchan->id);
+	ccr &= ~LOONGSON2_CMCDMA_CCR_EN;
+	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, lchan->id, ccr);
+
+	return 0;
+}
+
+/**
+ * loongson2_cmc_dma_resume() - Restart a paused channel
+ * @chan: DMA channel
+ *
+ * Restores CCR.EN. CNDTR is left untouched, so the transfer continues
+ * from the exact word where the pause happened.
+ *
+ * Return: 0 on success.
+ */
+static int loongson2_cmc_dma_resume(struct dma_chan *chan)
+{
+	struct loongson2_cmc_dma_chan *lchan = to_lmdma_chan(chan);
+	struct loongson2_cmc_dma_dev *lddev = lmdma_get_dev(lchan);
+	u32 ccr;
+
+	guard(spinlock_irqsave)(&lchan->vchan.lock);
+
+	if (!lchan->paused)
+		return 0;
+
+	lchan->paused = false;
+
+	/* No descriptor: nothing left to run */
+	if (!lchan->desc)
+		return 0;
+
+	ccr = loongson2_cmc_dma_read(lddev, LOONGSON2_CMCDMA_CCR, lchan->id);
+	ccr |= LOONGSON2_CMCDMA_CCR_EN;
+	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, lchan->id, ccr);
+
+	return 0;
+}
+
 static int loongson2_cmc_dma_probe(struct platform_device *pdev)
 {
 	const struct loongson2_cmc_dma_config *config;
@@ -676,6 +760,8 @@ static int loongson2_cmc_dma_probe(struct platform_device *pdev)
 	ddev->device_synchronize = loongson2_cmc_dma_synchronize;
 	ddev->device_tx_status = loongson2_cmc_dma_tx_status;
 	ddev->device_terminate_all = loongson2_cmc_dma_terminate_all;
+	ddev->device_pause = loongson2_cmc_dma_pause;
+	ddev->device_resume = loongson2_cmc_dma_resume;
 
 	ddev->max_sg_burst = LOONSON2_CMCDMA_MAX_DATA_ITEMS;
 	ddev->src_addr_widths = LOONGSON2_CMCDMA_BUSWIDTHS;
-- 
2.52.0

Re: [PATCH 4/4] dmaengine: loongson: loongson2-apb-cmc-dma: Add pause/resume support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dmaengine: loongson: loongson2_cmc_dma_start_transfer() clears pause state
- [Medium] dmaengine: loongson: loongson2_cmc_dma_tx_status() ignores pause if !state
--

commit 8e161a43d78d90e76eaddc39b1b96c7a28b1dd6c
Author: Binbin Zhou <zhoubinbin@loongson.cn>

dmaengine: loongson: loongson2-apb-cmc-dma: Add pause/resume support

This commit adds pause and resume support to the Loongson-2 APB CMC DMA
driver by clearing and restoring LOONGSON2_CMCDMA_CCR_EN while preserving
the hardware transfer position. It also updates status reporting and
cyclic scatterlist advancement to honor the paused state.

> diff --git a/drivers/dma/loongson/loongson2-apb-cmc-dma.c b/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> index c654e05729024..a4153ea854623 100644
> --- a/drivers/dma/loongson/loongson2-apb-cmc-dma.c
> +++ b/drivers/dma/loongson/loongson2-apb-cmc-dma.c

[ ... ]

> @@ -184,6 +185,8 @@ static void loongson2_cmc_dma_stop(struct loongson2_cmc_dma_chan *lchan)
>  	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, lchan->id, ccr);
>  
>  	loongson2_cmc_dma_irq_clear(lchan, LOONGSON2_CMCDMA_MASKI);
> +
> +	lchan->paused = false;
>  }

[Severity: Medium]
Can this reset lchan->paused and restart DMA when a segment completion
interrupt races with loongson2_cmc_dma_pause() on a non-cyclic transfer?

loongson2_cmc_dma_pause() allows pausing any channel with an active
lchan->desc, including non-cyclic slave_sg transfers. If a scatterlist
segment finishes and latches LOONGSON2_CMCDMA_TCI just before
loongson2_cmc_dma_pause() acquires lchan->vchan.lock and sets
lchan->paused = true, the interrupt handler will subsequently run:

  loongson2_cmc_dma_chan_irq()
    -> loongson2_cmc_dma_handle_chan_done()
      -> loongson2_cmc_dma_start_transfer()

Unlike the cyclic path in loongson2_cmc_dma_configure_next_sg(), the
non-cyclic path in loongson2_cmc_dma_handle_chan_done() calls
loongson2_cmc_dma_start_transfer():

	loongson2_cmc_dma_stop(lchan);
	...
	/* Start DMA */
	reg->ccr |= LOONGSON2_CMCDMA_CCR_EN;
	loongson2_cmc_dma_write(lddev, LOONGSON2_CMCDMA_CCR, lchan->id, reg->ccr);

This clears lchan->paused = false in loongson2_cmc_dma_stop() and
unconditionally sets LOONGSON2_CMCDMA_CCR_EN for the next scatterlist
entry or next queued descriptor. Should non-cyclic scatterlist
advancement also preserve lchan->paused and avoid re-enabling the channel
while paused?

[ ... ]

> @@ -534,6 +545,13 @@ static enum dma_status loongson2_cmc_dma_tx_status(struct dma_chan *chan, dma_co
>  			/* Active descriptor: compute residue from hardware progress */
>  			state->residue = loongson2_cmc_dma_desc_residue(lchan, lchan->desc,
>  									lchan->next_sg);
> +			/*
> +			 * A paused channel still has an active descriptor but is not making
> +			 * progress. Report DMA_PAUSED so clients can distinguish it from a
> +			 * running transfer.
> +			 */
> +			if (lchan->paused && status == DMA_IN_PROGRESS)
> +				status = DMA_PAUSED;
>  		} else if (vdesc) {
>  			/* Queued descriptor not yet started: residue is the total length */
>  			struct loongson2_cmc_dma_desc *ldesc = to_lmdma_desc(vdesc);

[Severity: Medium]
What happens if a client queries transfer status on a paused channel with
a NULL state pointer, such as via dmaengine_tx_status(chan, cookie, NULL)
or dma_async_is_tx_complete()?

Earlier in loongson2_cmc_dma_tx_status():

	status = dma_cookie_status(chan, cookie, state);
	if (status == DMA_COMPLETE || !state)
		return status;

When state is NULL, loongson2_cmc_dma_tx_status() returns early before
acquiring lchan->vchan.lock and checking lchan->paused, so it will return
DMA_IN_PROGRESS instead of DMA_PAUSED for the active paused descriptor.
Should the lchan->paused check still be performed when state is NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792988985.git.zhoubinbin@loongson.cn?part=4