On Fri, 7 Aug 2026 12:18:48 +0530 Gagandeep Singh <[email protected]> wrote:
> V2-changes: > - Added dependent patch: bus/platform: match device by devicetree compatible > string > - Fixed multiple AI reported issues includes: > - removed the alias from the driver, no need of it > - removed imx_edma5_write32 from data-path, reported as coherency > concern. > - fix submit for previously enqueued jobs. > - Added dsb in data path > - Added a commnet for rte_mem_iova2virt() NULL behaviour. > - Fixed: TCD NBYTES bits 31:30 are SMLOE/DMLOE — full 32-bit length > corrupts them > - Data path is fully synchronous: Acknowledged but intentionally deferred — > the synchronous design is a deliberate simplification for the initial > upstream submission. > Noted in docs. > - In-memory TCD64 pool is more complex than needed: > Noted/deferred — the pool structure was retained for forward > compatibility > with hardware SG chaining. > - Extended `imx_edma5_dump()` to read and print `CH_CSR` and `CH_ES` > registers > for every configured vchan. > - Double and stary lines removed. > > V1: > This patch series adds a new dmadev Poll-Mode Driver (PMD) for the NXP > i.MX95 eDMA5 (Enhanced DMA Type 5) controller. > > Key features supported by this driver: > - Memory-to-memory copy (RTE_DMA_OP_TYPE_MEMCPY) > - Scatter-gather memory copy (RTE_DMA_OP_TYPE_SG) > - 64-bit TCD (Transfer Control Descriptor) format > - Non-coherent DMA with explicit cache clean/invalidate > - Per-channel statistics and register dump for debug > > Patch breakdown: > [1/4] Skeleton: bus probe/remove, dmadev registration, MAINTAINERS, > doc index, and release notes for 26.11. > [2/4] Device configuration: vchan setup, TCD ring allocation, > start/stop, and capability reporting. > [3/4] Data path: enqueue (copy and sg), doorbell, completion poll. > [4/4] Statistics and dump: per-channel counters and register dump. > > Tested on NXP i.MX95 EVK with vfio-platform. > > Gagandeep Singh (5): > bus/platform: match device by devicetree compatible string > dma/imx_edma5: introduce eDMA5 dmadev skeleton > dma/imx_edma5: add device configuration > dma/imx_edma5: add data path > dma/imx_edma5: add statistics and dump > > MAINTAINERS | 5 + > doc/guides/dmadevs/imx_edma5.rst | 61 + > doc/guides/dmadevs/index.rst | 1 + > doc/guides/rel_notes/release_26_11.rst | 6 + > drivers/bus/platform/bus_platform_driver.h | 3 +- > drivers/bus/platform/platform.c | 67 +- > drivers/dma/imx_edma5/imx_edma5_dmadev.c | 1162 ++++++++++++++++++++ > drivers/dma/imx_edma5/imx_edma5_dmadev.h | 220 ++++ > drivers/dma/imx_edma5/imx_edma5_hw.h | 157 +++ > drivers/dma/imx_edma5/imx_edma5_logs.h | 16 + > drivers/dma/imx_edma5/meson.build | 10 + > drivers/dma/meson.build | 1 + > 12 files changed, 1707 insertions(+), 2 deletions(-) > create mode 100644 doc/guides/dmadevs/imx_edma5.rst > create mode 100644 drivers/dma/imx_edma5/imx_edma5_dmadev.c > create mode 100644 drivers/dma/imx_edma5/imx_edma5_dmadev.h > create mode 100644 drivers/dma/imx_edma5/imx_edma5_hw.h > create mode 100644 drivers/dma/imx_edma5/imx_edma5_logs.h > create mode 100644 drivers/dma/imx_edma5/meson.build > More AI feedback on V2 Review of [PATCH v2 0/5] dma/imx_edma5: introduce NXP i.MX95 eDMA5 driver Verification: each of the five commits builds independently with -Dwerror=true on x86_64 and aarch64 cross (gcc), full build including documentation is clean, check-git-log reports 5/5 valid. Confirmed by disassembly that the new DSB sequences are emitted on aarch64. Thanks for the v2. The bus matching, the SUBMIT-flag semantics, the missing DSB, the coherency contradiction and the style nits are all resolved. The NBYTES cap is applied to copy() but not to copy_sg(), and a few smaller items remain. Patch 1/5 (bus/platform: match by devicetree compatible): Warning: The quote-stripping in of_device_is_compatible() is unnecessary and should be dropped. Now that imx_edma5 sets .driver.alias directly in the driver struct, no in-tree caller passes a quoted string, so the branch is dead code. It exists to paper over RTE_PMD_REGISTER_ALIAS, which stringifies its argument via RTE_STR and therefore cannot express a compatible string containing a comma. Silently accepting a mangled alias in the bus hides that defect from the next driver that hits it. Either leave the macro alone and drop the quote handling, or fix the macro (e.g. a variant that takes the string unmodified) and drop it as well. Info: The last match block ends with "goto out;" immediately above the "out:" label; the goto is redundant. Info: The commit changes matching behaviour for every platform-bus driver and has no release notes entry. There is currently one in-tree consumer, so this is a judgement call, but a line under New Features would be reasonable. Patch 2/5 (skeleton): Warning: The driver documentation does not match the code. Both the Supported Features and Limitations sections describe scatter-gather as requiring equal-length or equal-sized source and destination segment pairs, but imx_edma5_copy_sg() walks the two lists as independent cursors and correctly handles unequal segmentation, requiring only equal totals. The doc understates the driver. Please update both sections. Info: IMX_EDMA5_CH_MATTR_COHERENT (and the RCACHE/WCACHE/RDOMAINS/WDOMAINS macros it is built from) is now unused after the CH_MATTR write was dropped in patch 3/5. Remove the definitions or note why they are kept. Patch 4/5 (data path): Error: The IMX_EDMA5_MAX_NBYTES cap is enforced in imx_edma5_copy() but not in imx_edma5_copy_sg(). Sub-transfer lengths there come from RTE_MIN(s_rem, d_rem) over rte_dma_sge.length, which is uint32_t, so a single segment larger than 1 GiB - 1 reaches imx_edma5_program_copy() and writes a count with bit 30 or 31 set into TCD_NBYTES - exactly the SMLOE/DMLOE corruption the copy() check was added to prevent. Validate each segment length (or each computed sub-transfer length) against IMX_EDMA5_MAX_NBYTES and return -EINVAL. Warning: An enqueue call can now block for a very long time. Each sub-transfer waits up to IMX_EDMA5_WAIT_TIMEOUT_MS (1000 ms), and imx_edma5_run_job() serialises SG sub-transfers, so a job with the maximum 32 sub-transfers can spin for up to 32 seconds inside rte_dma_copy_sg() with RTE_DMA_OP_FLAG_SUBMIT, or inside rte_dma_submit(). Consider a per-job deadline rather than a per-sub-transfer one. Warning: imx_edma5_wait_done() still cannot stop a transfer it gave up on. On timeout it calls imx_edma5_reset_hw_chan(), which writes CH_CSR.DONE, CH_ES and the TCD control fields but does not cancel an in-flight transfer (there is no MP_CSR.CX use in the driver), so a channel that is genuinely stuck rather than merely slow will still be reprogrammed while ACTIVE and the abandoned transfer keeps writing to the old destination. The length cap makes this much harder to hit, so this is no longer an error, but a cancel on the timeout path would close it properly. Warning: The data path remains fully synchronous - enqueue or submit programs the TCD, starts the channel and busy-waits for DONE - so the CPU spins for the duration of every copy and the offload gains nothing over memcpy. The hardware can run detached: program and START at submit time, poll CH_CSR.DONE in completed()/completed_status(), and serialise only when a second job needs the single register TCD. If this is deliberate for the first submission, please state it in the Limitations section of the driver doc rather than leaving it implicit. Warning: When rte_mem_iova2virt() returns NULL, cache maintenance is skipped and, on this non-coherent master, the transfer silently returns wrong data. The v2 comment documents this, but a code comment is not reachable by the application author. Either reject such addresses with -EINVAL or state the restriction in the driver documentation. Warning: rte_mem_iova2virt() is called per operation (source and destination, and per segment in copy_sg) in the fast path; it walks the memseg lists. In IOVA=VA mode the lookup is unnecessary and the value is the address itself.

