rx: gate PHY-status parsing on the RX-descriptor PHYST bit (Jaguar1/2/3)

CMakeLists.txt

@@ -1124,6 +1124,18 @@ target_link_libraries(TxPktBankSelftest PRIVATE devourer)
 target_include_directories(TxPktBankSelftest PRIVATE src)
 add_test(NAME txpkt_bank_policy COMMAND TxPktBankSelftest)
 
+# Headless guard for the Jaguar2/Jaguar3 RX PHY-status gate: the descriptor
+# PHYST bit (DW0 bit 26) that keeps A-MPDU subframes' reserved-but-unwritten
+# drvinfo bytes out of the RF EMAs, plus the PhyStsFill tier each parser
+# returns (which fields it actually filled). Both parsers are header-only and
+# self-contained, so the test builds whenever either generation is selected.
+if(DEVOURER_JAGUAR3 OR DEVOURER_JAGUAR2_8822B OR DEVOURER_JAGUAR2_8821C)
+    add_executable(RxPhystSelftest tests/rx_physt_selftest.cpp)
+    target_link_libraries(RxPhystSelftest PRIVATE devourer)
+    target_include_directories(RxPhystSelftest PRIVATE src)
+    add_test(NAME rx_physt_bit COMMAND RxPhystSelftest)
+endif()
+
 # Headless guard for the USB TX-aggregation URB packing (src/TxAggPlan.h) —
 # block alignment, the never-a-bulk-multiple boundary shim, the OQT
 # descs-per-bulk guard, the frame/byte caps — plus the HalMAC descriptor agg

src/RxPacket.h

@@ -19,9 +19,32 @@ enum class RX_PACKET_TYPE
     C2H_PACKET
 };
 
+/* How much of an rx_pkt_attrib's signal block a PHY-status report actually
+ * filled. Reports are paged/typed per generation — a CCK page carries only
+ * path-A power, while the per-stream EVM/SNR and the CFO tail live on one OFDM
+ * page only — so a plain bool cannot tell a caller which of the fields below
+ * are a measurement and which are still zero. Feeding an unfilled field into a
+ * running average is not a null operation: it drags the mean toward zero. */
+enum class PhyStsFill : uint8_t
+{
+    None,  /* nothing filled: no report, too short, or a layout not decoded */
+    Power, /* per-path RSSI, plus ldpc/stbc/bw on an OFDM page */
+    Full   /* Power, plus per-stream EVM/SNR and the path-A CFO tail */
+};
+
 struct rx_pkt_attrib
 {
     uint16_t pkt_len;
+    /* RX-descriptor PHY-status bit: the PHY wrote a status report into THIS
+     * frame's drvinfo area. It is the RAW descriptor bit on every generation
+     * that decodes it (Jaguar1, Jaguar2, Jaguar3, RTL8733B) — deliberately NOT
+     * "the report parsed" and NOT "the signal fields below are valid", since a
+     * parser can still decline an unrecognised page (that is PhyStsFill's job,
+     * kept in a local at the parse site). The drvinfo area is reserved on every
+     * frame (RX_DRVINFO_SZ is a global register), so this bit is the only thing
+     * separating a written report from stale bytes left by an earlier frame —
+     * notably on all-but-one subframe of an A-MPDU. Never set on Kestrel, whose
+     * PHY status arrives as its own PPDU-status frame rather than in drvinfo. */
     bool physt;
     uint8_t drvinfo_sz;
     uint8_t shift_sz;

src/RxQuality.h

@@ -43,6 +43,10 @@ struct RxQuality {
   int rssi_max_dbm = 0;  /* window peak — the strength signal (see LinkHealth) */
   double snr_mean_db = 0.0;
   double snr_min_db = 0.0;
+  /* false when no frame in the window carried SNR (a CCK-only or non-type1
+   * phy-status window); snr_mean_db / snr_min_db are 0 then, and are a mean
+   * over the reporting frames only otherwise — not over every decoded frame. */
+  bool snr_valid = false;
   double evm_mean_db = 0.0; /* 0 when evm_valid is false */
   bool evm_valid = false;
 
@@ -84,6 +88,7 @@ struct RxQualitySnapshot {
   int rssi_max_raw = 0;
   int snr_mean_raw = 0;
   int snr_min_raw = 0;
+  bool snr_valid = false; /* false when no frame in the window carried SNR */
   int evm_mean_raw = 0;
   bool evm_valid = false;
   double nf_mean_dbm = 0.0;
@@ -99,7 +104,10 @@ class RxQualityAccumulator {
   /* Raw path-A values straight off rx_pkt_attrib. A frame with no phy-status
    * power (rssi_raw <= 0) is not a quality sample and is skipped. SNR/EVM are
    * folded only when present (raw != 0 — CCK / non-type1 phy-status leaves them
-   * 0), so a mixed stream doesn't bias those means toward zero. */
+   * 0), so a mixed stream doesn't bias those means toward zero; snr_min_raw
+   * likewise ignores the absent ones rather than pinning itself to 0. Each
+   * carries its own sample count, so the window means are over the frames that
+   * actually reported the metric. */
   void add(int rssi_raw, int snr_raw, int evm_raw) {
     if (rssi_raw <= 0)
       return;
@@ -108,9 +116,12 @@ class RxQualityAccumulator {
     rssi_sum_ += rssi_raw;
     if (rssi_raw > rssi_max_)
       rssi_max_ = rssi_raw;
-    snr_sum_ += snr_raw;
-    if (snr_raw < snr_min_)
-      snr_min_ = snr_raw;
+    if (snr_raw != 0) {
+      snr_sum_ += snr_raw;
+      ++snr_n_;
+      if (snr_raw < snr_min_)
+        snr_min_ = snr_raw;
+    }
     if (evm_raw != 0) {
       evm_sum_ += evm_raw;
       ++evm_n_;
@@ -130,8 +141,11 @@ class RxQualityAccumulator {
     if (n_) {
       s.rssi_mean_raw = rssi_sum_ / static_cast<int>(n_);
       s.rssi_max_raw = rssi_max_;
-      s.snr_mean_raw = snr_sum_ / static_cast<int>(n_);
+    }
+    if (snr_n_) {
+      s.snr_mean_raw = snr_sum_ / static_cast<int>(snr_n_);
       s.snr_min_raw = snr_min_;
+      s.snr_valid = true;
     }
     if (evm_n_) {
       s.evm_mean_raw = evm_sum_ / static_cast<int>(evm_n_);
@@ -145,6 +159,7 @@ class RxQualityAccumulator {
     rssi_sum_ = 0;
     rssi_max_ = -128;
     snr_sum_ = 0;
+    snr_n_ = 0;
     snr_min_ = 127;
     evm_sum_ = 0;
     evm_n_ = 0;
@@ -158,6 +173,7 @@ class RxQualityAccumulator {
   uint32_t n_ = 0;
   int32_t rssi_sum_ = 0, rssi_max_ = -128;
   int32_t snr_sum_ = 0, snr_min_ = 127;
+  uint32_t snr_n_ = 0;
   int32_t evm_sum_ = 0;
   uint32_t evm_n_ = 0;
   double nf_sum_ = 0.0;
@@ -178,6 +194,7 @@ inline RxQuality build_rx_quality(const RxQualitySnapshot &s, const RxEnergy &e,
   q.rssi_max_dbm = s.rssi_max_raw - 110;
   q.snr_mean_db = s.snr_mean_raw / 2.0;
   q.snr_min_db = s.snr_min_raw / 2.0;
+  q.snr_valid = s.snr_valid;
   q.evm_valid = s.evm_valid;
   q.evm_mean_db = s.evm_mean_raw / 2.0;
   q.noise_floor_dbm = s.nf_mean_dbm;

src/jaguar1/FrameParser.cpp

@@ -228,13 +228,18 @@ std::vector<Packet> FrameParser::recvbuf2recvframe(std::span<uint8_t> ptr) {
 
       struct _phy_status_rpt_8812 driver_data = {};
       /* Only read the PHY-status report when the descriptor says one is
-       * present and it fits the remaining buffer. The kernel gates this
-       * on pattrib->physt (usb_ops_linux.c:179); drvinfo_sz >= the report
-       * size is the equivalent condition with the fields we carry —
-       * without it, frames with drvinfo_sz==0 had payload bytes decoded
-       * as RSSI/EVM/SNR, and a frame ending near the buffer tail
-       * over-read the transfer buffer. */
-      if (pattrib.drvinfo_sz >= sizeof(driver_data) &&
+       * present and it fits the remaining buffer. pattrib.physt (DW0 bit 26)
+       * is the per-frame fact — the same gate the kernel uses
+       * (usb_ops_linux.c:179 passes pbuf+RXDESC_OFFSET only when it is set).
+       * The size check alone is NOT equivalent: REG_RX_DRVINFO_SZ is a global
+       * register (_InitDriverInfoSize_8812A writes 4 = 32 bytes), so the
+       * drvinfo space is reserved on EVERY frame while the PHY writes a report
+       * only where the bit is set. On all-but-one subframe of an A-MPDU the
+       * area therefore holds bytes left by an earlier frame, and copying them
+       * decodes stale RSSI/SNR/EVM/CFO — including the per-chain values the
+       * 8814AU spatial-diversity work reads. The size check still guards the
+       * tail over-read it was added for. */
+      if (pattrib.physt && pattrib.drvinfo_sz >= sizeof(driver_data) &&
           pbuf.size() >= RXDESC_SIZE + sizeof(driver_data)) {
         memcpy(static_cast<void *>(&driver_data), pbuf.data() + RXDESC_SIZE,
                sizeof(driver_data));

src/jaguar1/RtlJaguarDevice.cpp

@@ -1550,7 +1550,13 @@ void RtlJaguarDevice::StartRxLoop(Action_ParsedRadioPacket packetProcessor) {
              std::span<uint8_t>{const_cast<uint8_t *>(data), (size_t)n})) {
       if (should_stop || g_devourer_should_stop)
         break;
-      if (!p.RxAtrib.crc_err) {
+      /* physt: the descriptor says the PHY wrote a status report for THIS
+       * frame. Without it FrameParser leaves the signal fields at 0 (the
+       * drvinfo space is reserved on every frame but written only where the
+       * bit is set), and folding those zeros would drag the running averages
+       * — the CFO tracker in particular, whose enable threshold a diluted
+       * average never crosses. */
+      if (!p.RxAtrib.crc_err && p.RxAtrib.physt) {
         _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]);
         _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm,
                      _eepromManager->numTotalRfPath);

src/jaguar2/FrameParserJaguar2.h

@@ -207,6 +207,11 @@ struct Rx8822bFrame {
   uint8_t shift;
   uint32_t tsfl;              /* hardware TSF-low at receive */
   bool paggr;                 /* MPDU arrived inside an A-MPDU */
+  bool physt;                 /* a PHY-status report was written for THIS frame;
+                               * REG_RX_DRVINFO_SZ is global, so the drvinfo
+                               * space is reserved on every frame and holds
+                               * stale bytes without this bit — notably on
+                               * all-but-one subframe of an A-MPDU */
   uint8_t ppdu_cnt;           /* 2-bit received-PPDU counter */
   uint32_t next_offset;
 };
@@ -227,6 +232,7 @@ inline bool parse_rx_8822b(const uint8_t *buf, size_t buflen,
   out.rx_rate = static_cast<uint8_t>(GET_RX_DESC_RX_RATE_8822B(buf));
   out.tsfl = static_cast<uint32_t>(GET_RX_DESC_TSFL_8822B(buf));
   out.paggr = GET_RX_DESC_PAGGR_8822B(buf) != 0;
+  out.physt = GET_RX_DESC_PHYST_8822B(buf) != 0;
   out.ppdu_cnt = static_cast<uint8_t>(GET_RX_DESC_PPDU_CNT_8822B(buf));
 
   uint32_t frame_off =
@@ -248,14 +254,19 @@ inline bool parse_rx_8822b(const uint8_t *buf, size_t buflen,
  * SNR rxsnr[i] and per-stream EVM rxevm[i] (both s(8,1), i.e. half-dB units, as
  * the vendor stores them). Values are the raw phy-status fields, matching the
  * Jaguar-1 FrameParser convention (rssi = per-path power byte, dBm = value-110).
- * CCK (type0) reports a single path-A pwdb. Requires physts_len >= 28. */
-inline void parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len,
-                               bool is_cck, rx_pkt_attrib &a) {
+ * CCK (type0) reports a single path-A pwdb. Requires physts_len >= 28.
+ * Returns which fields of `a` were filled (PhyStsFill): None on a null/short
+ * buffer, Power for the CCK type0 report (path-A power only — EVM/SNR and the
+ * CFO tail are NOT in that layout and stay 0), Full for type1. Callers must
+ * not fold a field the return value does not claim. */
+inline PhyStsFill parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len,
+                                     bool is_cck, rx_pkt_attrib &a) {
   if (physts == nullptr || physts_len < 28)
-    return;
+    return PhyStsFill::None;
   if (is_cck) {
     /* type0: DW0 = page_num(0), pwdb(1), ... */
     a.rssi[0] = physts[1];
+    return PhyStsFill::Power;
   } else {
     /* type1: DW0/1 pwdb[4] at bytes 1..4; DW4 rxevm[4] at bytes 16..19;
      * DW5 cfo_tail[4] at bytes 20..23; DW6 rxsnr[4] at bytes 24..27. */
@@ -276,6 +287,7 @@ inline void parse_phy_sts_jgr2(const uint8_t *physts, uint16_t physts_len,
     const uint8_t rxsc = (a.data_rate >= 4 && a.data_rate <= 11) ? l_rxsc : ht_rxsc;
     a.bw = rxsc >= 13 ? 2 : rxsc >= 9 ? 1 : 0;
   }
+  return PhyStsFill::Full;
 }
 
 } /* namespace jaguar2 */

src/jaguar2/RtlJaguar2Device.cpp

@@ -618,17 +618,31 @@ void RtlJaguar2Device::StartRxLoop(Action_ParsedRadioPacket packetProcessor) {
         /* Per-frame RSSI/SNR/EVM from the jgr2 PHY-status (present when
          * APP_PHYSTS is on, i.e. drvinfo carries the 32-byte report). CCK rates
          * (DESC_RATE1M..11M = 0..3) use type0, everything else type1. C2H has no
-         * phy-status (drvinfo=0), so the size guard already skips it. */
-        if (!is_c2h && f.drvinfo_size >= 28)
-          jaguar2::parse_phy_sts_jgr2(data + off + jaguar2::RXDESC_SIZE_8822B,
-                                      f.drvinfo_size, f.rx_rate <= 3, p.RxAtrib);
+         * phy-status (drvinfo=0), so the size guard already skips it.
+         * REG_RX_DRVINFO_SZ (0x060F) is a GLOBAL register, so the 32 drvinfo
+         * bytes are reserved on EVERY frame while the PHY writes a report only
+         * where the descriptor's PHYST bit (DW0 bit 26, f.physt) is set —
+         * on an A-MPDU's other subframes the area holds bytes left by an
+         * earlier frame, and parsing them anyway decodes garbage as
+         * rssi/snr/evm/cfo_tail. cfo_tail is the one that does damage: it
+         * steers the closed-loop XtalCap crystal trim below. */
+        PhyStsFill phy = PhyStsFill::None;
+        if (!is_c2h && f.physt && f.drvinfo_size >= 28)
+          phy = jaguar2::parse_phy_sts_jgr2(
+              data + off + jaguar2::RXDESC_SIZE_8822B, f.drvinfo_size,
+              f.rx_rate <= 3, p.RxAtrib);
+        /* The RAW descriptor bit, matching the field's meaning on Jaguar1 /
+         * Jaguar3 / RTL8733B; `phy` says which fields are safe to fold. */
+        p.RxAtrib.physt = f.physt;
         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 && phy != PhyStsFill::None) {
           _rxq.add(p.RxAtrib.rssi[0], p.RxAtrib.snr[0], p.RxAtrib.evm[0]);
           _rxpaths.add(p.RxAtrib.rssi, p.RxAtrib.snr, p.RxAtrib.evm,
                        _variant == jaguar2::ChipVariant::C8821C ? 1 : 2);
-          if (_cfg.tuning.cfo_track)
+          /* cfo_tail lives only in the type1 layout; the 0 a CCK report leaves
+           * would pull the tracker's average below its enable threshold. */
+          if (_cfg.tuning.cfo_track && phy == PhyStsFill::Full)
             _cfo.add(p.RxAtrib.cfo_tail); /* closed-loop CFO input (#217) */
         }
         _packetProcessor(p);

src/jaguar3/FrameParserJaguar3.h

@@ -217,6 +217,11 @@ struct Rx8822cFrame {
   uint8_t shift;         /* SHIFT_SZ */
   uint32_t tsfl;         /* hardware TSF-low at receive */
   bool paggr;            /* MPDU arrived inside an A-MPDU */
+  bool physt;            /* a PHY-status report was written for THIS frame;
+                          * drvinfo space is reserved on every frame
+                          * (RX_DRVINFO_SZ is global), so without this bit the
+                          * area holds stale bytes — notably on all-but-one
+                          * subframe of an A-MPDU */
   uint8_t ppdu_cnt;      /* 2-bit received-PPDU counter */
   uint32_t next_offset;  /* 8-byte-aligned offset of the next frame in an agg */
 };
@@ -239,6 +244,7 @@ inline bool parse_rx_8822c(const uint8_t *buf, size_t buflen,
   out.rx_rate = static_cast<uint8_t>(GET_RX_DESC_RX_RATE_8822C(buf));
   out.tsfl = static_cast<uint32_t>(GET_RX_DESC_TSFL_8822C(buf));
   out.paggr = GET_RX_DESC_PAGGR_8822C(buf) != 0;
+  out.physt = GET_RX_DESC_PHYST_8822C(buf) != 0;
   out.ppdu_cnt = static_cast<uint8_t>(GET_RX_DESC_PPDU_CNT_8822C(buf));
 
   uint32_t frame_off =
@@ -269,16 +275,21 @@ inline bool parse_rx_8822c(const uint8_t *buf, size_t buflen,
  * vendor's s(8,1) fields). The page type is taken from byte0 low nibble
  * (page_num) rather than guessed from the rate: 0 = CCK type0, else an OFDM
  * page; per-stream EVM/SNR are only present on the type1 OFDM page.
- * Requires physts_len >= 28. */
-inline void parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len,
-                               rx_pkt_attrib &a) {
+ * Requires physts_len >= 28. Returns which fields of `a` were filled
+ * (PhyStsFill): None on a null/short buffer, Full only on the type1 OFDM page
+ * that carries EVM/SNR/CFO, Power on the CCK page and on every other OFDM page
+ * — those share the common header, so their per-path power (and ldpc/stbc/bw)
+ * IS a measurement even though the type1-only fields are left at 0. Callers
+ * must not fold a field the return value does not claim. */
+inline PhyStsFill parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len,
+                                     rx_pkt_attrib &a) {
   if (physts == nullptr || physts_len < 28)
-    return;
+    return PhyStsFill::None;
   const uint8_t page_num = physts[0] & 0x0f;
   if (page_num == 0) {
     /* type0 (CCK): DW0 = page_num(0), pwdb_a(1). Single path-A power. */
     a.rssi[0] = physts[1];
-    return;
+    return PhyStsFill::Power;
   }
   /* OFDM header (valid for every jgr3 OFDM page): per-path pwdb[4] at bytes
    * 1..4, DW1 byte5 l_rxsc[3:0]/ht_rxsc[7:4], DW1 byte7 flags. */
@@ -303,7 +314,15 @@ inline void parse_phy_sts_jgr3(const uint8_t *physts, uint16_t physts_len,
       a.evm[i] = static_cast<int8_t>(physts[16 + i]);
       a.snr[i] = static_cast<int8_t>(physts[24 + i]);
     }
+    return PhyStsFill::Full;
   }
+  /* Pages 2..6: the common header above was parsed and is valid, so this is a
+   * Power fill rather than a failure — reporting None here would throw away
+   * real per-path RSSI, and would silently freeze GetRxQuality/GetActiveRxPaths
+   * if the BB page selector ever left type1 (devourer's BB table pins
+   * 0x8C0[25:22]=1, but the vendor auto-switch and debug page helpers do not
+   * restore it). */
+  return PhyStsFill::Power;
 }
 
 } /* namespace jaguar3 */