|SUCCESS| [v22, 27/27] doc: update release notes with NXP DPAA changes

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/170746

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-07

# DPAA Patch Series Review

Reviewing all 27 patches in the series. I'll organize findings by severity (Error, Warning, Info) as defined in AGENTS.md, with correctness bugs prioritized highest.

---

## SUMMARY

This series fixes several critical bugs (resource leaks, use-after-free, double close) in the DPAA PMD and introduces new features (offline port device support, runtime-configurable taildrop thresholds, DMA devargs).

**Key correctness findings:**
- Multiple resource-leak fixes (Patches 1-4, 16, 17): all appear correct, proper error-path unwind added
- Use-after-free fix (Patch 16): correct fix, clears field before free
- Integer multiply/shift fixes not present in this series (no new integer operations that require widening)
- No new `volatile` misuse introduced
- Secondary process support properly gated in Patches 9, 25

**Style/process findings:**
- Some log macro usage improvements (Patch 23)
- Some definition list opportunities in RST docs (minor, not flagged as errors)
- A few devargs could use clearer documentation of value ranges

---

## PATCH 1/27: net/dpaa: fix double close and null deref on remove

### Errors

None.

### Warnings

None.

**Analysis:** The fix is correct. The original code called `dpaa_eth_dev_close()` unconditionally and then again if state was not UNUSED, and it dereferenced `eth_dev` before checking the `rte_eth_dev_allocated()` result. The patch fixes both: it checks `eth_dev != NULL` before accessing it, closes only once (when the device is in use), logs the close result, and always releases the port. The release is done outside the condition because even if close fails, the port must be released.

---

## PATCH 2/27: net/dpaa: fix probe success when FMC init fails

### Errors

None.

### Warnings

None.

**Analysis:** Correct fix. The code jumped to `free_rx` without setting `ret`, so the function returned 0 (success) despite the failure. The patch propagates the error code from `dpaa_port_fmc_init()` (which is `num_rx_fqs` when negative).

---

## PATCH 3/27: net/dpaa: fix resource leak on probe failure

### Errors

None.

### Warnings

None.

**Analysis:** Correct error-path fix. When the SG pool creation failed, `dpaa_dev_init()` had already succeeded, so the early `return -ENOMEM` leaked all probe-time allocations. The patch unwinds through `close_dev` (calls `dpaa_eth_dev_close()`) and then `release_port`, matching the existing failure path for `dpaa_dev_init()` errors.

---

## PATCH 4/27: net/dpaa: fix free port resources on close

### Errors

None.

### Warnings

None.

**Analysis:** Correct fix. `dpaa_intf->tx_conf_queues` was allocated in `dpaa_dev_init()` but never freed. The offline/ONIC ports returned early from `dpaa_eth_dev_close()` before the common cleanup code that freed other resources. The patch adds a `clean_1` label after the early return for offline/ONIC ports, so they also release `fc_conf`, `cgr_rx`, `cgr_tx`, and the newly added `tx_conf_queues`. The patch also adds the `tx_conf_queues` free to the `dpaa_dev_init()` error paths, and nulls the pointer after freeing to make the cleanup idempotent.

---

## PATCH 5/27: bus/dpaa: fix FQD dest wq channel decoding

### Errors

None.

### Warnings

None.

**Analysis:** The fix is correct. The old code decoded `dest_wq` as `channel = dest_wq & 0x7` and `wq = dest_wq >> 3`, but the hardware layout is channel in bits 15:3 and wq in bits 2:0. The patch adds helper functions `qm_fqd_get_chan()` and `qm_fqd_get_wq()` that extract the fields correctly (channel shifted right by 3, wq masked by 0x7). This is a functional bug fix: the old code would never match a pool-channel FQ for the drain logic, causing incorrect shutdown behavior.

---

## PATCH 6/27: bus/dpaa: accept QDMA device name in devargs

### Errors

None.

### Warnings

None.

**Analysis:** The fix is correct. The `rte_dpaa_bus_parse()` function did not recognize the `dpaa_qdma-N` naming pattern, so passing a QDMA device through devargs failed. The patch adds parsing for `dpaa_qdma-<N>` (where `N` is 1..`RTE_DPAA_QDMA_DEVICES`, currently 1), allowing the documented devarg usage to work.

---

## PATCH 7/27: bus/dpaa: refine fman naming

### Errors

None.

### Warnings

None.

**Analysis:** This is a refactoring patch (renaming variables for clarity). It renames `ccsr_map` to `memac_map` and `bmi_map` to `rx_bmi_map`, and moves `GENMASK()` from `qman.c` to the header. No functional change. The naming is clearer and consistent with the hardware structure.

---

## PATCH 8/27: bus/dpaa: scan max BPID from DTS

### Errors

None.

### Warnings

None.

**Analysis:** This patch reads the `fsl,bpid-range` property from the device tree to determine the maximum BPID dynamically instead of hardcoding it. The code validates that the range does not exceed `BMAN_MAX_POOLS` (64), which is correct (the hardware register windows and code arrays are sized for 64 BPIDs). The fallback when no range is found is to retain the default `BMAN_MAX_POOLS` and log a warning. The only potential issue is if an operator misconfigures the DTS with a range that starts above 64 or wraps around, but the code clamps `start + count` to `BMAN_MAX_POOLS` if needed, so it's safe. The BMAN HW version default change (to BMAN_REV21) is also documented in the commit message.

---

## PATCH 9/27: drivers: add process-type guards for secondary process

### Errors

None.

### Warnings

None.

**Analysis:** Correct gating. The DMA/DPAA QDMA register bases are process-private mmaps created by the primary, so a secondary that probes would advertise a non-functional device. The patch rejects the probe with `-ENOTSUP` in secondary. The net/dpaa change is also correct: `rte_dpaa_remove()` manipulates `dpaa_valid_dev` and the shared TX SG pool, both owned by the primary, so a secondary must skip that logic.

---

## PATCH 10/27: drivers: shutdown DPAA FQ by fq descriptor

### Errors

None.

### Warnings

None.

**Analysis:** This changes `qman_shutdown_fq()` to take a `struct qman_fq *` instead of a bare `fqid`, and to use `fq->qp` when available (falling back to the affine portal otherwise). The function still validates the channel and drains the FQ through the correct portal. A wrapper `qman_shutdown_fq_by_fqid()` is added for the one caller (net/dpaa FQ cleanup) that only has an FQID and no descriptor yet. The change allows callers that own the FQ to drain it on the portal it is bound to, which is the correct place to do so.

---

## PATCH 11/27: drivers: add DPAA cgrid cleanup support

### Errors

None.

### Warnings

None.

**Analysis:** This adds cleanup logic for CGRs (congestion groups). A CGR must have no member FQs left when it is deleted, so the patch adds `qman_pending_fq_by_cgrid_range()` to scan for FQs still attached to a CGRID range, and calls it from `dpaa_eth_dev_close()` to shut down any stale FQs before deleting the CGR. The Tx CGRs are now also released (the old code only deleted the Rx ones). The patch also adds `qman_release_cgrid_range()` calls to return the CGRIDs to the allocator. The probe failure path is also updated to clean up the CGRs created before the failure.

**Note:** The FQID scan is a full 24-bit space walk (bounded at `QMAN_MAX_FQID`, 0x00FFFFFF). The patch does this at device close only, which is acceptable (not a fast path). A future optimization could be to batch-query FQIDs, but that is out of scope here. The current approach is safe.

---

## PATCH 12/27: bus/dpaa: improve FQ shutdown with channel validation

### Errors

None.

### Warnings

None.

**Analysis:** This reads the pool-channel range from the device tree (instead of using a hardcoded `qm_channel_pool1`), validates the channel before attempting to drain an FQ, and bounds the FQRN wait by `QMAN_FQRN_WAIT_MAX` iterations. The channel validation correctly rejects FQs scheduled on another portal's dedicated channel with `-EBUSY` (the drain would spin forever because the FQRN would never arrive). The FQRN wait loop now terminates after a fixed number of iterations and returns `-EBUSY` if the retire does not complete, which is the correct safe behavior.

---

## PATCH 13/27: drivers: add BMI Tx statistics

### Errors

None.

### Warnings

None.

**Analysis:** This extends the BMI statistics code to read and report Tx BMI counters in addition to the existing Rx ones. The functions now check for NULL register pointers and report zero if a register block is not mapped (which is correct for port types that do not have a Tx BMI, such as some offline ports). The code correctly iterates over both Rx and Tx register windows in `fman_if_bmi_stats_get_all()`, and the count constants are derived from the register offsets via `static_assert`, so they cannot drift from the layout.

---

## PATCH 14/27: net/dpaa: optimize FM deconfig

### Errors

None.

### Warnings

None.

**Analysis:** This consolidates FM deconfiguration so it is called only once, just before FQ cleanup, and only when the port has a valid handle. The patch moves the call from two places (one in the normal close path, one in probe failure) to a single place, and adds guards against running it when `port_handle` is NULL or when it was already run (via a null check at the start of `dpaa_fm_deconfig()`). The patch also adds a call to `dpaa_port_vsp_cleanup()` (which was previously missing in some paths) before the FM deconfig, so VSP resources are always released.

---

## PATCH 15/27: net/dpaa: optimize FMC MAC type parsing

### Errors

None.

### Warnings

None.

**Analysis:** This replaces MAC index arithmetic (`pport->number + DPAA_*_MAC_START_IDX`) with a call to `dpaa_port_fmc_get_idx_from_name()`, which parses the MAC index from the FMC port name. The MAC type checks (1G/2.5G/10G) are retained, but the index is now read from the "MAC/N" or "OFFLINE/N" string. This is more robust because the MAC type alone does not uniquely identify the index on hardware where MAC9/MAC10 can be either 10G or 2.5G/1G depending on serdes configuration.

---

## PATCH 16/27: mempool/dpaa: fix write after free on pool free

### Errors

None.

### Warnings

None.

**Analysis:** Correct fix. The code freed `mp->pool_data` (via `rte_free(mp->pool_data)`) and then accessed `bp_info->bp` (which is the same allocation). The patch clears `bp_info->bp` before freeing `bp_info` directly, so no write-after-free occurs.

---

## PATCH 17/27: drivers: release DPAA bpid on driver destructor

### Errors

None.

### Warnings

None.

**Analysis:** This adds a driver destructor (`RTE_FINI_PRIO(dpaa_mpool_finish, 104)`) that releases any BPIDs still allocated at process exit. The BPIDs are tracked in a per-process static table `s_dpaa_bpid_allocated_flag[]`, which is indexed by BPID and records the flags used when the pool was created. The destructor calls `bman_free_bpid(bpid, flags)` for each BPID marked in use. This is correct: the BPID is a kernel resource (allocated via ioctl), and without the destructor it leaks across application runs. The patch also documents that `rte_dpaa_bpid_info` (the EAL shared memory array) is deliberately not freed, because it is referenced by every Rx queue and cannot be released at destructor time (EAL memory is already detached).

The priority 104 is defined as a macro `RTE_PRIORITY_104` (following DPAA conventions elsewhere), and the destructor runs before the EAL cleanup detaches shared memory, so the `bman_free_bpid()` calls are safe.

---

## PATCH 18/27: dma/dpaa: add devargs for SG and errata workaround

### Errors

None.

### Warnings

None.

**Analysis:** This adds three devargs (`dpaa_dma_sg_disable`, `dpaa_dma_data_validation`, `dpaa_dma_pci_read_disable`) to control existing driver behavior (SG batching, post-DMA data validation, and ERR050757 programming) at runtime. The devargs are stored in static globals (`s_sg_enable`, `s_data_validation`, `s_pci_read`) and read at probe time. The code paths already existed; the patch just makes them configurable. The defaults are safe (SG on, data validation off, ERR050757 on when built with the compile-time flag). The documentation is updated to describe each devarg.

---

## PATCH 19/27: net/dpaa: support Rx/Tx taildrop threshold devarg

### Errors

None.

### Warnings

None.

**Analysis:** This adds `drv_rx_taildrop` and `drv_tx_taildrop` devargs to configure per-port taildrop thresholds at probe time. The code validates that the values are in the range [0, UINT16_MAX], clamping out-of-range values to the default (`CGR_RX_PERFQ_THRESH`). A value of 0 disables taildrop. The environment variable `DPAA_TX_TAILDROP_THRESHOLD` is still honored as a fallback when the devarg is absent, so existing scripts keep working. The thresholds are reset to their defaults at the start of `dpaa_dev_init()` so one port's value is not inherited by the next.

The patch uses the new `dpaa_get_devargs_int()` helper to parse the integer values, which is a clean approach. The documentation is updated.

---

## PATCH 20/27: net/dpaa: support fmcless rxq number as devargs

### Errors

None.

### Warnings

None.

**Analysis:** This adds a `drv_fmcless_rxq` devarg to set the number of Rx queues in FMCLESS mode. The default when the devarg is absent is changed from `rte_lcore_count()` to `DPAA_MAX_NUM_PCD_QUEUES` (which is the maximum supported per port), because multiple queues may be processed on the same core. The code validates that the value is at least 1 and clamps it to `DPAA_MAX_NUM_PCD_QUEUES` if larger. The documentation is updated.

---

## PATCH 21/27: net/dpaa: support non fmX-macY type of shared Ethernet name

### Errors

None.

### Warnings

None.

**Analysis:** This adds a `drv_sh_if_name` devarg to allow an application to provide the Linux kernel interface name of a shared MAC interface when it differs from the `fmX-macY` style name used by DPDK. The code