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

boards/z1: fix broken clock configuration #19705

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

maribu
Copy link
Member

@maribu maribu commented Jun 3, 2023

Contribution description

The MSP430F2xx family has on RSEL bit more than the MSP430x1xxx family. The first commit updates the clock calibration accordingly.

df5c319 from #19558 broke the clock configuration of the Z1 by relying on the incorrect documentation of what clock is actually used. Closely reading the convoluted clock initialization code revealed that no XT2 crystal is present (as also indicated by some comments in board.c), contradicting the #define MSP430_HAS_EXTERNAL_CRYSTAL 1 in the board.h.

The second commit should restore behavior (but with calibrated DCO than hard coded magic numbers).

Testing procedure

make BOARD=z1 -C examples/default flash term

should work again, but fail with master.

Issues/PRs references

Regression introduced in #19558

@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: MSP Platform: This PR/issue effects MSP-based platforms CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 3, 2023
@github-actions github-actions bot added Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Jun 3, 2023
@riot-ci
Copy link

riot-ci commented Jun 3, 2023

Murdock results

✔️ PASSED

971dc88 boards/z1: fix broken clock configuration

Success Failures Total Runtime
6933 0 6933 11m:01s

Artifacts

@maribu
Copy link
Member Author

maribu commented Jun 6, 2023

@kaspar030 tested this PR, see https://gitea.riot-labs.de/kaspar/results/src/branch/main/pr19705

The takeaway is:

  • The board is starting again
  • There still is lot of cleanup and bug fixing left to get MSP430 support back up to standard

@maribu
Copy link
Member Author

maribu commented Jun 7, 2023

I changed my mind regarding skipping the most significant RSEL bit in the calibration when an external resistor is used in the DCO: Just doing the normal calibration anyway reduces code complexity and size and only adds a minor delay in boot up time.

cpu/msp430fxyz/clock.c Outdated Show resolved Hide resolved
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and @kaspar030 confirmed it's still working

The MSP430F2xx family has on RSEL bit more than the MSP430x1xxx family.
This updates the clock calibration accordingly.
df5c319 from
RIOT-OS#19558 broke the clock
configuration of the Z1 by relying on the incorrect documentation of
what clock is actually used. Closely reading the convoluted clock
initialization code revealed that no XT2 crystal is present (as also
indicated by some comments in `board.c`), contradicting the
`#define MSP430_HAS_EXTERNAL_CRYSTAL 1` in the `board.h`.

This now should restore behavior (but with calibrated DCO than
hard coded magic numbers).
@maribu
Copy link
Member Author

maribu commented Jun 7, 2023

bors merge

bors bot added a commit that referenced this pull request Jun 7, 2023
19705: boards/z1: fix broken clock configuration r=maribu a=maribu

### Contribution description

The MSP430F2xx family has on RSEL bit more than the MSP430x1xxx family. The first commit updates the clock calibration accordingly.

df5c319 from #19558 broke the clock configuration of the Z1 by relying on the incorrect documentation of what clock is actually used. Closely reading the convoluted clock initialization code revealed that no XT2 crystal is present (as also indicated by some comments in `board.c`), contradicting the `#define MSP430_HAS_EXTERNAL_CRYSTAL 1` in the `board.h`.

The second commit should restore behavior (but with calibrated DCO than hard coded magic numbers).


19713: nanocoap: clean up coap_iterate_option(), make it public r=maribu a=benpicco



Co-authored-by: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@bors
Copy link
Contributor

bors bot commented Jun 7, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Jun 7, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 517abc7 into RIOT-OS:master Jun 7, 2023
@maribu maribu deleted the boards/z1/fix_clock branch June 7, 2023 18:39
@maribu
Copy link
Member Author

maribu commented Jun 7, 2023

Thx :)

@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports 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: MSP Platform: This PR/issue effects MSP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants