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

core: API: RTC interface should not use struct tm #2538

Closed
kaspar030 opened this issue Mar 5, 2015 · 8 comments
Closed

core: API: RTC interface should not use struct tm #2538

kaspar030 opened this issue Mar 5, 2015 · 8 comments
Assignees
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: stale State: The issue / PR has no activity for >185 days

Comments

@kaspar030
Copy link
Contributor

While collecting implementation ideas for a new timer subsystem, I stumbled about the fact that our real time clock interface can only be used using struct tm time representation.

While that might sound natural for a RTC, it seems inefficient:

  • struct tm is defined using at least 9 integers in newlib (-> 36bytes), where 8 would be enough for any conceivable use case if RTCs would be used not in calendar mode, but in counting mode (just counting seconds).

Also, while inefficient, it is very easy to convert an epoch (or something similar) to struct tm, should it be needed for presenting a date to the user.

I propose changing the low level interface to work with a single integer representing (epoch) seconds, maybe keeping the struct tm functions for convenience and backwards compatibility.

This would essentially reduce the RTCs to timers counting seconds, but we'd be able to use their capability of waking up MCUs from deep power down modes.

A quick survey of our rtc implementations and the capabilities of RTCs :
cc430: uses calendar mode. chip offers counting mode, but without alarm. can probably be easily worked around
lpc2387: only calendar mode capable
native: uses posix system calls, so it's already epoch based
sam3x8e: using calendar mode, but MCU offers a real time timer (RTT) matching counter mode with alarm.
samd21: using calendar mode, but RTC offers counter mode with alarm
kinetis: already uses counting mode

I say, let's keep the notion of days, years, (leap years) in software.
What do you think?

@kaspar030 kaspar030 added Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Mar 5, 2015
@jnohlgard
Copy link
Member

For all except lpc2387 we could just implement RTT and use the RTT->RTC wrapper layer used by Kinetis to provide RTC functionality. At least for now until a timer task force is active.

@kaspar030
Copy link
Contributor Author

@gebart So you'd approve of using RTT as time-keeping source and provide Calendar functionality in software?

@haukepetersen What's your take on this? IMHO this way we have the perfect "slow timer"?

@jnohlgard
Copy link
Member

@kaspar030 yes, I approve.

Additionally I think it's quite cumbersome to use a hardware clock split up into fields like the calendar RTCs are.

@OlegHahm OlegHahm self-assigned this Mar 5, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Mar 5, 2015

So, you're proposing to implement the counter mode and do the transformation to struct tm in software for all platforms that do not have already something like a RTT?

Keep in mind that a lot of upper layer stuff work on struct tm, so in many cases it will have to be transformed anyway.

@kaspar030
Copy link
Contributor Author

To get this to a solution:

  • most MCU's have a hardware module that can be used either in RTT or RTC mode.
  • the RTC interface is intrinsically bloated as the struct needed to supply year, month, hr, second, ... is either very large or needs a lot of code for packing/unpacking or both
  • an RTT interface is usually a lot simpler to implement (it takes just un integer instead of complex struct tm -> register conversions)
  • an RTC-like interface (e.g., something using struct tm) can easily be implemented on top of RTT (see kinetis/rtc: move to periph_common? #4565), in a generic way (shared code across all platforms)
  • not all use cases need RTC mode, so always having the bloated interface is wasteful

So I propose:

  • use RTx module in counter mode wherever possible, with RTC functionality added by soft-RTC
  • if RTx doesn't support counter mode, implementing the RTC interface is OK

@kaspar030
Copy link
Contributor Author

Keep in mind that a lot of upper layer stuff work on struct tm, so in many cases it will have to be transformed anyway.

If we wrote that upper layer stuff, we should re-evaluate if it wouldn't be simpler to just use RTT.
If that is external code, the bloat should be moved as high up as possible.

I've seen code that want's to sleep one day. So instead of sleeping USEC_PER_DAY microseconds, the code used C library code to break down that number of microseconds into struct tm, hands it to the rtc interface, which then writes it to the registers needed by the hardware. Horribly inefficient both code, memory and cycle wise.

@stale
Copy link

stale bot commented Aug 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019
@kaspar030
Copy link
Contributor Author

One of these "X should not do Y" that don't hurt enough... Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. State: stale State: The issue / PR has no activity for >185 days
Projects
None yet
Development

No branches or pull requests

3 participants