|WARNING| [v21, 27/27] doc: update release notes with NXP DPAA changes

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/170670

_AI Code Review_

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

# DPAA Patch Series Review

I'll review these 27 patches against the AGENTS.md guidelines, focusing on correctness bugs and critical issues first, then noting style and process concerns.

---

## CORRECTNESS BUGS (HIGH PRIORITY)

### Patch 01/27: net/dpaa: fix double close and null deref on remove
**No correctness issues.** The fix is correct: it checks `eth_dev != NULL` before accessing it, and ensures `dpaa_eth_dev_close()` is called only once (when `state != RTE_ETH_DEV_UNUSED`). The error-check-and-log before release is appropriate.

### Patch 02/27: net/dpaa: fix probe success when FMC init fails
**No correctness issues.** Correctly propagates `num_rx_fqs` (which is negative on error) into `ret`, fixing the bug where the function would return 0 (success) with `num_rx_fqs < 0`.

### Patch 03/27: net/dpaa: fix resource leak on probe failure
**No correctness issues.** The fix is correct: on mempool creation failure, the code now unwinds through `close_dev` and `release_port` labels, matching the existing error path for `dpaa_dev_init()` failure.

### Patch 04/27: net/dpaa: fix free port resources on close
**Correctness issues fixed, no new bugs introduced.**
- The patch adds `rte_free(dpaa_intf->tx_conf_queues)` and sets it to NULL in both the close path and the init error path.
- The `clean_1:` label is added so offline/ONIC ports also free `fc_conf`, `cgr_rx`, and `cgr_tx` memory.
- The patch correctly adds `goto free_tx` in the init error paths for CGR allocation failures instead of `goto free_rx`.

### Patch 05/27: bus/dpaa: fix FQD dest wq channel decoding
**No new correctness issues.** The fix is correct: the old code swapped the work queue and channel by using `dest_wq & 0x7` as the channel and `dest_wq >> 3` as the work queue, when the hardware layout is the opposite (channel is bits 15:3, work queue is bits 2:0). The new code uses the inline helpers `qm_fqd_get_chan()` and `qm_fqd_get_wq()` which correctly extract `be16_to_cpu(fqd->dest_wq) >> QM_FQD_CHAN_OFF` and `be16_to_cpu(fqd->dest_wq) & QM_FQD_WQ_MASK`.

### Patch 06/27: bus/dpaa: accept QDMA device name in devargs
**No correctness issues.** The patch adds parsing for `dpaa_qdma-%d` device names, bounding the index check `i > RTE_DPAA_QDMA_DEVICES` (returns `-EINVAL` if out of range). This allows the documented `-a dpaa_bus:dpaa_qdma-1,...` usage to work.

### Patch 07/27: bus/dpaa: refine fman naming
**No correctness issues.** This is a refactoring: `ccsr_map` is renamed to `memac_map` and `bmi_map` to `rx_bmi_map` for clarity. All usages are updated consistently. No functional change.

### Patch 08/27: bus/dpaa: scan max BPID from DTS
**Correctness issue fixed, no new bugs:**
- The old code hardcoded `bman_pool_max = 64` for BMAN_REV10 and BMAN_REV21.
- The new code reads `fsl,bpid-range` from the device tree and sets `bman_pool_max` accordingly, with a clamp to `BMAN_MAX_POOLS` (64) if the DTS value exceeds the hardware limit.
- The warning and clamp prevent out-of-bounds writes to the SCN registers and the depletion mask, which the patch comment explicitly notes.
- If no DTS entry is found, the code falls back to the default `BMAN_MAX_POOLS`.

### Patch 09/27: drivers: add process-type guards for secondary process
**No correctness issues.** The patch correctly rejects secondary-process probes for:
- `dma/dpaa` (QDMA): returns `-ENOTSUP` in `dpaa_qdma_probe()` for secondary, because the QDMA register mappings are process-private.
- `net/dpaa` remove path: the secondary only releases its local ethdev port and returns, skipping the driver-global state (`dpaa_valid_dev--`, `rte_mempool_free(dpaa_tx_sg_pool)`) which is owned by the primary.

### Patch 10/27: drivers: shutdown DPAA FQ by fq descriptor
**No new correctness issues.** The patch changes `qman_shutdown_fq()` to take `struct qman_fq *fq` instead of `u32 fqid`, and uses `fq->qp` when present (falling back to `get_affine_portal()` otherwise). A wrapper `qman_shutdown_fq_by_fqid()` is added for the one caller that only has an FQID. This is correct: it allows a caller that owns a queue to drain it on the portal the queue is bound to, and the wrapper provides the same signature for the caller that does not have a descriptor.

### Patch 11/27: drivers: add DPAA cgrid cleanup support
**Correctness issues fixed:**
- The patch adds `qman_pending_fq_by_cgrid_range()` which scans the FQID space for FQs still attached to a CGRID range, and calls `qman_shutdown_fq_by_fqid()` on any found. This is called before `qman_delete_cgr()` in the close path.
- The loop is bounded by `DPAA_CGR_STALE_FQ_MAX` iterations per CGRID to avoid spinning forever on an FQ that refuses to shut down.
- The CGRIDs are released with `qman_release_cgrid_range()` after deleting the CGRs.
- **Error in init path:** The patch adds `qman_delete_cgr()` and `qman_release_cgrid_range()` calls on the `free_tx` and `free_rx` error paths, but the counters `nb_tx_cgr` and `nb_rx_cgr` are only incremented if the CGR was successfully created *and* the FQ was successfully initialized. If a TX FQ init fails after the CGR is created, the CGR is not added to the counter, so the error path will not delete it. **This is a leak of the CGRID and the CGR itself on the portal's cgr_cbs list.** The same applies to the RX side.

**Error:** In `dpaa_dev_init()`, if `dpaa_tx_queue_init()` fails after `qman_create_cgr(&cgr_tx[loop], ...)` succeeds, the counter `nb_tx_cgr` is *not* incremented (because the increment is after the `dpaa_tx_queue_init()` call), so the error path `free_tx:` will not delete that CGR. The same bug exists on the RX side.

```c
for (loop = 0; loop < MAX_DPAA_CORES; loop++) {
	if (dpaa_intf->cgr_tx)
		dpaa_intf->cgr_tx[loop].cgrid = cgrid_tx[loop];

	ret = dpaa_tx_queue_init(&dpaa_intf->tx_queues[loop],
		dpaa_intf->cgr_tx ? &dpaa_intf->cgr_tx[loop] : NULL);
	if (ret)
		goto free_tx;
	if (dpaa_intf->cgr_tx)
		nb_tx_cgr++;  // <-- only incremented if FQ init succeeds
```

If `dpaa_tx_queue_init()` returns an error, the `goto free_tx` path will delete `cgr_tx[0..nb_tx_cgr)`, but `nb_tx_cgr` was never incremented for the current `loop`, so the CGR created in this iteration is not deleted.

**Fix needed:** Move the `nb_tx_cgr++` to immediately after `qman_create_cgr()` succeeds, so the counter always reflects the number of CGRs that have been created (and therefore must be deleted on the error path).

### Patch 12/27: bus/dpaa: improve FQ shutdown with channel validation
**Correctness issues fixed, no new bugs:**
- The patch adds DTS parsing for `pool-channel-range` and stores the pool channel start and count.
- The drain loop is bounded by `QMAN_FQRN_WAIT_MAX` instead of spinning forever.
- Rejects FQs on another portal's dedicated channel with `-EBUSY` instead of trying to drain them (which would hang).
- Rejects FQs on a DCP portal channel with `-EBUSY`.
- The hardcoded pool channel range check is replaced by a comparison against the DTS-derived values.

### Patch 13/27: drivers: add BMI Tx statistics
**No correctness issues.** The patch extends `fman_if_bmi_stats_get_all()` to read Tx BMI statistics registers in addition to Rx, and reports zero for register blocks not mapped for the port type. The caller always expects `FMAN_IF_BMI_STATS_COUNT` values, and the code now provides them.

### Patch 14/27: net/dpaa: optimize FM deconfig
**No correctness issues.** The patch moves the `dpaa_fm_deconfig()` call to a single location in `dpaa_eth_dev_close()` and adds a NULL check for `dpaa_intf->port_handle` inside the function, so callers no longer need to check it.

### Patch 15/27: net/dpaa: optimize FMC MAC type parsing
**No correctness issues.** The patch changes `dpaa_port_fmc_port_parse()` to derive the MAC index from the FMC port name string (parsing `"MAC/<idx>"` or `"OFFLINE/<idx>"`) instead of computing it from the port type and number. This is necessary because MAC9 and MAC10 can be 10G, 2.5G, or 1G depending on serdes configuration, so the port type alone is not sufficient to determine the index. The patch correctly handles the parsing and bounds-checks the result.

### Patch 16/27: mempool/dpaa: fix write after free on pool free
**Correctness issue fixed:**
- The old code did `rte_free(mp->pool_data); bp_info->bp = NULL;` where `bp_info` is `mp->pool_data`, so the write to `bp_info->bp` is into freed memory.
- The new code does `bp_info->bp = NULL; rte_free(bp_info);`, clearing the field before releasing the allocation.

