src/cyw43_ctrl: Lock the bus across BT HCI read/write#153
Open
beriberikix wants to merge 1 commit into
Open
Conversation
cyw43_bluetooth_hci_read() and cyw43_bluetooth_hci_write() call cyw43_btbus_read()/cyw43_btbus_write() without holding the cyw43 bus lock. The BT shared bus runs over the same gSPI/SDIO backplane as WiFi, so a caller that does not already serialise against WiFi will interleave WiFi and BT transfers on the bus. The btstack HCI transport in pico-sdk already wraps these calls in CYW43_THREAD_ENTER/EXIT, so it is unaffected. But any other consumer that calls the BT HCI API directly (e.g. a non-btstack or multi-threaded port) is left unprotected. Take CYW43_THREAD_ENTER across the whole transfer so the functions are self-serialising. The lock is recursive, so the existing wrapping in the btstack transport and the nested cyw43_ensure_bt_up() continue to work unchanged. Signed-off-by: Jonathan Beri <jmberi@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author
|
Smoke-tested on a Pico 2 W (RP2350): a patched georgerobotics-backend build brings up BT over the HCI path (reads the controller's BD_ADDR + HCI/LMP version through This exercises the recursive-lock nesting on real silicon (the BT HCI caller already holds the bus lock, so the added |
Collaborator
|
looks good |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
cyw43_bluetooth_hci_read()andcyw43_bluetooth_hci_write()callcyw43_btbus_read()/cyw43_btbus_write()without holding the cyw43 bus lock. The BT shared bus runs over the same gSPI/SDIO backplane as WiFi, so a caller that does not already serialise against WiFi will interleave WiFi and BT transfers on the bus.This change takes
CYW43_THREAD_ENTERacross the whole transfer so the two functions are self-serialising.Why
The btstack HCI transport in pico-sdk happens to wrap these calls in
CYW43_THREAD_ENTER/CYW43_THREAD_EXITitself (btstack_hci_transport_cyw43.c), so the stock pico-sdk + BTstack path is unaffected by this change. But any other consumer that calls the BT HCI API directly — a non-btstack integration, or a multi-threaded port where WiFi and BT run on separate threads — is left unprotected and can interleave bus transfers.The lock is recursive, so the existing wrapping in the btstack transport and the nested
cyw43_ensure_bt_up()continue to work unchanged. Behaviour is identical on the happy path; this only adds serialisation for direct callers.Context
This came out of a downstream Zephyr WiFi+BLE coexistence effort on the CYW43439 (gSPI), where WiFi and BT run as separate threads. The missing lock here let BT HCI transfers race the WiFi poll thread on the shared bus. Discussion and a related transport-robustness fix (re-read of transiently corrupt ring indices) are in raspberrypi/pico-sdk#3027; the broader effort is tracked in the Zephyr RFC zephyrproject-rtos/zephyr#111811.
Suggested by @peterharperuk in raspberrypi/pico-sdk#3027.
Testing
Builds clean under the repo host tests (
tests/sdiocompilescyw43_ctrl.c) and-Werror -fsyntax-onlywithCYW43_ENABLE_BLUETOOTH=1. Functionally verified on hardware (Pico 2 W) as part of the Zephyr coex port: with the bus held across BT transfers, WiFi+BLE ran a 2-hour soak with zero faults.