|WARNING| [v24, 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/171069

_AI Code Review_

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

# DPAA Patch Series Review

This is a comprehensive patch series for DPAA (Data Path Acceleration Architecture) drivers with 27 patches covering bug fixes, feature additions, and code improvements. Below is the review organized by severity.

---

## ERRORS

### Patch 02/27: net/dpaa: fix probe success when FMC init fails
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Line 2262:**
```c
ret = num_rx_fqs;
goto free_rx;
```

 **Correct fix.** The patch propagates the error from `dpaa_port_fmc_init()` instead of returning success (ret=0). The adjacent "not configured by FMC" case already sets `-ENODEV` before the same goto, so this brings the FMC initialization failure path in line with the existing error handling pattern.

---

### Patch 03/27: net/dpaa: fix resource leak on probe failure
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Lines 2625-2630:**
```c
goto close_dev;

close_dev:
    dpaa_eth_dev_close(eth_dev);
release_port:
    rte_eth_dev_release_port(eth_dev);
```

 **Correct fix.** When the shared Tx scatter-gather mempool creation fails, the early return leaked everything set up by `dpaa_dev_init()`. The patch adds cleanup labels matching the existing `dpaa_dev_init()` failure path, ensuring the device is closed and the port is released.

---

### Patch 04/27: net/dpaa: fix free port resources on close
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Lines 627-628:**
```c
rte_free(dpaa_intf->tx_conf_queues);
dpaa_intf->tx_conf_queues = NULL;
```

 **Correct fix.** The `tx_conf_queues` allocation is unconditional in `dpaa_dev_init()` but was never released. The patch adds the missing free and NULL assignment in `dpaa_eth_dev_close()` and also adjusts the probe error path to free it after the other allocations fail (`free_tx` label).

The new `clean_1` label placement is appropriate: it runs after the link update and MAC register writes but before the resource cleanup that now includes `tx_conf_queues`. The early return for offline/ONIC ports now jumps to `clean_1` to clean up the newly freed resources.

---

### Patch 05/27: bus/dpaa: fix FQD dest wq channel decoding
**File:** `drivers/bus/dpaa/base/qbman/qman.c`

**Lines 2825-2826:**
```c
channel = qm_fqd_get_chan(&mcr->queryfq.fqd);
wq = qm_fqd_get_wq(&mcr->queryfq.fqd);
```

 **Correct fix.** The old code extracted bits 2:0 as the channel and bits 15:3 as the work queue, but in the hardware FQD descriptor the work queue is bits 2:0 and the channel is bits 15:3. The new inline helpers in `qman.h` (lines 926-933) correctly decode `dest_wq`:
- `qm_fqd_get_chan()`: `be16_to_cpu(fqd->dest_wq) >> QM_FQD_CHAN_OFF` where `QM_FQD_CHAN_OFF = 3`
- `qm_fqd_get_wq()`: `be16_to_cpu(fqd->dest_wq) & QM_FQD_WQ_MASK` where `QM_FQD_WQ_MASK = GENMASK(2, 0)`

The old 0-7 channel value from the wrong bits could never match the pool-channel range, so pool-channel FQs were not drained correctly on shutdown.

---

### Patch 16/27: mempool/dpaa: fix write after free on pool free
**File:** `drivers/mempool/dpaa/dpaa_mempool.c`

**Lines 146-147:**
```c
bp_info->bp = NULL;
rte_free(bp_info);
```

 **Correct fix.** The old code freed `mp->pool_data` and then wrote to `bp_info->bp`, where `bp_info` is `DPAA_MEMPOOL_TO_POOL_INFO(mp)` (i.e., `mp->pool_data`). Both aliases refer to the same allocation, so the write accessed freed memory. The patch clears the field before releasing the allocation and frees `bp_info` directly instead of the alias.

---

## WARNINGS

### Patch 11/27: drivers: add DPAA cgrid cleanup support
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Lines 670-676 (CGR cleanup in close):**
```c
if (dpaa_intf->cgr_rx) {
    dpaa_cgr_stale_fq_cleanup(dev, dpaa_intf->cgr_rx[0].cgrid,
                              dpaa_intf->nb_rx_queues, "rx");
    for (loop = 0; loop < dpaa_intf->nb_rx_queues; loop++) {
        ret = qman_delete_cgr(&dpaa_intf->cgr_rx[loop]);
```

**Lines 2592-2599 (Error path cleanup in probe):**
```c
free_rx:
    for (loop = 0; loop < nb_rx_cgr; loop++)
        qman_delete_cgr(&dpaa_intf->cgr_rx[loop]);
    if (rx_cgrid_allocated)
        qman_release_cgrid_range(cgrid[0], num_rx_fqs);
```

 **Correct design.** A CGR must have no member FQs left when it is deleted. The patch adds `dpaa_cgr_stale_fq_cleanup()` to shut down any FQs still linked to the CGRID range before calling `qman_delete_cgr()` on each CGR. The CGRID range is then returned to the allocator with `qman_release_cgrid_range()`. The probe error path (lines 2598-2604) does the same for both Rx and Tx CGRs, deleting each CGR that was successfully created (`nb_rx_cgr`, `nb_tx_cgr`) and releasing the allocated CGRID range if any.

The `qman_pending_fq_by_cgrid_range()` scan (patch also adds this function) walks the FQID space to find any stale FQ still referencing the CGR range and shuts it down. The scan is bounded by the FQID space (24 bits), not O(N^2), because it resumes from the last found FQID + 1 instead of restarting each time.

---

### Patch 18/27: dma/dpaa: add devargs for SG and errata workaround
**File:** `drivers/dma/dpaa/dpaa_qdma.c`

**Lines 910-928 (new SG disable path):**
```c
} else if (!s_sg_enable) {
    while (fsl_queue->pending_num > 0) {
        ret = fsl_qdma_enqueue_desc_single(fsl_queue,
            fsl_queue->pending_desc[start].dst,
            fsl_queue->pending_desc[start].src,
            fsl_queue->pending_desc[start].len);
```

 **Feature addition.** The patch adds runtime control over scatter-gather batching (default on), post-transfer data validation (default off), and ERR050757 programming (default on when built with `RTE_DMA_DPAA_ERRATA_ERR050757`). These features already existed in the driver; the patch makes them configurable via device arguments (`dpaa_dma_sg_disable`, `dpaa_dma_data_validation`, `dpaa_dma_pci_read_disable`).

The new SG disable path (lines 910-928) submits each pending descriptor as an individual single-entry transfer instead of grouping them into one SG command. The existing code already had both paths; the devarg selects between them.

---

### Patch 19/27: net/dpaa: support Rx/Tx taildrop threshold devarg
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Lines 2449-2475 (Rx taildrop devarg):**
```c
ret = dpaa_get_devargs_int(dev->devargs, DRIVER_RX_TAILDROP, &td_val);
if (ret < 0) {
    DPAA_PMD_ERR("Invalid %s value", DRIVER_RX_TAILDROP);
    goto free_rx;
}
if (ret == 1) {
    if (td_val < 0 || td_val > UINT16_MAX) {
        DPAA_PMD_WARN("Rx tail drop threshold out of range, using %u",
                      CGR_RX_PERFQ_THRESH);
        td_threshold = CGR_RX_PERFQ_THRESH;
    } else {
        td_threshold = (unsigned int)td_val;
    }
```

 **Feature addition.** The Rx taildrop threshold was fixed at compile time (`CGR_RX_PERFQ_THRESH`). The patch adds a `drv_rx_taildrop` device argument to configure it per port, with 0 disabling taildrop. A value outside 0..UINT16_MAX falls back to the default. The same pattern is applied to the Tx threshold (`drv_tx_taildrop`), which previously could only be set through the `DPAA_TX_TAILDROP_THRESHOLD` environment variable (which is still honored as a fallback).

The `dpaa_get_devargs_int()` helper is added (lines 2304-2347) to parse integer valued device arguments, and the new arguments are advertised through `RTE_PMD_REGISTER_PARAM_STRING()` and documented in the DPAA NIC guide.

---

## INFO (Pre-existing or Style)

### Patch 01/27: net/dpaa: fix double close and null deref on remove
**File:** `drivers/net/dpaa/dpaa_ethdev.c`

**Lines 2692-2699 (old code, removed):**
```c
ret = dpaa_eth_dev_close(eth_dev);
if (eth_dev->state !=  RTE_ETH_DEV_UNUSED) {
    dpaa_eth_dev_close(eth_dev);
    ret = rte_eth_dev_release_port(eth_dev);
}
```

**Info:** The pre-existing code called `dpaa_eth_dev_close(eth_dev)` unconditionally, then again if the port was still in use. The first call ran on an eth_dev that `rte_eth_dev_allocated()` may not have found (NULL). The patch fixes this by checking `eth_dev != NULL && eth_dev->state != RTE_ETH_DEV_UNUSED` before closing, and logging the close result before releasing the port.

---

### Patch 06/27: bus/dpaa: accept QDMA device name in devargs
**File:** `drivers/bus/dpaa/dpaa_bus.c`

**Lines 524-530 (new QDMA parsing):**
```c
} else if (strncmp("dpaa_qdma", &name[delta], 9) == 0) {
    if (sscanf(&name[delta], "dpaa_qdma-%u", &i) != 1 ||
            i < 1 || i > RTE_DPAA_QDMA_DEVICES)
        return -EINVAL;
    max_name_len = sizeof("dpaa_qdma-.") - 1;
```

 **Bug fix.** The documented usage `-a dpaa_bus:dpaa_qdma-1,dpaa_dma_err_check=1` failed in `rte_devargs_parse()` because `rte_dpaa_bus_parse()` did not recognize the QDMA name pattern. The patch adds the missing case.

---

### Patch 07/27: bus/dpaa: refine fman naming
**File:** `drivers/bus/dpaa/base/fman/fman.c`

**Lines 468-469 (ccsr_map renamed to memac_map):**
```c
__if->memac_map = mmap(NULL, __if->regs_size,
    PROT_READ | PROT_WRITE, MAP_SHARED, fd, phys_addr);
```

**Info:** The patch renames `ccsr_map` to `memac_map` and `bmi_map` to `rx_bmi_map` for clarity. `ccsr_map` was misleading because it maps the MEMAC register space, not the generic CCSR. `bmi_map` was ambiguous because both Rx and Tx BMI exist. The rename improves code readability without changing behaviour.

---

### Patch 08/27: bus/dpaa: scan max BPID from DTS
**File:** `drivers/bus/dpaa/base/qbman/bman_driver.c`

**Lines 204-227 (new BPID range scan):**
```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]);
```

 **Feature improvement.** The patch calculates the maximum BPID dynamically from the device tree instead of using a hardcoded value based on the IP revision. The new code clamps the value to `BMAN_MAX_POOLS` (64) if the DTS range exceeds it, because the depletion mask and SCN registers only describe BPID 0..63 and `bman_create_portal()` walks this bound with a `u8`.

---

### Patch 09/27: drivers: add process-type guards for secondary process
**File:** `drivers/dma/dpaa/dpaa_qdma.c`

**Lines 1443-1447 (secondary reject):**
```c
if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
    DPAA_PMD_WARN("%s: not supported in secondary process",
        dpaa_dev->device.name);
    return -ENOTSUP;
}
```

 **Bug fix.** The `struct fsl_qdma_engine` is allocated from shared memory, but `reg_base` in it is a process-private mmap of the QDMA CCSR region created by the primary. A secondary that probes the device would advertise a dmadev whose MMIO pointers are not mapped in its own address space. The patch rejects the probe with `-ENOTSUP` until a secondary can establish its own mapping.

In `net/dpaa`, the patch adds a guard in `rte_dpaa_remove()` to let a secondary release just its local ethdev port without running the primary-only bookkeeping (decrement device count, free shared Tx mempool).

---

### Patch 10/27: drivers: shutdown DPAA FQ by fq descriptor
**File:** `drivers/bus/dpaa/base/qbman/qman.c`

**Lines 2789-2799 (new signature):**
```c
int
qman_shutdown_fq(struct qman_fq *fq)
{
    struct qman_portal *p = fq->qp;
```

 **API improvement.** The patch changes `qman_shutdown_fq()` to take the FQ descriptor instead of just an fqid, and uses `fq->qp` when the caller has one, falling back to the affine portal otherwise. This lets a caller that ownswarning: review has no Review-Result line, severity inferred from prose (response may be truncated)