|WARNING| [v19, 26/26] 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/170484

_AI Code Review_

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

# DPAA Patch Series Review

## Critical Correctness Issues (Errors)

### Patch 01/26: net/dpaa: fix double close and null deref on remove
**Error**: The conditional check `eth_dev != NULL` is inside an `if` block that was already true for `eth_dev` in the original code, so the original logic was correct. However, the new code changes behavior:

- **Line 2682**: `eth_dev->state != RTE_ETH_DEV_UNUSED` is checked only when `eth_dev != NULL`, which is an improvement.
- **Line 2689**: But the code then calls `dpaa_eth_dev_close(eth_dev)` on `line 2683`, and if that fails, only warns but continues to release the port on `line 2687`. This error path should still return the error code from `dpaa_eth_dev_close()` rather than overwriting it with the return from `rte_eth_dev_release_port()`.

**Suggested fix:**
```c
if (eth_dev != NULL && eth_dev->state != RTE_ETH_DEV_UNUSED) {
	ret = dpaa_eth_dev_close(eth_dev);
	if (ret != 0)
		DPAA_PMD_WARN("%s: close failed(%d), releasing port",
			dpaa_dev->device.name, ret);
	int rel_ret = rte_eth_dev_release_port(eth_dev);
	if (rel_ret && !ret)
		ret = rel_ret;
}
```
This preserves the close error but also reports release failures.

### Patch 03/26: net/dpaa: fix free port resources on close
**No correctness errors found.** The patch correctly adds cleanup of `tx_conf_queues` on both the normal and error paths, and moves cleanup of O/H/ONIC ports before the `goto clean_1` label so they also free the resources.

### Patch 04/26: bus/dpaa: fix FQD dest wq channel decoding
**No correctness errors found.** The bit field extraction was indeed wrong (channel and wq were swapped), and the fix correctly uses the FQD descriptor layout.

### Patch 09/26: drivers: shutdown DPAA FQ by fq descriptor
**Error**: The function `qman_shutdown_fq()` now takes `struct qman_fq *` instead of `u32 fqid`, but some callers still pass an FQID directly. The code adds a wrapper `qman_shutdown_fq_by_fqid()` for the one known call site (`dpaa_ethdev.c:2361`), but if there are other call sites in files not shown in this patch, they would break at compile time.

**Verification needed**: Confirm that all call sites of `qman_shutdown_fq()` in the entire codebase have been converted or use the new wrapper.

### Patch 10/26: drivers: add DPAA cgrid cleanup support
**Warning**: The new `qman_pending_fq_by_cgrid_range()` function scans the entire FQID space (24-bit, 0 to 0x00FFFFFF) looking for FQs that reference a given CGRID. This is potentially a very long loop. The function includes a break on `-ERANGE` when an FQID is not implemented, which helps on some devices, but on devices where the FQID space is sparsely populated, this could still iterate millions of times.

**Suggested mitigation**: Document that this function is only expected to run during device close, so the cost is acceptable. The existing `DPAA_BUS_DEBUG("Scanning FQs for cgrid(0x%x..0x%x)")` log already hints at this.

**No action required** if performance on close is acceptable. Otherwise, consider maintaining a per-port list of allocated FQIDs to avoid the full space scan.

### Patch 15/26: mempool/dpaa: fix write after free on pool free
**No correctness errors found.** The patch correctly moves the `bp_info->bp = NULL` assignment before the `rte_free(bp_info)`, and changes the argument from `mp->pool_data` to `bp_info` (which are the same pointer, but using `bp_info` is clearer).

### Patch 25/26: crypto/dpaa_sec: improve crypto fq resource handling
**Error**: The patch changes `dpaa_sec_init_tx()` to take a `uint32_t fqid` parameter instead of using `QMAN_FQ_FLAG_DYNAMIC_FQID`, but on the error paths `init_error1:` and `init_error2:`, the code calls `qman_release_fqid_range(fqids[0], N)` using `fqids[0]`, which is only valid if at least one FQID was successfully allocated. If `qman_alloc_fqid_range()` fails completely (returns < 0), `fqids[]` is uninitialized.

**Suggested fix:**
```c
init_error3:
	for (j = 0; j < i; j++) {
		ret = qman_shutdown_fq(&internals->inq[j]);
		if (ret) {
			DPAA_SEC_ERR("%s: Failed(%d) to shutdown RXQ(0x%08x)",
				cryptodev->data->name, ret, internals->inq[j].fqid);
		}
	}
	if (num > 0)  /* Only release if we allocated some */
		qman_release_fqid_range(fqids[0], num);
init_error2:
	i = internals->max_nb_queue_pairs;
	fqids[0] = internals->qps[0].outq.fqid;
init_error1:
	for (j = 0; j < i; j++) {
		qp = &internals->qps[j];
		ret = qman_shutdown_fq(&qp->outq);
		if (ret) {
			DPAA_SEC_ERR("%s: Failed(%d) to shutdown TXQ(0x%08x)",
				cryptodev->data->name, ret, qp->outq.fqid);
		}
	}
	if (fqids[0] != 0)  /* Check that we have a valid FQID range to release */
		qman_release_fqid_range(fqids[0], internals->max_nb_queue_pairs);
```
(Note: the second `init_error1:` path reads `fqids[0] = internals->qps[0].outq.fqid;` which is safe only if at least one `qps` was initialized. The patch should track `num` from the first allocation and use that instead.)

---

## Warnings (Should Fix)

### Patch 07/26: bus/dpaa: scan max BPID from DTS
**Warning**: The loop `for_each_compatible_node(dt_node, NULL, "fsl,bpid-range")` iterates over all matching nodes, and the last one's `start + count` is used as `bman_pool_max`. If the DTS has multiple `fsl,bpid-range` nodes (not typical but possible), the patch logs `pr_info("Max BPID: %d, fixed BPID < %d", bman_pool_max, start);` inside the loop, which could print multiple times. The message should be moved outside the loop or the loop should break after the first match.

**Suggested fix:**
```c
for_each_compatible_node(dt_node, NULL, "fsl,bpid-range") {
	range = of_get_property(dt_node, "fsl,bpid-range", &len);
	if (!range || len < (int)(2 * sizeof(rte_be32_t)))
		continue;
	start = rte_be_to_cpu_32(range[BPID_RANGE_START_INDEX]);
	count = rte_be_to_cpu_32(range[BPID_RANGE_COUNT_INDEX]);
	if (!count) {
		pr_warn("Empty BPID range in DTS, using default pool max\n");
		continue;
	}
	if (start + count > BMAN_MAX_POOLS) {
		pr_warn("BPID range %u+%u exceeds max %u, clamping\n",
			start, count, BMAN_MAX_POOLS);
		bman_pool_max = BMAN_MAX_POOLS;
	} else {
		bman_pool_max = start + count;
	}
	break;  /* Use the first valid range only */
}
if (count)
	pr_info("Max BPID: %d, fixed BPID < %d", bman_pool_max, start);
else
	pr_warn("No BPID range found in DTS, using default pool max\n");
```

### Patch 11/26: bus/dpaa: improve FQ shutdown with channel validation
**Warning**: The new code in `qman_shutdown_fq()` loops up to `QMAN_FQRN_WAIT_MAX` (10000000) iterations waiting for the FQRN. If the notification never arrives, the loop exits after ~10M iterations and returns `-EBUSY`, which is correct. However, spinning that many times could take a significant amount of time on slow hardware. Consider adding a `rte_delay_us_sleep(1)` every N iterations to avoid burning CPU, or accept that a stuck FQ will cause a long delay.

**No action required** if the current behavior is acceptable for a shutdown path.

---

## Style and Process Notes (Informational)

### General: Commit message formatting
These are checked by `checkpatches.sh` and are not flagged here per the instructions.

### Patch 06/26: bus/dpaa: refine fman naming
**Info**: The patch renames `ccsr_map` to `memac_map` and `bmi_map` to `rx_bmi_map` for clarity. This is a good refactor with no functional change. The `struct __fman_if` now has `memac_map`, `rx_bmi_map`, and `tx_bmi_map`, which is self-documenting.

### Patch 17/26: dma/dpaa: add devargs for SG and errata workaround
**Info**: The patch adds three new device arguments (`dpaa_dma_sg_disable`, `dpaa_dma_data_validation`, `dpaa_dma_pci_read_disable`) and gates existing functionality with them. The code correctly checks these at runtime and the documentation in `doc/guides/dmadevs/dpaa.rst` describes their usage.

### Patch 18/26: net/dpaa: support Rx/Tx taildrop threshold devarg
**Info**: The patch adds `dpaa_get_devargs_int()` and uses it to parse `drv_rx_taildrop` and `drv_tx_taildrop`. The implementation is straightforward and the fallback to environment variables for Tx is preserved for backward compatibility.

### Patch 19/26: net/dpaa: support fmcless rxq number as devargs
**Info**: The default number of Rx queues in FMCLESS mode is changed from `rte_lcore_count()` to `DPAA_MAX_NUM_PCD_QUEUES` (16), and the new `drv_fmcless_rxq` devarg allows overriding this. The change is documented in `doc/guides/nics/dpaa.rst`.

### Patch 20/26: net/dpaa: support non fmX-macY type of shared Ethernet name
**Info**: The `drv_sh_if_name` devarg lets the application provide the kernel interface name when it differs from the DPDK `fmX-macY` convention. The `dpaa_get_devargs_str()` helper is added and used in `dpaa_eth_dev_configure()` to fetch the name and pass it to `ioctl(SIOCGIFMTU)`. The implementation correctly handles the case where the devarg is absent or too long.

### Patch 21/26: bus/dpaa: optimize DPAA multi-entry buffer pool operations
**Info**: The patch replaces the hardcoded `8` with `FSL_BM_BURST_MAX` and adds a `struct bm_hw_buf_desc` overlay to `union bm_buffer` so that a release or acquire entry can be built in one 8-byte store. The `bm_buffer_set64_to_be()` macro clears the reserved byte and bpid, which is good. The `memcpy(&r->bufs[1], &bm_bufs[1], ...)` on the release path is correct: entry 0 is written separately to avoid triggering the ring valid bit.

### Patch 23/26: net/dpaa: enhance VSP port support
**Info**: The patch adds `fman_onic` handling to `get_rx_port_type()`, consolidates the per-profile VSP state into `struct dpaa_if_vsp`, and adds bounds checking on the VSP ID. The refactor improves clarity and fixes the ONIC port type mapping.

### Patch 24/26: drivers: add offline (O/H) port device support
**Info**: This is a large patch that adds support for the DPAA offline (O/H) port. The device is created with `FSL_DPAA_OL` type and probed only in the primary process. The Rx/Tx FQIDs and buffer pool are programmed into the kernel via `ioctl()` on the `/dev/cdx_ctrl` device. The patch adds `dpaa_oldev.c` and the public header `rte_pmd_dpaa_oldev.h` with experimental APIs for classification and LGW configuration. The implementation correctly checks for secondary process and returns `-ENOTSUP`.

---

## Summary

**Critical issues to fix:**
1. **Patch 01/26**: Preserve the `dpaa_eth_dev_close()` error code even when `rte_eth_dev_release_port()` succeeds.
2. **Patch 09/26**: Verify that all call sites of `qman_shutdown_fq()` have been converted or use the wrapper `qman_shutdown_fq_by_fqid()`.
3. **Patch 25/26**: Guard `qman_release_fqid_range()` calls against uninitialized `fqids[]` when allocation fails.

**Warnings:**
1. **Patch 07/26**: Move the `pr_info("Max BPID: ...")` log outside the loop or break after the first match.
2. **Patch 11/26**: Consider rate-limiting the FQRN wait loop to avoid spinning at full speed.

**Style/process notes:**
- The series adds several new device arguments and features, all of which are documented in the respective guides.
- Release notes are updated in patch 26/26 with a single consolidated entry.
- No forbidden tokens, deprecated API usage, or ABI breaks were found (all new APIs are marked experimental and use the export macros).

Overall, the series makes good progress on DPAA correctness and flexibility. The critical issues above should be addressed before merging.