RTIC v2.0.0 support

[kyp44]

Hi, I'm new here. I'm working on a game targeting the PyGamer, which has a nice BSP here. Being a game, using async Rust is extremely valuable for me, and information on using async on SAMD processors seems to be very scare. It doesn't seem like Embassy supports SAMD processors at all, but RTIC v2 now uses async for software tasks, whereas v1 did not.

The HAL crate here does support RTIC (via the rtic feature), but it only seems to support RTIC v1 by implementing the old Monotonic trait. Things have changed with v2 and now there is a different Monotonic trait that has to be implemented, which involves some async functions.

The current paradigm for RTIC v2 is to select the time driver from the rtic_monotonics crate, in which the type that implements Monotonic is created using macros instead of passing it to the app RTIC macro as before. Looking at the code for the supported microcontrollers (noting that SAMD is not among them), it seems like Monotonic is most easily implemented indirectly by instead implementing the simpler TimerQueueBackend trait, which does not involve any async methods.

I think I have enough embedded experience to implement an RTC-based driver for RTIC v2. Given that the v1 driver was implemented in this HAL instead of within the RTIC project, I wanted to solicit opinions on whether a new v2 driver should also just live with the HAL here (including relevant macros), or by adding SAMD support to the RTIC project? Thoughts?

NOTE: I am aware that I can already use RTIC v2 on SAMD chips using the SysTick driver that ships with RTIC, but I want a more efficient driver that does not generate frequent, periodic interrupts.

[jbeaurivage]

Hi @kyp44 and welcome!

Ideally the implementation would live in this crate. By all means, feel free to open up a PR with a RTIC v2 monotonic implementation. I've actually been meaning to do it too, just haven't found the time yet - I'd be happy to assist you with that.

By the way, if you're looking for async support on more peripherals: I've been working on adding async APIs to the HAL. The PR is pretty much complete at this point, it just hasn't gotten merged yet. Having some extra hands testing it out would absolutely help with getting it over the finish line.

[kyp44]

@jbeaurivage I appreciate the response!

While I have never used RTIC v1, my impression is that the Monotonic driver was a critical component in that you couldn't even use RTIC without one (if I'm wrong about that please let me know!). In v2, however, the new Monotonic is created by the user (via a macro) and seems to only provide global (note that the methods are all associated functions) access to async delay functions. In that sense it seems to be completely optional and you wouldn't really need one if you did not need any async delays. I imagine that you probably also need a driver if you want to have periodic hardware tasks as well, though these aren't really discussed in the RTIC book.

I do need async delays for my application, but looking at the async methods you've implemented for TimerFuture, I could probably get away with just using these. However this would be less ergonmic than having a global delay provider as I would have to pass peripheral or TimerFuture objects around for safety reasons.

So I think I still would like to try to implement the global TimerQueueBackend driver, and I agree that it should just live in this HAL. It can be implemented along side the old Monotonic driver so that Rtc can be used with RTIC v1 and v2.

I'll fork and start working on this right away, and I'm happy to have you help or at least have another set of eyes on my implementation. I'm also happy to help you test your async HAL extensions, though I only have a PyGamer in terms of ATSAMD hardware so I may be limited in what I can test.

[jbeaurivage]

No problem! Yeah having a global monotonic is entirely optional on v2.
Regarding testing the async APIs, just the pygamer is fine, I've done extensive testing on a D21 target. Testing on a SAMx5x chip would likely give us the confidence that it's virtually bug-free and useable.

[jbeaurivage]

@kyp44 By the way, do reach out on Matrix if you haven't already: https://matrix.to/#/#atsamd-rs:matrix.org

[Sympatron]

Most rtic-monotonic implementations for other platforms live in rtic-monotonics. So that is where I would look for reference.

[kyp44]

Status update: here I have got an RTC-based monotonic pretty much fully working, but am still working on ironing out some async behaviors that may or may not even be the monotonic misbehaving.

Currently, the monotonic is implemented directly in the HAL but @jbeaurivage and I have been discussing on Matrix, and it may be more appropriate to submit it to the rtic-monotonics project. More details on this to come once I can be sure that the monotonic is behaving correctly.

[maximeborges]

Timer-based implementation proposal: rtic-rs/rtic#994

[kyp44]

Update

While I am still putting them through their paces a bit: I have completed RTIC v2 monotonics using the RTC.

@jbeaurivage and I have had some discussion about whether these monotnonics belong as part of this HAL (as the RTIC v1 monotonic does), or part of the rtic-monotonics crate. Note that as currently implemented they are part of the HAL, but I think more discussion is warranted on this as there are issues/concerns with either option.

What to include?

There are two monotonics: mode0 (32-bit counter) and mode1 (16-bit counter with half-period counting). They each have pros and cons.

• mode0: Uses the RTC in 32-bit mode. The advantage is that it is a little more efficient in that there are no overflow or half-period interrupts. In heavy stress testing, this also seems more robust. The disadvantage is that the monotonic wraps at only 36 hours for the faster 32.768 kHz clock. This mode could also be used with half-period counting to extend the wrapping time (and the additional interrupts would be extremely infrequent), but not every chip has the required two compares in this mode, so I didn't bother. This could be a nice enhancement down the road though for chips that do support it.

• mode1: Uses the RTC in 16-bit mode with half-period counting. The advantage is that monotonic wrapping takes millions of years! The disadvantage is less efficincy with a half-period or overflow intterrupt occurring every second for the faster 32.768 kHz clock. This is also less robust in very heavy stress testing. Note that every chip has at least two compares in this mode.

Should both be included to give the end user a choice, or just one for simplicity (probably the mode1 monotonic since the wrapping period is infinite for all practical purposes)?

RTC clock source

In order to use the montonics, the RTC clock source need to be setup. There two clock frequencies available: 1.024 kHz or 32.768 kHz, and two sources: internal or external. So this a total of four possible configurations. The way I see it, there are two ways to ensure the clock is setup:

1. Directly when setting up the monotonic via the macros, that is different macros are used for different clocks sources, e.g. rtc_monotonic_1k_int or rtc_monotonic_32k_ext. This is how it is currently implemented.

2. By requiring an instance of RtcClock as verification that the RTC clock has been set up.

Where do we belong?

Whether residing in the HAL, or in rtic-monotonics, there are issues:

• If residing in the HAL, we can take advantage of option 2 above to setup the RTC clock, i.e. the monotonic doesn't have to duplicate that functionality directly and there should be no potential for RTC clock configuration conflicts using safe Rust. However, the monotonic does depend on private functions from rtic-monotonics to enable and set the RTC interrupt priority to the highest level, namely set_monotonic_prio and cortex_logical2hw. These are small functions so diplicating them in the HAL may not be a big deal, though note that set_monotonic_prio does use the external RTIC_ASYNC_MAX_LOGICAL_PRIO variable from RTIC. The HAL does not seem to provide similar functionality to edit the NVIC.

• It is not customary to include HAL dependencies in rtic-monotonics, only PAC crates. As such we would likely need to go with option 1 above by having the monotonic setup the RTC clock manually, duplicating HAL functionality, and opening up the possiblity of RTC clock conflicts even in safe Rust. The advtange here is obviation of the need to duplicate the private functions from rtic-monotonics, and also this would likely make ATSAMD RTIC v2 support more discoverable for end users as rtic-monotonics is a central location for supported chips. Of course the monotonics residing in rtic-monotonics could depend on the HAL to render option 2 viable, but both communities would have to be alright with this.

What are thoughts on this topic?

Remaining tasks:

• Put into the final form depending on where it ends up (atsamd-hal or rtic-monotonics).
• I can only actually test on a atsamd51j, but need to ensure that they at least compiles for all other supported chips so that others can test.

[maximeborges]

Setting up clocks is just a bunch of instructions to write in the right registers at the end of the day; I don't think duplicating such little amount of code in rtic-monotonics to be a major problem here.
I agree that from a safety perspective, it would be nice to have tighter integration with the HAL so we would be able to avoid any conflicts.
This is currently not what rtic-monotonics implementations have been aiming for, looking at the code for other chips. So I think having a PAC-only implementation there makes most sense for coherency for now, and maybe having at feature-gated variant that uses the HAL to bring that safer variant in would be nice to have (I think most users would use the HAL anyway, and that's what I'd like to propose for my timer monotonic also).
I don't really see the point of having the monotonic implementation in the HAL, it would create more confusion than anything I think.

[kyp44]

@maximeborges You are correct, and actually, in this case, setting the clock is only a single line of code.

I also forgot to mention above that the RTC clock frequency is actually needed at compile time anyway (currently accomplished by having different macros to setup the monotonic), so simply just passing in an RtcClock as proof that the clock has been configured would be insufficient, as there is no way to get the frequency from this at compile time.

Given your insights here, I agree that it makes sense for the monotonics to live in rtic-monotonics and to just depend on the PAC.

I'm going to finish the tasks above, clean things up, and then submit a PR to rtic-monotonics. Unless anyone advises against it, I'll include both the mode0 and mode1 monotonics to give users a choice. Obvisouly, I'll ensure that the documentation explains the differences. I may also enhance the mode0 monotonic to take advantage of chips that support it, especially since the PyGamer chip (atsamd51j) supports it and that's what I'm using for my project.

@maximeborges It looks like you only added support in rtic-monotonics for the atsamd21g. For the RTC monotonics, I've only been able to test it on the atsamd51j, but it does include alternate code paths for other ATSAMD chips (mainly copied from RTC code from the HAL). Does it make sense to currently include the RTC monotonics only for tested chips or all chips so others can test it and submit issues if they don't work correctly? Not sure whether rtic-monotonics has a policy on this. Also, maybe you could test the RTC monotonics on the atsamd21g? I don't mind testing your timer monotonic on the atsamd51j and adding alternate code paths for that if it doesn't work initially.

[Sympatron]

Maybe adding the implementation to the HAL for now would be easier, because it already handles all the differences between all supported chips and it would be easier to implement for all of them by reusing a lot of the timer stuff that is already present in the HAL. It would also be easier to experiment here until we settle on a set of useful implementations.
Once that is done, we could port that to rtic-monotonics to be more in line with the rest of the ecosystem.

[Sympatron]

The disadvantage is that the monotonic wraps at only 36 hours for the faster 32.768 kHz clock.

@kyp44 A disadvantage compared to what? mode1 wraps a lot more often.

This is also less robust in very heavy stress testing.

What do you mean by that? Do you mean overflows can be missed if the interrupt is not processed for more than a second or does anything else break?

I don't know if a timer that can not set intervals greater than a second would be very useful and even 32 seconds for the 1kHz variant wouldn't be sufficient for many use cases.

All in all I think mode0 and @maximeborges's 32 bit timer implementation in rtic-rs/rtic#994 are most useful, but mode1 could be added for anyone who cannot spare the second timer or the RTC with the appropriate disclaimers in the documentation.

[kyp44]

So the mode1 monotonic does half-period counting using the convenient rtic_time::half_period_counter. Because of this, the monotonic counter is a u64 value, which extends it into the millions of years even with the faster clock. Yes, the actual 16-bit hardware counter wraps frequently, but the overall monotonic does not, because of the half period counting. Half-period counting requires two RTC compares (one for the half period at 0x8000 and one for the RTIC timers), and interrupts to handle the period counting on half-periods and counter overflows. For this reason, half-period counting was implemented for mode1 and not mode0 because all ATSAMD chips have at least two compares in mode0 but not all have two compares in mode1. Hence, the mode0 monotonic rollover corresponds with the 32-bit hardware counter rollover at only 36 hours (for the faster 32 kHz clock), which is a disadvantage relative to the mode1 monotonic with period counting.

Regarding the robustness issues, I first implemented the RTC monotonic using mode0 without period counting because it's simpler, which is basically the whole reason this monotonic even exists. Then I converted this to mode1 with period counting. To test the monotonics, I wrote a stress test for the PyGamer that spawns over 700 RTIC async tasks, all syncronized (just using async delays using the monotonic) to draw small colored boxes on the display every second at different locations. I initially had issues with both monotonics that I was able to fix. However, currently the mode1 monotonic still has robustness issues under these heavy conditions. Namely, after about 15 minutes (with the 32 kHz clock) or 40 minutes (with the 1 kHz clock) everything just freezes. I suspect this has something to do with the extra interrupts needed for period counting because, in contrast, I've had the mode0 monotonic running with the same stress test for over 24 hours without issues (at which point I just turned it off under the assumption that it would just go forever).

I very much would like to resolve this mode1 issue but it is difficult to troubleshoot for two main reasons:

