Skip to content

cpu/cc2538: RTT: implement missing API functions#14700

Merged
benpicco merged 3 commits intoRIOT-OS:masterfrom
fjmolinas:to_cc2538_enhance_rtt
Aug 7, 2020
Merged

cpu/cc2538: RTT: implement missing API functions#14700
benpicco merged 3 commits intoRIOT-OS:masterfrom
fjmolinas:to_cc2538_enhance_rtt

Conversation

@fjmolinas
Copy link
Contributor

Contribution description

This PR is a takeover of #13630, rest of the description is as in the original PR, I only removed comments made in #13630 (review). This PR should help addressing #13686 since the application does not compile because of the missing functions.

The RTT implementation of cpu/cc2538 is based on the sleep timer, a 32 bit 32kHz timer that can only be read and that only supports one alarm. (No overflow interrupt)

This implements functions that are defined by the RIOT API, but are not directly supported by the hardware:

  • rtt_set_counter(): as we can't set the counter directly, add an offset to the raw counter value instead
  • rtt_get_alarm(): we can't read back the alarm time, so store it in a local variable. This is also needed for
  • rtt_set_overflow_cb(): In the previous implementation the overflow callback would always overwrite the alarm callback and vice versa.
    To fix this, check which event will occur first and set that alarm as well as a flag to tell which alarm it was.
    When the alarm rings, set the alarm to the other event if there is one.

Testing procedure

There are no tests for those functions.

Issues/PRs references

I implemented those for #13519, but then discovered I don't need them.
So I'm not sure if this should be added just for completeness' sake when we could just as well do without.

We can't set the hardware counter directly, so always add an offset.
We can't read back the alarm, so just store it in a variable.
Previously the setting the alarm would overwrite the overflow callback
and vice versa.

Since we can only set one alarm in hardware, always set the alarm to the
closest event of the two.
@fjmolinas fjmolinas added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: cpu Area: CPU/MCU ports labels Aug 4, 2020
@fjmolinas fjmolinas requested a review from benpicco August 4, 2020 14:33
@fjmolinas fjmolinas requested a review from MrKevinWeiss as a code owner August 4, 2020 14:33
@fjmolinas
Copy link
Contributor Author

ping @benpicco

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.

The overhead is just a few instructions and if those functions are useful after all, we might as well add them.

It's a pity there are no tests for those / the API is not clear here.

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 7, 2020
@benpicco benpicco merged commit 574676b into RIOT-OS:master Aug 7, 2020
@fjmolinas fjmolinas deleted the to_cc2538_enhance_rtt branch July 30, 2021 11:28
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 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.

2 participants