nrf5x: request HFCLK in USB_EVT_READY to fix post-SoftDevice deadlock#3633
Merged
Conversation
USB_EVT_DETECTED runs hfclk_enable() which, when SoftDevice is not yet
enabled, starts HFXO via direct CLOCK register access. After
sd_softdevice_enable() takes over CLOCK, HFXO is physically off again
and SD's HFCLK reference count is 0.
If USB_EVT_READY is fired post-SD (e.g. on nRF52 via Bluefruit's
usb_softdevice_post_enable() when the pre-SD nrfx_power READY callback
didn't get to run before nrfx_power was uninited), the wait loop
'while (!hfclk_running()) {}' calls sd_clock_hfclk_is_running() which
returns false forever -> deadlock that blocks both USB enumeration and
any further app code on the calling thread.
Call hfclk_enable() right before the wait so HFCLK is requested in
whichever context (SD or direct) is current. hfclk_enable() is
idempotent.
Reproduces reliably with bleuart on Feather nRF52840 Express flashed
via JLink: chip wedges in sd_clock_hfclk_is_running SVC, no USB
enumeration, no BLE advertising. With the fix, USB enumerates and BLE
advertises as expected.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a potential deadlock in the nRF5x DCD power-event handling when USB_EVT_READY is delivered after the SoftDevice has been enabled, by ensuring HFCLK is (re-)requested in the currently active clock-control context before waiting for it to become running.
Changes:
- Request HFCLK again in
USB_EVT_READYviahfclk_enable()immediately before thehfclk_running()wait loop. - Add an explanatory comment describing the pre-SoftDevice vs post-SoftDevice HFCLK ownership scenario that can otherwise lead to an infinite wait.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in sizeNo entries. Changes <1% in size
No changes
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| adafruit_clue/dfu_runtime | 11,960 → 11,984 (+24) | — | — | — | 12,968 → 12,992 (+24) | +0.2% |
| adafruit_clue/hid_generic_inout | 13,080 → 13,104 (+24) | — | — | — | 13,912 → 13,936 (+24) | +0.2% |
| adafruit_clue/hid_multiple_interface | 13,700 → 13,724 (+24) | — | — | — | 14,708 → 14,732 (+24) | +0.2% |
| adafruit_clue/hid_boot_interface | 13,756 → 13,780 (+24) | — | — | — | 14,716 → 14,740 (+24) | +0.2% |
| adafruit_clue/hid_composite | 13,788 → 13,812 (+24) | — | — | — | 14,892 → 14,916 (+24) | +0.2% |
| adafruit_clue/midi_test | 13,964 → 13,988 (+24) | — | — | — | 14,892 → 14,916 (+24) | +0.2% |
| adafruit_clue/cdc_dual_ports | 14,632 → 14,656 (+24) | — | — | — | 15,816 → 15,840 (+24) | +0.2% |
| adafruit_clue/printer_to_cdc | 14,852 → 14,876 (+24) | — | — | — | 15,840 → 15,864 (+24) | +0.2% |
| adafruit_clue/audio_test | 15,116 → 15,140 (+24) | — | — | — | 16,016 → 16,040 (+24) | +0.1% |
| adafruit_clue/webusb_serial | 14,920 → 14,944 (+24) | — | — | — | 16,200 → 16,224 (+24) | +0.1% |
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.
Summary
Fix a deadlock in
tusb_hal_nrf_power_event(USB_EVT_READY)when fired after the SoftDevice has been enabled.USB_EVT_DETECTEDrunshfclk_enable()which, when SD is not yet enabled, starts HFXO via direct CLOCK register access. Aftersd_softdevice_enable()takes over the CLOCK peripheral, HFXO is physically off again and SD's HFCLK reference count is 0.If
USB_EVT_READYis then fired post-SD (e.g. on nRF52 via Bluefruit'susb_softdevice_post_enable()when the pre-SDnrfx_powerREADY callback didn't get to run beforenrfx_powerwas uninited), the wait loopcalls
sd_clock_hfclk_is_running()which returns false forever — deadlock that blocks both USB enumeration and any further app code on the calling thread.The fix calls
hfclk_enable()immediately before the wait, so HFCLK is requested in whichever context (SD or direct) is current.hfclk_enable()is idempotent.Reproduction
Adafruit Feather nRF52840 Express running the
bleuartexample, flashed via JLink (cold start, race-prone). Without the patch the chip wedges insd_clock_hfclk_is_runningSVC: no USB enumeration, no BLE advertising. With the patch USB enumerates as239a:8029and BLE advertises asFeather nRF52840 Express.Test plan
hfclk_enable()is idempotent and takes the same direct-register branch as before, so behavior should be unchanged