jaguar3: honour rx.keep_corrupted in the monitor RCR (ACRC32|AICV)

[gilankpam]

DEVOURER_RX_KEEP_CORRUPTED / DeviceConfig::rx.keep_corrupted was a silent
no-op on Jaguar3 (RTL8812CU/8822CU, RTL8812EU/8822EU).

monitor_rx_cfg wrote a hardcoded RCR of 0xF410400F | BIT28, and its comment
claimed that value already accepted CRC/ICV-error frames — it listed
AAP/APM/AM/AB/ACF/AICV/ACRC32 + APP_PHYSTS. Decoding the literal against
halmac_bit_8822c.h / halmac_bit_8822e.h shows ACRC32 (BIT8) and AICV (BIT9)
are not in it. The WMAC filter therefore dropped every FCS-failed frame no
matter how the config field was set, so the corrupted-frame retention the
fused-FEC salvage layer is built on never worked on this generation.

The fix gates the two bits on _cfg.rx.keep_corrupted, which is what Jaguar1
(RadioManagementModule.cpp), Jaguar2 (HalJaguar2.cpp) and the RTL8733B
(Halmac8733bMac::configure_monitor_rx) already do. Both misleading comments
are corrected, including a full decode of the base literal so the next reader
does not have to redo it.

Default behaviour is unchanged: keep_corrupted defaults to false, so an
unconfigured Jaguar3 monitor session programs exactly the RCR it programmed
before this patch.

Also widens the keep_corrupted doc comment in DeviceConfig.h, which said
"Jaguar1 + Jaguar2": it now names Jaguar3 and the RTL8733B, and states that
Kestrel has no port and the knob is inert there.

Validation

Bench-verified on one RTL8812EU unit: with the bits set, FCS-failed frames
reach the host with crc_err marked in the RX descriptor; with them clear the
count is zero regardless of channel conditions. One unit, one bench — the A/B
is its own control, but the result is not replicated across parts.

No headless selftest: monitor_rx_cfg writes straight to the device with no
seam to assert against, which is also why the Jaguar1 and Jaguar2 equivalents
have none. cmake --build + ctest green (55/55, la_csi_math skipped — it
needs python numpy/scipy).

Note the Jaguar1 measurement in docs/pseudo-preamble-puncturing.md ("zero
FCS-failed frames" under a jammed slice) is unaffected — that experiment used
an 8814AU receiver, whose path already gated correctly.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JiAS32956Z3cmTbnmcXYkT

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Honor keep_corrupted in Jaguar3 monitor receive configuration

🐞 Bug fix 📝 Documentation 🕐 10-20 Minutes

AI Description

• Enables Jaguar3 WMAC acceptance of CRC32 and ICV-failed frames when configured.
• Preserves existing monitor behavior when corrupted-frame retention remains disabled.
• Documents supported chip families and Kestrel's inert configuration behavior.

Diagram

graph TD
  CFG["RX config"] --> MON["Jaguar3 monitor"] --> KEEP{"Keep corrupted?"}
  KEEP -->|No| BASE["Base RCR"] --> WMAC["WMAC filter"] --> HOST["Host RX"]
  KEEP -->|Yes| BITS["Add error bits"] --> WMAC

Loading

High-Level Assessment

The conditional RCR-bit approach is optimal because it matches Jaguar1, Jaguar2, and RTL8733B behavior while preserving Jaguar3's default register value. Always enabling corrupted-frame delivery was considered but would change default filtering semantics and increase unwanted host traffic.

Files changed (2) +18 / -5

Bug fix (1) +16 / -4

HalJaguar3.cppConditionally accept corrupted Jaguar3 monitor frames +16/-4

Conditionally accept corrupted Jaguar3 monitor frames

• Builds the Jaguar3 monitor RCR dynamically and adds ACRC32 and AICV only when rx.keep_corrupted is enabled. Clarifies the base RCR decode, hardware filtering behavior, and descriptor error reporting while retaining the previous default value.

src/jaguar3/HalJaguar3.cpp

Documentation (1) +2 / -1

DeviceConfig.hDocument expanded keep_corrupted chipset support +2/-1

Document expanded keep_corrupted chipset support

• Extends the configuration comment to include Jaguar3 and RTL8733B support. It also explicitly documents that the option remains inert on Kestrel.

src/DeviceConfig.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

Correct fix, and the decode in the new comment holds up.

I checked 0xF410400F | BIT28 against reference/rtl88x2cu/hal/halmac/halmac_bit_8822c.h:15954-15986: APP_FCS(31) APP_MIC(30) APP_ICV(29) APP_PHYSTS(28) VHT_DACK(26) PKTCTL_DLEN(20) HTC_LOC_CTRL(14) AB(3) AM(2) APM(1) AAP(0). ACRC32(8) and AICV(9) are genuinely absent, and the old comment's ACF was never in the literal either — the correction is right on both counts.

Downstream is wired, so the RCR bits really are the whole gate: FrameParserJaguar3.h:242-243 decodes crc_err/icv_err, RtlJaguar3Device.cpp:299-300 propagates them into RxAtrib, and the RX loop delivers the frame to the packetProcessor without a software drop (only the PHY-status fold at :343 gates on crc_err). The fix does reach the salvage layer.

Two things beyond the inline note:

A behaviour change the description doesn't cover. examples/doctor/main.cpp:229 sets cfg.rx.keep_corrupted = true unconditionally. On Jaguar3 the doctor's rx_frames_crc goes from structurally-always-0 to a real count. No verdict flip — ClassifyAdapterHealth reads rx_frames_ok separately — but the printed report changes on this generation, and "default behaviour is unchanged" reads as covering the shipped tools. Worth a line in the body.

Needs a rebase. The branch is BEHIND master; #415 renamed IRtlDevice→IRadio. Trivial here since this diff touches neither.

No objection to merging once rebased.

[josephnef]

| (1u << 28) is redundant — bit 28 is already in the leading 0xF nibble of 0xF410400F, which is exactly what the new comment above now spells out (... + APP_FCS/APP_MIC/APP_ICV/APP_PHYSTS). Pre-existing, but now that the base is decoded in the comment the separate OR reads as if APP_PHYSTS were being added on top. Drop one or the other.