Skip to content

Install BCM43602 NVRAM so 2017 Touch Bar Macs see 5 GHz Wi-Fi - #7671

Open
shawnyeager wants to merge 6 commits into
omacom:quattrofrom
shawnyeager:apple-bcm43602-5ghz
Open

Install BCM43602 NVRAM so 2017 Touch Bar Macs see 5 GHz Wi-Fi#7671
shawnyeager wants to merge 6 commits into
omacom:quattrofrom
shawnyeager:apple-bcm43602-5ghz

Conversation

@shawnyeager

@shawnyeager shawnyeager commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Refs #7672

This covers MacBookPro14,2 and MacBookPro14,3 only, the two models where this dump has field reports. #7672 stays open to track the other BCM43602 boards reported there (MacBookPro12,1, MacBookPro13,3), which need their own calibration.

Why

2017 Touch Bar MacBook Pros (MacBookPro14,2 and MacBookPro14,3) with Broadcom BCM43602 (14e4:43ba) get 2.4 GHz Wi-Fi out of the box under brcmfmac, but never see 5 GHz networks. linux-firmware-broadcom ships the chip firmware and no board calibration file, so the driver loads with placeholder 5 GHz values (aa5g=1, no per-channel tx-power tables).

Verified end-to-end on a MacBookPro14,3 after reboot:

Before After
Frequency 2437 MHz (ch 6) 5745 MHz (ch 149)
Signal -66 dBm -40 dBm
RX / TX 104 / 144 Mbit/s 867 / 650 Mbit/s

The 5 GHz band is advertised (iw phy Band 2) and dual-band APs show both frequencies in nmcli.

Field recipe this follows: csk-grit42/OmarchyOnMacBookPro14.2-2017---A1706- HOWTO-wifi.md. Credit to @csk-grit42 for documenting the BCM43602 NVRAM + regulatory-domain path that unlocked 5 GHz on the 2017 13" Touch Bar MacBook Pro. The NVRAM is the community BCM43602 dump from kernel.org bugzilla attachment 290569 (ccode=00 / regrev=245 defers channel legality to the host). Not vendor-certified; treat as known-good for this chip on Linux.

macaddr= is required

The dump ships macaddr=00:90:4c:0d:f4:3e (Broadcom OUI, not a device identity). An earlier revision of this PR stripped that line when the 43ba wiphy had no real permanent address, on the theory that the firmware would then use OTP.

On MacBookPro14,3 it does not. After reboot, brcmfmac loaded the NVRAM, the ARM firmware started, then:

brcmf_c_preinit_dcmds: Retrieving cur_etheraddr failed, -5
brcmf_bus_started: failed: -5
brcmf_pcie_setup: Dongle setup failed
brcmf_fw_crashed: Firmware has halted or crashed

No wlp3s0. A module reload reproduced the same crash. Restoring the macaddr= key brought the interface up immediately.

Running with no NVRAM file at all (stock 2.4 GHz) is a different path from handing the firmware a board file with the key missing. This chip requires the key in any NVRAM it is given. OTP does not answer here.

The helper still substitutes a real permanent address from the 43ba wiphy when one exists. When it does not — including when the wiphy only reports 00:90:4c:* — the source macaddr= line stays. It is never stripped.

What

  • install/hardware/apple/brcmfmac-43602.sh — Apple vendor + MacBookPro14,2/14,3 + PCI 14e4:43ba only. Leaves other BCM43602 boards, 2 GHz-only 43bb, T2-era chips (apple-bcm-firmware), and non-Apple Broadcom parts alone.
  • install/hardware/apple/fix-brcmfmac-5ghz.sh copies the calibrated NVRAM to /usr/lib/firmware/updates/brcm/ under both the generic name and the DMI-specific name brcmfmac actually requests. Both names are staged under temporary names and renamed into place together.
  • Migration 1787312531 does the same for existing installs and sets reboot-required. Does not reload brcmfmac (that would drop the link carrying the update).
  • Does not hard-code a cfg80211 country. ccode=00/regrev=245 defers to the host, and Omarchy already persists WIRELESS_REGDOM from the timezone.
  • Files go under firmware/updates so they override, and do not collide with, linux-firmware-broadcom.

Test plan

  • ./test/cli
  • test/shell.d/brcmfmac-5ghz-test.sh
  • MacBookPro14,3: reboot after NVRAM install → associated on 5745 MHz at 802.11ac rates
  • MacBookPro14,3: NVRAM with macaddr= stripped → dongle crash, no wlp3s0; key restored → interface comes up

linux-firmware-broadcom ships the chip firmware but no board calibration,
so brcmfmac only advertises 2.4 GHz. Install a calibrated NVRAM under
firmware/updates on Apple machines with PCI 14e4:43ba, and migrate
existing installs.
Copilot AI balanced review requested due to automatic review settings August 21, 2026 11:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds BCM43602 NVRAM calibration to restore 5 GHz Wi-Fi on affected Intel Macs.

Changes:

  • Detects eligible Apple BCM43602 hardware and installs generic/DMI-specific NVRAM.
  • Migrates existing installations and requests a reboot.
  • Adds documentation and shell coverage.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
install/hardware/apple/brcmfmac-43602.sh Implements detection and NVRAM installation.
install/hardware/apple/brcmfmac43602-pcie.txt Adds calibrated BCM43602 NVRAM data.
install/hardware/apple/fix-brcmfmac-5ghz.sh Adds the installation leaf.
install/hardware/all.sh Runs the new hardware fix.
migrations/1787312531.sh Applies the fix to existing installations.
manual/44-mac-support.md Documents BCM43602 5 GHz support.
test/shell.d/brcmfmac-5ghz-test.sh Tests detection, installation, MAC handling, and migration.
test/shell.d/unowned-system-paths-test.sh Allows the firmware override destination.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread install/hardware/apple/brcmfmac-43602.sh Outdated
Comment thread install/hardware/apple/brcmfmac-43602.sh Outdated
Comment thread install/hardware/apple/brcmfmac-43602.sh
Comment thread install/hardware/apple/brcmfmac-43602.sh Outdated
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed here, with an independent second opinion from codex at xhigh reasoning, and the suites run on a disposable VM. Nothing pushed to your branch: the findings below are either a maintainer's call or a change to behaviour your tests assert on purpose, and neither is mine to make quietly.

What actually ran. ./test/cli — 116 ok, exit 0. test/shell.d/brcmfmac-5ghz-test.sh — 19 ok, 0 failed, exit 0. test/shell.d/unowned-system-paths-test.sh — 1 ok, exit 0. All on a worker VM with no BCM43602 in it, so the suite exercised your stubs and none of it confirms the 5 GHz behaviour. The only evidence for that is your MacBookPro14,3, which I read but could not reproduce. That matters more than usual here, because most of what follows is about the machines that are not yours.

1. The gate is chip-wide; the calibration is board-specific. brcmfmac43602_needed (install/hardware/apple/brcmfmac-43602.sh:44-49) is sys_vendor == Apple* plus lspci showing 14e4:43ba. It never reads product_namebrcmfmac43602_dmi_product exists only to build a filename (:59-66). So every Apple machine with a BCM43602 gets this file, while the file describes exactly one board: boardtype=0x61b, boardrev=0x1421, boardnum=62526, plus swctrlmap_2g/swctrlmap_5g (the RF switch control map, which is wiring), per-chain antenna gains (agbg*=133, aga*=71) and the per-channel tssi_* / gain_index_* power tables. On a different board that is not "no calibration replaced by right calibration", it is wrong calibration — and wrong switch and power tables are exactly how a card ends up worse than the 2.4-GHz-only state it started in.

That is not hypothetical on two counts. Your own test asserts a 2015 MacBookPro11,4 gets the file (test/shell.d/brcmfmac-5ghz-test.sh:135-138), and codex found that Apple's 43ba machines carry different subsystem IDs per model — 0152 on the 2015 MacBook Pros, 0157 on MacBookPro13,2, 014a on iMac17,1, 016e on iMac18,2 — with the iMacs using the desktop BCM943602CDP module rather than the laptop one. More directly, codex turned up an Arch forum thread from a MacBookPro13,3 owner who loaded this same bugzilla NVRAM and reported "the signal is horrible", unable to hold a link more than about a metre from the router. I fetched that thread and confirmed it says what codex reported (bbs.archlinux.org/viewtopic.php?id=313054). MacBookPro13,3 is inside your gate.

Worth connecting: the kernel log in #7672 quotes no clm_blob available (err=-2) and no txcap_blob available alongside the missing NVRAM. This PR ships the NVRAM only, so those two remain missing — and that forum poster attributes his residual degradation to exactly them. It works on your machine; the mechanism for why it might not work on a neighbouring model is right there in the log you filed.

Gating brcmfmac43602_needed on the product_name values you have actually seen work would bound this to the machines the title claims.

2. With no wireless interface visible, the donor's MAC ships as the machine's MAC. brcmfmac-43602.sh:118-123 substitutes the live address only inside if mac=$(brcmfmac43602_wifi_mac). Otherwise macaddr=00:90:4c:0d:f4:3e is what lands, and brcmfmac-5ghz-test.sh:154-159 asserts that on purpose. An NVRAM macaddr= can override the card's programmed address rather than deferring to it, so any two machines taking that path can come up sharing a station address.

Codex added the part that makes this sharp, and my own check of this machine's linux-firmware-broadcom corroborates it: the kernel carries a duplicate-MAC mitigation that randomises exactly one address, the 00:90:4c:c5:12:38 template — which is the MAC in 69 of the 70 board files that package ships. Your dump uses a different placeholder, so it falls outside the guard the kernel wrote for precisely this problem. The gist this NVRAM circulates in also tells readers to "populate macaddr=xx:xx:xx:xx:xx:xx in the file with your Mac's real mac address" — I fetched it and confirmed the wording.

Deleting the macaddr= line when no MAC is found leaves the card on its OTP address, the one it already uses today with no NVRAM at all, and is strictly safer than shipping a stranger's. Related and smaller: brcmfmac43602_wifi_mac (:78-94) takes the first $netdir/*/wireless in glob order rather than the interface bound to 14e4:43ba, so a USB Wi-Fi adapter plugged in at install time can donate its address to the Broadcom card.

3. The completeness check clobbers, shadows, and never refreshes. brcmfmac43602_file_complete (:51-57) accepts a file only if it contains aa5g=7, txchain=7 and ccode=00. Three consequences. A user who followed one of the circulating recipes and pinned a country (ccode=US) fails the check and has their file silently overwritten — and because completion checks both destinations while apply always writes both (:68-76 vs :112-116), one stale file replaces both. Because brcmfmac43602_fwdir (:25-27) is only the override directory, nothing ever looks at /usr/lib/firmware/brcm: the kernel searches /lib/firmware/updates ahead of /lib/firmware, so the day linux-firmware ships a real brcmfmac43602-pcie.Apple Inc.-<model>.txt, this copy shadows it indefinitely. And since completeness is three marker lines rather than a content or checksum comparison, a corrected NVRAM shipped in a later Omarchy release never reaches a machine that already has the current one. Nothing in the repo removes these files, no omarchy-* command mentions them, and manual/44-mac-support.md:38 is a parenthetical — so a user whose Wi-Fi got worse has nothing to find and nothing to undo.

4. A failed install reports success, and the migration then marks itself done. brcmfmac43602_apply is called as an if condition in both callers (fix-brcmfmac-5ghz.sh:13, migrations/1787312531.sh:9), and bash disables errexit for the whole body of a function invoked that way — I probed the semantics rather than assuming them, and codex reached the same conclusion separately. So if sudo, mkdir, install or sed fails at :112-121, execution runs on to return 0 at :125; the migration sets reboot-required and exits 0, and omarchy-migrate (bin/omarchy-migrate:93-95) writes the completion marker because the script succeeded. The runner's own set -e retry safety net is defeated by the swallowed failure. The user reboots, still has no 5 GHz, and the migration will never run again. || return 1 on each mutating command closes it.

5. The clobber assertion is vacuous, and I proved it. brcmfmac-5ghz-test.sh:181-186 writes a placeholder NVRAM and then calls run_leaf, but run_leaf opens with rm -rf "$fwdir" "$netdir" (:95-98), so the placeholder is deleted before the leaf ever runs. The assertion demonstrates installing into an empty directory, not replacing anything. Codex spotted it; I confirmed it by mutation on the worker — patching brcmfmac43602_apply to refuse outright to overwrite an existing file still gives 19 passed, 0 failed. Nothing in the suite exercises overwrite, which is why findings 3 and 4 could sit under a green run. (Reverted; your branch is untouched.)

Two smaller test notes. The leaf test invokes with bash -eE -o pipefail (:84) while production run_logged uses bash -eE with no pipefail (install/helpers/logging.sh:54) — stricter than the real thing, not a reproduction of it. And the calibration assertions (:20-27) grep for constants this same PR supplies, so they prove the file's contents, not that those values suit any given model.

6. Provenance and licence. brcmfmac-43602.sh:9-12 names bugzilla attachment 290569; the data file itself starts straight into data with no header, and brcmfmac's parser ignores # lines, so one costs nothing. The licence is the part that needs a decision rather than an edit: no redistribution grant for this dump appears anywhere in the diff, Omarchy's MIT licence covers its authors' own work rather than third-party Broadcom calibration data, and the gist this file circulates in states outright that "it would be distributed with Linux if Broadcom would assist but because they are not willing to help it would be illegal to distribute this with Linux." I fetched that page and confirmed the sentence. That is the maintainer's call, not yours or mine, but it should be visible next to the bytes.

What is right, since a review that lists only problems misleads. /usr/lib/firmware/updates is the correct destination and a better choice than writing into pacman's /usr/lib/firmware/brcm. The <sys_vendor>-<product_name> filename matches what the driver actually builds — linux-firmware's own brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt confirms the shape. You used grep … >/dev/null rather than grep -q behind lspci, which is the #6608 trap. The migration is genuinely idempotent across reruns, correctly does not reload brcmfmac, is mode 0644 with no shebang, and its timestamp sorts last. sudo is the right escalation for a migration that runs in a visible terminal. The nullglob save/restore is correct and tested. And the file placement under install/ does ship — omarchy's PKGBUILD copies that tree wholesale.

On the allowlist entry. test/shell.d/unowned-system-paths-test.sh:33-36 records /usr/lib/firmware/updates, and that test's covers() matches by path prefix in both directions, so the entry permits any future Omarchy script to write anywhere below the kernel's firmware override tree — and, because the match is symmetric, excuses a write naming /usr/lib/firmware itself. Your stated reason (pacman cannot conflict there) is correct as far as it goes. It is worth the maintainer knowing the entry is broader than this change needs, rather than a defect in your change.

Overlap you should know about. #7487 by @csvenke fixes the same defect on the same chip, verified on the same MacBookPro14,3, with a different NVRAM (bugzilla 193121, Boot Camp lineage, ccode=0/regrev=1) and different trade-offs: it strips macaddr= when it cannot find one, resolves the interface through the PCI device rather than glob order, refuses to touch a destination that already exists, and carries provenance in the data file — but it writes into /usr/lib/firmware/brcm/, which is pacman's territory, and it has no migration. Yours is better on destination, packaging and existing-install coverage; theirs is better on the MAC and no-clobber questions. Both edit the same block of install/hardware/all.sh and both install a brcmfmac43602-pcie.txt with different contents to the same runtime path, so they cannot both land as they stand. Which one is the maintainer's call, and I have left #7487 untouched.

Where the second opinion stands. Codex agreed with findings 1 through 4, which I had already reached — its independence is not currently guaranteed, since it runs as the same user and could read this machine, though I grepped its log and found no transcript reads. What it contributed on its own terms stands regardless: the per-model subsystem IDs, the MacBookPro13,3 degradation report, the kernel's duplicate-MAC guard covering a different placeholder than yours, the vacuous clobber assertion, and the licence statement in the upstream gist. I verified each of those against the source or fetched the page before repeating it here.

Waiting on: the maintainer, for the licence question and for the choice between this and #7487; and on you, for findings 1 through 5. Nothing needs another pass from me until the branch moves.

Gate on the 2017 Touch Bar models this dump has actually been seen to
work on. Resolve the NIC through its PCI BDF, strip macaddr= when none
is found, never overwrite an existing board file, and stop wrapping
install in `if` so a failed write cannot mark the migration done.
@shawnyeager

Copy link
Copy Markdown
Contributor Author

Addressed the review on the branch.

1. Gate. brcmfmac43602_needed now requires MacBookPro14,2 or MacBookPro14,3 as well as Apple + 14e4:43ba. 14,2 is the HOWTO machine; 14,3 is the one this PR was verified on. MacBookPro13,3 is the same PCI ID and is excluded — that Arch thread is why. Tests assert both the include and the 13,3/11,4 excludes.

2. MAC. Live address comes from /sys/bus/pci/devices/<bdf>/net/*/address for the 14e4:43ba BDF (lspci -Dnn + awk over the whole stream). If none is found, macaddr= is deleted so the card keeps its OTP address. The dump's 00:90:4c:0d:f4:3e is never installed. The first-*/wireless glob is gone.

3. Completeness / clobber / shadow. Three-marker check is gone. If a generic or DMI-specific board file already exists in /usr/lib/firmware/updates/brcm or /usr/lib/firmware/brcm, we skip. User-placed files and a future linux-firmware board file both win. No refresh of an already-installed copy; a later dump would need its own migration.

4. Failed install looking like success. if brcmfmac43602_apply is gone. The leaf/migration do needed / installed in if, then call brcmfmac43602_install unguarded so set -e holds. Each write is || return 1. Tests stub install(1) to fail and assert the leaf/migration exit non-zero with no reboot-required.

5. Vacuous clobber test. The existing-file case now uses invoke_leaf (no rm -rf "$fwdir"). Refusing to overwrite is what the suite actually exercises.

6. Provenance. The data file now starts with # comments naming attachment 290569, stating it is not provided or licensed by Broadcom or Apple, and noting that macaddr= is substituted or stripped. Licence to redistribute the bytes is still a maintainer call.

vs #7487. Kept /usr/lib/firmware/updates (pacman-safe), the DMI-specific filename the driver actually requests, the migration, and the dump that produced 5 GHz at 802.11ac on this 14,3 (ccode=00/regrev=245, deferring to the host regdom Omarchy already sets). Took #7487's PCI MAC lookup, macaddr= strip, and no-clobber. The two PRs still cannot both land as written — different dumps to related dest paths — but this side no longer has the MAC/clobber/gate problems that made them a straight swap.

Arch ships everything under /usr/lib/firmware zstd-compressed — all 118 files in /usr/lib/firmware/brcm on a current install, including NVRAM board files for other chips such as `brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt.zst`. The existence check looked only for the uncompressed name, so the rule that a packaged linux-firmware board file outranks this copy could never fire on Arch: the day linux-firmware ships one for the BCM43602, the check would miss it and write the override on top, shadowing it for good.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex XHigh <noreply@openai.com>
@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at f81f4d5. All five findings from the last pass are closed, and I proved each one by breaking the code it guards rather than by reading it. Three new things, one of which I fixed on your branch.

What ran. test/shell.d/brcmfmac-5ghz-test.sh — 25 ok, exit 0. test/shell.d/unowned-system-paths-test.sh — exit 0. ./test/cli — exit 0. All on a disposable worker VM with no BCM43602 in it, so this exercises your stubs and none of it confirms 5 GHz behaviour on real hardware. Your MacBookPro14,3 is still the only evidence for that.

The five.

  1. Gate — fixed. brcmfmac-43602.sh:61 is an anchored ^MacBookPro14,[23]$, so 13,3, 11,4, 14,1 and the iMacs cannot match. Deleting that line makes MacBookPro13,3 is outside the model gate fail, so the exclusions are real assertions. The model string comes from /sys/class/dmi/id/product_name (:46), populated from SMBIOS at boot and already what three sibling leaves gate on (fix-spi-keyboard.sh:2, fix-suspend-nvme.sh:3, fix-surface-keyboard.sh:5) — it is there that early.
  2. Donor MAC — fixed, and the duplicate-MAC question is moot. 00:90:4c:0d:f4:3e never reaches disk: :117-129 substitutes the live address or deletes the line. Replacing the delete with a cat fails the macaddr line is stripped when no MAC is discoverable.
  3. Clobber — fixed. Shadowing — see the commit below.
  4. Failed install returning 0 — fixed. I checked the production path rather than trusting the test: bin/omarchy-migrate:93 runs bash -euo pipefail "$file" and only touches the marker at :95 on success, which is exactly what your test reproduces, and the leaf test now uses bash -eE -c 'source "$1"' to match install/helpers/logging.sh:55-60 instead of being stricter than it. Making the install failure path return 0 fails a failed install does not look like success.
  5. Vacuous clobber assertion — fixed. Making brcmfmac43602_installed always return false overwrites the user-owned file and fails an existing NVRAM is never clobbered. It now tests what it claims.

Pushed to your branch: f81f4d5, "See a packaged board file that arrives compressed". brcmfmac43602_installed looked only for brcmfmac43602-pcie.txt, but Arch compresses everything under /usr/lib/firmware — all 118 files in /usr/lib/firmware/brcm on a current install are .zst, board NVRAM included, and brcmfmac43241b4-sdio.Intel Corp.-VALLEYVIEW C0 PLATFORM.txt.zst is one that ships today. So the rule your header states, that a future linux-firmware board file outranks this copy, could never fire on Arch: the check would miss the packaged file and write the override on top of it. I reproduced that (a .txt.zst in the packaged directory, override written anyway), added .zst and .xz to the check, and added an assertion; reverting the fix fails that assertion and nothing else.

Still open, and yours to decide how: a failed install strands a half-done state that the skip guard then makes permanent. brcmfmac43602_installed answers "is a file there", and the callers use it for two different questions — "do not clobber someone else's file" and "we already did this". Those come apart when a write fails. Two runs on the worker:

  • An install(1) that truncates the destination and then fails — the shape an ENOSPC failure takes — leaves a 0-byte brcmfmac43602-pcie.txt. The migration correctly exits 1 that time. Next login the guard sees the file, exits 0, and omarchy-migrate writes the marker: an empty NVRAM sits at the kernel's highest-priority firmware path permanently, and nothing will replace it.
  • The generic write succeeding and the DMI-specific one failing leaves the generic file and exit 1. The rerun skips, exits 0, marker lands — so omarchy-state set reboot-required is never called on either run. The user has the NVRAM but is never told to reboot, and the migration will not run again.

Your test at brcmfmac-5ghz-test.sh:267 cannot catch either: the stub install exits before creating anything, so a failed install leaves no dest file is true regardless of what the code does. A stub that truncates its last argument and then exits 1 fails it. I have not pushed anything here, because the fix is a design choice — install to a temp name and rename, or separate "we installed it" from "a file exists" — and which one is yours to make.

Smaller. fix-brcmfmac-5ghz.sh:21 prints Installed BCM43602 5 GHz NVRAM before line 22 does the work, so a failed install logs the success line and then the failure. And the same file uses two early return 0 guards where AGENTS.md asks for a full if/else and every other hardware leaf uses a positive if — nesting the call in the then body still propagates failure, since errexit is suspended only for an if condition, not for its body.

Two things the move made non-obvious, both fine. The data file now lives in default/, which the omarchy package does not ship — but omarchy-settings does (cp -a default/. into /usr/share/omarchy/default/), the two are built from one _commit, and omarchy depends on omarchy-settings=${pkgver}, so the migration and its data land together. And the # header parses: mainline brcmf_nvram_strip starts in its idle state, takes # there into a comment state and eats through the newline, so a comment at byte zero is fine, and 6.8 KB is far under the 64,000-byte limit.

Neighbours. #7644 still conflicts with this textually — git merge-tree reports CONFLICT (content): Merge conflict in install/hardware/all.sh, because both of you add a run_logged line immediately after fix-brcmfmac-supplicant.sh. Keeping both lines resolves it; whichever lands second needs the rebase. #7487 has not moved since 19 Aug and is still a competing implementation with a different dump; you have taken its MAC lookup, macaddr= strip and no-clobber, so what remains between you is the dump itself, the destination, the migration, and how wide the gate is. Still not both landable, still the maintainer's call, and I have left it untouched.

Second opinion. codex at xhigh found the partial-install/permanent-skip problem, the compressed-firmware gap and the vacuous leaves no dest file assertion — none of which I had reasoned about, and I reproduced all three on the worker before repeating them here. It also flagged that brcmfmac43602_wifi_mac reads net_addrs[0] out of a glob, so glob order would pick the address if the PCI function ever exposed more than one netdev; I am passing that on but rating it low, because the realistic second interface is the P2P device and current kernels register that as a wdev with no netdev. Its independence is not guaranteed in general, since it runs as the same user — but every command it ran this time was inside the worktree, and it excluded .jsonl and .log from its own searches.

Waiting on: you, for the half-install and lost-reboot-prompt question; the maintainer, for the licence on the vendored dump and for the choice between this and #7487.

@caueguerra

Copy link
Copy Markdown

Test report from an Omarchy 4.0.2-1 system. This was a manual NVRAM test, not a checkout of this PR branch.

Hardware/software:

  • DMI: MacBookPro13,3
  • Wi-Fi: Broadcom BCM43602, PCI 14e4:43ba, Apple subsystem 106b:015a
  • Driver: brcmfmac
  • Kernel: 7.1.8-arch1-3
  • linux-firmware-broadcom: 20260810-2
  • NetworkManager: 1.58.1-1

Before installing NVRAM:

  • iw phy exposed only 2.4 GHz frequencies
  • Connected at 2412 MHz/channel 1, 20 MHz width
  • Signal approximately -68 dBm
  • RX bitrate approximately 21.6 Mbit/s
  • Active rescan found no 5 GHz BSSes
  • Kernel logged missing clm_blob/txcap_blob warnings

Manual change:

  • Installed the MacBookPro13,3-specific brcmfmac43602-pcie.txt from https://github.com/a17t/mbp13-3
  • Changed ccode=X3 / regrev=15 to ccode=0 / regrev=1
  • Resulting relevant values: boardtype=0x073e, boardrev=0x1101, boardflags=0x00400001, boardflags2=0xC080101B, boardflags3=0x40000100, aa5g=7, rxchain=7, txchain=7
  • Reloaded brcmfmac

After:

  • Band 2/5 GHz frequencies appeared immediately
  • Automatically reconnected to the same SSID at 5805 MHz/channel 161, 80 MHz width
  • Signal improved to approximately -35 dBm
  • Observed RX link rate up to 780 Mbit/s and TX up to 650 Mbit/s
  • Gateway and internet ping tests had 0% packet loss
  • The missing clm_blob/txcap_blob warnings remain, but 5 GHz works

This validates that a calibrated NVRAM restores 5 GHz on MacBookPro13,3 / subsystem 106b:015a, but it does not validate applying the PR's current donor calibration unchanged to this model. The working file here has different board values from the PR description, which supports gating calibration by model/subsystem.

One concern: before NVRAM, the interface reported 00:90:4c:0d:f4:3e; that same value was written into the test file. This appears to be Broadcom's placeholder rather than a confirmed factory MAC. The installer should avoid treating an existing 00:90:4c:* address as authoritative and should not ship or retain a shared donor MAC.

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at f81f4d5. The branch has not moved since the last pass, so this is about what the thread learned rather than about new code. Nothing pushed. The second opinion is codex at xhigh reasoning; where it agreed with something already concluded I say so, and its independence is not guaranteed in general because it runs as the same user, though every file it read this time was inside the worktree or upstream kernel source.

Housekeeping first, so nothing gets re-litigated. Copilot's review and its four inline comments are dated 21 Aug 11:54, before your 14:09 rewrite, and all four describe code you deleted in 53796d7: brcmfmac43602_file_complete is gone, the */wireless glob is now a PCI BDF lookup, the donor MAC is stripped rather than retained, and brcmfmac43602_apply no longer exists so neither caller invokes anything as an if condition. I checked each against this head. None of them hold.

What ran. test/shell.d/brcmfmac-5ghz-test.sh (25 ok), test/shell.d/unowned-system-paths-test.sh and ./test/cli, all exit 0, all on a disposable worker VM with no BCM43602 in it. That exercises your stubs; none of it says anything about 5 GHz on real hardware.

Both things I left open last time are still open, and I re-proved them at this head rather than assuming. Driving the migration with an install(1) that truncates its destination and then fails leaves a 0-byte brcmfmac43602-pcie.txt and exit 1; the next run sees the file, exits 0, and omarchy-migrate writes the completion marker, so an empty NVRAM sits at the kernel's highest-priority firmware path permanently. Driving it with an install(1) that succeeds once and fails on the second call leaves the correct 6868-byte generic file with the DMI-specific name missing, exit 1, and omarchy-state set reboot-required called on neither run — the user has a working NVRAM and is never told to reboot, and the migration will not run again. And brcmfmac-5ghz-test.sh:279 still cannot catch either: patching the stub to truncate its last argument before exiting 1 turns a failed install leaves no dest file from passing to failing, with nothing else in the suite changing. The fix is still a design choice — install to a temp name and rename, or separate "we installed it" from "a file exists" — so it is still yours rather than mine.

@caueguerra's report, read against this branch. It is a MacBookPro13,3, which line 61 excludes, and it installs a different file, so it is not a test of this PR. It is corroboration of your design rather than a counter-example, and you should read it that way: the file that worked there is boardtype=0x073e / boardrev=0x1101 against this dump's 0x61b / 0x1421, which is the board-specific argument for the narrow gate, made from a machine outside it. @nanwangjkl on #7672 makes the same point from the other direction — a MacBookPro12,1 that declined to install this NVRAM precisely because the gate excluded it.

The one part of that report that does land on this branch, and it is new. Before any NVRAM, that card reported 00:90:4c:0d:f4:3e — byte-for-byte the placeholder at line 22 of your dump. Codex traced why: it is defaultsromvars_43602 in Broadcom's own source, the value a BCM43602 boots with when it has no usable SROM/OTP data, and upstream brcmfmac's duplicate-address mitigation randomises only the other template, 00:90:4c:c5:12:38, so it does not catch this one. The consequence for your code is that on such a machine brcmfmac43602_wifi_mac (:90-102) returns a syntactically valid address, brcmfmac43602_install (:122-123) takes the substitution branch, and the strip branch at :130 never runs — so "the dump's 00:90:4c:0d:f4:3e is never installed" is not true for the machines where it matters most. The immediate effect is nil, since the card already has that address, but it persists a non-unique address that two machines on one segment can collide on. Codex also killed the obvious fix: stripping macaddr= would not help either, because the fallback is what appears when there is no OTP address to fall back to.

A second, separate MAC problem codex found that I had not reasoned about. /sys/.../net/*/address is ndev->dev_addr, the current address, not the permanent one. NetworkManager's disconnected scan randomisation is on by default, cloned-mac-address can be random or stable, and a systemd .link MACAddress= rule or a plain ip link set address all change it too. Omarchy ships no NetworkManager MAC configuration — etc/NetworkManager/conf.d/omarchy-wifi-powersave.conf is the only file — so upstream defaults apply. If the migration lands while a randomised address is set, that transient address is written into both NVRAM files and survives indefinitely at the highest-priority firmware path. /sys/class/ieee80211/phy*/macaddress exposes wiphy->perm_addr, which brcmfmac sets from the firmware's boot-time cur_etheraddr and does not update when the netdev address changes, so it is a sounder source — resolved through this PCI device rather than the first global phy. It does not solve the paragraph above, since perm_addr can itself be the fallback.

On the gate, which is the part I want to be careful about. Codex rates MacBookPro14,2 a High: it reads aa2g/aa5g/txchain/rxchain=7 as RF-path bitmaps that brcmfmac copies into device RAM without validating against the board, so a board with two usable paths told it has three can end up with stream selection over a path that is not there, and a wrong swctrlmap_* or antenna-gain table biases transmit-power control rather than being inert. That mechanism is drawn from the open brcmsmac sibling and Infineon's NVRAM application note, not from the closed BCM43602 firmware, so treat it as a plausible mechanism rather than a demonstration. Its premise that 14,2 is admitted without evidence is wrong, though, and I checked: the csk-grit42 HOWTO your description cites is written for a MacBookPro14,2 and instructs installing this same attachment 290569, verifying aa5g=7, txchain=7, rxchain=7, ccode=00, regrev=245. Both models inside your gate have a field report of this exact dump, which is more than "works on mine" and is worth saying plainly, because it is the thing most likely to be doubted.

Something worth deciding before this merges. The body says Fixes #7672, and #7672 now carries reports from MacBookPro12,1, 13,3, 14,2 and 14,3 across at least three different boards. This gate covers two of the four. Merging as written would auto-close an issue for two users it deliberately does not help.

Smaller, and both from codex. fix-brcmfmac-5ghz.sh:21-22 installs without setting reboot-required while the migration does, and omarchy-apply-hardware:16-18 documents itself as rerunnable — though no install leaf anywhere in this repo sets reboot-required, so changing that here would break a repo-wide pattern rather than fix a local bug, and I would leave it. And the header at :16-19 claims a future linux-firmware board file outranks this copy, which is true only before installation: once the override exists, /lib/firmware/updates is searched first and the guard at :76-85 skips forever, so there is no handoff or cleanup path. That is the same "never refreshes" thread from the first review rather than a new one.

Neighbours. #7644 still conflicts textually — git merge-tree against this head reports CONFLICT (content): Merge conflict in install/hardware/all.sh, both of you adding a run_logged line after fix-brcmfmac-supplicant.sh; keeping both lines resolves it and whoever lands second rebases. #7487 has a new confirmation from @bernardcosta on a MacBookPro14,3. His antenna note is correct and it is about that PR's file, not yours: #7487 declares rxchain=3/txchain=3/aa5g=3 with only a0/a1 per-chain tables, where yours declares 7 with a0/a1/a2 and per-channel gain_index_c0..c2. The more useful reading for whoever decides between you: the two dumps carry different board identities (0x61b/0x1421 against 0x073e/0x1101) and both are now reported working on a MacBookPro14,3, so "5 GHz appeared" does not discriminate between correct and incorrect calibration on this chip. One more confirmation from one more machine is not evidence about the machines that are not that one, in either direction. I have left #7487 untouched.

Waiting on: you, for the partial-install state and the lost reboot prompt, and for whether Fixes #7672 should stay; the maintainer, for the licence on the vendored dump and for the choice between this and #7487.

A write that failed part-way could leave a truncated or half-written NVRAM
pair under the name the driver loads, where the skip guard then kept it
forever and the migration never asked for the reboot. Stage both names under
temporary names and rename them into place together, rolling back on failure.

Read the MAC from the wiphy's permanent address rather than the netdev's
current one, which NetworkManager randomises while scanning, and treat the
Broadcom 00:90:4c placeholder as no address so it is never persisted.

Give the leaf and the migration the positive if that the other hardware
leaves use, and log the install after it succeeds.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QH9zheAn7LKZeTzgwpHqBF
@shawnyeager

Copy link
Copy Markdown
Contributor Author

Pushed 21b1f92, which closes the items still open from the last two passes and @caueguerra's report.

a failed install strands a half-done state that the skip guard then makes permanent

Fixed by staging. brcmfmac43602_install now writes both names to <name>.tmp in the firmware directory and renames them into place together, removing everything it staged or placed if any step fails. A truncating install(1) leaves nothing under a name the driver loads, a stub that succeeds once and fails on the second call leaves nothing either, and the suite now drives the migration with the truncating stub and then reruns it: the rerun installs and asks for the reboot.

The installer should avoid treating an existing 00:90:4c:* address as authoritative and should not ship or retain a shared donor MAC.

Agreed, and fixed. brcmfmac43602_wifi_mac now treats a 00:90:4c:* address as no address, so the strip branch runs and the placeholder is never persisted, even on a card that boots on it.

/sys/.../net/*/address is ndev->dev_addr, the current address, not the permanent one.

Fixed. The lookup now prefers ieee80211/phy*/macaddress under the 14e4:43ba PCI device and falls back to the netdev address only when no wiphy attribute exists. A test writes a randomised netdev address next to a permanent one and asserts the permanent one lands.

fix-brcmfmac-5ghz.sh:21 prints Installed BCM43602 5 GHz NVRAM before line 22 does the work, so a failed install logs the success line

Both the leaf and the migration now use the positive if the other hardware leaves use, and the leaf logs after the install returns.

the header at :16-19 claims a future linux-firmware board file outranks this copy, which is true only before installation

Reworded to say that.

Still deciding whether Fixes #7672 stays, given that issue now carries reports from models outside the gate. The licence on the vendored dump and the choice against #7487 remain the maintainer's.

@shawnyeager

Copy link
Copy Markdown
Contributor Author

Merging as written would auto-close an issue for two users it deliberately does not help.

Changed the body to Refs #7672 and stated the scope there: this PR covers MacBookPro14,2 and 14,3, and #7672 stays open to track the other BCM43602 boards.

@monomyth

monomyth commented Sep 4, 2026

Copy link
Copy Markdown

Hardware validation data for the MacBookPro13,3 case that this PR intentionally leaves out:

  • DMI: MacBookPro13,3 (2016 15-inch)
  • Wi-Fi: BCM43602 14e4:43ba, Apple subsystem 106b:015a
  • Driver/firmware: brcmfmac, BCM43602/2 firmware 7.35.177.61

Important: this does not validate this PR's attachment 290569 on 13,3. We used the separate community calibration from kernel.org Bugzilla attachment 285753, so 13,3 needs model-specific source selection rather than widening the existing 290569 model gate.

Before:

  • iw phy exposed Band 1 only
  • only 2.4 GHz networks were visible
  • connected at 2412 MHz
  • the device used Broadcom's placeholder 00:90:4c:0d:f4:3e address
  • the kernel logged no clm_blob and warned that channels may be limited

Tested installation:

  • attachment 285753
  • installed atomically under /usr/lib/firmware/updates/brcm/ using both generic and DMI-specific names
  • placeholder MAC replaced with a stable locally administered address
  • ccode=US, regrev=0 for this US test machine

After reboot:

  • iw phy exposes Band 2, including channels 36 through 165
  • connected to a 5 GHz AP at 5600 MHz / channel 120
  • signal: -53 dBm
  • RX: 866.6 Mbit/s
  • TX: 702.0 Mbit/s
  • 20/20 gateway pings, 0% loss, 3.736 ms average
  • user also ran a speed test and reported normal results
  • mDNS and SSH are working on the new DHCP address

I am preparing a dependent branch that adds the 13,3 calibration as a separate fixture/path and preserves this PR's 14,2/14,3 gate.

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at 21b1f92. Everything left open from the last pass is closed, and each fix is proved by mutation on a disposable VM rather than by the suite passing: reverting the atomic staging fails a failed install leaves no dest file, neutering the staging rollback fails a failed install leaves nothing behind, preferring the netdev address over the wiphy fails the permanent address wins over a randomised netdev address, dropping the 00:90:4c guard fails the Broadcom placeholder address is never persisted, and removing the model regex fails MacBookPro13,3 is outside the model gate. 29 assertions pass at head and ./test/cli is 116 ok, both on a worker VM with no BCM43602 in it — so none of that is evidence about 5 GHz itself, which still rests on your MacBookPro14,3.

Three things are new.

The 00:90:4c guard is right about the address this dump ships, and narrower than it reads. brcmfmac compares against one exact address rather than the OUI: brcmf_default_mac_address is 00:90:4c:c5:12:38, matched with ether_addr_equal_unaligned in brcmf_c_preinit_dcmds, and on a match the driver calls eth_random_addr(), sets NET_ADDR_RANDOM, and copies the result into wiphy->perm_addr. A card reporting 00:90:4c:0d:f4:3e — the address in this dump, and the one @monomyth observed on a 13,3 — therefore reaches sysfs unchanged and your prefix check catches it. A card reporting the driver's exact default never shows a 00:90:4c address at all; it shows a kernel-generated locally administered one, and brcmfmac43602_wifi_mac (install/hardware/apple/brcmfmac-43602.sh:100-108) would write that into NVRAM as the card's permanent identity. The netdev fallback has the same shape with a NetworkManager cloned address. Rejecting any locally administered address (bit 0x02 in the first octet) closes both, but I have not pushed it: #10141 deliberately writes a stable locally administered MAC on the 13,3, so which way this goes is a policy question across the two branches rather than a defect in this one.

An interrupted rename leaves a state the skip guard makes permanent. brcmfmac-43602.sh:161-166 renames the generic name before the DMI-specific name. A kill between the two renames leaves the generic file final and the DMI file as .tmp; on the next run brcmfmac43602_installed (:82) accepts the generic file, the if body is skipped, and omarchy-state set reboot-required never runs, so the migration marks itself done with no reboot prompt. The driver falls back to the generic name, so 5 GHz still arrives at whatever the next reboot is — what is lost is the prompt, not the fix. The window is two renames in one directory, so this is a note rather than something to hold the PR for.

The rename rollback at :162-165 is not covered by the suite. Making the second mv fail with the rollback intact, and again with the rollback replaced by true, produces identical results: the leaf's errexit takes the harness down in both cases and no assertion distinguishes them. The staging rollback is covered (a half-written NVRAM pair is rolled back); this second one is not. I did not push a test, because #10141 is stacked on this head.

On the four Copilot comments: all four are moot or wrong at 21b1f92, and your replies are accurate — each was checked against the source rather than taken from either side. One caveat for anyone reading the thread cold: GitHub has re-anchored the macaddr one to the current head, where the else branch at :143 strips the line, so it reads as live and is not.

Two smaller things. The title still says "2016–2017 Macs" while the gate is ^MacBookPro14,[23]$, which is 2017 only — 13,3 is excluded, and tested to be excluded — and that title is what the merge commit and the changelog carry. And the install/hardware/all.sh conflict reported last run is gone; the branch merges cleanly into quattro as it stands.

Unchanged and still the maintainer's: the licence on a dump whose own header says it is "not provided or licensed by Broadcom or Apple", and the overlap with #7487, which vendors a genuinely different calibration to the same path (boardtype=0x073e / aa5g=3 against this branch's 0x61b / aa5g=7) and gates on chip rather than model. Confirmed on a current worker that linux-firmware-broadcom 20260810-2 ships brcmfmac43602-pcie.bin.zst and no .txt board file at all, so nothing packaged conflicts with either today.

Reviewed here by Claude Opus 5, with a second opinion from codex (gpt-5.6-sol) at xhigh reasoning; the interrupted rename and the uncovered rename rollback are its findings, and its independence is not guaranteed, since read-only sandboxing restricts writes and not reads. Nothing pushed to your branch.

@shawnyeager shawnyeager changed the title Install BCM43602 NVRAM so 2016–2017 Macs see 5 GHz Wi-Fi Install BCM43602 NVRAM so 2017 Touch Bar Macs see 5 GHz Wi-Fi Sep 4, 2026
@shawnyeager

Copy link
Copy Markdown
Contributor Author

Re-reviewed at 21b1f92. No code change.

The 00:90:4c prefix stays. It matches the address this dump ships and the address a 13,3 shows in sysfs. A card on the kernel's exact default (00:90:4c:c5:12:38) already presents a locally administered address; writing that into NVRAM is how this branch persists it. Rejecting U/L bit 0x02 here would collide with #10141, which writes a stable locally administered MAC on 13,3 on purpose.

The kill between the two mv calls and the uncovered rename rollback stay as notes. The driver still loads the generic name, so 5 GHz is not lost; what drops is the reboot prompt. The window is two renames in one directory.

Title updated to match the gate (MacBookPro14,2 / 14,3, 2017 only). Licence and #7487 remain the maintainer's.

@monomyth

monomyth commented Sep 4, 2026

Copy link
Copy Markdown

Thanks for considering #10141. Our MacBookPro13,3 requirement is a stable per-machine fallback when the firmware exposes the Broadcom placeholder. We generate that address explicitly; we don't require accepting arbitrary kernel- or NetworkManager-randomized addresses as permanent hardware identity.

Stricter validation of discovered addresses could therefore coexist with our explicit 13,3 fallback. Please don't treat the stacked branch as a reason to defer the rename-rollback test either; we can adapt/rebase #10141.

Our hardware validation remains specific to attachment 285753 on MacBookPro13,3, not attachment 290569 or a wider model gate.

On MacBookPro14,3, an NVRAM file with no macaddr= key makes brcmfmac
time out on cur_etheraddr and fail dongle setup. OTP does not answer.
Keep the source line when the 43ba wiphy has no real permanent address;
still substitute a live address when it does.
@shawnyeager

Copy link
Copy Markdown
Contributor Author

Follow-up: stripping macaddr= to force OTP is not viable on MacBookPro14,3. Firmware times out on cur_etheraddr (-5) and the dongle never creates wlp3s0. The key stays in the NVRAM; a real 43ba wiphy address is still substituted when one exists. Details in the description.

@monomyth

monomyth commented Sep 6, 2026

Copy link
Copy Markdown

Thanks for the concrete reboot/reload result. We do not have MacBookPro14,3 evidence that contradicts it. A required macaddr= key and a requirement to keep the shared donor value are different questions.

On our MacBookPro13,3, attachment 285753 plus an explicitly generated stable locally administered address brought up 5 GHz: 5600 MHz/channel 120, 866.6 Mbit/s reported RX, and 20/20 gateway pings without loss on the tested custom 7.2.2 kernel. That is positive evidence for a populated key with a per-installation value on 13,3; it does not validate attachment 290569, the same approach on 14,3, or removal of the key.

Our draft fallback derives an address from the local machine-id instead of preserving 00:90:4c:0d:f4:3e. It is not ready to generalize unchanged: cloned/absent machine IDs need explicit handling and tests. Retaining the identical donor address on multiple machines leaves a duplicate-identity risk even if one machine associates successfully; we have not run a two-machine collision test.

A useful next hardware test on 14,3 would be a populated, unique locally administered MAC in the same calibration, with recovery access available, checking firmware startup, association and identity after reboot. This is a proposed test, not a completed one. We can adapt #10141 to the updated shared installer; its older stripping fallback should not be treated as overriding your new hardware finding.

Additional 13,3 follow-up: custom 7.2.3-arch1-2-mbp133 is now verified live with the same intended stable local MAC and a traffic-carrying 5.6 GHz / 80 MHz association. This adds a new-kernel result for that 13,3 configuration, not a test of the 14,3 calibration or fallback. Our empty/missing machine-id helper issue is now explicitly recorded as a merge-readiness defect in draft #10141; that generator should not be copied unchanged.

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at ffda5d4. One commit since the last pass, and it reverses the previous one's MAC handling, so this is about that.

What the delta does. At 21b1f92 the installer stripped macaddr= when the 43ba wiphy showed no usable address; at ffda5d4 it copies the source verbatim first and substitutes only on success, so the no-address branch now ships the dump's own macaddr=00:90:4c:0d:f4:3e. The crash evidence for keeping the key is solid and I am not disputing it.

The thing that settles most of the thread, and I do not think either of you has it yet. That address is compiled into the chip firmware. On a current worker running linux-firmware-broadcom 20260810-2:

$ strings /usr/lib/firmware/brcm/brcmfmac43602-pcie.bin | grep 00:90:4c
macaddr=00:90:4c:0d:f4:3e

One match in the whole blob. So it is not a donor's board address that leaked into attachment 290569 — it is the default the BCM43602 firmware itself carries. That is why @caueguerra and @monomyth both saw exactly that address on a 13,3 with no board file, and it is the mechanism behind "OTP does not answer here" on your 14,3. Two consequences:

  • The vendored dump does not carry anyone's real MAC, which is worth knowing about a community dump.
  • On the branch @monomyth is worried about — the wiphy reporting 00:90:4c:* — keeping the key preserves the address the card already had rather than assigning a new shared one. Those machines were already colliding on stock Arch before Omarchy shipped anything. This PR does not create that collision. It also does not fix it, and it pins it: once the file is at /usr/lib/firmware/updates/brcm, brcmfmac43602_installed (install/hardware/apple/brcmfmac-43602.sh:77-87) skips forever, so nothing later can change the address.

Where it does take something away, and this is the finding. brcmfmac43602_wifi_mac also returns 1 when the sysfs candidates at :101 are simply absent — brcmfmac unbound, not yet initialised, or failed. lspci still lists the device, so brcmfmac43602_needed passes and the install proceeds anyway. A machine in that state with a perfectly good address gets the firmware default written over it, permanently, by the skip guard above. At 21b1f92 the same event produced a keyless file, which on a card with usable SROM was self-correcting — and on yours crashed the dongle. The delta trades a crash for a pinned shared identity in that one branch.

That branch is reachable in the ordering install/hardware/all.sh already has, and #10306 makes it more so: it adds omarchy-pkg-add apple-bcm-firmware to fix-brcmfmac-supplicant.sh at line 38, immediately before your leaf at line 39, and does not reload the module. A Mac that booted without usable Broadcom firmware still has no wiphy when your leaf runs.

So the disagreement is narrower than the thread reads. @monomyth's 6 Sep comment already separates the two questions, and that separation is right: the key must exist (your evidence), and the value should be per-machine (his). #10141's brcmfmac43602_stable_mac02: plus the first 10 hex of sha256(machine-id) — is such a value, and he has already recorded its empty-machine-id handling as a defect in his own draft. Which value goes in the required key is a policy call spanning both branches, so I have pushed nothing.

One thing only you can answer, and it decides how much of the above is theoretical: on the 14,3 you verified, what does grep '^macaddr=' /usr/lib/firmware/updates/brcm/brcmfmac43602-pcie.txt say? If it is your own address, the substitution branch is the normal path for this population. If it is 00:90:4c:0d:f4:3e, it is not.

Prior findings. All three from the last pass are untouched by this delta and still open, and all three remain notes rather than blockers: the 00:90:4c prefix guard still misses a card presenting the kernel's exact brcmf_default_mac_address; an interrupted rename still loses the reboot prompt through the same skip guard; the rename rollback at :162-165 is still uncovered. On the last one, @monomyth withdrew the stacked-branch objection, so that reason is gone — but a test for it needs restructuring how the harness drives a failing second mv, which is your call about your own suite rather than a defect for me to patch.

What ran. On a disposable worker with no BCM43602 in it: test/shell.d/brcmfmac-5ghz-test.sh 29 ok, test/shell.d/unowned-system-paths-test.sh, and ./test/cli 116 ok, all exit 0. The two new assertions are mutation-proved rather than just green: restoring the strip fails placeholder wiphy keeps the source macaddr= key, and deleting the substitution fails the installed NVRAM carries the NIC's live MAC while printing macaddr=00:90:4c:0d:f4:3e straight out of the installed file. I also checked whether the rewritten grep -qx "$(grep '^macaddr=' "$nvram")" form goes vacuous if the source ever loses the key — it does on its own terms, but the suite is not blind to it: deleting macaddr= from the vendored file still fails the installed NVRAM carries the NIC's live MAC. None of that is evidence about 5 GHz on real hardware, which still rests on your 14,3.

Second opinion. Reviewed here by Claude Opus 5, with a second opinion from codex (gpt-5.6-sol) at xhigh reasoning, pinned to ffda5d4 and checked against the tree. It contributed two things I did not have: it disassembled the brcmfmac.ko.zst actually installed on this machine and confirmed brcmf_c_preinit_dcmds compares against the single constant 00:90:4c:c5:12:38, and it found the upstream platform-MAC discussion stating that the NVRAM macaddr= overrides an SROM address and that the parser strips an existing key to append the platform one — which is the citation for treating the key as authoritative rather than a fallback, something I had only inferred from your crash log. It also noted that sed -i needs a second temp file beside $work, so it can fail on a nearly full or permission-changed TMPDIR where the old streaming sed would not; both legs check status and remove $work, and nothing reaches a firmware path, so that is a note. Its independence is not guaranteed in general, since read-only sandboxing restricts writes and not reads. I disagree with its severity: it rates this High and merge-blocking on the premise that the donor address is newly assigned, which the firmware blob above contradicts for the branch it itself calls practically reachable.

Neighbours. #7487, #7644, #10211, #10306, #10314 and #10332 all merge-tree clean against this head today; #10141 conflicts by design, since it rewrites this same shared installer to add a 13,3 source and widen the gate. #10332 reloads brcmfmac around every suspend on 43ba/43bb/43bc, which would pick this NVRAM up without waiting for a reboot — and equally would reapply a wrong address at every wake. #10306 is the ordering interaction above.

Waiting on: you, for the grep above and for whether the required key should carry a per-machine value; the maintainer, for the licence on the vendored dump and the overlap with #7487. Nothing pushed to your branch.

@omarchybot

Copy link
Copy Markdown
Collaborator

Re-reviewed at ffda5d4. The last comment on this thread was written against 21b1f92, so this answers only the new commit, "Keep macaddr= in BCM43602 NVRAM; stripping it crashes the dongle".

What ran, on a disposable worker VM with no BCM43602 in it. test/shell.d/brcmfmac-5ghz-test.sh 29 ok, test/shell.d/unowned-system-paths-test.sh 1 ok, ./test/cli 116 ok, all exit 0. None of that is evidence about 5 GHz or about the crash you reported; that still rests on your MacBookPro14,3, and I have no way to reproduce it here.

The four findings closed at 21b1f92 are still closed at ffda5d4, and I re-proved each by breaking the code it guards rather than by reading it. Installing straight to $target instead of $target.tmp takes the suite down at assertion 4; replacing the staging rollback with true fails a failed install leaves nothing behind; putting net/*/address ahead of ieee80211/phy*/macaddress in the candidate list fails the permanent address wins over a randomised netdev address; removing the anchored model regex fails MacBookPro13,3 is outside the model gate. A revert-flavoured commit is where a closed finding usually reopens, and these four did not.

The fifth one is reversed on purpose, and that is the whole of this review. At 21b1f92 the strip branch meant 00:90:4c:0d:f4:3e never reached disk. At ffda5d4 it does, whenever brcmfmac43602_wifi_mac returns non-zero — which is all three of its exits: no 14e4:43ba BDF from the second lspci -Dnn (brcmfmac-43602.sh:100), a first valid candidate in the 00:90:4c:* range (:104-106), and no readable wiphy or netdev address at all (:110). In every one of those the installed file at both names carries the dump's own macaddr=00:90:4c:0d:f4:3e, and brcmfmac43602_installed (:82) then skips forever, so nothing revisits it. So the answer to "can two Omarchy machines end up with the same MAC on one network" is yes, and the address is that one.

Two things worth saying plainly about how much that is worth. It is not a duplicate this PR creates: @caueguerra and @monomyth each reported a card presenting exactly 00:90:4c:0d:f4:3e before any NVRAM was installed, so those machines already collide today. What the commit changes is that the value moves to /usr/lib/firmware/updates/brcm, the kernel's highest-priority firmware path, where the install guard never looks again — a collision that a later firmware or kernel change could have lifted becomes one that only deleting both files lifts. And the kernel will not lift it: disassembling this machine's own brcmfmac.ko.zst, brcmf_c_preinit_dcmds compares cur_etheraddr against exactly 00:90:4c:c5:12:38 and randomises only on that, so 00:90:4c:0d:f4:3e is not a value it recognises.

The 00:90:4c guard has lost its test, and its effect has inverted. Deleting the guard at :104-106 outright leaves the suite at 29 passed, 0 failed — at 21b1f92 that same mutation failed the Broadcom placeholder address is never persisted, and the assertion that caught it was rewritten in this commit. It is worth restoring coverage, but not by pinning the current behaviour, because the guard now does the opposite of what it did: pointing the wiphy and netdev fixtures at 00:90:4c:aa:bb:cc and rerunning, the suite's own placeholder wiphy keeps the source macaddr= key assertion still passes, meaning a card reporting a 00:90:4c address that is not the dump's has its own distinct address discarded in favour of the shared one. Rejecting the OUI made sense when rejection meant stripping the key. Now that rejection means falling back to one fixed address, it only helps for cards whose fallback is byte-for-byte the dump's.

@monomyth's distinction is the one I would put to the maintainer: "a required macaddr= key and a requirement to keep the shared donor value are different questions". Your crash evidence settles the first and I am not disputing it — the key must be present. It does not settle the second, and #10141's machine-id-derived stable local address is the other answer to it. Which of those two lands is a policy call across the two branches rather than a defect in either, so I have pushed nothing.

One item from the second opinion that I checked and can bound. The three rewritten assertions (brcmfmac-5ghz-test.sh:219, :230, :365) derive their expected value from $(grep '^macaddr=' "$nvram"), so if the source ever lost that line the pattern would be empty and grep -qx '' would match a blank line in the installed file. Each is individually vacuous that way. The suite as a whole is not: deleting macaddr= from default/firmware/apple/brcmfmac43602-pcie.txt and rerunning fails at the installed NVRAM carries the NIC's live MAC before any of the three is reached. Worth tightening to grep -Fqx over a value the test asserts is non-empty, but it is not a hole today.

Neighbours. #7487 still adds the same default/firmware/apple/brcmfmac43602-pcie.txt with a different calibration, so the two cannot both land. #10141 is stacked on this and its author has said the stacking should not defer test work here. #10306 merges cleanly against this branch. #10332 does not — git merge-tree reports a conflict in install/hardware/all.sh, both of you adding a run_logged line in the Apple block; keeping both lines resolves it and whoever lands second rebases.

Unchanged and still the maintainer's: the licence on a dump whose own header says it is not provided or licensed by Broadcom or Apple, and the choice between this and #7487.

Reviewed here by Claude Opus 5, with a second opinion from codex (gpt-5.6-sol) at xhigh reasoning pinned to ffda5d4 and checked against this tree. The brcmfmac.ko disassembly and the vacuity of the three assertions are its work; the duplicate-address conclusion it reached independently of mine, though its independence is not guaranteed, because read-only sandboxing restricts writes and not reads. Nothing pushed to your branch.

@omarchybot

Copy link
Copy Markdown
Collaborator

Housekeeping, and the last from me on this pass: the two comments above are both ours, three minutes apart, from one triage run that reviewed this branch twice by mistake. The second one says the previous comment was written against 21b1f922. That is wrong — both were written against ffda5d49, and neither is stale.

Read them as one review. They agree on the verdict and on the substance: the macaddr= key has to be present, and the open question is whether its value may be the dump's shared 00:90:4c:0d:f4:3e. Their findings differ in emphasis rather than in conclusion, so nothing in either needs discounting.

Nothing was pushed to your branch.

Keep the macaddr= key (stripping it crashes the dongle). Never write the
dump's shared 00:90:4c:0d:f4:3e. When the 43ba wiphy has no real permanent
address, substitute a locally-administered MAC from machine-id so install
with no interface up still works. Tests fail if the donor is written.
@shawnyeager

Copy link
Copy Markdown
Contributor Author

c0fea4e keeps the macaddr= key (stripping it still crashes the dongle on cur_etheraddr) and stops persisting the dump donor 00:90:4c:0d:f4:3e.

When the 43ba wiphy has a real permanent address, that is what is written. When it does not — no sysfs address, or any 00:90:4c:* including a distinct placeholder and the kernel's 00:90:4c:c5:12:38 — the key is filled with a locally-administered unicast address from this machine's /etc/machine-id (salt bcm43602-wifi). Empty or missing machine-id with no live MAC fails the install and leaves nothing on disk. The 00:90:4c:* rejection in brcmfmac43602_wifi_mac is unchanged; it no longer falls through to the shared donor.

Tests now fail if that donor is written. They also cover two machine-ids producing two addresses, and the inverted-guard case where a card's own 00:90:4c:* must not be persisted.

Licence and overlap with #7487 stay a maintainer call. #10141 remains the 13,3 stack; this does not take its dump, widen this gate, or reuse its mbp133-wifi salt.

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.

5 participants