Skip to content

Feat nrf ticks support #6171

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

Closed
wants to merge 12 commits into from
Closed

Conversation

hoihu
Copy link
Contributor

@hoihu hoihu commented Jun 18, 2020

This is a follow up / replacement of #5470

It supports time.ticks_ms(x), time.ticks_add(x) and time.ticks_diff(x) for two configurable timebases:

  1. Ticker timebase (1MHz), precise, app 5uA, 32bits
  2. RTC timebase (32kHz), less precise, app 0.5uA, 24bits

both are capabale of running in WFI low power mode (in contrast to the SYSTICK module)
see above link for further discussions / insights / thoughts).

@hoihu hoihu force-pushed the feat-nrf-ticks-support branch from 8a7eada to 9b8df90 Compare June 18, 2020 20:46
hoihu added 2 commits June 18, 2020 22:56
The time tick support is enabled with the MICROPY_PY_TIME_TICKS
compiler switch.

By default it uses the Ticker's 1MHz clock to schedule a CCR0
callback in 1msec intervals (MICROPY_PY_TIME_USE_TICKER_BASE)
Using the 1MHz clock typically consumes app. 5uA.
It's a 32bit counter and overflows every 50 days

For very power sensitive applications the RTC can also be used
as timebase using the MICROPY_PY_TIME_USE_RTC_BASE compiler switch.
In this mode, an error of app 0.7% is introduced due to the granularity
of the 32kHz ticks vs 1msec ticks. The RTC clock consumes app. 0.5uA.
Since it's a 24bit counter, overflows occurs every 4h.

Both modes are operational in WFI power save mode, in contrast
to the SYSTICK module which switches off.
@hoihu hoihu force-pushed the feat-nrf-ticks-support branch from 9b8df90 to aea3a8e Compare June 18, 2020 21:00
@hoihu
Copy link
Contributor Author

hoihu commented Jun 20, 2020

And thinking on this a bit further...

With the proposal of using the RTC as by @dpgeorge , the advantage of using the Ticker's timebase is almost neglectable IMO. Actually the RTC will be more precise for longer intervals this way than the ticker's 1MHz because it is sourced by the high precision 32kHz crystal (if it is present..).

So I wonder if we should use CCR0 of the ticker for the usec timebase, and make the RTC the default and only implementation for the msec timebase?

Or we invest CCR1 to support the usec timebase and leave the rest configurable as is (RTC or ticker based msec timebase).

this approach uses a prescaler of 0 and handles the overflow
event in an irq
@hoihu
Copy link
Contributor Author

hoihu commented Jun 20, 2020

Ok so I've pushed an update on how it could look with RTC overflow bit handling. It doesn't add much code.

It works on my PCA10040 devkit (running more than 512 sec didn't overflow).

@dpgeorge
Copy link
Member

Actually the RTC will be more precise for longer intervals this way than the ticker's 1MHz because it is sourced by the high precision 32kHz crystal (if it is present..).

So I wonder if we should use CCR0 of the ticker for the usec timebase, and make the RTC the default and only implementation for the msec timebase?

I think this is a good approach, to have RTC1 as the only implementation of msec.

And in fact it can also be used for usec, just with a ~30us resolution. Simply return RTC1->COUNTER * 1000000 / 32768. For low-power applications I think 30us resolution is pretty good considering it has minimal overhead, simple implementation, and no additional resource usage.

In summary so far, if only for my own understanding of the situation:

  • the nrf port doesn't have ticks_ms or ticks_us
  • SysTick could be used to implement these in the same way it's done on stm32, except that SysTick on nrf doesn't run during a WFI
  • instead RTC is a good choice, it's essentially a low-power replacement for SysTick which runs during sleep
  • RTC0 is used by soft-device, not all MCUs have RTC2, so use RTC1 (dedicate it to internal timing, not for user use)
  • set RTC1->PRESCALER=0 to get maximum resolution in COUNTER, implement overflow to get more than 24-bit precision, and use this to construct a millisecond counter for ticks_ms
  • can also construct a microsecond counter for ticks_us with 30us resolution, at no extra cost
  • could use ticker's CCR0/1 for a 1us-resolution ticks_us, at extra power cost and usage of ticker peripheral (IMO this is not worth doing at this stage)
  • could eventually use RTC1's CCRs to implement an efficient softtimer for machine.Timer(), following stm32's approach (but that's out of scope of this PR)

@hoihu
Copy link
Contributor Author

hoihu commented Jun 21, 2020

yes that is the situation from my point of view too.

I‘ll cleanup the PR accordingly: add the usec support using RTC, remove the tickers msec support.

should there be a check in the rtccounter code that will prevent using RTC1 when it is used by the time module, raising a error?

@dpgeorge
Copy link
Member

should there be a check in the rtccounter code that will prevent using RTC1 when it is used by the time module, raising a error?

On the one hand it might be useful to let the user inspect RTC1 and maybe configure it if they know what they're doing (even when it's used by the time module). But OTOH it will be very confusing for those that don't know RTC1 is used by the time module, when they change it and all the time functions start behaving strangely. So I'd say raise an exception.

@glennrub
Copy link
Contributor

@hoihu, As i mentioned in the earlier PR, i was hunting for a timer replacement for nrf91 sockets. I have tested the current code, and it works very well :) Thanks a lot for doing this!

@hoihu
Copy link
Contributor Author

hoihu commented Jun 22, 2020

So this is the work in progres so far, not yet cleaned up and not handling the overflow condition within mp_hal_ticks_ms(). Couple of things I've pointed out, in mphalconfigport.c as comments.

also revert ticker's changes and SOFT_PWM settings
guard mp_hal_ticks_ms() against preempting of RTC overflow irq
@glennrub
Copy link
Contributor

sigh sorry for coming back to the subject, but I think this doesn't work either since mp_hal_ticks_ms can be preempted by the RTC irq.

Could it be an option to handle counter read in IRQ context as well, a bit like it is done in machine/rtcounter.c? Even if not exactly the same implementation, you can get both the Compare match event and Overflow event in the same nrfx-hal interrupt loop. Then use the counter value and overflow information to update a shared volatile variable with mp_hal_ticks_ms.

@hoihu
Copy link
Contributor Author

hoihu commented Jun 24, 2020

Could it be an option to handle counter read in IRQ context as well, a bit like it is done in machine/rtcounter.c?

Maybe.. But how can we trigger the irq (e.g. from mp_hal_ticks_ms())? Is there a way of manually fire a RTC irq? And there are still race conditions left I believe.. since the overflow event is still an IRQ & If at the same time mp_hal_ticks_ms() has fired it's irq requests they may conflict.

and add MICROPY_EVENT_POLL_HOOK support by setting rtc to app fire 1msec irqs
rewrite mp_hal_delay_us to use ticks_us and mp_hal_delay_ms to use ticks_ms
mp_hal_delay_ms is using MICROPY_EVENT_POLL_HOOK
@hoihu
Copy link
Contributor Author

hoihu commented Jun 25, 2020

Ok thanks @dpgeorge and @glennrub for all your inputs!

I've added in addition the changes to mp_hal_delay_ms and mp_hal_delay_us. mp_hal_delay_ms now introduces MICROPY_EVENT_POLL_HOOK support with WFI. In order to leave WFI I've setup a irq for CC1 that gets called every 1msec (and not handled, but it wakes up the CPU). This is then the foundation on which select.polland asyncio should work. I think it makes a lot of sense to introduce these changes in the PR too, but let me know your opinion about it.

@dpgeorge
Copy link
Member

mp_hal_delay_ms now introduces MICROPY_EVENT_POLL_HOOK support with WFI. In order to leave WFI I've setup a irq for CC1 that gets called every 1msec (and not handled, but it wakes up the CPU). This is then the foundation on which select.polland asyncio should work.