1. The problem doesn't manifest for ~15 minutes, so every time I want to try something or look at something I have to wait ~15 minutes.
2. I do not have debugging hardware to actually see what's going on when it freezes, so I currently have no idea what's causing it to freeze.

I've been thinking about purchasing a debugger to be able to troubleshoot this, but haven't yet.

I have no problem keeping the monotonics in the HAL temporarily for experimentation / troubleshooting / enhancement and moving to rtic-monotonics later when its fully robust. The current implmentation only depends on the PAC though (atsamd variant infrastructure aside), and I don't think it makes sense to start adding actual HAL dependencies if it's going to move. My concern would also be that, if it's in a released version of the HAL, people could start depending on it, only to have it moved/removed later. Maybe this is not a concern since the HAL is not yet at a stable version 1.0.0.

EDIT: All ATSAMD chips have at least two compares in mode1 but not all have two compares in mode0. These are incorrectly reversed above, see subsequent comments.

[jbeaurivage]

Feel free to ignore this idea, but for the time being, the RTC monotonic impl could live in its own crate, owned by atsamd-rs, until it has been proven, after which we could port the code over to rtic-monotonics.

We'd just have to be very deliberate about the fact that this new crate would be a temporary solution, because temporary solutions have a nasty tendency to become the de facto permanent solution if not looked after.

[kyp44]

I like the idea in principle, however all the ATSAMD variant infrastructure is in the HAL, not the workspace, so we wouldn't be able to take advantage of that. Really though, I don't think duplicating that (either in a new crate in the atsamd workspace or in rtic-monotonics) is really that big of a deal. Would you intend to publish this temporary crate on crates.io, because that would seem like a sure-fire way to ensure that it becomes a de facto permanent solution? I'm guessing the intention is just to have it here locally for developers to work on and test before moving to rtic-monotonics for permanent publication.

EDIT: I also like this because it keeps it isolated, making it easier to move later IMO. For purposes of awareness, testing, and troubleshooting, it certainly makes more sense than just living as a branch in my fork like it does currently.

EDIT 2: Can anyone recommend debugger hardware that a) works well on ATSAMD chips and b) plays nicely with Rust on Linux?

[Sympatron]

Ok, now I get it. I think I looked at the wrong code while reviewing and got confused.

all ATSAMD chips have at least two compares in mode0 but not all have two compares in mode1

Just to be sure: You mean the other way round?

after about 15 minutes (with the 32 kHz clock) or 40 minutes (with the 1 kHz clock) everything just freezes.

I don't know why this might be either, but it should be fixed before it is being published. Your test might be unrealistic, but for now we don't know if it won't happen under less stress just after more time. If it happens after 15min under stress it might happen under normal circumstances after weeks or months which would be very bad in production and extremely hard to debug.

The problems you seemed to be having look like further testing is needed to confirm/prevent your findings.
The SAMD5x errata lists RTC problems, but only 2.17.2 is relevant here I think and this is already dealt with in _start(), so no luck there...

Does anyone have experience with the SAMD RTC? I have never really used the RTC interrupts so I can only speculate...

Regarding mode0: I still think overflowing an Instant is not expected by RTIC, especially if it's u64 and overflows at u32::MAX. With a 32 kHz Timer it will overflow after ~37.3 hours. Could you run your test for longer than that to confirm whether it works?

RTC monotonic impl could live in its own crate, owned by atsamd-rs

This will probably become a permanent temporary solution...

If it lives in the HAL it can easily be stripped later while bumping the minor version. The fact that it won't be available in the HAL forever can be documented, to reduce surprises in the future.

[kyp44]

Just to be sure: You mean the other way round?

You are correct, good catch! Added correcting edit note in the original comment.

