|WARNING| drivers/bus: remove blocklist evaluation when probing a device

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Summary
This patch removes redundant device blocklist checks from bus driver probe functions. The EAL already evaluates blocklist policy before calling `.probe_device`, making these checks unnecessary.

---

## Errors

None found.

---

## Warnings

### 1. Missing Cc: stable at dpdk.org tag
**Location:** Commit message  
**Issue:** This is a bug fix (indicated by the `Fixes:` tag) but does not include `Cc: stable at dpdk.org` for backporting consideration.  
**Fix:** Add `Cc: stable at dpdk.org` after the `Fixes:` line in the commit message.

### 2. Inconsistent error return values
**Location:** All four modified files  
**Issue:** The removed code returned different values on different buses:
- `auxiliary_common.c`: returns `-1`
- `fslmc_bus.c`: returns `0`
- `pci_common.c`: returns `1`
- `vmbus_common.c`: returns `1`

While the patch removes this inconsistency (which is good), the commit message does not mention that these buses previously had different behaviors when a device was blocklisted. This inconsistency suggests the blocklist check was either never working correctly on all buses, or the return value semantics differed across buses.

**Recommendation:** Add a note in the commit message explaining that this also fixes an inconsistency where different buses returned different values for blocklisted devices (0, 1, or -1), though the EAL already prevented reaching this code.

---

## Info

### 1. Commit message clarity
The commit message is concise but could be clearer about the functional impact:
- "EAL already evaluated if the device is blocklisted" - consider stating *where* in the call chain this happens (e.g., "in `rte_bus_probe()` before calling each bus's `.probe_device` callback")
- The message implies this is dead code removal but doesn't explicitly state that the removed checks could never trigger in practice

**Suggested improvement:**
```
.probe_device is called from EAL's rte_bus_probe(), which already
evaluates whether a device is blocklisted before invoking each bus
driver's probe callback. These bus-level checks are therefore
redundant dead code.

This also removes an inconsistency where different buses returned
different error codes (0, 1, or -1) for blocklisted devices.
```

### 2. Potential for follow-up
The FSLMC bus includes a comment "/* FIXME: probe_device should allocate intr_handle */" that could be addressed in a future patch, though it's unrelated to this change.

---

## Positive Observations

1. **Clean refactoring:** Removes duplicated logic across four bus drivers
2. **Code simplification:** Reduces code paths and potential for bugs
3. **Correct architectural fix:** Centralizes policy enforcement in the EAL rather than scattering it across bus drivers
4. **No functional change:** Since the EAL already blocks these devices, the removal is safe

---

## Overall Assessment

This is a **valid cleanup patch** that removes redundant code. The only substantive concern is the missing `Cc: stable at dpdk.org` tag for backporting. The commit message could be slightly more explicit about where the EAL performs the blocklist check, but this is a minor clarity issue.

**Recommendation:** Add `Cc: stable at dpdk.org` and consider enhancing the commit message for clarity.