Skip to content

cpu/nrf5x_common: properly calibrate RC-based low-frequency clock #20669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2024

Conversation

mguetschow
Copy link
Contributor

Contribution description

@BOZHENG001 encountered the issue where all NimBLE-based examples would fail to establish connection while advertising works normally on feather-nrf52840-sense, while they work properly on nrf52840dk. As suggested by @andrzej-kaczmarek in apache/mynewt-nimble#1777 (comment), this is an issue with the internal RC low-frequency clock which needs to be calibrated.

While RIOT already tries to do so, it currently ignores the fact that the high-frequency clock needs to be manually started for the calibration to succeed. This is documented for nRF51 here on page 53 and for nRF52-series for example here for nRF52832 and here for nRF52840. The documentation of nRF5240 does not explicitly mention the necessity to manually start the HFCLK, but it shouldn't harm there either.

Testing procedure

make -C examples/nimble_heart_rate_sensor BOARD=feather-nrf52840-sense flash term fails to establish a connection (e.g. to the Android app "nRF Connect"). This is also possible to reproduce with BOARD=nrf52840dk after switching to the internal RC oscillator:

diff --git a/boards/common/nrf52xxxdk/include/periph_conf_common.h b/boards/common/nrf52xxxdk/include/periph_conf_common.h
index 0b7bbdd019..af35d672ff 100644
--- a/boards/common/nrf52xxxdk/include/periph_conf_common.h
+++ b/boards/common/nrf52xxxdk/include/periph_conf_common.h
@@ -22,7 +22,7 @@
 #define PERIPH_CONF_COMMON_H
 
 #include "periph_cpu.h"
-#include "cfg_clock_32_1.h"
+#include "cfg_clock_32_0.h"
 #include "cfg_i2c_default.h"
 #include "cfg_rtt_default.h"
 #include "cfg_timer_default.h"

With this PR, connection can be reliably established.

Issues/PRs references

The issue has also been discussed on Matrix with @maribu, thanks for your input!

@mguetschow mguetschow requested a review from aabadie as a code owner May 14, 2024 14:47
@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels May 14, 2024
@mguetschow mguetschow requested review from maribu and MrKevinWeiss May 14, 2024 15:15
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 14, 2024
@benpicco benpicco requested a review from dylad May 14, 2024 15:25
@maribu maribu enabled auto-merge May 14, 2024 15:34
@riot-ci
Copy link

riot-ci commented May 14, 2024

Murdock results

✔️ PASSED

6af8090 cpu/nrf5x_common: properly calibrate RC-based low-frequency clock

Success Failures Total Runtime
10082 0 10083 11m:59s

Artifacts

@maribu maribu added this pull request to the merge queue May 14, 2024
Merged via the queue into RIOT-OS:master with commit 28beadd May 14, 2024
@mguetschow mguetschow deleted the nrf52-lfclk_rc-calibrate branch May 15, 2024 07:14
@mguetschow mguetschow added this to the Release 2024.07 milestone Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants