* [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors
@ 2026-07-20 14:25 Mahad Ibrahim
2026-07-20 14:40 ` sashiko-bot
2026-07-21 7:32 ` Keguang Zhang
0 siblings, 2 replies; 4+ messages in thread
From: Mahad Ibrahim @ 2026-07-20 14:25 UTC (permalink / raw)
To: Keguang Zhang, Vinod Koul
Cc: Frank Li, linux-mips, dmaengine, linux-kernel, Mahad Ibrahim
ls1x_dma_tx_status() locates the descriptor actively being processed by
walking the LLI list and comparing the hardware reported next descriptor
pointer against each element's next-descriptor pointer.
A list_for_each_entry macro is used in the comparison phase, which
internally expands to the container_of macro to find the current
ls1x_dma_lli's next-descriptor pointer to compare against. However a
problem occurs when the loop reaches the final lli list element and wraps
around to the head. The head is a different struct ls1x_dma_desc. It goes
down to the offset the node member would be at in ls1x_dma_lli inside of
ls1x_dma_desc.
This leaves the lli pointer somewhere inside of ls1x_dma_desc at the end
of the loop. The subsequent phys debug value read therefore would print
garbage as a valid phys address.
The primary bug occurs in the residue calculation list_for_each_entry_from
macro. The lli pointer still points to somewhere inside of ls1x_dma_desc,
when the list_for_each_entry_from performs the same wrong offset
calculation on ls1x_dma_desc to determine the stop condition of the loop,
it lands at the same offset that lli pointer is on, and the loop
immediately stops.
This produces a 0 residue value, while it should be the entire length of
the lli list chain. The driver advertises DMA_RESIDUE_GRANULARITY_SEGMENT,
so a 0 residue would mean the transfer has completed.
Found by the following Coccinelle check:
scripts/coccinelle/iterators/use_after_iter.cocci
drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
reference to the index variable of the iterator on line 450
Note the check still reports this line after the fix; the remaining use
is safe (the cursor is only dereferenced on the match path, or reset to
the first entry otherwise).
I did not see a bug upstream detailing this error, nor do I have the
hardware to confirm this bug or error, all this is from a pure code
examination.
As I do not possess the hardware, I cannot test the patch. Compile tested
only with mips64-linux-gnu-gcc.
Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
drivers/dma/loongson/loongson1-apb-dma.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
index 89786cbd20ab..8bb6e6e5719d 100644
--- a/drivers/dma/loongson/loongson1-apb-dma.c
+++ b/drivers/dma/loongson/loongson1-apb-dma.c
@@ -439,6 +439,7 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
struct ls1x_dma_lli *lli;
dma_addr_t next_phys;
+ bool found = false;
/* get the current lli */
if (ls1x_dma_query(chan, &chan->curr_lli->phys))
@@ -447,11 +448,17 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
/* locate the current lli */
next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
list_for_each_entry(lli, &desc->lli_list, node)
- if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
+ if (lli->hw[LS1X_DMADESC_NEXT] == next_phys) {
+ found = true;
break;
-
- dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
- &lli->phys);
+ }
+
+ if (!found)
+ lli = list_first_entry(&desc->lli_list,
+ struct ls1x_dma_lli, node);
+ else
+ dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
+ &lli->phys);
/* count the residues */
list_for_each_entry_from(lli, &desc->lli_list, node)
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors
2026-07-20 14:25 [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors Mahad Ibrahim
@ 2026-07-20 14:40 ` sashiko-bot
2026-07-21 7:32 ` Keguang Zhang
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-20 14:40 UTC (permalink / raw)
To: Mahad Ibrahim; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
Pre-existing issues:
- [High] Calling `issue_pending` while a transfer is active forcefully restarts the active transfer from the beginning.
- [High] The DMA IRQ handler fails to start the next pending descriptor when the current transfer completes.
- [High] Zero-length scatterlists or cyclic configurations with buf_len < period_len cause NULL pointer dereferences and memory corruption.
- [High] IRQ and devres leak on resource allocation failure.
- [High] Lockless read-modify-write race on the shared global `LS1X_DMA_CTRL` register corrupts commands and stops wrong channels.
- [High] Idle hardware causes false-positive LLI matches, reporting partial residue for pending transfers.
- [Medium] The driver stalls the CPU for up to 3ms inside a `spinlock_irqsave` critical section.
- [Medium] The driver tracks `bus_width` globally per channel rather than per descriptor, corrupting residue calculations.
--
--- Patch [1]: [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors ---
commit 62e04cef060bef1de9eb6ca4a2d6840c563a954f
Author: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors
This commit fixes a bug in ls1x_dma_tx_status() where the loop locating the
current descriptor could wrap around to the list head. The wrapped pointer
caused the subsequent list_for_each_entry_from() to stop immediately, resulting
in a residue of 0 being reported.
> diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
> index 89786cbd20ab5..8bb6e6e5719df 100644
> --- a/drivers/dma/loongson/loongson1-apb-dma.c
> +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> @@ -439,6 +439,7 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
[ ... ]
> /* locate the current lli */
> next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
> list_for_each_entry(lli, &desc->lli_list, node)
> - if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
> + if (lli->hw[LS1X_DMADESC_NEXT] == next_phys) {
> + found = true;
> break;
> -
> - dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> - &lli->phys);
> + }
[Severity: High]
This is a pre-existing issue, but might idle hardware cause false-positive
LLI matches and bypass the newly added fallback logic?
The final LLI of all non-cyclic descriptors is initialized with NEXT == 0.
When the hardware is idle after a previous transfer, its next_phys register
remains 0.
If a client queries the status of a pending, unstarted descriptor, will the
driver incorrectly match next_phys = 0 to the descriptor's final LLI, setting
found = true, and report only the length of the last segment as the residue
instead of the full size?
> +
> + if (!found)
> + lli = list_first_entry(&desc->lli_list,
> + struct ls1x_dma_lli, node);
> + else
> + dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> + &lli->phys);
>
> /* count the residues */
> list_for_each_entry_from(lli, &desc->lli_list, node)
[Severity: Medium]
This isn't a bug introduced by this patch, but will tracking bus_width
globally per channel rather than per descriptor corrupt residue calculations?
In ls1x_dma_prep_lli(), the bus width for a specific transfer is cached
globally in chan->bus_width. Later, ls1x_dma_tx_status() calculates the
residue:
bytes += lli->hw[LS1X_DMADESC_LENGTH] * chan->bus_width;
If a client queues multiple transfers with different configurations or
directions on the same channel, will querying the status of older transfers
use the width of the most recently prepared transfer and result in an
incorrect byte count?
[Severity: High]
This is a pre-existing issue, but does calling vchan_issue_pending() while a
transfer is active forcefully restart the active transfer from the beginning?
When vchan_issue_pending() is called, it moves submitted descriptors to the
issued list and returns true if the list is not empty.
In ls1x_dma_issue_pending(), it unconditionally fetches the first descriptor:
if (vchan_issue_pending(&chan->vc)) {
struct virt_dma_desc *vd = vchan_next_desc(&chan->vc);
...
ls1x_dma_start(chan, &lli->phys);
}
Because vchan_next_desc() returns the currently running transfer (it is only
removed upon completion), this immediately calls ls1x_dma_start() and
instructs the hardware to begin a new transfer at the active descriptor's
start address.
Can this destroy the current hardware state machine and restart the ongoing
transfer, leading to data corruption?
[Severity: High]
This isn't a bug introduced by this patch, but is it possible that the DMA IRQ
handler fails to start the next pending descriptor when the current transfer
completes?
When a non-cyclic transfer completes, ls1x_dma_irq_handler() removes the
completed descriptor from the issued list and marks it complete:
} else {
list_del(&vd->node);
vchan_cookie_complete(vd);
}
It does not fetch the next queued descriptor or call ls1x_dma_start() to
initiate the next transfer.
If a client submits multiple descriptors, will the channel stall indefinitely
after finishing the first one?
[Severity: High]
This is a pre-existing issue, but can zero-length scatterlists or cyclic
configurations with buf_len < period_len cause NULL pointer dereferences and
memory corruption?
In ls1x_dma_prep_dma_cyclic(), if buf_len < period_len, sg_len becomes 0.
ls1x_dma_prep_lli() is then called with sg_len == 0, bypassing the iteration
loop, leaving lli and first pointers uninitialized.
If is_cyclic is true, the driver immediately dereferences the NULL first
pointer:
if (is_cyclic) {
lli->hw[LS1X_DMADESC_NEXT] = first->phys | LS1X_DMA_NEXT_VALID;
chan->is_cyclic = is_cyclic;
}
If is_cyclic is false, it returns success with an empty lli_list.
When ls1x_dma_issue_pending() processes it:
lli = list_first_entry(&desc->lli_list, struct ls1x_dma_lli, node);
ls1x_dma_start(chan, &lli->phys);
Can this force list_first_entry() to return a garbage offset and instruct
the hardware to execute from random memory addresses?
[Severity: High]
This isn't a bug introduced by this patch, but is there an IRQ and devres
leak on resource allocation failure?
ls1x_dma_alloc_chan_resources() dynamically registers an IRQ handler using
devm_request_irq().
If subsequent allocations like dma_pool_create() fail:
if (!chan->lli_pool)
return -ENOMEM;
It returns an error without calling devm_free_irq(). The DMA engine core
handles the failure but does not call the corresponding free_chan_resources()
callback on allocation failure.
Will the IRQ handler remain permanently registered, allowing devm allocations
to accumulate and potentially exhausting memory or registering duplicate
handlers upon repeated client requests?
[Severity: High]
This is a pre-existing issue, but can a lockless read-modify-write race on
the shared global LS1X_DMA_CTRL register corrupt commands and stop wrong
channels?
The LS1X_DMA_CTRL register is a single global register mapped to
chan->reg_base and shared across all DMA channels. ls1x_dma_stop() performs a
lockless read-modify-write:
int val = readl(chan->reg_base + LS1X_DMA_CTRL);
writel(val | LS1X_DMA_STOP, chan->reg_base + LS1X_DMA_CTRL);
Because it does not clear the previous channel ID field in val, does it send
the STOP command to whichever channel was last written to the register, even
if ls1x_dma_stop() was called for a different channel?
Will concurrent operations using per-channel locks in ls1x_dma_start() and
ls1x_dma_query() also silently race and corrupt the register?
[Severity: Medium]
This isn't a bug introduced by this patch, but could stalling the CPU for up
to 3ms inside a spinlock_irqsave() critical section be problematic?
ls1x_dma_tx_status() acquires a per-channel spinlock and disables interrupts.
It then calls ls1x_dma_query(), which loops polling the hardware:
ret = readl_poll_timeout_atomic(chan->reg_base + LS1X_DMA_CTRL, val,
!(val & LS1X_DMA_ASK_VALID), 0, 3000);
Can stalling a CPU for 3000 microseconds with interrupts masked degrade system
latency and trigger soft lockup detectors under worst-case hardware delays?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720142538.2766-1-mahad.ibrahim.dev@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors
2026-07-20 14:25 [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors Mahad Ibrahim
2026-07-20 14:40 ` sashiko-bot
@ 2026-07-21 7:32 ` Keguang Zhang
2026-07-21 16:14 ` Mahad Ibrahim
1 sibling, 1 reply; 4+ messages in thread
From: Keguang Zhang @ 2026-07-21 7:32 UTC (permalink / raw)
To: Mahad Ibrahim; +Cc: Vinod Koul, Frank Li, linux-mips, dmaengine, linux-kernel
Hi Mahad,
Thanks for catching this use-after-iterator issue.
On Mon, Jul 20, 2026 at 10:25 PM Mahad Ibrahim
<mahad.ibrahim.dev@gmail.com> wrote:
>
> ls1x_dma_tx_status() locates the descriptor actively being processed by
> walking the LLI list and comparing the hardware reported next descriptor
> pointer against each element's next-descriptor pointer.
>
> A list_for_each_entry macro is used in the comparison phase, which
> internally expands to the container_of macro to find the current
> ls1x_dma_lli's next-descriptor pointer to compare against. However a
> problem occurs when the loop reaches the final lli list element and wraps
> around to the head. The head is a different struct ls1x_dma_desc. It goes
> down to the offset the node member would be at in ls1x_dma_lli inside of
> ls1x_dma_desc.
>
> This leaves the lli pointer somewhere inside of ls1x_dma_desc at the end
> of the loop. The subsequent phys debug value read therefore would print
> garbage as a valid phys address.
>
> The primary bug occurs in the residue calculation list_for_each_entry_from
> macro. The lli pointer still points to somewhere inside of ls1x_dma_desc,
> when the list_for_each_entry_from performs the same wrong offset
> calculation on ls1x_dma_desc to determine the stop condition of the loop,
> it lands at the same offset that lli pointer is on, and the loop
> immediately stops.
>
> This produces a 0 residue value, while it should be the entire length of
> the lli list chain. The driver advertises DMA_RESIDUE_GRANULARITY_SEGMENT,
> so a 0 residue would mean the transfer has completed.
>
> Found by the following Coccinelle check:
>
> scripts/coccinelle/iterators/use_after_iter.cocci
>
> drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
> reference to the index variable of the iterator on line 450
>
> Note the check still reports this line after the fix; the remaining use
> is safe (the cursor is only dereferenced on the match path, or reset to
> the first entry otherwise).
>
> I did not see a bug upstream detailing this error, nor do I have the
> hardware to confirm this bug or error, all this is from a pure code
> examination.
>
> As I do not possess the hardware, I cannot test the patch. Compile tested
> only with mips64-linux-gnu-gcc.
>
> Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
> ---
> drivers/dma/loongson/loongson1-apb-dma.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
> index 89786cbd20ab..8bb6e6e5719d 100644
> --- a/drivers/dma/loongson/loongson1-apb-dma.c
> +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> @@ -439,6 +439,7 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
> struct ls1x_dma_lli *lli;
> dma_addr_t next_phys;
> + bool found = false;
>
> /* get the current lli */
> if (ls1x_dma_query(chan, &chan->curr_lli->phys))
> @@ -447,11 +448,17 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> /* locate the current lli */
> next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
> list_for_each_entry(lli, &desc->lli_list, node)
> - if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
> + if (lli->hw[LS1X_DMADESC_NEXT] == next_phys) {
> + found = true;
> break;
> -
> - dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> - &lli->phys);
> + }
> +
> + if (!found)
> + lli = list_first_entry(&desc->lli_list,
> + struct ls1x_dma_lli, node);
However, I don't think falling back to list_first_entry() when no
match is found is appropriate. Since next_phys should always match one
of the LLIs in the descriptor chain, failing to find a match should be
treated as an unexpected condition rather than silently falling back
to the first LLI.
I would suggest something like:
scoped_guard(spinlock_irqsave, &chan->vc.lock) {
vd = vchan_find_desc(&chan->vc, cookie);
if (vd) {
struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
struct ls1x_dma_lli *lli;
dma_addr_t next_phys;
/* get the current lli */
if (ls1x_dma_query(chan, &chan->curr_lli->phys))
return status;
/* locate the current lli */
next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
list_for_each_entry(lli, &desc->lli_list, node) {
if (lli->hw[LS1X_DMADESC_NEXT] != next_phys)
continue;
dev_dbg(chan2dev(dchan), "current
lli_phys=%pad\n", &lli->phys);
/* count the residues */
list_for_each_entry_from(lli,
&desc->lli_list, node)
bytes +=
lli->hw[LS1X_DMADESC_LENGTH] * chan->bus_width;
dma_set_residue(state, bytes);
return status;
}
dev_warn(chan2dev(dchan), "unable to locate
current LLI\n");
}
}
return status;
> + else
> + dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> + &lli->phys);
>
> /* count the residues */
> list_for_each_entry_from(lli, &desc->lli_list, node)
> --
> 2.54.0
>
--
Best regards,
Keguang Zhang
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors
2026-07-21 7:32 ` Keguang Zhang
@ 2026-07-21 16:14 ` Mahad Ibrahim
0 siblings, 0 replies; 4+ messages in thread
From: Mahad Ibrahim @ 2026-07-21 16:14 UTC (permalink / raw)
To: Keguang Zhang; +Cc: Vinod Koul, Frank Li, linux-mips, dmaengine, linux-kernel
Hello Keguang,
Thank you for the quick response and feedback. I'll adopt the single-loop
structure.
Additionally, Sashiko pointed out another possible flaw, which occurs when
the hardware is idle. The hw[LS1X_DMADESC_NEXT] of the last terminator LLI
in a chain is 0 and the current next_phys the hardware returns with
next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
is also 0. Which would match, however I do not think it's the preferred
behavior, the residue would only be the length of the last LLI. If
you wish I can add it as an additional patch on top of the single-loop
structure.
Best regards,
Mahad Ibrahim
On Tue, Jul 21, 2026 at 12:32 PM Keguang Zhang <keguang.zhang@gmail.com> wrote:
>
> Hi Mahad,
>
> Thanks for catching this use-after-iterator issue.
>
> On Mon, Jul 20, 2026 at 10:25 PM Mahad Ibrahim
> <mahad.ibrahim.dev@gmail.com> wrote:
> >
> > ls1x_dma_tx_status() locates the descriptor actively being processed by
> > walking the LLI list and comparing the hardware reported next descriptor
> > pointer against each element's next-descriptor pointer.
> >
> > A list_for_each_entry macro is used in the comparison phase, which
> > internally expands to the container_of macro to find the current
> > ls1x_dma_lli's next-descriptor pointer to compare against. However a
> > problem occurs when the loop reaches the final lli list element and wraps
> > around to the head. The head is a different struct ls1x_dma_desc. It goes
> > down to the offset the node member would be at in ls1x_dma_lli inside of
> > ls1x_dma_desc.
> >
> > This leaves the lli pointer somewhere inside of ls1x_dma_desc at the end
> > of the loop. The subsequent phys debug value read therefore would print
> > garbage as a valid phys address.
> >
> > The primary bug occurs in the residue calculation list_for_each_entry_from
> > macro. The lli pointer still points to somewhere inside of ls1x_dma_desc,
> > when the list_for_each_entry_from performs the same wrong offset
> > calculation on ls1x_dma_desc to determine the stop condition of the loop,
> > it lands at the same offset that lli pointer is on, and the loop
> > immediately stops.
> >
> > This produces a 0 residue value, while it should be the entire length of
> > the lli list chain. The driver advertises DMA_RESIDUE_GRANULARITY_SEGMENT,
> > so a 0 residue would mean the transfer has completed.
> >
> > Found by the following Coccinelle check:
> >
> > scripts/coccinelle/iterators/use_after_iter.cocci
> >
> > drivers/dma/loongson/loongson1-apb-dma.c:461:6-9: ERROR: invalid
> > reference to the index variable of the iterator on line 450
> >
> > Note the check still reports this line after the fix; the remaining use
> > is safe (the cursor is only dereferenced on the match path, or reset to
> > the first entry otherwise).
> >
> > I did not see a bug upstream detailing this error, nor do I have the
> > hardware to confirm this bug or error, all this is from a pure code
> > examination.
> >
> > As I do not possess the hardware, I cannot test the patch. Compile tested
> > only with mips64-linux-gnu-gcc.
> >
> > Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
> > ---
> > drivers/dma/loongson/loongson1-apb-dma.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
> > index 89786cbd20ab..8bb6e6e5719d 100644
> > --- a/drivers/dma/loongson/loongson1-apb-dma.c
> > +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> > @@ -439,6 +439,7 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> > struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
> > struct ls1x_dma_lli *lli;
> > dma_addr_t next_phys;
> > + bool found = false;
> >
> > /* get the current lli */
> > if (ls1x_dma_query(chan, &chan->curr_lli->phys))
> > @@ -447,11 +448,17 @@ static enum dma_status ls1x_dma_tx_status(struct dma_chan *dchan,
> > /* locate the current lli */
> > next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
> > list_for_each_entry(lli, &desc->lli_list, node)
> > - if (lli->hw[LS1X_DMADESC_NEXT] == next_phys)
> > + if (lli->hw[LS1X_DMADESC_NEXT] == next_phys) {
> > + found = true;
> > break;
> > -
> > - dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> > - &lli->phys);
> > + }
> > +
> > + if (!found)
> > + lli = list_first_entry(&desc->lli_list,
> > + struct ls1x_dma_lli, node);
>
> However, I don't think falling back to list_first_entry() when no
> match is found is appropriate. Since next_phys should always match one
> of the LLIs in the descriptor chain, failing to find a match should be
> treated as an unexpected condition rather than silently falling back
> to the first LLI.
> I would suggest something like:
>
> scoped_guard(spinlock_irqsave, &chan->vc.lock) {
> vd = vchan_find_desc(&chan->vc, cookie);
> if (vd) {
> struct ls1x_dma_desc *desc = to_ls1x_dma_desc(vd);
> struct ls1x_dma_lli *lli;
> dma_addr_t next_phys;
>
> /* get the current lli */
> if (ls1x_dma_query(chan, &chan->curr_lli->phys))
> return status;
>
> /* locate the current lli */
> next_phys = chan->curr_lli->hw[LS1X_DMADESC_NEXT];
> list_for_each_entry(lli, &desc->lli_list, node) {
> if (lli->hw[LS1X_DMADESC_NEXT] != next_phys)
> continue;
>
> dev_dbg(chan2dev(dchan), "current
> lli_phys=%pad\n", &lli->phys);
>
> /* count the residues */
> list_for_each_entry_from(lli,
> &desc->lli_list, node)
> bytes +=
> lli->hw[LS1X_DMADESC_LENGTH] * chan->bus_width;
>
> dma_set_residue(state, bytes);
> return status;
> }
>
> dev_warn(chan2dev(dchan), "unable to locate
> current LLI\n");
> }
> }
>
> return status;
>
> > + else
> > + dev_dbg(chan2dev(dchan), "current lli_phys=%pad",
> > + &lli->phys);
> >
> > /* count the residues */
> > list_for_each_entry_from(lli, &desc->lli_list, node)
> > --
> > 2.54.0
> >
>
>
> --
> Best regards,
>
> Keguang Zhang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-21 16:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20 14:25 [PATCH] dmaengine: loongson1-apb-dma: fix residue calculation for queued descriptors Mahad Ibrahim
2026-07-20 14:40 ` sashiko-bot
2026-07-21 7:32 ` Keguang Zhang
2026-07-21 16:14 ` Mahad Ibrahim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox