Re: [PATCH v4 3/4] dma/ae4dma: add data path operations

[Raghavendra Ningoji <raghavendra.ningoji@amd.com>]

On Mon, 7 Jul 2026 at 06:49, fengchengwen <fengchengwen@huawei.com> wrote:
>
> > +	if (last_idx != NULL)
> > +		*last_idx = (uint16_t)(cmd_q->next_read - 1);
>
> 1\ the last_idx always non-NULL for driver, so no need for
>    'if (last_idx != NULL)'
> 2\ last_idx should be the last success completed index

Both fixed in v5. The NULL check is dropped, and last_idx now reports
the ring_idx of the last *successfully* completed op.

> Consider four reqests, the hardware mark each request as following:
>     req1   success
>     req2   err
>     req3   success
>     req4   err
> the cpl_count will be 4, and err_count will be 2
> It will return 2 in current impl, and last_idx will be the req4's
> But in this function, it should return 1, and last_idx should be req1's

Agreed - thanks for the clear example. rte_dma_completed() is reworked
in v5 to stop at the first failed descriptor: it returns only the run of
successful ops preceding the error (1 in your example), sets *has_error,
and leaves the failed op in place with last_idx pointing at the last
success (req1). The failed op and everything after it are then drained
and reported by rte_dma_completed_status().

On a related note, while implementing the above I found the last_idx
documentation for rte_dma_completed() and rte_dma_completed_status()
to be identical:

    @param[out] last_idx
      The last completed operation's ring_idx.

Since rte_dma_completed() only reports operations that completed
*successfully* (it stops at the first error), its last_idx is really the
last successfully completed operation's ring_idx, whereas for
rte_dma_completed_status() it is the last completed operation regardless
of status. Would a small doc clarification along these lines be welcome
(as a separate patch)?

    rte_dma_completed():
    @param[out] last_idx
      The last successfully completed operation's ring_idx.

I'm happy to send that as a separate dmadev doc patch if you agree.

Thanks,
Raghavendra