That's OK for now. Eventually the plan is to move to something more event based so there's not a constant polling (see #6110) but for now a WFI with wake-up is OK.

@glennrub
Copy link
Contributor

CC1 that gets called every 1msec (and not handled, but it wakes up the CPU).

You need to handle the compare0 event, to set a new CC. I'll point it out in the code. When doing that, the interval measures 0.9276 ms on logic analyzer.

@hoihu
Copy link
Contributor Author

hoihu commented Jun 26, 2020

pushed an update

  • use macros for common "guarding" code of RTC overflow (as by review @dpgeorge)
  • fixed a bug with the CCR handling (now it correctly uses CCR0 and ~1msec intervals for wakeup)
  • reintroduce legacy delay functions if tick support is not enabled
  • some minor cleanup

I tested this always with the PCA10040 btw.
If that looks ok, I'll start cleanup the commits. Looking forward for feedback!

hoihu added a commit to hoihu/micropython that referenced this pull request Jun 27, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
hoihu added a commit to hoihu/micropython that referenced this pull request Jun 27, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
@hoihu hoihu mentioned this pull request Jun 27, 2020
@hoihu
Copy link
Contributor Author

hoihu commented Jun 27, 2020

I've placed #6202 for a cleaned up version.

hoihu added a commit to hoihu/micropython that referenced this pull request Jun 27, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
hoihu added a commit to hoihu/micropython that referenced this pull request Jun 27, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
@tannewt
Copy link

tannewt commented Jun 30, 2020

FYI, I switched CircuitPython to the RTC (from SysTick) on our supported ports as well. In cases where we need higher precision (such as PulseIn) we use a separate timer.

@hoihu
Copy link
Contributor Author

hoihu commented Jun 30, 2020

FYI, I switched CircuitPython to the RTC (from SysTick) on our supported ports as well. In cases where we need higher precision (such as PulseIn) we use a separate timer.

Good to know, thanks for the information.

hoihu added a commit to hoihu/micropython that referenced this pull request Jul 2, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
glennrub pushed a commit to glennrub/micropython that referenced this pull request Jul 4, 2020
Add time.ticks_ms / time.ticks_us support using RTC1 as timebase.
Also add the time.ticks_add resp. time.ticks_diff helper functions.

This feature can be enabled using MICROPY_PY_TIME_TICKS. If disabled
the system uses the legacy sleep methods.

In addition support for MICROPY_EVENT_POLL_HOOK was added
to the time.sleep_ms(x) function, making this function more power
efficient and allows support for select.poll / asyncio. To support
this, the RTC's CCR0 was used to schedule a ~1msec event to wakeup
the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are app. 30usec, time.ticks_us
is not perfect, but moreless usable. For tighter measurments, the
ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an irq with
higher prio than the RTC overflow irq (3). If so, it introduces
a race condition and possibly leads to wrong tick calculations.

- See also micropython#6171
for further informations.
dpgeorge pushed a commit that referenced this pull request Jul 8, 2020
This commit adds time.ticks_ms/us support using RTC1 as the timebase.  It
also adds the time.ticks_add/diff helper functions.  This feature can be
enabled using MICROPY_PY_TIME_TICKS.  If disabled the system uses the
legacy sleep methods and does not have any ticks functions.

In addition support for MICROPY_EVENT_POLL_HOOK was added to the
time.sleep_ms(x) function, making this function more power efficient and
allows support for select.poll/asyncio.  To support this, the RTC's CCR0
was used to schedule a ~1msec event to wakeup the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are approx 30usec, time.ticks_us is
not perfect, does not have 1us resolution, but is otherwise quite usable.
For tighter measurments the ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an IRQ with higher prio than
the RTC overflow irq (3).  If so it introduces a race condition and
possibly leads to wrong tick calculations.

See #6171 and #6202.
@dpgeorge
Copy link
Member

dpgeorge commented Jul 8, 2020

I'll close this because it's superseded by the implementation in 15574cd

@dpgeorge dpgeorge closed this Jul 8, 2020
amirgon pushed a commit to lvgl/lv_micropython that referenced this pull request Sep 4, 2020
This commit adds time.ticks_ms/us support using RTC1 as the timebase.  It
also adds the time.ticks_add/diff helper functions.  This feature can be
enabled using MICROPY_PY_TIME_TICKS.  If disabled the system uses the
legacy sleep methods and does not have any ticks functions.

In addition support for MICROPY_EVENT_POLL_HOOK was added to the
time.sleep_ms(x) function, making this function more power efficient and
allows support for select.poll/asyncio.  To support this, the RTC's CCR0
was used to schedule a ~1msec event to wakeup the CPU.

Some important notes about the RTC timebase:

- Since the granularity of RTC1's ticks are approx 30usec, time.ticks_us is
not perfect, does not have 1us resolution, but is otherwise quite usable.
For tighter measurments the ticker's 1MHz counter should be used.

- time.ticks_ms(x) should *not* be called in an IRQ with higher prio than
the RTC overflow irq (3).  If so it introduces a race condition and
possibly leads to wrong tick calculations.

See micropython#6171 and micropython#6202.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants