On Fri, 27 Jun 2026 at 08:23, fengchengwen <[email protected]> wrote: > > > This commit adds: > > - copy enqueue (rte_dma_copy): ... > I don't think it's necessary to write in such detail because the ops > implemented are defined by the framework. If needed, you can supplement > by explaining what special features this driver has.
Condensed the commit log in v4. It now just lists the ops and explains the driver-specific completion handling (the engine signals completion by advancing read_idx rather than reliably writing a status word back). > > + cmd_q->next_write = (uint16_t)((write + 1) % nb); > the next_write is [0, nb_desc-1], and it will as return value as copy, > but the dmadev framework expect as [0, 0xFFFF], I doubt your drvier was > not passed in any DMA test (e.g. dpdk-test, dpdk-dma-perf or examples/dma) > ... > > + *last_idx = (uint16_t)((cmd_q->next_read - 1 + nb) % nb); > the last_idx should be in range of [0, 0xFFFF] Rather than having two counters,framework counter and h/w slot count in dricer, we had verified internally by wrapping counters of dma-test apps to h/w slots(32). But again as you mentioned everything has to go with framework.v4 reworks the indexing so next_write/next_read are free-running 16-bit counters in [0, 0xFFFF]: - rte_dma_copy() returns the pre-increment free-running ring_idx. - completed()/completed_status() always set last_idx to the last completed ring_idx (next_read - 1), even when no new op is reaped. - the HW ring slot is derived as (hw_base + idx) & (nb_desc - 1), and in-flight as (next_write - next_read), which is why the depth is kept a power of two. With this plus RTE_DMA_CAPA_OPS_COPY (patch 2/4), the driver follows the framework ring-index contract. > > + case AE4DMA_DMA_ERR_INV_ALIGN: > > + /* Name matches DPDK public enum spelling. */ > > + return RTE_DMA_STATUS_DATA_POISION; > Suggest add RTE_DMA_STATUS_INVALID_ALIGN enum in rte_dmadev.h Agreed that reusing DATA_POISION was wrong. For v4 the alignment error maps to RTE_DMA_STATUS_ERROR_UNKNOWN so we no longer misuse an unrelated code. Adding a dedicated RTE_DMA_STATUS_INVALID_ALIGNMENT to the public rte_dmadev.h touches the dmadev API, so I would prefer to do that as a separate follow-up patch with its own justification - happy to send it if you agree. > > + if (nb < 2 || !rte_is_power_of_2(nb)) > > + return 0; > No need to check this Removed; vchan_setup already guarantees a valid power-of-two depth. burst_capacity now just computes (nb - 1) - in_flight. Thanks, Raghavendra
