Skip to content

Expose the EFUSE MAC as a per-unit adapter identity - #383

Merged
josephnef merged 4 commits into
OpenIPC:masterfrom
snokvist:feat/permanent-mac-identity
Aug 7, 2026
Merged

Expose the EFUSE MAC as a per-unit adapter identity#383
josephnef merged 4 commits into
OpenIPC:masterfrom
snokvist:feat/permanent-mac-identity

Conversation

@snokvist

@snokvist snokvist commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

The problem

A consumer that keeps per-adapter state — a measured TX-power curve, a
calibration, anything tied to one specific dongle — has to answer "is this the
same physical adapter I measured last time?"
across a re-plug, a reboot, and a
port change. Applying one unit's measurements to another silently is the failure
to avoid.

devourer currently exposes no key that can answer it:

  • USB bus path (1-1, 3-1.2) identifies a port, not a device. It changes
    whenever the dongle moves, orphaning the state.
  • USB serial descriptor is not unique — see below.

The serial is a burned-in constant; the MAC beside it is per-unit

Both live in the same EFUSE, a few bytes apart. Dumped through the vendor kernel
driver (/proc/net/rtl88x2{eu,cu}/<iface>/efuse_map) on two adapters:

RTL8822EU — netdev wlx84fc1450bcde:

0x150   DA 0B 1A A8 FF 7E 02 84   FC 14 50 BC DE 09 03 52
0x170   20 4E 49 43 08 03 31 32   33 34 35 36 FF FF FF FF

RTL8822CU — netdev wlx40a5ef2f2308:

0x150   DA 0B 12 C8 FF 7E 02 40   A5 EF 2F 23 08 09 03 52
0x170   20 4E 49 43 08 03 31 32   33 34 35 36 FF FF FF FF
logical offset contents per-unit?
0x157 the 6-byte MAC yes
0x15D / 0x166 "Realtek" / "802.11ac NIC" string descriptors no
0x174 USB serial string descriptor — "123456" no, identical on both

So the serial is not missing, it is a placeholder burned identically into every
unit. Keying on it would be worse than the bus path: two adapters in one host
would share state and silently apply each other's measurements.

The MAC is also exactly where Linux gets it — the vendor driver programs it into
the netdev, and udev derives the stable wlx<mac> name from that. That name is
stable across re-plug because it comes from the chip.

What this adds

IRtlDevice::GetPermanentMacAddress(uint8_t out[6]), defaulting to false so
unimplemented chips degrade gracefully and no existing consumer changes
behaviour.

  • Jaguar1 — routes to the existing EepromManager::GetMacAddress. The read,
    the per-chip offsets (from hal_pg.h: 8812AU 0xD7, 8814AU 0xD8, 8821AU
    0x107) and the unprogrammed-value rejection were all already implemented;
    only a route to a caller was missing.
  • Jaguar3 — logical 0x157. On 8822E the value is captured during the
    existing rtw_hal_init efuse pass, because that OTP is not reliably readable
    after TX/coex bring-up — the same constraint _efuse_cache exists for. One
    walk decodes far enough for both, and _efuse_cache keeps its size so
    probe_efuse_map's compare surface is unchanged. On 8822C the map is
    decoded on demand.
  • doctor prints it, which is also how to check the offset on a chip nobody
    has measured: compare against the wlx<mac> name the vendor driver gives the
    same dongle.

Jaguar2 and Kestrel keep the default false. Kestrel's offset constant already
exists in-tree (EFUSE_USB_MAC_ADDR_8852B = 0x488, kestrel/MacRegAx.h:179) if
someone with the hardware wants to finish it.

Hardware verification

Three adapters, all three implemented code paths, doctor output vs the kernel's
own netdev MAC:

adapter path exercised kernel devourer
RTL8812AU (Jaguar1) EepromManager, offset 0xD7 20:0d:b0:c4:a7:6a 20:0d:b0:c4:a7:6a match
RTL8822EU (Jaguar3, C8822E) captured in rtw_hal_init 84:fc:14:50:bc:de 84:fc:14:50:bc:de match
RTL8822CU (Jaguar3, C8822C) on-demand decode 40:a5:ef:2f:23:08 40:a5:ef:2f:23:08 match (needs #384)

The 8822C row needed a separate fix, and it is not in this PR. On that unit the
shared non-EU walk in read_efuse_logical_map returned an empty logical map, so
the MAC read found 0xFF — independently visible in the same run as
Jaguar3: rfe_type=0x00 while the kernel reads logical 0xCA = 0x03 from the same
adapter. Root cause is now known and fixed in #384 (tracked as #385): the
walk stopped as soon as a section's base passed the requested byte, which assumes
sections are burned in ascending base order — they are append-ordered. My original
guess in this PR (the if (hdr == 0xFF) break; padding termination) was wrong;
@josephnef's control experiment on a second 8822CU showed EU-style 0xFF-run
tolerance alone changes nothing.

The walk fix stays out of this PR deliberately: it feeds RFE and per-channel
TX-power base for every 8822C user and deserves its own validation rather than
riding along with a new accessor. The two changes are independent — this one has no
unverified path once #384 lands.

Caveats worth stating

  • 0x157 is measured, not read from a datasheet. Resolved — it is the
    vendor constant. include/hal_pg.h (rtl88x2cu 20230728 / rtl88x2eu 20230815):
    EEPROM_MAC_ADDR_8822CU 0x157, EEPROM_MAC_ADDR_8822EU 0x157. The caveat's worry
    was right in kind — the other variants do differ (…CS/ES 0x16A, …CE/EE 0x120)
    — but devourer is USB-only, so 0x157 is correct for both Jaguar3 parts. The
    Jaguar1 offsets served by EepromManager check out against the same header
    (8812AU 0xD7, 8814AU 0xD8, 8821AU 0x107), and the Jaguar2 follow-up already
    has its constant there too (8822BU and 8821CU are both 0x107).
  • A MAC is a hardware identity rather than a secret, but it is a stable device
    identifier; consumers logging it should treat it as they treat any other
    adapter identity.

Build is clean; no new warnings.

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

qodo-free-for-open-source-projects Bot commented Aug 6, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Expose EFUSE MAC as a stable per-adapter identity (Jaguar1/Jaguar3)

✨ Enhancement 🐞 Bug fix 🕐 40+ Minutes

Grey Divider

AI Description

• Add IRtlDevice API to fetch permanent EFUSE MAC as per-unit adapter identity.
• Implement EFUSE MAC retrieval for Jaguar1 and Jaguar3, with safe caching/locking.
• Fix Jaguar3 EFUSE logical-map walk to decode unordered sections and avoid wraparound reads.
Diagram

graph TD
  C([Consumer / doctor]) --> I["IRtlDevice::GetPermanentMacAddress()"] --> J1["Jaguar1 device"] --> E1["EepromManager"] --> F[("EFUSE/EEPROM")]
  C([Consumer / doctor]) --> I["IRtlDevice::GetPermanentMacAddress()"] --> J3["Jaguar3 device"] --> H3["HalJaguar3"] --> F[("EFUSE/EEPROM")]
  subgraph Legend
    direction LR
    _app(["App/CLI"]) ~~~ _mod["Module/API"] ~~~ _hw[("Hardware/OTP")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use OS/netdev MAC (or wlx) instead of EFUSE access
  • ➕ No EFUSE decode complexity or chip-specific offsets
  • ➕ Works for any chipset once the interface exists
  • ➖ Requires OS integration and/or netdev presence (not purely device-library)
  • ➖ Can be affected by admin MAC randomization/spoofing or driver behavior
  • ➖ Harder to use in environments without udev/predictable names
2. Expose a higher-level 'AdapterId' (e.g., hash(chip + EFUSE MAC))
  • ➕ Single canonical identity format for consumers
  • ➕ Avoids leaking raw MAC in logs by default
  • ➖ Still depends on the same underlying EFUSE MAC correctness
  • ➖ Adds new format/versioning questions and reduces debuggability
  • ➖ Consumers that need the actual MAC still need another API

Recommendation: Keep the PR’s approach: exposing the EFUSE MAC via IRtlDevice provides a stable, per-unit identity without introducing OS dependencies, and it aligns with how the vendor driver derives the netdev MAC. The on-8822E capture (during the existing reliable-OTP window) and the on-8822C on-demand decode are pragmatic trade-offs; just ensure callers treat false as 'no stable identity' (as documented) and do not silently fall back to weaker identifiers.

Files changed (8) +168 / -16

Enhancement (7) +88 / -3
main.cppPrint EFUSE MAC identity in doctor report +15/-0

Print EFUSE MAC identity in doctor report

• Adds a new report line that attempts to read and print the permanent EFUSE MAC via IRtlDevice. Falls back to a clear 'unavailable' message when unsupported or unprogrammed, and runs even if bring-up failed to aid validation.

examples/doctor/main.cpp

IRtlDevice.hAdd GetPermanentMacAddress() to IRtlDevice with safe default +23/-0

Add GetPermanentMacAddress() to IRtlDevice with safe default

• Introduces a new virtual method to retrieve a per-unit EFUSE MAC as a stable identity key. The default implementation returns false to preserve behavior on unsupported chips and to force callers to handle missing identity explicitly.

src/IRtlDevice.h

RtlJaguarDevice.cppImplement permanent MAC identity for Jaguar1 via EepromManager +8/-0

Implement permanent MAC identity for Jaguar1 via EepromManager

• Implements IRtlDevice::GetPermanentMacAddress by routing to the existing EepromManager MAC accessor. Includes null checks to degrade gracefully when EEPROM state is unavailable.

src/jaguar1/RtlJaguarDevice.cpp

RtlJaguarDevice.hDeclare Jaguar1 GetPermanentMacAddress() override +4/-0

Declare Jaguar1 GetPermanentMacAddress() override

• Adds the method override declaration with documentation noting it is served from the already-read EEPROM map and uses existing upstream-derived offsets.

src/jaguar1/RtlJaguarDevice.h

HalJaguar3.hAdd HalJaguar3 perm_mac() API and storage for cached MAC +25/-3

Add HalJaguar3 perm_mac() API and storage for cached MAC

• Adds a HalJaguar3 entry point for retrieving the EFUSE MAC (with variant-specific behavior) and the associated state: offset constant, programmed-value check, cached bytes, and one-shot probe flag.

src/jaguar3/HalJaguar3.h

RtlJaguar3Device.cppExpose Jaguar3 permanent MAC identity under device register lock +10/-0

Expose Jaguar3 permanent MAC identity under device register lock

• Implements IRtlDevice::GetPermanentMacAddress by calling into HalJaguar3::perm_mac under the existing mutex. This serializes potential on-demand EFUSE reads with other register/EFUSE access paths (e.g., coex tick).

src/jaguar3/RtlJaguar3Device.cpp

RtlJaguar3Device.hDeclare Jaguar3 GetPermanentMacAddress() override +3/-0

Declare Jaguar3 GetPermanentMacAddress() override

• Adds the method override declaration and documents the split behavior: 8822E captured during init vs 8822C decoded on demand.

src/jaguar3/RtlJaguar3Device.h

Bug fix (1) +80 / -13
HalJaguar3.cppFix EFUSE logical-map walk and add Jaguar3 permanent MAC support +80/-13

Fix EFUSE logical-map walk and add Jaguar3 permanent MAC support

• Updates the EFUSE logical-map decoder to walk the full programmed area (sections are not ordered by base) and adds a bounds guard to prevent 10-bit address wraparound. Adds permanent-MAC extraction at logical offset 0x157: captured during 8822E init alongside the existing cache, and decoded on-demand once on 8822C with unprogrammed-value rejection.

src/jaguar3/HalJaguar3.cpp

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

qodo-free-for-open-source-projects Bot commented Aug 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Doctor MAC read can throw 🐞 Bug ☼ Reliability
Description
examples/doctor calls GetPermanentMacAddress() outside any try/catch, including when bring-up
fails; on Jaguar3/8822C this can perform EFUSE register I/O and throw, terminating the doctor
report. This makes the diagnostic tool less reliable precisely in failure scenarios it’s meant to
analyze.
Code

examples/doctor/main.cpp[R303-308]

+    if (dev->GetPermanentMacAddress(mac))
+      std::printf("efuse MAC:       %02x:%02x:%02x:%02x:%02x:%02x\n", mac[0],
+                  mac[1], mac[2], mac[3], mac[4], mac[5]);
+    else
+      std::printf("efuse MAC:       unavailable (unsupported chip, or "
+                  "unprogrammed)\n");
Evidence
The new doctor report block calls GetPermanentMacAddress() after bring-up handling, but without
any additional exception handling. On 8822C, permanent MAC retrieval can decode EFUSE and uses
register reads that throw in the USB transport on failure, so an exception can escape and terminate
the tool.

examples/doctor/main.cpp[246-255]
examples/doctor/main.cpp[292-309]
src/jaguar3/RtlJaguar3Device.cpp[2084-2092]
src/jaguar3/HalJaguar3.cpp[768-785]
src/jaguar3/HalJaguar3.cpp[646-662]
src/RtlAdapter.cpp[169-201]
src/UsbTransport.h[159-168]

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

## Issue description
`doctor` invokes `IRtlDevice::GetPermanentMacAddress()` without exception handling. For Jaguar3/8822C the implementation may touch hardware (EFUSE reads) which can throw on USB control-transfer failures, aborting `doctor` instead of producing a report.
### Issue Context
- `UsbTransport::ctrl_read()` throws `std::ios_base::failure` on a failed register read.
- The 8822C permanent MAC path can trigger EFUSE reads via `RtlAdapter::efuse_OneByteRead()`.
### Fix Focus Areas
- examples/doctor/main.cpp[292-309]
- examples/doctor/main.cpp[246-255]
### Suggested fix
Wrap the `GetPermanentMacAddress()` call in `try/catch (const std::exception&)` and print the existing "unavailable" line (optionally also print the exception message). Alternatively/additionally, in `RtlJaguar3Device::GetPermanentMacAddress`, return `false` when `_brought_up == false` to avoid attempting EFUSE I/O on an unpowered chip.

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


2. perm_mac latches after exception 🐞 Bug ☼ Reliability
Description
HalJaguar3::perm_mac sets _perm_mac_probed=true before performing the potentially-throwing EFUSE
decode; if a transient USB read exception occurs, subsequent calls will never retry and will return
false permanently for that device instance. This turns a recoverable transport glitch into a
persistent loss of adapter identity.
Code

src/jaguar3/HalJaguar3.cpp[R776-779]

+    _perm_mac_probed = true;
+    uint8_t map[kMacLogicalOff + 0x10] = {};
+    read_efuse_logical_map(map, sizeof(map));
+    memcpy(_perm_mac, map + kMacLogicalOff, sizeof(_perm_mac));
Evidence
The code sets _perm_mac_probed before the EFUSE map decode and does not reset it if an exception
occurs. The EFUSE decode reads through the USB transport, which throws on failed control reads, so
this state can be latched incorrectly after a transient failure.

src/jaguar3/HalJaguar3.cpp[768-785]
src/jaguar3/HalJaguar3.cpp[646-662]
src/RtlAdapter.cpp[169-201]
src/UsbTransport.h[159-168]

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

## Issue description
`HalJaguar3::perm_mac()` sets `_perm_mac_probed = true` before calling `read_efuse_logical_map()`. If the EFUSE walk throws (e.g., USB read failure), `_perm_mac_probed` remains true and the code will never retry, causing all future `perm_mac()` calls to return false.
### Issue Context
EFUSE walking uses `_device.efuse_OneByteRead()` which calls `rtw_read*/rtw_write*` on the transport; the USB transport throws on failed control reads.
### Fix Focus Areas
- src/jaguar3/HalJaguar3.cpp[768-785]
- src/UsbTransport.h[159-168]
### Suggested fix
Move `_perm_mac_probed = true` to after a successful `read_efuse_logical_map()` completes (or guard it with a small RAII rollback), and optionally catch `std::exception` to keep `_perm_mac_probed` false on failure so a later call can retry.

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


3. Unserialized EFUSE MAC read ✓ Resolved 🐞 Bug ☼ Reliability
Description
RtlJaguar3Device::GetPermanentMacAddress calls HalJaguar3::perm_mac without taking _reg_mu; on
RTL8822C this can trigger an on-demand EFUSE logical-map decode that performs register I/O. This
violates the device’s existing “serialize vs the coex tick” locking discipline and can cause
concurrent hardware access and racy initialization of _perm_mac/_perm_mac_valid if multiple threads
query the identity at once.
Code

src/jaguar3/RtlJaguar3Device.cpp[R2084-2086]

+bool RtlJaguar3Device::GetPermanentMacAddress(uint8_t out[6]) {
+  return _hal.perm_mac(out);
+}
Evidence
The new Jaguar3 accessor is not serialized on _reg_mu, while other EFUSE/register entry points
explicitly are; HalJaguar3::perm_mac can perform a fresh EFUSE decode on 8822C, which relies on
low-level EFUSE/register reads and therefore should follow the same serialization discipline to
avoid concurrent hardware access with the coex runtime tick.

src/jaguar3/RtlJaguar3Device.cpp[2082-2086]
src/jaguar3/RtlJaguar3Device.cpp[1598-1605]
src/jaguar3/RtlJaguar3Device.cpp[443-454]
src/jaguar3/HalJaguar3.cpp[747-761]
src/jaguar3/HalJaguar3.cpp[622-639]

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

## Issue description
`RtlJaguar3Device::GetPermanentMacAddress()` calls into `_hal.perm_mac()` without acquiring `_reg_mu`. On 8822C, `perm_mac()` may perform an EFUSE walk (`read_efuse_logical_map`) which uses EFUSE controller register operations; this should be serialized with the coex runtime thread and other register-touching entry points.
### Issue Context
- The coex runtime loop takes `_reg_mu` around HAL register/H2C work.
- EFUSE stability probing also takes `_reg_mu` before reading EFUSE.
- The new identity accessor introduces a new EFUSE read entry point that bypasses this established mutex.
### Fix Focus Areas
- src/jaguar3/RtlJaguar3Device.cpp[2082-2087]
- src/jaguar3/RtlJaguar3Device.cpp[1598-1605]
- src/jaguar3/RtlJaguar3Device.cpp[443-454]
- src/jaguar3/HalJaguar3.cpp[747-761]
### Suggested fix
- In `RtlJaguar3Device::GetPermanentMacAddress`, wrap the `_hal.perm_mac(out)` call with `std::lock_guard<std::mutex> lk(_reg_mu);` (matching `ProbeEfuseStability` and other register-touching entry points).
- (Optional) If you want to minimize lock hold time, you can lock only when `_variant == C8822C` (the only path that does on-demand EFUSE reads), but simplest/most consistent is to always lock.

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



Informational

4. Unqualified memcpy portability 🐞 Bug ⚙ Maintainability
Description
New code in HalJaguar3.cpp includes ` but calls memcpy without std::` qualification, which can
fail to compile on stricter standard library configurations that only expose std::memcpy. This is
inconsistent with other files that use std::memcpy.
Code

src/jaguar3/HalJaguar3.cpp[R756-759]

+  memcpy(_efuse_cache, map, sizeof(_efuse_cache));
_efuse_cache_valid = true;
+  memcpy(_perm_mac, map + kMacLogicalOff, sizeof(_perm_mac));
+  _perm_mac_valid = mac_programmed(_perm_mac);
Evidence
The new additions in HalJaguar3.cpp use memcpy unqualified, while the file includes ``.
Elsewhere in the repository std::memcpy is used, indicating the intended style and avoiding
toolchain differences.

src/jaguar3/HalJaguar3.cpp[1-4]
src/jaguar3/HalJaguar3.cpp[754-760]
src/jaguar3/RtlJaguar3Device.cpp[2076-2079]

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

## Issue description
`HalJaguar3.cpp` includes `<cstring>` but uses unqualified `memcpy(...)`. In standard C++, `<cstring>` guarantees `std::memcpy`; a global `::memcpy` may not be declared on all toolchains.
### Issue Context
Other code in the repo already uses `std::memcpy` (e.g., `RtlJaguar3Device.cpp`).
### Fix Focus Areas
- src/jaguar3/HalJaguar3.cpp[754-785]
### Suggested fix
Replace `memcpy(...)` with `std::memcpy(...)` (or include `<string.h>` if global C names are the project’s convention, but consistency suggests `std::memcpy`).

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


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/jaguar3/RtlJaguar3Device.cpp
@snokvist

snokvist commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the 8822C gap in the table above is now root-caused and fixed in #384, and with that fix this PR's MAC read works on 8822C too.

The cause was not this change: read_efuse_logical_map stopped walking as soon as a section's base passed the requested byte, which assumes sections appear in ascending base order. On the measured RTL8822CU the third section jumps to base 0x100, so the walk ended after three sections and returned a map that was 0xFF almost everywhere — which is also why read_efuse_rfe_type() returned 0 there while the kernel reads 0x03.

With #384 applied, all three implemented paths verify against the kernel's netdev MAC:

adapter path kernel devourer
RTL8812AU (Jaguar1) EepromManager, 0xD7 20:0d:b0:c4:a7:6a 20:0d:b0:c4:a7:6a match
RTL8822EU (Jaguar3, C8822E) captured in rtw_hal_init 84:fc:14:50:bc:de 84:fc:14:50:bc:de match
RTL8822CU (Jaguar3, C8822C) on-demand decode 40:a5:ef:2f:23:08 40:a5:ef:2f:23:08 match

The two PRs are independent — #384 is a standalone bug fix (it affects RFE/BB config for every 8822C user regardless of this feature), and this one no longer has a known unverified path once it lands. Happy to reorder or squash them if you'd prefer them as one change.

@snokvist

snokvist commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Review finding addressed — good catch.

RtlJaguar3Device::GetPermanentMacAddress now takes _reg_mu before calling into the HAL, matching the discipline ProbeEfuseStability and the other register/EFUSE entry points already follow. On 8822C perm_mac can run a fresh on-demand map decode, which is real register I/O and would otherwise race the coex tick; on 8822E it is a cached lookup so the lock is uncontended. It also makes the lazy fill of _perm_mac/_perm_mac_valid single-writer.

Jaguar1's override is left unlocked deliberately: it reads out of the EEPROM map that EepromManager already parsed at bring-up, so it touches no hardware.

Re-verified on hardware after the change — all three paths unchanged:

adapter kernel devourer
RTL8812AU (Jaguar1) 20:0d:b0:c4:a7:6a 20:0d:b0:c4:a7:6a match
RTL8822EU (Jaguar3, C8822E) 84:fc:14:50:bc:de 84:fc:14:50:bc:de match
RTL8822CU (Jaguar3, C8822C) 40:a5:ef:2f:23:08 40:a5:ef:2f:23:08 match

(The 8822C row needs #384 to decode its map at all.)

josephnef
josephnef previously approved these changes Aug 6, 2026

@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.

Reviewed with independent hardware verification on a second rig (RTL8814AU, RTL8822EU, RTL8822CU, RTL8822BU). Approving — the motivation is sound (confirmed the placeholder "123456" serial on this rig's units too), the code is careful, and the _reg_mu serialization commit was exactly right.

Independent verification

Build clean, 49/49 ctest. doctor run twice per adapter, requiring run-to-run stability:

adapter path result
RTL8814AU (Jaguar1) EepromManager 20:0d:b0:c7:e4:b3, stable
RTL8822EU (Jaguar3-E) init capture 98:03:cf:cf:a4:49, stable
RTL8822BU (Jaguar2) default graceful unavailable
RTL8822CU (Jaguar3-C) on-demand decode unavailablereproduces your failure, 2/2 units

The 8822C limitation: your deferral was right, the diagnosis isn't

I tested the suspected mechanism (the hdr == 0xFF early-break on a padded map): applying EU-style 0xFF-run tolerance to the non-EU walk changes nothing on this unit.

A physical dump of the OTP shows the real cause. The map is fully programmed (the MAC words are there, at phys 0xd2+), but the sections are append-ordered, not logical-ordered — a logical-0x100+ section sits at phys 0x12. The other termination is what kills the walk:

if (base > upto + 8)
  break; /* past the byte we need */

Any upto ≤ 0xFA walk bails after ~3 blocks. With that exit removed (walk to the 64-byte-0xFF-run end, like the EU branch), the same unit decodes MAC a8:b5:8e:6a:94:ea and rfe_type flips 0x00 → 0x03 — so this pre-existing bug is mis-selecting PHY tables on append-ordered 8822C units today, and exposes the TX-power-base walk to the same truncation. Agreed it deserves its own change and on-air validation; filed as a follow-up issue with the dump and experiment.

Minor, non-blocking

  • doctor: the MAC line prints mid-probe (above the RX smoke), detached from the == adapter doctor == report block, and is skipped entirely when bring-up fails — consider moving it into the report section.
  • perm_mac on 8822C: a failed decode is retried with a full OTP walk (real register I/O under _reg_mu) on every call; a "probed" flag would cap it at one attempt.
  • The "serial is 123456" rationale is written out three times (IRtlDevice.h, HalJaguar3.h, PR body); the interface doc-comment could be the single home, per the no-duplication rule.
  • Worth a nod that Jaguar2 (HalMAC has efuse APIs) and Kestrel (EFUSE_USB_MAC_ADDR_8852B already in-tree) are expected follow-ups rather than permanent gaps.

@snokvist

snokvist commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the second-rig verification, and for pushing the review fixes yourself (b39300d) — nothing is outstanding from that review on my side. The approval was auto-dismissed by that very commit, so this is a re-request rather than a new round.

Two follow-ups.

The 0x157 caveat is resolved — it is the vendor constant

The PR body flagged that 0x157 was measured rather than read from a datasheet, and asked for a hal_pg.h cross-check before merge. Done, and it lands exactly on the value:

include/hal_pg.h (rtl88x2cu 20230728 and rtl88x2eu 20230815, identical)
  #define EEPROM_MAC_ADDR_8822CU   0x157
  #define EEPROM_MAC_ADDR_8822EU   0x157

The caveat's worry was right in kind — the variants do differ (…CS/ES 0x16A, …CE/EE 0x120) — but devourer is USB-only, so 0x157 is correct for both Jaguar3 parts, and it is one constant rather than two because the vendor uses the same offset for both.

The same header confirms the Jaguar1 offsets EepromManager already applies (8812AU 0xD7, 8814AU 0xD8, 8821AU 0x107), and hands the Jaguar2 follow-up its constant for free: 8822BU and 8821CU are both 0x107 — so your 8822BU's graceful unavailable has a known one-line answer whenever someone wants it. (Kestrel's is already in-tree as EFUSE_USB_MAC_ADDR_8852B.)

I've updated the PR body: the caveat is marked resolved with the constants, the stale 8822C table row now reads 40:a5:ef:2f:23:08 match (with #384), and the paragraph that guessed at the hdr == 0xFF padding termination is replaced — your control experiment showed that guess was wrong, and the body shouldn't keep a wrong diagnosis where someone will find it later. That mis-numbered 0xCA = 0x15 in the body is now 0x03 as well.

This PR and #384 conflict — in both directions

GitHub reports both as mergeable because each is only tested against master. This PR calls read_efuse_logical_map(map, sizeof(map), kMacLogicalOff + 6) in cache_efuse_8822e and perm_mac; #384 removes that parameter. Whichever merges second needs the third argument dropped at those two call sites — 2 tokens, nothing semantic: with upto gone, the 8822C on-demand walk just becomes the full walk it always wanted, which is why the MAC decodes there at all.

I merged both onto master locally with that resolution: build clean, 49/49 ctest. Merge in whichever order you prefer and I'll push the fixup to the trailing PR right away — or if you'd rather have it conflict-free up front, I'll rebase this branch onto #384.

@snokvist
snokvist force-pushed the feat/permanent-mac-identity branch from b39300d to 39ff28d Compare August 6, 2026 18:46
@snokvist

snokvist commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto #384 so this lands conflict-free — b39300d939ff28d.

The only conflict was the two read_efuse_logical_map() call sites (cache_efuse_8822e and perm_mac); the third argument is dropped at both, nothing else changed. The resulting tree is byte-identical to the merge resolution I verified earlier, and your review-fixes commit keeps its authorship. Build clean, 49/49 ctest.

Reading the PR now: it shows six commits, the first three being #384's. Only the last three are this PR — feat: expose the EFUSE MAC…, fix: serialize…, review fixes: …. GitHub drops the borrowed ones automatically once #384 is on master.

One behaviour change worth a re-run on your rig. With the walk fixed underneath it, the 8822CU no longer degrades to unavailable — it should now print a8:b5:8e:6a:94:ea, the value your own #385 experiment produced on that unit, which is also the cross-check that the offset is right on a third adapter. Everything else is unchanged from what you verified: the Jaguar1 and 8822E paths don't touch the modified walk, and 8822BU still takes the Jaguar2 default.

On the red CI on the previous head — that was infrastructure, not the code. Every failing job died in Set up job with Failed to resolve action download info. Error: Service Unavailable (run 31118635032, 16:44Z); the macOS jobs that did get runners passed. The rebase re-triggers the matrix.

@josephnef josephnef closed this Aug 7, 2026
@josephnef josephnef reopened this Aug 7, 2026
Comment thread examples/doctor/main.cpp
Comment thread src/jaguar3/HalJaguar3.cpp Outdated
Comment thread src/jaguar3/HalJaguar3.cpp Outdated
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 39ff28d

josephnef pushed a commit that referenced this pull request Aug 7, 2026
…ad rfe_type=0) (#384)

## The bug

`HalJaguar3::read_efuse_logical_map` stopped walking as soon as a
section's
logical base passed the byte the caller asked for:

```c
if (base > upto + 8)
  break; /* past the byte we need */
```

That is only valid if sections appear in **ascending base order**. They
do not.

Physical EFUSE dumped off an RTL8822CU (`0bda:c812`), decoded by hand:

```
phys 0x00  hdr=0x00         -> base 0x000   ok
phys 0x09  hdr=0x10         -> base 0x008   ok
phys 0x12  hdr=0x0F ext=48  -> base 0x100   <- early exit fires here
phys 0x2C  hdr=0x4F ext=5D  -> base 0x150   never reached
phys 0xD5  hdr=0x4F ext=4E  -> base 0x110   never reached
phys 0xDA  hdr=0x4F ext=5E  -> base 0x150   never reached
```

The third section on the chip jumps to base 0x100, so **any** request
below that
— including `EEPROM_RFE_OPTION_8822C` at logical 0xCA, which is the
whole reason
`read_efuse_rfe_type()` calls this — ended the walk after three sections
and
returned a map that was 0xFF almost everywhere.

## Why it matters

On the affected adapter `read_efuse_rfe_type()` returned **0**, while
the vendor
kernel driver reads **0x03** from the same chip
(`/proc/net/rtl88x2cu/<iface>/efuse_map`, logical 0xCA). The RFE type
gates BB /
RFE configuration, so those units were being brought up against an
unprogrammed
default rather than their actual front-end.

It is silent: nothing errors, the map just reads unprogrammed.

## The fix

Walk the whole programmed area (the existing 0xFF-header terminator and
`kPhysMax` bound already stop it). The `upto` parameter is removed
rather than
left unused — a parameter that still *looks* like it bounds the walk is
how this
comes back.

The 8822E branch is untouched: it never used `upto`, terminating on a
long 0xFF
run instead, which is why only the C path was affected.

## Hardware verification

| adapter | before | after | kernel (`efuse_map` 0xCA) |
|---|---|---|---|
| RTL8822CU (`0bda:c812`, C8822C) | `rfe_type=0x00` | `rfe_type=0x03` |
**0x03** |
| RTL8822EU (`0bda:a81a`, C8822E) | `rfe_type=0x15` | `rfe_type=0x15` |
**0x15** |

The EU is the regression check — unchanged, and its `efuse decoded
(0x22=46
0x4c=51 0xca=15)` line is identical before and after. The 8822C EFUSE
stability
probe also now reports a valid `0x8129` EEPROM ID.

Found while implementing #383 (EFUSE MAC as a per-unit identity), which
could not
read the MAC on 8822C for this reason. With this fix that adapter's MAC
decodes
correctly — `40:a5:ef:2f:23:08`, matching its netdev exactly. The two
changes are
independent; this one stands on its own regardless of what happens to
#383.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
snokvist and others added 4 commits August 7, 2026 07:39
A consumer that keeps per-adapter state — a measured TX-power curve, a
calibration, anything tied to one specific dongle — has to answer "is this the
same physical adapter I measured last time?" across a re-plug, a reboot and a
port change. devourer offers no key that can: a USB bus path identifies a port,
not a device, and the USB serial descriptor is not unique.

That second point is worth stating precisely, because it looks like the obvious
answer. Dumped through the vendor kernel driver, both values live in the same
EFUSE a few bytes apart:

  0x157  the 6-byte MAC          per-unit
  0x174  USB serial descriptor   the constant "123456" on every unit measured

The MAC is also where Linux gets it: the vendor driver programs it into the
netdev, and udev derives the stable `wlx<mac>` name from that.

Adds IRtlDevice::GetPermanentMacAddress, defaulting to false so unimplemented
chips degrade gracefully and no existing consumer changes behaviour.

  Jaguar1  — routes to the existing EepromManager::GetMacAddress. The read, the
             per-chip offsets (hal_pg.h) and the unprogrammed-value rejection
             were all already there; only a route to a caller was missing.
  Jaguar3  — logical offset 0x157. On 8822E the value is captured during the
             existing rtw_hal_init efuse pass, because that OTP is not reliably
             readable after TX/coex bring-up — the same constraint _efuse_cache
             exists for. One walk decodes far enough for both, and _efuse_cache
             keeps its size so the health probe's compare surface is unchanged.
             On 8822C the map is decoded on demand.

doctor prints the value, which is also how to check the offset on a chip nobody
has measured: compare it against the `wlx<mac>` name the vendor driver gives
the same dongle.
…dedup

- doctor: the efuse-MAC line moves into the report block (and is attempted
  even after a failed bring-up, where it degrades to the unavailable line).
- 8822C: a failed on-demand decode is no longer retried on every call —
  the walk is real register I/O under the device lock and an unprogrammed
  EFUSE stays unprogrammed, so one attempt is kept, positive or negative.
- The identity rationale lives once, on the interface declaration; the HAL
  comment points there instead of restating it.
- The interface doc names Jaguar2/Kestrel as expected follow-ups rather
  than permanent gaps.

Hardware re-verified (doctor, two stable runs each): 8814AU + 8822EU show
their programmed MACs in the report; 8822BU and 8822CU show the
unavailable line (the 8822C decode gap is issue OpenIPC#385).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 8822C on-demand decode is thousands of control-IN reads, any of which
can throw std::ios_base::failure on a USB glitch — most likely exactly when
a caller probes identity on a dead or unpowered adapter (doctor does, after
a failed bring-up). GetPermanentMacAddress now catches at the device layer
and returns the contract's false; the one-attempt latch is already set by
then, so a glitched walk is not silently retried either. memcpy sites take
the std:: qualification the subtree already uses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@josephnef
josephnef force-pushed the feat/permanent-mac-identity branch from 175a489 to bc28f22 Compare August 7, 2026 04:40
@josephnef
josephnef merged commit 8bd594c into OpenIPC:master Aug 7, 2026
20 checks passed
josephnef added a commit that referenced this pull request Aug 7, 2026
…very generation (#386)

## What this adds

#383 shipped `IRtlDevice::GetPermanentMacAddress` on Jaguar1 and Jaguar3
and left Jaguar2 and Kestrel on the graceful default. This wires the
remaining two generations — the identity is now every-generation.

**Jaguar2** — logical EFUSE `0x107`, one offset for both dies
(`hal_pg.h`: `EEPROM_MAC_ADDR_8822BU == EEPROM_MAC_ADDR_8821CU`). Served
from the logical map `HalJaguar2` already caches for RFE/TX-power, so
post-bring-up it is a lookup; a pre-init call triggers the existing lazy
walk. The device entry point serializes on `_reg_mu` and folds a
USB-glitch throw into the contract's `false`, matching the Jaguar3
shape.

**Kestrel** — a route, not a new read: the bring-up efuse parse already
extracts the MAC at logical `0x488` and `autoload_ok` is exactly the
programmed-value check. One constant serves both dies **by vendor
dispatch**: mac_ax's USB efuse-info table has no 8852C entry and falls
back to the 8852B offsets (`reference/rtl8852cu` `mac_ax/efuse.c`, the
`else info = efuse_info_usb_8852b` arm).

The `IRtlDevice.h` doc drops the "expected follow-ups" paragraph —
current state only; the default stays `false` so a future generation
degrades gracefully.

## Hardware verification

Two stable `doctor` runs per adapter; the Jaguar2 values are
cross-checked against the **vendor kernel driver** built from
`reference/rtl88x2bu` on the same host:

| adapter | path | devourer | vendor driver |
|---|---|---|---|
| RTL8822BU | Jaguar2, 0x107 | `40:a5:ef:57:37:0c` | **match** |
| Archer T3U (8822BU) | Jaguar2, 0x107 | `8c:86:dd:48:00:9d` | **match**
|
| TP-Link TX50UH (8852C) | Kestrel, 0x488 | `cc:ba:bd:61:57:6b` | see
below |
| RTL8814AU / RTL8822CU | regression | unchanged | — |

The TX50UH has no same-host vendor-driver run; its offset stands on the
vendor-source dispatch above plus the fact that the same parse already
feeds the on-air-working rfe/xtal/thermal fields. Its USB iSerial is the
Realtek placeholder `00e04c000001` — the constant-serial premise that
motivated #383, re-confirmed on AX silicon.

The 8821C variant (RTL8811CU/8821CU/8821CE) shares the Jaguar2 path and
constant but no unit was on the rig for this run.

Build clean, 49/49 ctest.

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

---------

Co-authored-by: Claude Opus 4.8 <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