Expose the EFUSE MAC as a per-unit adapter identity - #383
Conversation
PR Summary by QodoExpose EFUSE MAC as stable per-adapter identity
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1.
|
|
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: With #384 applied, all three implemented paths verify against the kernel's netdev MAC:
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. |
|
Review finding addressed — good catch.
Jaguar1's override is left unlocked deliberately: it reads out of the EEPROM map that Re-verified on hardware after the change — all three paths unchanged:
(The 8822C row needs #384 to decode its map at all.) |
josephnef
left a comment
There was a problem hiding this comment.
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 | unavailable — reproduces 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_macon 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_8852Balready in-tree) are expected follow-ups rather than permanent gaps.
- The comment said the kernel reads 0x15 from the affected 8822CU. 0x15 is the 8822EU's value (the regression-check adapter); the 8822CU reads 0x03, as both the PR's hardware table and the fix's before/after show. - The block documents read_efuse_logical_map but sat above probe_efuse_map (pre-existing); moved onto the function it describes. - Present-tense: the invariant is that sections are not in ascending base order, with the measured phys dump as its evidence. Git carries the "it used to" part. Comment-only. Build clean, 49/49 ctest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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
|
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>
b39300d to
39ff28d
Compare
|
Rebased onto #384 so this lands conflict-free — The only conflict was the two Reading the PR now: it shows six commits, the first three being #384's. Only the last three are this PR — One behaviour change worth a re-run on your rig. With the walk fixed underneath it, the 8822CU no longer degrades to On the red CI on the previous head — that was infrastructure, not the code. Every failing job died in |
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:
1-1,3-1.2) identifies a port, not a device. It changeswhenever the dongle moves, orphaning the state.
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:RTL8822CU — netdev
wlx40a5ef2f2308:"Realtek"/"802.11ac NIC"string descriptors"123456"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 isstable across re-plug because it comes from the chip.
What this adds
IRtlDevice::GetPermanentMacAddress(uint8_t out[6]), defaulting tofalsesounimplemented chips degrade gracefully and no existing consumer changes
behaviour.
EepromManager::GetMacAddress. The read,the per-chip offsets (from
hal_pg.h: 8812AU0xD7, 8814AU0xD8, 8821AU0x107) and the unprogrammed-value rejection were all already implemented;only a route to a caller was missing.
0x157. On 8822E the value is captured during theexisting
rtw_hal_initefuse pass, because that OTP is not reliably readableafter TX/coex bring-up — the same constraint
_efuse_cacheexists for. Onewalk decodes far enough for both, and
_efuse_cachekeeps its size soprobe_efuse_map's compare surface is unchanged. On 8822C the map isdecoded on demand.
has measured: compare against the
wlx<mac>name the vendor driver gives thesame dongle.
Jaguar2 and Kestrel keep the default
false. Kestrel's offset constant alreadyexists in-tree (
EFUSE_USB_MAC_ADDR_8852B = 0x488,kestrel/MacRegAx.h:179) ifsomeone with the hardware wants to finish it.
Hardware verification
Three adapters, all three implemented code paths,
doctoroutput vs the kernel'sown netdev MAC:
EepromManager, offset 0xD720:0d:b0:c4:a7:6a20:0d:b0:c4:a7:6artw_hal_init84:fc:14:50:bc:de84:fc:14:50:bc:de40:a5:ef:2f:23:0840:a5:ef:2f:23:08The 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_mapreturned an empty logical map, sothe MAC read found
0xFF— independently visible in the same run asJaguar3: rfe_type=0x00while the kernel reads logical0xCA = 0x03from the sameadapter. 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
Resolved — it is the0x157is measured, not read from a datasheet.vendor constant.
include/hal_pg.h(rtl88x2cu 20230728 / rtl88x2eu 20230815):EEPROM_MAC_ADDR_8822CU 0x157,EEPROM_MAC_ADDR_8822EU 0x157. The caveat's worrywas right in kind — the other variants do differ (
…CS/ES 0x16A,…CE/EE 0x120)— but devourer is USB-only, so
0x157is correct for both Jaguar3 parts. TheJaguar1 offsets served by
EepromManagercheck out against the same header(8812AU
0xD7, 8814AU0xD8, 8821AU0x107), and the Jaguar2 follow-up alreadyhas its constant there too (8822BU and 8821CU are both
0x107).identifier; consumers logging it should treat it as they treat any other
adapter identity.
Build is clean; no new warnings.