Re: [PATCH v3 0/5] dma/imx_edma5: introduce NXP i.MX95 eDMA5 driver

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 11 Aug 2026 16:19:28 +0530
Gagandeep Singh <g.singh@nxp.com> wrote:

> V3-changes:
>  - Fixed below AI comments:
>    - Dead quote-stripping removed (`of_device_is_compatible`)
>    - Redundant `goto out` removed
>    - New Features entry added for bus platform changes.
>    - Scatter-gather doc corrected in both sections
>    - Unused CH_MATTR macros removed
>    - Fixed SG sub-transfer length cap (`imx_edma5_copy_sg`)
>    - Updated Per-job timeout logic.
>    - Updated `rte_mem_iova2virt` per-op in IOVA=VA mode logic
>    - Updated comments and documentation as per the AI comments.
> 
> 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           |   69 ++
>  doc/guides/dmadevs/index.rst               |    1 +
>  doc/guides/rel_notes/release_26_11.rst     |   15 +
>  drivers/bus/platform/bus_platform_driver.h |    3 +-
>  drivers/bus/platform/platform.c            |   52 +-
>  drivers/dma/imx_edma5/imx_edma5_dmadev.c   | 1190 ++++++++++++++++++++
>  drivers/dma/imx_edma5/imx_edma5_dmadev.h   |  234 ++++
>  drivers/dma/imx_edma5/imx_edma5_hw.h       |  144 +++
>  drivers/dma/imx_edma5/imx_edma5_logs.h     |   16 +
>  drivers/dma/imx_edma5/meson.build          |   10 +
>  drivers/dma/meson.build                    |    1 +
>  12 files changed, 1738 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
> 

Still lots of AI review feedback if run with stronger model.
Nothing major

Review of [PATCH v3 0/5] dma/imx_edma5: introduce NXP i.MX95 eDMA5 driver



Verification: the full series applies cleanly to main. Each of the five
commits builds independently with -Dwerror=true on x86_64 and on aarch64
cross, for both the bus and the driver objects. Documentation renders
without sphinx warnings and both release-notes entries appear, ordered
bus-before-PMD per the suggested ordering in the template. check-git-log
reports 5/5 valid. No trailing whitespace, no lines over 100 characters.

Patch 1/5 addresses both v2 findings: the quote-stripping block is gone
and the redundant goto before the out: label is gone. The release-notes
entry has been added. Two small things remain, neither of them defects:

Info: With the quote handling removed, the `want` buffer in
of_device_is_compatible() is a straight copy of `compat` and is no longer
needed - strcmp(s, compat) would do. Dropping it also removes 8 KB
(BUFSIZ) of stack from a function that currently reserves about 20 KB
across path[], buf[] and want[]. The rte_strscpy() return value is also
unchecked, which becomes moot once the copy goes away.

Info: The working pattern for a device-tree compatible is now to set
.driver.alias directly in the rte_platform_driver initializer, because
RTE_PMD_REGISTER_ALIAS stringifies its argument and cannot express a
string containing a comma. That is a reasonable resolution, but nothing
in bus_platform_driver.h says so, and the next driver author will
reach for the macro first. A sentence in the comment above
RTE_PMD_REGISTER_ALIAS would save someone the debugging session.

Info: imx_edma5_hw.h line 77 has a double blank line left where the
CH_MATTR attribute macros were removed, and IMX_EDMA5_CH_MATTR (line 55)
is now the only CH_MATTR definition remaining and has no user. Keeping a
complete register-offset map is defensible.

Info: The cover letter's "Patch breakdown" still lists [1/4] through
[4/4] from v1; the series has been five patches since v2.

The correctness issues from v1 and v2 - the unreachable probe, the broken
alias, the missing DSB, the SUBMIT-flag semantics, the NBYTES truncation
in both copy paths, the per-sub-transfer timeout, and the coherency
contradiction - are all resolved and verified against the applied tree.