-
-
Notifications
You must be signed in to change notification settings - Fork 25
jaguar3: gate PHY-status parsing on the RX-desc PHYST bit (DW0 bit 26) #409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -322,12 +322,17 @@ void RtlJaguar3Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { | |
| * present (monitor_rx_cfg enables APP_PHYSTS + RX_DRVINFO_SZ=4, so the | ||
| * 32-byte report is counted in drvinfo). Skips C2H reports and any | ||
| * frame whose drvinfo is too short (e.g. CCK, which carries no OFDM | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This retained sentence is wrong on both counts and now contradicts the new text three lines below. |
||
| * report). The report sits immediately after the 24-byte descriptor. */ | ||
| if (!is_c2h && f.drvinfo_size >= 28) | ||
| jaguar3::parse_phy_sts_jgr3(data + off + jaguar3::RXDESC_SIZE_8822C, | ||
| f.drvinfo_size, p.RxAtrib); | ||
| * report). The report sits immediately after the 24-byte descriptor. | ||
| * f.physt (RX desc DW0 bit 26) says the PHY actually WROTE a report | ||
| * for this frame — the drvinfo space itself is reserved on every | ||
| * frame, so on A-MPDU subframes without the bit it holds stale bytes | ||
| * whose page nibble can alias 0/1 (contaminated RSSI/SNR tails). */ | ||
| if (!is_c2h && f.physt && f.drvinfo_size >= 28) | ||
| p.RxAtrib.physt = jaguar3::parse_phy_sts_jgr3( | ||
| data + off + jaguar3::RXDESC_SIZE_8822C, f.drvinfo_size, | ||
| p.RxAtrib); | ||
|
Comment on lines
+330
to
+333
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Physt semantics diverge by chipset Jaguar3 now reports RxAtrib.physt=false when the descriptor bit is set but the report is short or uses an unsupported page, while Jaguar1 and RTL8733B expose the raw descriptor bit. Consumers of the shared packet callback can therefore no longer use physt consistently to identify which A-MPDU subframe carried PHY status. Agent Prompt
|
||
| p.Data = std::span<uint8_t>(const_cast<uint8_t *>(f.frame), f.frame_len); | ||
| if (!p.RxAtrib.crc_err) { | ||
| if (!p.RxAtrib.crc_err && p.RxAtrib.physt) { | ||
| _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CCK page-0 passes the gate having filled only |
||
| _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm, | ||
| 2); /* 8822C/8822E are 2T2R */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* Headless guard for the Jaguar3 RX-descriptor PHYST bit (DW0 bit 26, | ||
| * "the drvinfo area of THIS frame holds a written PHY-status report"). | ||
| * Inside an A-MPDU the drvinfo space is reserved on every subframe | ||
| * (RX_DRVINFO_SZ is a global register) but the PHY writes a report only | ||
| * where this bit is set — parsing the reserved bytes anyway reads stale | ||
| * garbage that can alias a valid page number, which contaminates the | ||
| * RSSI/SNR tails. A bit-position or plumbing regression here fails ctest | ||
| * instead of poisoning the RF EMAs. */ | ||
| #include <cstdio> | ||
| #include <cstring> | ||
|
|
||
| #include "jaguar3/FrameParserJaguar3.h" | ||
|
|
||
| static int g_fail = 0; | ||
| #define CHECK(cond, ...) \ | ||
| do { \ | ||
| if (!(cond)) { \ | ||
| ++g_fail; \ | ||
| std::printf("FAIL: " __VA_ARGS__); \ | ||
| std::printf("\n"); \ | ||
| } \ | ||
| } while (0) | ||
|
|
||
| /* 24-byte descriptor + 32-byte drvinfo + a 60-byte PSDU. */ | ||
| static constexpr uint32_t kDrvInfo = 32; | ||
| static constexpr uint32_t kFrameLen = 60; | ||
| static constexpr size_t kBufLen = | ||
| jaguar3::RXDESC_SIZE_8822C + kDrvInfo + kFrameLen; | ||
|
|
||
| static void make_desc(uint8_t *buf, bool physt) { | ||
| std::memset(buf, 0, kBufLen); | ||
| /* DW0: PKT_LEN[13:0] = 60, DRV_INFO_SIZE[19:16] = 4 (units of 8 bytes), | ||
| * SHIFT[25:24] = 0, PHYST = bit 26. */ | ||
| buf[0] = kFrameLen; | ||
| buf[2] = kDrvInfo / 8; | ||
| if (physt) | ||
| buf[3] |= 0x04; | ||
| } | ||
|
|
||
| static void test_physt_bit_decoded() { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The selftest covers only the descriptor-bit decode — |
||
| uint8_t buf[kBufLen]; | ||
| jaguar3::Rx8822cFrame f; | ||
|
|
||
| make_desc(buf, true); | ||
| CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=1 desc must parse"); | ||
| CHECK(f.physt, "PHYST set in DW0 bit 26 -> Rx8822cFrame.physt true"); | ||
|
|
||
| make_desc(buf, false); | ||
| CHECK(jaguar3::parse_rx_8822c(buf, kBufLen, f), "physt=0 desc must parse"); | ||
| CHECK(!f.physt, "PHYST clear -> Rx8822cFrame.physt false"); | ||
| } | ||
|
|
||
| int main() { | ||
| test_physt_bit_decoded(); | ||
| if (g_fail == 0) | ||
| std::printf("rx_physt_selftest: all checks passed\n"); | ||
| return g_fail == 0 ? 0 : 1; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutate-then-return-false breaks the new contract for OFDM pages 2-6. By this point the function has already written
a.rssi[0..3],ldpc,stbc,bwfrom the OFDM common header — which is valid on every jgr3 OFDM page (per the comment above and the vendorphy_sts_rpt_jgr3_ofdm_cmnstruct) — and then returns false, telling the caller "do not trusta's signal fields". Two consequences: the half-mutated attrib flows to the packetProcessor taggedphyst=false, and genuinely valid per-path RSSI is dropped from the EMAs. If the BB page selector ever leaves type1 (vendor auto-switch via 0x8c4, or the vendor debug page-switch helper, which never restores the page), every OFDM frame readsphyst=falseandGetRxQuality/GetActiveRxPathsfreeze with no diagnostic. Latent under devourer's own config (the BB table pins 0x8C0[25:22]=1), but the contract is wrong today: either fill nothing on the false path, or return true for pages whose common header was parsed.