Add WiFi resume hook for T2 MacBooks#5140
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an installer-time systemd sleep/resume hook for T2 MacBooks to reload the Broadcom (brcmfmac) WiFi module on resume, addressing post-suspend connectivity failures.
Changes:
- Extend the T2 detection/install script to create a
systemd/system-sleepresume hook. - On
postresume, unload and reloadbrcmfmacand log outcomes vialogger.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| KERNEL_CMDLINE[default]+=" intel_iommu=on iommu=pt pcie_ports=compat" | ||
| EOF | ||
|
|
||
| cat <<'HOOK' | sudo tee /usr/lib/systemd/system-sleep/wifi-resume >/dev/null |
There was a problem hiding this comment.
This installs the system-sleep hook into /usr/lib/systemd/system-sleep, which is typically reserved for distribution/package-managed units and may be overwritten on upgrades. Elsewhere in this repo, locally-installed systemd artifacts are placed under /etc/systemd/... (e.g., install/config/hardware/apple/fix-suspend-nvme.sh:13). Consider writing the hook to /etc/systemd/system-sleep/wifi-resume instead (and ensure the target directory exists, e.g., via mkdir -p).
1eaad95 to
485c431
Compare
|
We need a migration for this too, no? |
We do! 😅 I'm on it David! |
20b6104 to
4a7e847
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sudo mkdir -p /etc/systemd/system-sleep | ||
| cat <<'HOOK' | sudo tee /etc/systemd/system-sleep/wifi-resume >/dev/null | ||
| #!/bin/bash | ||
| if [[ $1 == "post" ]]; then | ||
| logger -t wifi-resume "Reloading brcmfmac after resume" | ||
| modprobe -r brcmfmac 2>/dev/null | ||
| modprobe brcmfmac || logger -t wifi-resume "Failed to reload brcmfmac" | ||
| fi | ||
| HOOK | ||
| sudo chmod +x /etc/systemd/system-sleep/wifi-resume | ||
| fi |
There was a problem hiding this comment.
The WiFi resume hook installation code is duplicated in both this migration file and in install/config/hardware/apple/fix-t2.sh (lines 36-45). This duplication creates a maintenance burden: any bugs or improvements to the hook logic must be made in both places, increasing the risk of inconsistency. Consider consolidating this logic into a shared function or having one location call the other to avoid divergence. Additionally, this migration lacks error handling - if the hook installation fails, there's no indication in the logs, and no way to recover.
| sudo mkdir -p /etc/systemd/system-sleep | |
| cat <<'HOOK' | sudo tee /etc/systemd/system-sleep/wifi-resume >/dev/null | |
| #!/bin/bash | |
| if [[ $1 == "post" ]]; then | |
| logger -t wifi-resume "Reloading brcmfmac after resume" | |
| modprobe -r brcmfmac 2>/dev/null | |
| modprobe brcmfmac || logger -t wifi-resume "Failed to reload brcmfmac" | |
| fi | |
| HOOK | |
| sudo chmod +x /etc/systemd/system-sleep/wifi-resume | |
| fi | |
| if [[ -x "install/config/hardware/apple/fix-t2.sh" ]]; then | |
| if ! sudo "install/config/hardware/apple/fix-t2.sh"; then | |
| echo "Error: fix-t2.sh failed while installing WiFi resume hook" >&2 | |
| fi | |
| else | |
| echo "Error: install/config/hardware/apple/fix-t2.sh not found or not executable; WiFi resume hook not installed" >&2 | |
| fi | |
| if [[ ! -f /etc/systemd/system-sleep/wifi-resume ]]; then | |
| echo "Error: WiFi resume hook was not created at /etc/systemd/system-sleep/wifi-resume" >&2 | |
| fi | |
| fi |
| always_full_speed=false | ||
| EOF | ||
|
|
||
| sudo mkdir -p /etc/systemd/system-sleep |
There was a problem hiding this comment.
The WiFi resume hook installation code is duplicated in both this file and in migrations/1774703846.sh. Additionally, unlike the migration file, this location lacks the idempotency check ([[ ! -f /etc/systemd/system-sleep/wifi-resume ]]) that prevents reinstalling the hook if it already exists. If fix-t2.sh is run after the migration (or if the script is executed multiple times), the hook file will be unnecessarily overwritten. Consider adding the same guard condition used in the migration.
|
Did you or anyone else test this? |
systemd-sleep only scans /usr/lib/systemd/system-sleep (confirmed in the man page and the binary itself), so the hook in /etc/ was never executed. Move it to the path systemd-sleep actually reads, and drop the redundant mkdir since /usr/lib/systemd/system-sleep is shipped by the systemd package.
|
Hey David! I just tested on my T2 mac, and I caught a bug. I installed the hook to It appeared to work during my initial review because an older copy existed at Pushed f25bb1f to install at the correct path and drop the redundant mkdir -p (the Verified on my T2 MacBook Air:
|
Summary
The Broadcom WiFi driver (
brcmfmac) on T2 MacBooks loses connection state after suspend and silently fails — WiFi appears connected but can't transmit data. This hook unloads and reloads the kernel module on resume, restoring connectivity.Tested:
fix-t2.shon a T2 MacBook