Yeah, the issue about the counter being one tick behind in the interrupt handler is odd, and I did have a look at the errata before, as you have, and did not find anything about it. The behavior seems to be consistent though. It's possible that I'm not understanding how the counter syncing works (it's supposed to be automatic on the SAMx51x chips) so perhaps that's the issue. Independent verification of this behavior would be good.

I agree that the freezing issue should be addressed. It actually used to freeze sooner under that stress test (in about 90 seconds for the 1 kHz) clock. This I was able to troubleshoot successfully by moving some code around. In particular, RtcBackend::on_interrupt currently has two parts, the first that increments the half-period counter if needed, and the second that waits for the hardware counter to catch up (due to the quirk discussed above). The latter part used to be up in the RTC() ISR directly, which caused RTIC tasks to be popped off the TimerQueue, but then not be ready when polled, causing them to stall forever. The details of this are not well documented in the commit history, but you can see the code jump down to fix it. I can go into more detail on this if you like, but it was a subtle issue that I could never quite fully reproduce with 100% reliability, despite trying for hours to do so.

I suspect the current task stalling issue is something similarly subtle that manifests only when the timing is just right. Note that the stall time (e.g. the 15 minutes I gave for the 1 kHz clock) is not consistent, the numbers above are just averages. Just like the prior problem, this could very well be caused by the need to have to wait for the clock to catch up due to the above quirk that you pointed out. Based on the following, it seems clear that it is related to the extra interrupts needed for half-period counting:

1. It does not seem happen at all for the mode0 counter with no period counting.
2. It happens sooner for the faster clock. I suspect that this is because those extra interrupts are occurring more frequently (half-period incremented every second vs. every 32 seconds with the 1 kHz clock), and so there is a generally higher probability that the necessary timing conditions will happen.

Lastly, the mode0 monotonic Instant should be, a u32, not a u64. A u32 is not even capable of representing a time longer than about 36 hours (for the 32 kHz clock). It is currently a u64 only so I could easily swap out the monotonics in my stress test without needing to change anything in the stress test. In retrospect this was dumb and unnecessary. Since I think it make sense to keep both monotonics, I'm going to combine the branches to have two distinct monotonics with a feature in my stress test to switch between them.

It sounds like there is some disagreement about where to put the monotonics, even on a temporary basis. I keep waffling myself, but maybe the HAL would be better to make it usable for people, but with the documentation clearly stating that it will later be moved to rtic-monotonics, and also to disclose the current robustness issues. This seems less likely to become a permanent solution than having it as a separate crate and publishing it on crates.io. If the intention was not to publish the separate crate and just have it used internally, having it the HAL instead makes it more discoverable and usable by people in the meantime.

[kyp44]

Status update

In preparation for temporary publication in the HAL, I merged both monotonics into a new branch with a clean commit history, refactored a bit, and documented everything.

I also broke the stress-test PyGamer program out into a separate project, since it does not really have anything to do with the PyGamer game I am working on (i.e. the project it was a branch in before).

Remaining tasks:

• Investigate the mode 1 monotonic robustness issue further. I have an idea of how I can make it freeze faster for easier troubleshooting.
• Make sure both monotonics at least compile for all SAMD chips, correcting things where necessary. I briefly tried this before and it definitely did not compile for all chips, but I need to go back and revisit it.

[AfoHT]

Hi there! Thanks for reaching out in rtic-rs/rtic#995 :)

Exciting with monotonics / more RTIC v2 support in atsamd-hal.

We discussed this endeavor in RTIC's most recent weekly meeting, but tl;dr is that we're happy to help out with alpha/regular release(s) of rtic-monotonics to ease this work.
One approach could be to just flag/document atsamd support "experimental" while you still iron out the last wrinkles.

How does this align with the other PR for atsamd21?

[kyp44]

@AfoHT The other PR is for a timer-based monotonic, whereas this uses the RTC peripheral. For sure though they would share some general ATSAMD infrastructure in rtic-monotonics, e.g. chip selection features and being in the same high-level atsamd module, though it looks like the timer monotonic is specifically for the atsamd21. The aim with these RTC monotonics is to support all atsamd chips, but I can only test on the atsamd51j myself. At some point, I am also interesting in testing and/or adding atsamd51j support for the timer-based monotonic.

If there is a mechanism to mark the RTC monotonic as experimental in rtic-monotonics, I do not mind publishing it there from the get-go. As of now, it only exists in my atsamd-hal fork (i.e. it hasn't yet been merged into atsamd-hal) and there isn't even a PR for it yet. Somewhat inadvertently, I recently discovered some kind of major issue with both the RTC monotonics (i.e. mode 0 and mode 1) that I am trying to troubleshoot. My time is a bit limited at the moment so it's slow going, and I wouldn't feel comfortable publishing it anywhere until this issue is figured out. So there is time to further discuss where best to publish them when they are ready.

[sjroe]

Slightly related, I've been working on an implemenation of embassy-time-driver using the atsamd-hal RTC which is essentially embassys own version of RTICs monotonic.

@kyp44 I've come across the same issues that you are having with the counter lagging the compare interrupt and always stepping in 4. I have found that enabling the RTC prescaler to at least div4 rtc.mode0().ctrla().modify(|_, w| w.prescaler().div4()) resolved this issue. Obviously the downside to this is the tick frequency is dropped from 32kHz to 8kHz and 1024Hz to 128Hz.

[kyp44]

@sjroe Glad to hear this! I'm not sure how embassy time drivers compare with RTIC monotonics, but it would be great to be able to collaborate on this and maybe even have some code re-use if possible.

My next experiment was actually going to be to play around with the prescaler and see how that affects the counter jumping, though you just provided the answer! I'll confirm these results as well. I'm still testing things and dealing with a few other strange issues, but the way I'm currently solving the compare lag issue is to simply wait for the counter to change first in the interrupt handler before doing anything else. So far, this seems to suffice to ensure that the counter is at least the compare value (which is critical for RTIC in particular) before any other logic is done. In the past I had a more complicated solution, but simply always waiting is much more simple and straightforward.

What chip are you using to test with? You probably already read that I am using the PyGamer, which features the SAMD51J. I'd also invite you to stop over at the ATSAMD Matrix channel for live discussion. My time to work on this stuff is a bit limited lately, but I typically have that tab open when working on it.

[kyp44]

Status Update

@sjroe I observed the same behavior when I enabled the prescalar, which was the same in both mode0 and mode1 and for either clock rate. Dividing by 2 causes the count to increment by two each time, and diving by 4 or more then increments by one. Upon reading the Microchip documentation more closely, as best I can figure this ticking issue is due to the sync delay. According to this, the delay can be up to over 3 times the source clock period, which might explain why it seems to only sync every 4 ticks, though it does say that 2 clock periods is more typical. This also explains why enabling the prescalar improves this, since doing so increases the tick period but does not affect the source clock rate or increase the sync delay.

I have finally gotten both monotonics working robustly, with even the stress test using the 32 kHz clock and the mode1 mononotic running solidly for at least 12 hours straight. However, I want to refactor a bit so as to implement period counting in mode0 for supported chips, and update the documentation accordingly before publishing. Again, the updated monotonics are here if the code would be useful for your work @sjroe .

@AfoHT I would like to at least get these compiling for all chips while it still resides in the HAL, but I am only able to test them on the atsamd51j. I notice that that the proposed timer-based monotonic is only supported for the atsamd21 (e.g. it only even adds a crate feature for the atsamd21g, not for all chips). Does it make sense to also just enable the RTC monotonics for the atsamd51j until others can validate support on other chips? The alternative is to publish for all chips and let issues be filed if they do not work correctly on untested chips.

[AfoHT]

@AfoHT I would like to at least get these compiling for all chips while it still resides in the HAL, but I am only able to test them on the atsamd51j. I notice that that the proposed timer-based monotonic is only supported for the atsamd21 (e.g. it only even adds a crate feature for the atsamd21g, not for all chips). Does it make sense to also just enable the RTC monotonics for the atsamd51j until others can validate support on other chips? The alternative is to publish for all chips and let issues be filed if they do not work correctly on untested chips.

Sounds good to me, I think enabling others to test it out and report issues, just as you suggested in the "alternative approach", is the way to go. To compare, to my knowledge we have not tested every single stm32, every single nrf etc. 👍

[kyp44]

Want to solicit opinions from both HAL and RTIC folks. Though I cannot test it, I am adding SAMD11/21 support to these RTC monotonics, which is requiring only some pretty minor alternate compilation paths so far. However, I discovered that the RTC source clock is set in a completely different way for these chips. In particular, on the SAMx5x chips it is simply set to one of four values in a OSC32KCTRL register. However, on the SAMD11/21 chips one must setup a normal peripheral clock using the generic clock controller, which is more complicated.

This led me to do some research, and I noticed that:

• The Rtc abstraction in the HAL does enable the RTC clock (via the mask in the power manager) but does not set the clock source, assuming it to have already been set. It's worth noting that on the SAMx5x, the RTC is sourced from the internal 1 kHz clock on reset, but evidently the source is not automatically setup for anything on the SAMD11/21 chips.
• The monotonics I checked in rtic-monotonics (e.g. on the nrf chips) also do not set the RTC clock sources themselves unless they do so completely unsafely by stealing peripherals. Granted, I'm assuming that this family of chips have configuration RTC clock sources.

Currently these SAMD RTC monotonics do set up the RTC clock source, assuming the SAMx5x way of doing that. So I am asking if it makes more sense to likewise have these SAMD RTC monotonics not set the RTC clock sources, perhaps mentioning in their documentation that it needs to be configured ahead of time. This avoids re-inventing that wheel, especially in the more complicated case of the SAMD11/21 chips.

EDIT: I would actually pose the same question about the ensuring that the RTC clock is enabled via the power manager (SAMD11/21) or the main clock (SAMx5x), noting that, on reset, the RTC clock is already enabled for all chips. As an example of when this can be problematic, requiring an Mclk reference when setting up the monotonic on a SAMx5x prevents using the v2 of the clock API to first set the RTIC clock source and then start the monotonic.

TLDR: Should it be the RTC monotonics' responsibility to set and/or enable the RTC clock source?

[bradleyharden]

@kyp44, I haven't looked at atsamd-hal code in quite a while now, but I'd like to at least partially weigh in here.

Should it be the RTC monotonics' responsibility to set and/or enable the RTC clock source?

I would argue no. Generally speaking, I don't think libraries should set up clocks. That should be the user's responsibility. The obvious exception is if that library's explicit purpose is to set up clocks, but I don't think this case fits. The purpose of the monotonic implementation isn't to set up the clock, it's to implement a feature for RTIC. Instead, I think the implementation should require proof that you've set up a clock correctly. I think that's probably easy with clock::v2, but perhaps it's more difficult with v1. I don't remember.

[kyp44]

@bradleyharden I agree that simply requiring proof that the clock has been configured would be ideal. Looks like this is easy with v2 but that no corresponding proof type exists in v1 for the SAMx5x chips. A proof type does exist for the SAMD11/21 chips though (for which there is a only a single clock API). I chalk this situation up to the fact that the RTC source is just another generic clock on the SAMD11/21 but is a bit of a special animal on the SAMx5x. Additionally it seems like, with v1 (on SAMx5x) you actually can only set the RTC clock to be the 1 kHz internal clock or the 32 kHz external clock. So for example, there is no way to select the internal 32 kHz clock. For these reasons, if we want to require proof, I would recommend requiring v2 on SAMx5x platforms.

In both cases there is also the issue that the clock APIs make the RTC clock frequency available only at run time, whereas the monotonics need it at compile time, so the end user would have to specify this anyway when setting up the monotonic.

The last issue is there was some uneasiness about having rtic-monotonics depend on the HAL both from the HAL folks (stability concerns since the HAL is not yet at v1.0.0), and based on the fact that the Monotonics there typically depend only on PAC crates but not HAL crates. Perhaps an exception to this convention ought to be made here for the sake of the "if it compiles it should work" philosophy.

[kyp44]

Status update

I am confident that the montonics are now ready for prime time. The robustness issues have all been resolved, and I refactored things so that the RTC mode and the monotonic type (basic or half-period counting) are decoupled. This enables using half-period counting in mode 0 for SAMx5x chips that have two compares in this mode, which it now does. Everything has been documented.

I am ready to migrate them to rtic-monotonics, but I am still not sure how exactly to handle the RTC clock source. Currently no proof is required but the documentation makes it clear that the RTC clock needs to be set up before starting the monotonic. On the one hand I would like to be able to require proof, but this is only possible using the clock v2 API, which is not really supported throughout the rest of the HAL as has been discussed, making it difficult to use. On the other hand, I am a little hesitant to publish the monotonics in rtic-monotonics because this crate is at a stable version (currently v2.0.3), and requiring proof later would be a breaking change. There are smaller clocking v2 efforts going on here and there, but it seems like a more holistic approach of porting v2 to the SAMD11/21 chips then migrating the entire HAL would be better.

Another approach might be to temporarily release/merge the montonics into the HAL so they can start being used, as we were initially planning to do, and then move them to rtic-monotonics once the full clocking v2 migration is complete and they can be fully stable.

I also tested the HAL Rtc abstraction on the PyGamer, and it does not seem to work correctly, so this is something that I may tackle next. It's worth noting that my abstraction of the RTC modes is general and low-level enough that, perhaps with some modifications, it could serve as a basis for all ATSAMD RTC applications such as the HAL Rtc, these RTC montonics, and the embassy time driver that @sjroe is working on. I'll add that these mode abstractions handle all register synchronizing and chip variants. This would require publishing this as a crate though, and I don't know whether that makes sense.

Lastly, currently mode0 and mode1 monotonics are made available for all chip variants. However, the mode0 monotonic may not be very useful on SAMD11/21 platforms as it rolls over after only 36 hours (when using the 32 kHz clock), violating the monotonic contract. Conversely, the mode1 monotonic offers nothing on SAMx5x platforms because half-period counting can be used in mode0, resulting in much less frequent extraneous interrupts. See the atsamd_hal::rtc::rtic documentation for more details. I wonder whether it makes sense to always provide both modes and allow to the end user to make poor choices, or to only provide the best choice for each variant.

RFC

Requesting comments for:

• The release strategy for the monotonics given the RTC clock source issue
• Whether it makes sense to publish a low-level RTC abstraction crate that handles all chip variants and register syncing, and that can be used for all RTC applications.
• Whether to provide monotonics that are not great choices given the chip variant.

[jbeaurivage]

Great work! While I don't have complete answers to your questions, I do have some thoughts that may be useful.

The release strategy for the monotonics given the RTC clock source issue

That is definitely tricky. I think it would make sense for rtic-monotonics to require proof of a correct clock setup, although that means that it now depends on atsamd-hal. I don't think rtic-monotonics has historically depended on a HAL directly; maybe we could check with their maintainers to see if it's something they wish to support. One thing to watch out for is that our hal is still being heavily iterated on, and we publish breaking changes fairly regularly. While not ideal, this can somewhat be worked around.

Whether it makes sense to publish a low-level RTC abstraction crate that handles all chip variants and register syncing, and that can be used for all RTC applications.

Absolutely, I don't see a downside to that.

Whether to provide monotonics that are not great choices given the chip variant.

The way I see rtic-monotonics, and I'm happy to be corrected if I'm wrong, is that it provides an easy to use and opinionated way of getting a global timer working. There's something to be said about limiting the options and providing a sane set of defaults; users who have more specialized requirements can dive deeper in the specifics of the different peripherals.

[kyp44]

@jbeaurivage Thanks!

Regarding the release issue, it is definitely the case that rtic-monotonics does not currently depend on any HAL crates, but do depend on PAC crates. It seems that monotonics for other chip families in rtic-monotonics do have safety in that starting the monotonic typically consumes the PAC peripheral object. However, in cases where other things need to be setup first (e.g. clock sources), proof is typically not typically required. As a specific example, consider the RTC-based monotonic for nRF chips.
The RTC on these chips requires that the LFCLK is setup and enabled in order for the RTC to work, noting that the start method of the monotonic only requires the appropriate RTC peripheral from the PAC. This start method could also require a reference to a Clocks<H, L, LfOscStarted> to better ensure that the clock has been started, but this is not done, perhaps to avoid a HAL dependency.

Regarding the RTC abstraction crate, I'm going to move forward with this as well as work on fixing Rtc and have it use the abstraction crate.

I just posted a comment here in the hopes of getting some feeback on the above issue was well as limiting monotonic options.

[jbeaurivage]

Following a very productive conversation with the RTIC team during their weekly meeting (minutes), it was decided that the better course of action is to support the monotonics drivers directly in atsamd-hal. For better visibility, the rtic-monotonics will redirect ATSAMD users to our HAL.

I think @AfoHT summarized well the essence of what rtic-monotonics tries to achieve:

It's great with options, and I can see this hybrid approach working well in that regard
For HALs that are willing to implement the RTIC specifics, gets better integration etc., the plumbing for clock among other things
Those who do not, rtic-monotonics can bridge the gap until they do xD

With that in mind, I suggested that @kyp44 open a PR here with his work. We'll have a chance to review and merge in a timely manner. Also, this gives us better control of when we want to integrate clock::v2 into the mix.

We've also discussed on Matrix that the same course of action should be followed for integration of embassy-time drivers, which @sjroe is currently working on. Therefore, there's no need for a separate crate that abstracts away the RTC specifics. It can all be contained in atsamd-hal, thus greatly reducing the complexity.