### Patch 17/27: drivers: release DPAA bpid on driver destructor
**Correctness issues:**
- The patch adds a per-BPID table `s_dpaa_bpid_allocated_flag[]` to track which BPIDs were allocated by this process, and a `RTE_FINI_PRIO` destructor that releases them.
- The destructor cannot touch `rte_dpaa_bpid_info` because it is EAL memory that may already be unmapped by the time the destructors run, so `bman_free_bpid()` is added to release the BPID from the flags alone.
- `rte_dpaa_bpid_info` is deliberately *not* freed by the destructor, as the patch comment explains: it is referenced by every Rx queue (including in secondaries where the Rx path installs the primary's array), so releasing it when the last local mempool goes away would leave those references dangling.
- `dpaa_mbuf_free_pool()` now clears `rte_dpaa_bpid_info[bpid].mp` and `.bp` to NULL when the pool is freed, and marks `s_dpaa_bpid_allocated_flag[bpid].used = false`.

**Error:** The patch sets `s_dpaa_bpid_allocated_flag[bpid].used = true` in `dpaa_mbuf_create_pool()` after the BPID is allocated, but does *not* set it to `false` if a subsequent failure (e.g., `bp_info = rte_zmalloc(...)` returning NULL) causes the function to return an error. This leaves the BPID marked as "used" in the local table, so the destructor will try to free it even though the mempool was never successfully created.

**Error:** In `dpaa_mbuf_create_pool()`, if `rte_zmalloc()` fails, the function calls `bman_free_pool(bp);` and returns `-ENOMEM`, but does not set `s_dpaa_bpid_allocated_flag[bpid].used = false`. The BPID is therefore leaked in the local table (the destructor will try to free it, which is correct), but the `params.flags` were never saved, so `s_dpaa_bpid_allocated_flag[bpid].flags` is zero (from the static initializer). If `params.flags` included `BMAN_POOL_FLAG_THRESH`, the destructor will not call `bm_pool_set(bpid, zero_thresholds)` (because the local flags are zero), so the threshold settings are leaked in the kernel.

**Fix needed:** In `dpaa_mbuf_create_pool()`, move the line `s_dpaa_bpid_allocated_flag[bpid].flags = params.flags;` to immediately after `bp = bman_new_pool(&params);` succeeds, so the flags are always saved if the pool was created. And if a later step fails, set `s_dpaa_bpid_allocated_flag[bpid].used = false` before returning, so the destructor does not try to free a BPID that was never fully set up.

### Patch 18/27: dma/dpaa: add devargs for SG and errata workaround
**No correctness issues.** The patch adds `dpaa_dma_sg_disable`, `dpaa_dma_data_validation`, and `dpaa_dma_pci_read_disable` device arguments to control existing driver behavior at runtime. The code paths they control are not changed, only gated by boolean flags.

### Patch 19/27: net/dpaa: support Rx/Tx taildrop threshold devarg
**Correctness issues:**
- The patch adds `dpaa_get_devargs_int()` to parse integer device arguments, and adds `DRIVER_RX_TAILDROP` and `DRIVER_TX_TAILDROP` devargs to set `td_threshold` and `td_tx_threshold`.
- **Error:** In `dpaa_dev_init()`, the code resets `td_threshold = CGR_RX_PERFQ_THRESH` and `td_tx_threshold = 0` at the start, before parsing the devargs. If the devarg is not present, the environment variable `DPAA_TX_TAILDROP_THRESHOLD` is read and parsed. But the environment variable is parsed *before* the `drv_tx_taildrop` devarg, so if the environment variable is set and the devarg is also set, the devarg wins (because it is parsed later and overwrites `td_tx_threshold`). This is the correct precedence (devarg should override environment variable), but the code does *not* reset `td_tx_threshold` to zero before parsing the environment variable, so if the environment variable was set in a previous call to `dpaa_dev_init()` (e.g., for a different port), the value persists. **Wait, no:** the function is `dpaa_dev_init()`, which is called once per port, and `td_threshold` and `td_tx_threshold` are local variables, so they are re-initialized to the default values at the start of each
  call. **This is correct.**

**No error.** The local variables `td_threshold` and `td_tx_threshold` are reset to their defaults at the start of `dpaa_dev_init()`, so the environment variable and devarg parsing is per-port and does not leak between ports.

### Patch 20/27: net/dpaa: support fmcless rxq number as devargs
**No correctness issues.** The