From a55ea569afb46e8d6855bcd2127620087c0efb18 Mon Sep 17 00:00:00 2001 From: Erik Bourget Date: Mon, 9 Nov 2026 14:45:03 -0500 Subject: [PATCH] wifi: brcmfmac: coordinate retained firmware with J713 system sleep Drain reset work before suspending the managed radio and require completed asynchronous setup. Report the D3 acknowledgment and resume result to the host owner, keeping reset work blocked until both radio functions finish recovery. Preserve the existing path on other hosts. Signed-off-by: Erik Bourget --- MAINTAINERS | 1 + .../broadcom/brcm80211/brcmfmac/pcie.c | 65 +++- tools/asahi/brcmfmac-sleep-lifetime-tests.c | 351 ++++++++++++++++++ tools/asahi/brcmfmac-sleep-lifetime-tests.py | 60 +++ 4 files changed, 474 insertions(+), 3 deletions(-) create mode 100644 tools/asahi/brcmfmac-sleep-lifetime-tests.c create mode 100644 tools/asahi/brcmfmac-sleep-lifetime-tests.py diff --git a/MAINTAINERS b/MAINTAINERS index 0a06b6c9eee48..82291a41d6f20 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5572,6 +5572,7 @@ L: brcm80211-dev-list.pdl@broadcom.com S: Supported F: drivers/net/wireless/broadcom/brcm80211/ F: include/linux/platform_data/brcmfmac.h +F: tools/asahi/brcmfmac-sleep-lifetime-tests.* BROADCOM BRCMSTB GPIO DRIVER M: Doug Berger diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c index aeaa156fbcc22..c1e5656ef2696 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -398,6 +399,10 @@ struct brcmf_otp_params { struct brcmf_pciedev_info { enum brcmf_pcie_state state; + /* Published only after the asynchronous firmware setup has completed. */ + bool setup_complete; + /* This binding blocked reset work for the active radio transaction. */ + bool sleep_reset_blocked; bool in_irq; struct pci_dev *pdev; char fw_name[BRCMF_FW_NAME_LEN]; @@ -1717,6 +1722,10 @@ static int brcmf_pcie_reset(struct device *dev) struct brcmf_fw_request *fwreq; int err; + if (apple_radio_sleep_owned(dev)) + return -EBUSY; + + WRITE_ONCE(devinfo->setup_complete, false); brcmf_pcie_intr_disable(devinfo); brcmf_pcie_bus_console_read(devinfo, true); @@ -2594,6 +2603,8 @@ static void brcmf_pcie_setup(struct device *dev, int ret, brcmf_pcie_bus_console_read(devinfo, false); brcmf_pcie_fwcon_timer(devinfo, true); + /* Publish completed firmware setup before suspend admission. */ + smp_store_release(&devinfo->setup_complete, true); return; @@ -3071,19 +3082,56 @@ brcmf_pcie_remove(struct pci_dev *pdev) #ifdef CONFIG_PM +static struct pci_driver brcmf_pciedrvr; + +static void brcmf_pcie_sleep_finish(struct device *dev) +{ + struct brcmf_pciedev_info *devinfo; + struct brcmf_bus *bus; + + /* Async setup failure may unbind us after suspend refused admission. */ + device_lock(dev); + if (dev->driver != &brcmf_pciedrvr.driver) + goto unlock; + bus = dev_get_drvdata(dev); + if (!bus) + goto unlock; + devinfo = bus->bus_priv.pcie->devinfo; + /* A replacement binding has no cleanup pending from this transaction. */ + if (!devinfo->sleep_reset_blocked) + goto unlock; + devinfo->sleep_reset_blocked = false; + brcmf_bus_allow_reset_work(bus); +unlock: + device_unlock(dev); +} + +static int brcmf_pcie_pm_leave_D3(struct device *dev); static int brcmf_pcie_pm_enter_D3(struct device *dev) { struct brcmf_pciedev_info *devinfo; struct brcmf_bus *bus; struct brcmf_if *ifp; - int ifidx; + int ifidx, ret; brcmf_dbg(PCIE, "Enter\n"); bus = dev_get_drvdata(dev); devinfo = bus->bus_priv.pcie->devinfo; + ret = apple_radio_sleep_endpoint_prepare(dev, brcmf_pcie_sleep_finish); + if (ret) + return ret; + /* Drain a recovery already running when the platform acquired its lease. */ + if (apple_radio_sleep_owned(dev)) { + brcmf_bus_cancel_reset_work(bus); + devinfo->sleep_reset_blocked = true; + /* A reset can have returned while firmware loading is still async. */ + if (!smp_load_acquire(&devinfo->setup_complete)) + return -EBUSY; + } + /* Stop new packets and drain ndo_start_xmit before taking the bus down. * The BUS reason prevents completion flow control from waking the queue. */ @@ -3112,8 +3160,10 @@ static int brcmf_pcie_pm_enter_D3(struct device *dev) } devinfo->state = BRCMFMAC_PCIE_STATE_DOWN; - - return 0; + ret = apple_radio_sleep_endpoint_suspended(dev); + if (ret) + brcmf_pcie_pm_leave_D3(dev); + return ret; } @@ -3124,6 +3174,9 @@ static int brcmf_pcie_pm_leave_D3(struct device *dev) struct pci_dev *pdev; int err; + err = apple_radio_sleep_endpoint_resume(dev); + if (err) + return err; brcmf_dbg(PCIE, "Enter\n"); bus = dev_get_drvdata(dev); @@ -3144,11 +3197,17 @@ static int brcmf_pcie_pm_leave_D3(struct device *dev) brcmf_bus_change_state(bus, BRCMF_BUS_UP); brcmf_msgbuf_resume(bus->drvr); brcmf_pcie_fwcon_timer(devinfo, true); + apple_radio_sleep_endpoint_resumed(dev, 0); return 0; } cleanup: devinfo->state = BRCMFMAC_PCIE_STATE_DOWN; + if (apple_radio_sleep_owned(dev)) { + /* Lost firmware/rings violate retention; reset/reprobe is not recovery. */ + apple_radio_sleep_endpoint_resumed(dev, -EUCLEAN); + return -EUCLEAN; + } brcmf_chip_detach(devinfo->ci); devinfo->ci = NULL; pdev = devinfo->pdev; diff --git a/tools/asahi/brcmfmac-sleep-lifetime-tests.c b/tools/asahi/brcmfmac-sleep-lifetime-tests.c new file mode 100644 index 0000000000000..a39c9bdcf8586 --- /dev/null +++ b/tools/asahi/brcmfmac-sleep-lifetime-tests.c @@ -0,0 +1,351 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Execute the real suspend/finish hooks around serialized driver removal. */ +#include +#include +#include +#include +#include +#include + +#define BRCMF_MAX_IFS 16 + +struct device_driver { + int unused; +}; +struct pci_driver { + struct device_driver driver; +}; +struct device { + pthread_mutex_t lock; + struct device_driver *driver; + void *data; +}; +struct brcmf_pciedev_info { + bool setup_complete, sleep_reset_blocked, mbdata_completed; + int state, mbdata_resp_wait; +}; +struct brcmf_pciedev { + struct brcmf_pciedev_info *devinfo; +}; +struct brcmf_if { + void *ndev; +}; +struct brcmf_pub { + struct brcmf_if *iflist[BRCMF_MAX_IFS]; +}; +struct brcmf_bus { + struct { + struct brcmf_pciedev *pcie; + } bus_priv; + struct brcmf_pub *drvr; + bool removing; +}; +static struct pci_driver brcmf_pciedrvr; +static struct device_driver foreign_driver; +static struct device endpoint = { .lock = PTHREAD_MUTEX_INITIALIZER }; +static _Thread_local bool endpoint_locked; +static bool owned; +static void (*finish_callback)(struct device *dev); +static unsigned int allows, cancels; + +/* Synchronize the two overlap cases without sleeps or scheduler assumptions. */ +static pthread_mutex_t sync_lock = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t sync_cond = PTHREAD_COND_INITIALIZER; +static bool watch_lock, lock_attempted, pause_allow, in_allow, release_allow; + +static void device_lock(struct device *dev) +{ + assert(dev == &endpoint); + pthread_mutex_lock(&sync_lock); + if (watch_lock) { + lock_attempted = true; + pthread_cond_broadcast(&sync_cond); + } + pthread_mutex_unlock(&sync_lock); + pthread_mutex_lock(&dev->lock); + endpoint_locked = true; +} + +static void device_unlock(struct device *dev) +{ + assert(endpoint_locked); + endpoint_locked = false; + pthread_mutex_unlock(&dev->lock); +} + +static void *dev_get_drvdata(struct device *dev) +{ + /* This is a lifetime assertion, not a simulated lock. */ + assert(endpoint_locked); + return dev->data; +} + +static void brcmf_bus_allow_reset_work(struct brcmf_bus *bus) +{ + assert(endpoint_locked && bus && bus == endpoint.data); + pthread_mutex_lock(&sync_lock); + in_allow = true; + pthread_cond_broadcast(&sync_cond); + while (pause_allow && !release_allow) + pthread_cond_wait(&sync_cond, &sync_lock); + pthread_mutex_unlock(&sync_lock); + bus->removing = false; + allows++; +} + +static void brcmf_bus_cancel_reset_work(struct brcmf_bus *bus) +{ + assert(endpoint_locked && bus == endpoint.data); + bus->removing = true; + cancels++; +} + +static bool apple_radio_sleep_owned(struct device *dev) +{ + assert(dev == &endpoint); + return owned; +} + +static int +apple_radio_sleep_endpoint_prepare(struct device *dev, + void (*finish)(struct device *dev)) +{ + assert(dev == &endpoint); + if (owned) + finish_callback = finish; + return 0; +} + +static int apple_radio_sleep_endpoint_suspended(struct device *dev) +{ + assert(dev == &endpoint); + return 0; +} + +static int brcmf_pcie_pm_leave_D3(struct device *dev) +{ + (void)dev; + return 0; +} + +#define BRCMF_NETIF_STOP_REASON_BUS 0 +#define BRCMF_BUS_DOWN 0 +#define BRCMF_BUS_UP 1 +#define BRCMFMAC_PCIE_STATE_DOWN 0 +#define BRCMF_H2D_HOST_D3_INFORM 0 +#define BRCMF_PCIE_MBDATA_TIMEOUT 100 +#define brcmf_dbg(...) ((void)0) +#define brcmf_err(...) ((void)0) +/* The asynchronous setup publication is supplied before the callback starts. */ +#define smp_load_acquire(p) (*(p)) +#define wait_event_timeout(wait, condition, timeout) \ + ((void)(wait), (void)(condition), (void)(timeout)) +static void brcmf_txflowblock_if(struct brcmf_if *ifp, int reason, bool block) +{ + (void)ifp; + (void)reason; + (void)block; +} +static void netif_tx_disable(void *ndev) +{ + (void)ndev; +} +static void brcmf_msgbuf_suspend(struct brcmf_pub *drvr) +{ + (void)drvr; +} +static void brcmf_msgbuf_resume(struct brcmf_pub *drvr) +{ + (void)drvr; +} +static void brcmf_pcie_fwcon_timer(struct brcmf_pciedev_info *info, bool on) +{ + (void)info; + (void)on; +} +static void brcmf_bus_change_state(struct brcmf_bus *bus, int state) +{ + (void)bus; + (void)state; +} +static void brcmf_pcie_send_mb_data(struct brcmf_pciedev_info *info, int value) +{ + (void)value; + info->mbdata_completed = true; +} + +/* ACTUAL_FUNCTIONS */ + +static void bind(bool setup_complete) +{ + struct brcmf_bus *bus = calloc(1, sizeof(*bus)); + + assert(endpoint_locked && !endpoint.data && bus); + bus->bus_priv.pcie = calloc(1, sizeof(*bus->bus_priv.pcie)); + assert(bus->bus_priv.pcie); + bus->bus_priv.pcie->devinfo = + calloc(1, sizeof(*bus->bus_priv.pcie->devinfo)); + assert(bus->bus_priv.pcie->devinfo); + bus->bus_priv.pcie->devinfo->setup_complete = setup_complete; + endpoint.driver = &brcmf_pciedrvr.driver; + endpoint.data = bus; +} + +static void remove_binding(void) +{ + struct brcmf_bus *bus = endpoint.data; + + assert(endpoint_locked && bus); + free(bus->bus_priv.pcie->devinfo); + free(bus->bus_priv.pcie); + free(bus); + endpoint.data = NULL; + endpoint.driver = NULL; +} + +static void fresh(bool setup_complete) +{ + assert(!endpoint.data); + watch_lock = lock_attempted = pause_allow = in_allow = release_allow = + false; + allows = cancels = 0; + finish_callback = NULL; + owned = true; + device_lock(&endpoint); + bind(setup_complete); + device_unlock(&endpoint); +} + +static int suspend_endpoint(void) +{ + int ret; + + device_lock(&endpoint); + ret = brcmf_pcie_pm_enter_D3(&endpoint); + device_unlock(&endpoint); + return ret; +} + +static void host_complete(void) +{ + /* Match apple_radio_finish: end ownership before invoking endpoint hooks. */ + owned = false; + if (finish_callback) + finish_callback(&endpoint); +} + +static void *finish_thread(void *unused) +{ + (void)unused; + host_complete(); + return NULL; +} + +static void *remove_thread(void *unused) +{ + (void)unused; + device_lock(&endpoint); + remove_binding(); + device_unlock(&endpoint); + return NULL; +} + +static void wait_flag(bool *flag) +{ + pthread_mutex_lock(&sync_lock); + while (!*flag) + pthread_cond_wait(&sync_cond, &sync_lock); + pthread_mutex_unlock(&sync_lock); +} + +int main(void) +{ + pthread_t finisher, remover; + struct brcmf_bus *replacement; + + /* A refused suspend with a live binding must undo its reset-work block. */ + fresh(false); + assert(suspend_endpoint() == -EBUSY && cancels == 1); + host_complete(); + assert(allows == 1); + remove_thread(NULL); + + /* The reported race: async setup failure removes the failed PM device. */ + fresh(false); + assert(suspend_endpoint() == -EBUSY); + remove_thread(NULL); + host_complete(); + assert(!allows); + + /* An endpoint can keep its driver pointer while drvdata is already clear. */ + endpoint.driver = &brcmf_pciedrvr.driver; + host_complete(); + assert(!allows); + endpoint.driver = NULL; + + /* Same-driver replacement must not inherit the old binding's cleanup. */ + fresh(false); + assert(suspend_endpoint() == -EBUSY); + remove_thread(NULL); + device_lock(&endpoint); + bind(true); + replacement = endpoint.data; + replacement->removing = true; + device_unlock(&endpoint); + host_complete(); + assert(!allows && replacement->removing); + endpoint.driver = &foreign_driver; + replacement->bus_priv.pcie->devinfo->sleep_reset_blocked = true; + host_complete(); + assert(!allows && replacement->removing); + remove_thread(NULL); + + /* Removal holds the device lock while the finish callback arrives. */ + fresh(false); + assert(suspend_endpoint() == -EBUSY); + device_lock(&endpoint); + watch_lock = true; + assert(!pthread_create(&finisher, NULL, finish_thread, NULL)); + wait_flag(&lock_attempted); + remove_binding(); + device_unlock(&endpoint); + assert(!pthread_join(finisher, NULL)); + assert(!allows); + + /* Conversely, removal must wait until finish stops using the bus. */ + fresh(false); + assert(suspend_endpoint() == -EBUSY); + pause_allow = true; + assert(!pthread_create(&finisher, NULL, finish_thread, NULL)); + wait_flag(&in_allow); + pthread_mutex_lock(&sync_lock); + watch_lock = true; + pthread_mutex_unlock(&sync_lock); + assert(!pthread_create(&remover, NULL, remove_thread, NULL)); + wait_flag(&lock_attempted); + pthread_mutex_lock(&sync_lock); + release_allow = true; + pthread_cond_broadcast(&sync_cond); + pthread_mutex_unlock(&sync_lock); + assert(!pthread_join(finisher, NULL)); + assert(!pthread_join(remover, NULL)); + assert(allows == 1 && !endpoint.data); + + /* Completed setup admits D3; completion releases the block exactly once. */ + fresh(true); + assert(!suspend_endpoint()); + host_complete(); + host_complete(); + assert(allows == 1); + remove_thread(NULL); + + /* Other hosts do not acquire a deferred reset-work cleanup. */ + fresh(true); + owned = false; + assert(!suspend_endpoint() && !finish_callback && !cancels); + host_complete(); + assert(!allows); + remove_thread(NULL); + puts("PASS: Wi-Fi abort, binding replacement and overlapping remove/finish"); + return 0; +} diff --git a/tools/asahi/brcmfmac-sleep-lifetime-tests.py b/tools/asahi/brcmfmac-sleep-lifetime-tests.py new file mode 100644 index 0000000000000..b697a2eea600e --- /dev/null +++ b/tools/asahi/brcmfmac-sleep-lifetime-tests.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-only +"""Exercise production Wi-Fi sleep hooks against asynchronous binding removal.""" + +from pathlib import Path +import resource +import subprocess +import tempfile + +from sleep_test_utils import function + +resource.setrlimit(resource.RLIMIT_CORE, (0, 0)) +root = Path(__file__).resolve().parents[2] +source = (root / "drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c").read_text() +actual = "\n".join( + function(source, name) + for name in ("brcmf_pcie_sleep_finish", "brcmf_pcie_pm_enter_D3") +) +fixture = Path(__file__).with_suffix(".c").read_text() +variants = { + "production": actual, + "unlocked-cleanup": actual.replace("device_lock(dev);", "").replace( + "device_unlock(dev);", "" + ), + "replacement-binding-cleanup": actual.replace( + "if (!devinfo->sleep_reset_blocked)", "if (false)" + ), + "wrong-driver-cleanup": actual.replace( + "if (dev->driver != &brcmf_pciedrvr.driver)", "if (false)" + ), + "missing-pending-cleanup": actual.replace( + "devinfo->sleep_reset_blocked = true;", "" + ), +} +with tempfile.TemporaryDirectory(prefix="brcmf-sleep-lifetime-") as tmp: + c, exe = Path(tmp) / "test.c", Path(tmp) / "test" + for name, code in variants.items(): + c.write_text(fixture.replace("/* ACTUAL_FUNCTIONS */", code)) + subprocess.run( + [ + "cc", + "-std=gnu11", + "-pthread", + "-Wall", + "-Wextra", + "-Werror", + str(c), + "-o", + str(exe), + ], + check=True, + ) + result = subprocess.run([str(exe)], text=True, capture_output=True, timeout=10) + if name == "production": + assert result.returncode == 0, result.stderr + print(result.stdout.strip()) + else: + assert code != actual + assert result.returncode != 0, f"mutation survived: {name}" + print(f"PASS: rejected {name}")