RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.93k stars 1.99k forks source link

ztimer and Power Management #16327

Closed jue89 closed 1 year ago

jue89 commented 3 years ago

Description

I'm the initial author of ztimer's interaction with pm_layered (#13722). Basically, it works like this: For each ztimer_clock_t, a minimum required PM mode can be defined. If so, that PM mode is blocked, once a ztimer_t is added to the ztimer_clock_t and unblocked, once all ztimer_t have expired. This works in my applications for more than a year, but I came to the conclusion that this approach isn't a clean solution.

Long story short: ztimer interacts with pm_layered to ensure that the underlying ztimerperiph[timer|rtt|rtc|ptp] doesn't stall. In my opinion, this should be done by the timer/rtt/rtc/ptp driver. Instead of blocking and unblocking certain PM modes, ztimer should call timer_start()/timer_stop(), rtt_poweron()/rtt_poweroff(), ... The respective driver should be in charge of blocking and unblocking PM modes.

Would you agree, that this is at least a cleaner solution?

Before any change can happen, we have some homework left to do:

I'd love to hear your opinion on this!

Useful links

Original PR: #13722

kaspar030 commented 3 years ago

Would you agree, that this is at least a cleaner solution?

Yes it is! We should work towards that. Though let's not underestimate the simplicity of the current solution.

This means a simple ztimer_now() can't be used for time measurement anymore.

Probably this needs solving on the API level as first step?

kfessel commented 3 years ago

Turing off the underlying peripheral will stall the respective ztimer_clock_t

maybe some other system is need to keep track of time (not of alerts) while xtimer had absolute time (ticks) ztimer has relative time (since the last overflow) and some additions to remove that relative overflow semantic (extended)

jue89 commented 3 years ago

Yeah, but if used correctly, ztimer_now(ZTIMER_USEC) can be used to measure (short) time differences. (We need some acquire and release function to tell ztimer not to turn off the peripheral while measuring.) ztimer_now(ZTIMER_MSEC) can be used as a device uptime clock - as the underlying periph_rtt is likely to use less energy. It will keep running with a 1kHz clock, not matter if ztimer_ts has been added. Or did I get you wrong?

kfessel commented 3 years ago

you are right in the energy point ztimer_msec is very low power (~1 µA for stm32l0). but with 32Bit MSEC is only absolute to 42days. I think a generic clock should be build on top of ztimer that would be able to provide {uint64_t sec;uint64_t ns} that adapts to the current power level

if ptp (nanosec) is running use it 
else if usec is running  use it 
else if msec is running use it
else if there is any sec source use it and try to keep it running

and if one asks for now he get a best effort answer in {uint64_t sec;uint64_t ns}

jue89 commented 3 years ago

If you add the module ztimer_now64, you'll get a uint64_t now value for every ztimer clock. I'd rather stick to the initial concept of ztimer. (Which doesn't mean another module may offer an interface, you've described!)

jue89 commented 2 years ago

This issue got a little bit dusty. But I'm picking it up again.

Due to the chip shortage we are forced to switch MCU vendors. Thankfully, RIOT's driver system and its hardware abstraction allow us to do that! :-) To be more precise, we are switching from the samr30 to the efm32 family.

You may ask: How is that related to ztimer? The samr30 uses a periph_timer implementation that doesn't care about pm_layered at all, whereas the periph_timer of the efm32 does! This creates the following problem with the efm32 family:

The dirty quick-fix is of course just to unblock the PM mode, that periph_timer has blocked during init, and let ztimer handle the power management.

The more cleaner solution would be to remove the power management code from periph_timer - but that feels wrong to me.

The cleanest solution would be to make ztimer start and stop the underlying periph_timer and make periph_timer handling all the power management. I'd like to put some effort into this.

To get there, some decisions have to be made:

  1. How to extend the ztimer_ops_t interface? I guess, the trivial solution would be to add an on() and off() method. This enables ztimer to turn the backing ztimer_periph_* resp. ztimer_convert_* on and off. The backend can be described with the following state diagram. Of course, not every backend must implement these on() and off() methods. states

  2. How to extend the user-facing API of ztimer? I would add ztimer_acquire() and ztimer_release() to the public API. (A ztimer_set call will call them, as well.) This allows the user to tell ztimer if timers are expected to be running. This may come in handy if ztimer_now() is used for measuring time distances.

Are there any thoughts on this?

jue89 commented 2 years ago

16891 is also experiencing the problem I've described above.

jue89 commented 2 years ago

@kaspar030 @chrysn @kfessel had a little chat on RIOT Matrix. Little recap - please add details if I missed an important one!

ztimer_now() creates problems if underlying clocks are turned off. @kaspar030 suggested:

Maybe it makes sense to make the current ztimer_now() an internal function, and only provide now() on an ztimer_clock_handle_t (which keeps track of acquired state), and which can only be created through ztimer_acquire()? Or is that too much type system thinking?

@kfessel suggested to add a prove that you got a running timer on the clock:

i like the idea of providing prove that that u know ztimer is running for external ztimer_now use eg by setting up a timer which by definition keeps a ztimer clock running until it expires

Both suggestions are solving the same problem.

I suggested the following:

Re making ztimer_now() internal: I'd suggest to bring in ztimer_acquire() and ztimer_release() into RIOT as a first step (with all the required mechanics). ztimer_ondemand will become opt-in and no harm is made if ztimer_ondemand isn't compiled in. In the next step we can deprecate ztimer_now(), ztimer_acquire() and ztimer_release() as external interfaces and offer an interface for working with ztimer clock's now values. The third step is to migrate all users of ztimer_now().

chrysn commented 2 years ago

One point I'd like to add is that for deprecating the publicity of ztimer_now or moving people off it, we'll need alternatives; ztimer_until (#17229) is IMO one of the more promising ones.

Ad "type system thinking", I think it's going a bit too far, especially because while "gotta have a handle to read now()" is certainly is a valid restriction, the much more relevant one is to have "the same" (or a continuous) acquisition on both of two compared timers, and checking that would come at a runtime cost (while still not having the precise semantics of the type system; the best I can imagine is that the timestamps store not only their u32 but also the clock pointer in hopes that the clock is immortal, and the "how many times have I stopped" counter of the clock which it'd need to track all the time). I'd leave that to languages with type systems that can express these things actually in the type (and am not sure I know any).