Skip to content
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

cpu/msp430: clean up and fix clock driver #20601

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

maribu
Copy link
Member

@maribu maribu commented Apr 19, 2024

Contribution description

  • The validity test for the high frequency crystal did not take into account the higher range supported by the MSP430 F2xx / G2xx family. This fixes the issue.
    • The CPU family used is exposed to C as CPU_FAM_<NAME> macro
  • Unused headers where dropped
  • The status register is aliased SR, so let's use that more readable name.

Testing procedure

This does not change any binaries. It would allow boards using an MSP430 F2xx/G2xx MCU to configure a CPU frequency of 16 MHz from a crystal without a compile time check triggering. (Both upstream boards use 16 MHz, but from a DCO instead of a crystal.)

One could test this with the following patch:

diff --git a/boards/olimex-msp430-h2618/include/periph_conf.h b/boards/olimex-msp430-h2618/include/periph_conf.h
index 23d223a40b..20b981a6f7 100644
--- a/boards/olimex-msp430-h2618/include/periph_conf.h
+++ b/boards/olimex-msp430-h2618/include/periph_conf.h
@@ -28,19 +28,21 @@
 extern "C" {
 #endif
 
-#define CLOCK_CORECLOCK     msp430_dco_freq
+#define CLOCK_CORECLOCK     MHZ(16)
 
 /**
  * @brief   Clock configuration
  */
 static const msp430_clock_params_t clock_params = {
-    .target_dco_frequency = MHZ(16),
+    .target_dco_frequency = 0,
     .lfxt1_frequency = 32768,
-    .main_clock_source = MAIN_CLOCK_SOURCE_DCOCLK,
-    .submain_clock_source = SUBMAIN_CLOCK_SOURCE_DCOCLK,
+    .main_clock_source = MAIN_CLOCK_SOURCE_XT2CLK,
+    .submain_clock_source = SUBMAIN_CLOCK_SOURCE_XT2CLK,
     .main_clock_divier = MAIN_CLOCK_DIVIDE_BY_1,
     .submain_clock_divier = SUBMAIN_CLOCK_DIVIDE_BY_1,
     .auxiliary_clock_divier = AUXILIARY_CLOCK_DIVIDE_BY_1,
+    .xt2_frequency = CLOCK_CORECLOCK,
+    .has_xt2 = true,
 };
 
 /**

make BOARD=olimex-msp430-h2618 -C examples/default

yields undefined reference to xt2_frequency_invalid'inmaster`, but works with this PR.

Issues/PRs references

None

- The validity test for the high frequency crystal did not take
  into account the higher range supported by the MSP430 F2xx / G2xx
  family. This fixes the issue.
    - The CPU family used is exposed to C as `CPU_FAM_<NAME>` macro
- Unused headers where dropped
- The status register is aliased `SR`, so let's use that more readable
  name.
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 19, 2024
@github-actions github-actions bot added Platform: MSP Platform: This PR/issue effects MSP-based platforms Area: cpu Area: CPU/MCU ports labels Apr 19, 2024
@riot-ci
Copy link

riot-ci commented Apr 19, 2024

Murdock results

✔️ PASSED

024832a cpu/msp430: clean up and fix clock driver

Success Failures Total Runtime
10066 0 10066 12m:53s

Artifacts

@benpicco benpicco enabled auto-merge April 22, 2024 08:44
@benpicco benpicco added this pull request to the merge queue Apr 22, 2024
Merged via the queue into RIOT-OS:master with commit 5a7bcc9 Apr 22, 2024
30 checks passed
@maribu maribu deleted the cpu/msp430/clock branch April 22, 2024 16:00
@maribu
Copy link
Member Author

maribu commented Apr 22, 2024

Thx :)

@maribu
Copy link
Member Author

maribu commented Apr 22, 2024

IMO this doesn't need a backport, as this bug is not triggered by any boards supported by RIOT. New boards get added to master and won't be backported.

@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 5, 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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants