Skip to content

kestrel: 8852C TX path selection from P-MAC (dead ACK bring-ups) - #492

Merged
josephnef merged 3 commits into
OpenIPC:masterfrom
agustinglez:kestrel-8852c-resp-txpath
Oct 16, 2026
Merged

josephnef merged 3 commits into
OpenIPC:masterfrom
agustinglez:kestrel-8852c-resp-txpath

Conversation

@agustinglez

Copy link
Copy Markdown
Contributor

Fixes the dead ACK bring-ups on the 8852C (item 1 of #490).

In short

Some 8852C bring-ups armed the hardware ACK responder and then answered no ACK for the whole session. The chip did build the ACK, but every frame also carries a selection of which transmit antenna paths to use, and the one the MAC attached to the ACK was garbage in those bring-ups: it enabled a path this 2T2R chip does not have, or wired path A to the wrong signal, so no antenna sent it. The selection comes from MAC state nothing initialises, which is why some bring-ups were fine and others were not.

The fix makes the baseband take the path selection from its own registers instead of from the MAC, using the vendor's own function for it, right after the existing path routing at bring-up. 8852C only.

The change

  • kestrel_halbb_ctrl_tx_path (halbb glue) calls halbb_ctrl_tx_path_pmac_8852c(RF_PATH_AB) after halbb_ctrl_tx_path_tmac_8852c: 0x09a4 [4:2] selects P-MAC as the source of the path enable and the path A/B maps, set to paths A+B with A on IFFT 0 and B on IFFT 1.
  • ack_responder_ok is true on the 8852C too, still as an explicit list of measured variants.
  • docs/kestrel-ack-responder.md: the dead-bring-up section now gives the cause, how it was found, what was ruled out, and the numbers. AdapterCaps.h and the SetAckResponder contract follow.

Measured

TX50UH responder, TX20U Nano soliciting, ch161, MCS1, 8 s per session:

dead sessions
without the fix (three series, the last on master with #491) 3 of 12, 6 of 15, 4 of 30
with the fix (register-level override and this vendor call; 30 on this exact commit) 0 of 123, all 96.8-100%
  • The bad selection (path A mapped to 1) died 10 of 10 times without the fix and answered 15 of 15 times with it.
  • Live disarm with the solicitor running across it: 3791/3845 then 0/1602, and 3829/3843 then 0/1639. Never armed: 0/1591.
  • Your bench on 9efa6ac (the register-level version): 0 dead of 20, both units, ch6 and ch161.
  • One-stream injection from the 8852C to the 8852B is unchanged: MCS1 and 6M ~1900/2000 on ch6, and on ch161 with carrier sense on.
  • MSVC build, ctest 79/79 on Windows.

Your two notes

  • TSSI reset pulse. Included: the change now calls the vendor function instead of writing the three registers by hand, so the per-path 0x58dc / 0x78dc pulse runs as in the vendor. The 30 sessions and the injection cells above were taken on this version. The function also sets halbb's bb->tx_path to RF_PATH_AB, as in the vendor driver.
  • Scope. The switch applies to every PPDU, and the measurements cover one stream only. Two-stream injection from the 8852C does not decode on master with or without this (MCS12 and VHT2SS_MCS1: 0/2000 at the 8852B, while the 8852B's own two-stream frames reach the 8852C), so what the change does to two-stream TX is not measured.

Not in this PR

Some 8852C bring-ups armed the ACK responder, read the entry back, and
answered no ACK for the whole session (OpenIPC#490). The chip built the ACK,
but the TX path selection the MAC attached to it was garbage in those
bring-ups: it enabled a path this 2T2R die does not have, or mapped path
A off the signal, so no antenna sent it. The selection comes from MAC
state nothing initialises, which is why it changed per bring-up.

The halbb glue now follows the T-MAC path routing at bring-up with
halbb_ctrl_tx_path_pmac_8852c(RF_PATH_AB): the BB takes the path enable
and the path A/B maps from its own P-MAC registers (0x09a4 [4:2]), paths
A+B with A on IFFT 0 and B on IFFT 1, plus the vendor's per-path TSSI
reset pulse. 8852C only; the switch applies to every PPDU.

- ack_responder_ok is true on the 8852C as well (still an explicit list
  of measured variants).
- docs/kestrel-ack-responder.md: the dead-bring-up section now gives the
  cause, how it was found (the TX-info debug port, decoded with the BB's
  INTF_TXINFO layout), what was ruled out, and the fix's numbers.

Measured (TX50UH responder, TX20U Nano soliciting, ch161, MCS1, 8 s per
session): before, 3 of 12, 6 of 15 and 4 of 30 sessions dead (the last
on master with OpenIPC#491); the map-A-1 selection died 10 of 10 times. After,
0 dead of 123 armed sessions across the register-level override and this
vendor call (30 of them on this exact code), 15 of them with the bad
selection, all 96.8-100%; solicitor across a live disarm 3791/3845 then
0/1602 and 3829/3843 then 0/1639; never armed 0/1591. Second bench: 0 of
20 on ch6 and ch161, both units. One-stream injection unchanged (MCS1 and
6M ~1900/2000 to the 8852B, ch6 and ch161 with carrier sense on). Two-
stream injection from the 8852C does not decode with or without this,
so its effect there is not measured.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix intermittent 8852C ACK loss by selecting TX paths from P-MAC

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Select 8852C transmit paths from baseband P-MAC registers to prevent ACKs from being built but not
 radiated.
• Advertise 8852C ACK responder support after successful on-air bring-up measurements.
• Document the diagnosis, measured results, and unmeasured two-stream impact.
Diagram

graph TD
  A["8852C bring-up"] --> B["halbb glue"] --> C["T-MAC routing"] --> D["P-MAC selection"] --> E["Baseband TX"] --> F["Radiated ACK"]
  D --> G["Responder capability"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Initialize the MAC's response path state
  • ➕ Could preserve MAC-controlled path selection for other PPDUs.
  • ➖ The source of the MAC's path selection remains unidentified, making this impractical for this fix.
2. Override path selection only for ACKs
  • ➕ Would avoid changing path selection for ordinary transmissions, including unmeasured two-stream traffic.
  • ➖ Requires a response-specific control point not established by the current investigation.

Recommendation: Use the existing vendor P-MAC function for this fix: it sets the intended paths and includes the per-path TSSI reset, while avoiding unidentified MAC state. Keep the every-PPDU scope explicit and validate two-stream TX separately before claiming it is unaffected.

Files changed (7) +90 / -71

Enhancement (2) +8 / -7
HalKestrel.cppLog P-MAC path selection during PHY bring-up +2/-1

Log P-MAC path selection during PHY bring-up

• Extends the TX path bring-up log to identify P-MAC selection of paths A and B alongside T-MAC routing.

src/kestrel/HalKestrel.cpp

RtlKestrelDevice.cppAdvertise measured 8852C ACK responder support +6/-6

Advertise measured 8852C ACK responder support

• Sets ack_responder_ok for the explicitly listed 8852B and 8852C variants. Updates the capability rationale to reflect the 8852C P-MAC path fix.

src/kestrel/RtlKestrelDevice.cpp

Bug fix (1) +12 / -1
kestrel_halbb_glue.cSelect 8852C TX paths through the vendor P-MAC API +12/-1

Select 8852C TX paths through the vendor P-MAC API

• Calls halbb_ctrl_tx_path_pmac_8852c with RF_PATH_AB after existing T-MAC routing on 8852C only. This selects baseband-owned path enable and A/B mapping and runs the vendor TSSI reset pulses.

hal/halbb/g6/kestrel_halbb_glue.c

Documentation (4) +70 / -63
kestrel-ack-responder.mdExplain the 8852C dead-ACK cause and measured fix +53/-46

Explain the 8852C dead-ACK cause and measured fix

• Replaces the unexplained-bring-up account with TX-info evidence identifying invalid MAC-supplied path selections. Documents the P-MAC remedy, ruled-out causes, session results, and the unresolved two-stream impact.

docs/kestrel-ack-responder.md

kestrel_halbb_glue.hDocument combined T-MAC routing and P-MAC selection +4/-2

Document combined T-MAC routing and P-MAC selection

• Clarifies the TX path helper's two bring-up steps and its 8852C-only scope.

hal/halbb/g6/kestrel_halbb_glue.h

AdapterCaps.hRecord corrected 8852C ACK responder measurements +9/-10

Record corrected 8852C ACK responder measurements

• Replaces the prior false-capability rationale with the diagnosed path-selection cause and post-fix armed, disarmed, and never-armed results.

src/AdapterCaps.h

RtlKestrelDevice.hUpdate the 8852C responder contract +4/-5

Update the 8852C responder contract

• Replaces the warning about unreliable armed sessions with the requirement that bring-up select TX paths from baseband P-MAC registers.

src/kestrel/RtlKestrelDevice.h

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. The cause lacks comparison evidence ✓ Resolved
Description
The dead-session investigation records TX-info and register observations but does not show the
required comparison of dead and good firmware RAM states or a same-unit vendor-driver A/B result.
Without those results, the path-selection attribution and the vendor comparison cannot be
independently verified against the investigation requirement.
Code

docs/kestrel-ack-responder.md[R123-127]

+The selection is not taken from the per-macid CMAC table (searched for in all
+128 entries, every session), the MAC TX power tables (a good session's tables
+written before every arm still gave dead sessions), the firmware version (the
+Windows driver's 0.29.123.4 gave dead sessions too), the slot, temperature, or
+the 5 GHz EDCCA level (#491). Where the MAC takes it from is still not found.
Evidence
The checklist requires dead/good firmware RAM comparison and a vendor-driver A/B test with results.
The changed investigation section documents TX-info, MAC-register, firmware-version, and other
exclusions, but does not provide either required comparison.

Investigate intermittent 8852C ACK responder failures
docs/kestrel-ack-responder.md[109-127]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updated investigation documents TX-info and register evidence but omits the required dead-versus-good firmware RAM comparison and same-unit vendor-driver A/B results.
## Fix Focus Areas
- docs/kestrel-ack-responder.md[109-127]
## Recommended Fix
Add the firmware RAM dump comparison for dead and good sessions, then document the same-unit vendor-driver A/B test, including whether the vendor bring-up reproduced the failure and which observations distinguish the sessions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Five-gigahertz results lack a baseline ✓ Resolved
Description
The documentation presents five-gigahertz injection results as unchanged and reports their reception
counts without a same-unit vendor-driver injection comparison. Those measurements therefore do not
establish whether the near-silent host injection behavior is specific to this implementation before
the results are used as evidence.
Code

docs/kestrel-ack-responder.md[R141-146]

+session. On the second bench (two TX50UH, an 8812CU soliciting): 0 dead of 20
+on ch6 and ch161, both units. Injection from the 8852C is unchanged for one
+stream (MCS1 and 6M ~1900/2000 to the 8852B on ch6 and on ch161 with carrier
+sense on). Two-stream injection from the 8852C does not decode with or without
+the fix (MCS12, VHT2SS_MCS1: 0/2000, while the 8852B's are received by the
+8852C), so what the fix does to two-stream TX is not measured.
Evidence
The checklist requires a same-unit five-gigahertz vendor-driver A/B before treating Kestrel TX
measurements as reliable. The changed section reports ch161 injection counts and two-stream results
but contains no vendor-driver baseline.

Validate 5 GHz Kestrel host injection against the vendor driver
docs/kestrel-ack-responder.md[141-146]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The five-gigahertz injection measurements are documented without the required same-unit vendor-driver A/B comparison.
## Fix Focus Areas
- docs/kestrel-ack-responder.md[135-146]
## Recommended Fix
Run the five-gigahertz injection test on the same TX50UH unit with the vendor driver, document both results and test conditions, and qualify or remove the Kestrel measurement until that comparison is available.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Bench results omit time-of-day context ✓ Resolved
Description
The responder matrix reports the new 123-session and 20-session measurements without stating when
during the day either matrix row was collected. Because the observed dead-session rate varied over
repeated sessions across a day, later readers cannot interpret these favorable rates against the
required time-of-day context.
Code

src/AdapterCaps.h[R212-219]

+   * session and says nothing). Some bring-ups used to answer no ACK at all
+   * (3 of 12, 6 of 15 and 4 of 30 here, up to ~85% on a second bench): the
+   * TX path selection the MAC attached to the ACK was uninitialised. With
+   * the path selection taken from the BB's P-MAC registers
+   * (docs/kestrel-ack-responder.md): 0 dead of 123 armed sessions here,
+   * every one 96.8-100%, 0 of 20 on the second bench; solicitor across a
+   * live disarm 3791/3845 then 0/1602 and 3829/3843 then 0/1639, never
+   * armed 0/1591).
Evidence
The checklist requires the relevant AdapterCaps.h matrix rows to state when measurements were taken.
The changed comments provide session counts, benches, and outcomes, but no date or time-of-day
context.

Record time-of-day context for responder bench results
src/AdapterCaps.h[212-219]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The responder capability comments include favorable session counts but omit the time-of-day context required to interpret measurements whose failure rate varied during the day.
## Fix Focus Areas
- src/AdapterCaps.h[212-219]
## Recommended Fix
Add the measurement date and time-of-day or clearly identified bench-session windows for each relevant responder matrix row, preserving the existing session counts and adverse results.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can pick a display mode instead of tweaking every Display option by hand

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread docs/kestrel-ack-responder.md
Comment thread docs/kestrel-ack-responder.md Outdated
Comment thread src/AdapterCaps.h Outdated
…heck

Review follow-up on the OpenIPC#490 requirements:

- AdapterCaps.h and the doc give the date and time window of every
  responder series (the dead rate drifts through a day, OpenIPC#490 item 4).
  The 4-of-30 series without the fix ran between series with it, so the
  difference is not the time of day.
- The doc states that OpenIPC#490's suggested experiments (firmware RAM dump of
  dead against good sessions, vendor-driver A/B on the same unit) were
  not run, and what the attribution rests on instead.
- The injection numbers are scoped as a before/after check of this
  change on the same build, not an absolute 5 GHz TX figure; the
  vendor-driver injection A/B of OpenIPC#490 item 2 was not run.
The two experiments OpenIPC#490 proposed, run after the cause was found:

- Firmware RAM: the firmware's writable data memory (about 590 KB, read
  through the indirect window at the CPU's own addresses) dumped in 26
  sessions and grouped by the TX-info path word. No bit separates the
  groups, and the firmware's own few-valued words take each value in
  both, so the path selection is not recorded in firmware memory.
- Vendor driver: the same TX50UH on the Realtek Windows driver,
  associated to an AP on ch161, through 45 full driver restarts: 0 dead,
  88.6-98.3% ACKed, against 4 dead of 30 for devourer without the fix on
  the same unit. The vendor bring-up initialises the response's path
  selection; which write does it is still not found.

@josephnef josephnef left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-run on ea0552d on this bench (two TX50UH, 8812CU soliciting with CCX tx.report, 8 s per session, 2026-10-16 morning; yesterday afternoon the same cells were ~85% dead):

cell sessions result
bus-3 unit, ch6 8 all alive, 98.2-99.0%
bus-3 unit, ch161 6 all alive, 100% each
bus-9 unit, ch6 4 all alive, 88.6-96.3%
live disarm, solicitor across it (3 cells) 3 ~40% of the first 40% of reports, then 0: clean step
never armed (3 cells) 3 0

0 dead of 18 armed sessions. Injection from the bus-3 unit with the default carrier-sense configuration (master with #491): ch6 224/444, ch161 229/442, same as before the change. The bring-up logs the P-MAC line once; Linux build + ctest 83/83.

Code: one vendor call after the T-MAC routing, 8852C-only, with the per-path pulse now included; the caps flag stays an explicit list of measured variants; the contract and doc say what the response depends on. The doc's firmware-RAM and vendor-driver sections answer the two experiments #490 asked for, and the 5 GHz injection numbers are scoped as a before/after of this change only, which is the right claim. The three Qodo threads were posted against the earlier head and read as answered by ea0552d (comparison evidence, the scoped 5 GHz claim, the times in AdapterCaps.h); they need resolving for the gate. Approving.

@josephnef
josephnef merged commit 8da7af2 into OpenIPC:master Oct 16, 2026
49 of 51 checks passed
josephnef added a commit that referenced this pull request Oct 16, 2026
The `build-mingw` job has failed on every run since 2026-10-16 (twice on
#492's head after a re-run): `pacman: error: target not found:
mingw-w64-x86_64-libusb`. The action also warns that MINGW64 is
deprecated. The package file is still on the mirror, but the
`mingw64.db` database no longer lists libusb, while `ucrt64.db` ships
every package the job installs at the same versions (gcc 16.2.0, cmake
4.4.4, ninja 1.13.2, libusb 1.0.30, pkgconf 3.0.8, openssl 3.6.5).

The job moves to UCRT64 with the `mingw-w64-ucrt-x86_64-*` package
names. Same gcc, same pkg-config libusb path, same binary-stdin and
station-crypto test coverage; the configure and build steps are
unchanged. Not a required check, so this is the only thing that turns it
green again.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants