RFC-RTC

[brentward]

Hi everyone, I'm working on the RTC for the kernel. I am sharing my plan here for comment.
rp6502 0.1+rtc1 release: https://github.com/brentward/rp6502/releases/tag/v0.1%2Brtc1
rp6502-sdk 0.1+rtc1 release: https://github.com/brentward/rp6502-sdk/releases/tag/v0.1%2Brtc1

This change includes a driver for the RTC and implementations of the POSIX clock functions in the SDK.

The purpose of the driver is to add kernel APIs for enabling/setting the RTC and getting the time from the RTC, and to implement the get_fattime() function that FatFs requires to add timestamps to the file system. The driver will not activate the RTC until a time is set and FatFs will use the current default values if the RTC is not set. The driver APIs accept and receive 32-bit timestamps so they can be passed in and out of the kernel with the AXSreg rather than manipulating the stack directly. The time is set on the RTC without including timezone information, so the time defaults to UTC.

The implementation of the POSIX functions ignores the clockid_t parameter, which is part of the prototype. I did this because the implementaion of time_t __fastcall__ time (time_t* t) that CC65 uses does not pass a clock_id parameter into get_time (see https://github.com/cc65/cc65/blob/master/libsrc/common/time.s#L28). This causes calls to time() to fail if the clock_id paramater is validated to match CLOCK_REALTIME; this is contrary to the documentation which says it should only succeed if it does match CLOCK_REALTIME. Because of this I chose to ignore clock_id in all of the POSIX clock prototypes.

The API calls and the POSIX functions both use the _mappederrno() function to set the errno to standard POSIX error codes and set the _oserror to new error codes specific to the RTC, which will indicate the RTC not being set and invalid datetime formats.

I chose to not set a time zone on the RIA so the timestamps returned could be in UTC and a time zone can be set by updating the _tz variable that CC65 uses.

I would appreciate any feedback on the design. In particular I'd like feedback on the choice to ignore the clock_id, how the error responses should behave, how time zones are handled, and any ideas on a system for indicating that the RTC is waiting for a NTP response to set the time.

I've written some utilities that utilize the optimized calls, the POSIX wrappers to the calls, and the C time functions:
https://github.com/brentward/rp6502-rtc/releases/tag/v0.1 (use https://github.com/brentward/rp6502-sdk/releases/tag/v0.1%2Brtc1 for the SDK when building)

RTC Clock Driver

Typedefs

unsigned long time_t

Kernal API calls

API calls are setup to be similar to POSIX calls but are optimized to only require the axsreg rather than using the stack.

$14 clock_gettime_

int clock_gettime_(time_t* timep)

Reads the RTC and writes it as a timestamp to timep.

Parameters: timep: Destination for the returned time_t value.
Returns: On success 0 is returned. On error -1 is returned and errno and oserror are set to indicate the error.
Errno: EINVAL
Oserror: RTC_NOT_SET

$15 clock_settime_

int clock_settime_(const time_t* timep)

Turns on the RTC if not already active and sets time to the timestamp in timep

Parameters: timep: Location of the timestamp used to set the RTC.
Returns: On success 0 is returned. On error -1 is returned and errno and oserror are set to indicate the error.
ASX regs: timep
Errno: EINVAL
Oserror: RTC_INVALID_DATETIME

Kernel events

Typedefs

uint32_t DWORD

get_fattime

DWORD get_fattime (void)

The get_fattime function shall return any valid time even if the system does not support a real time clock. If a zero is returned, the file will not have a valid timestamp. If the RTC is not running it will return the default NORTC values.

Returns: Always returns a valid timestamp packed into DWORD

POSIX wrapper

Wrappers to standard POSIX calls are implemented to enable the ISO C time functions located in time.h.

clock_getres

int __fastcall__ clock_getres (clockid_t clock_id, struct timespec *res)

Determine the realtime clock resolution. As implemented it ignores clock_id. Currently a non 0 response is not possible.

Parameters: *res: Destination for the returned timespec value of the clock resolution
Returns: On success 0 is returned.

clock_gettime

int __fastcall__ clock_gettime (clockid_t clock_id, struct timespec *tp)

Get the time from the real time clock.

Parameters: *tp: Destination for the returned timespec value of the current time in RTC
Returns: On success 0 is returned. On error -1 is returned and errno and oserror are set to indicate the error.
Errno: EINVAL
Oserror: RTC_NOT_SET

clock_settime

int __fastcall__ clock_settime (clockid_t clock_id, const struct timespec *tp)

Set the time on the real time clock

Parameters: *tp: Source of the timespec containing the current time to set the RTC
Returns: On success 0 is returned. On error -1 is returned and errno and oserror are set to indicate the error.
Errno: EINVAL
Oserror: RTC_INVALID_DATETIME

[rumbledethumps]

This seem about right.

Since the clock_id can be passed on a short stack, and an empty short stack is a zero aka CLOCK_REALTIME, there's no reason to avoid implementing the full POSIX-y. POSIX is the direction I'll always try to take. Returning a full timespec with a tv_nsec set to 0 is fine.

Did you look into timezones? I expect timezone to be a RIA setting one day. Don't want this to get weird, so it deserves some thought now.

If the xstack seems daunting, hang tight and watch the upcoming API improvements.

[brentward]

I agree that it should be as close as possible to POSIX. Once the API improvements are available I will make a revision to directly implement the POSIX calls rather than using a wrapper.

I did look into time zones and I did try a few things with it. I have some thoughts on implementation.

I propose to have the time stored in the RIA's RTC in UTC and have the clock_gettime respond with UTC. Timezone info could be stored on the RIA and settz() could be implemented as an API call which would set the _tz variable that CC65 uses so calls to localtime() would work. We could also implement an API call to set the time zone, which could be stored as a setting in the config.sys file. If we can, we should implement the time zone database. This method of time zones is compatible with my current version.

I did play around with setting the time zone on the RIA and doing conversion to local time there, but I couldn't figure out a way to use the conversion there since the POSIX standard is for CLOCK_REALTIME to be in UTC.

Once there is more feedback on how time zones should be implemented I can work on a revision that incorporates that.

[rumbledethumps]

Once I allow settings be changed by applications, the kernel complexity goes up and process isolation goes down. I think the only thing needed is a call to clock_gettimezone(CLOCK_REALTIME, &_tz) in a constructor. That makes four kernel calls to fully support time.h and we'll have an ABI as stable as CC65 time internals. For now, clock_gettimezone can return a constant UTC. This is a great demarcation because the tzinfo project will "only" have to implement clock_gettimezone on the Pi Pico.

Seeing this as epoch-1970 and knowing FAT is epoch-1980 gives me an idea. Instead of waiting to start the clock, start it on boot at 1970 and don't use it for FAT until it's > FF_NORTC. Apps can test <1980 and assume it's not been set. NTP status can get an API call of its own if it ever happens. I think this is what time.h users will expect instead of errors.

[brentward]

I agree it isn't necessary for apps to set the system time zone. I'll use this approach in my next revision.

I definitely like the second idea. This will allow the use of the timestamps for stuff like measuring time duration even when the RTC isn't set.

[rumbledethumps]

The SDK side is complete except maybe the name clock_gettimezone could be up for debate. Leave clock_gettimezone unimplemented on the RIA since the default of returning an error leaves the standard library at UTC and apps can later use the error to know if the RIA version supports timezones.

Everything in the SDK ended up as the thinnest possible implementation, just like all the other standard library calls, so this feels like the correct plan. I think you have everything worked out on the RIA side. Did you want to finish the coding or should I?

[brentward]

I’ve been working on it again for the last few days. Everything looks good to me except the timezones. I will finalize a version without timezones and show it to you. If it is ok I’d also like to show you what I have with timezones to see what you think. Sent from my iPhoneOn Oct 23, 2023, at 3:22 PM, rumbledethumps ***@***.***> wrote: The SDK side is complete except maybe the name clock_gettimezone could be up for debate. Leave clock_gettimezone unimplemented on the RIA since the default of returning an error leaves the standard library at UTC and apps can later use the error to know if the RIA version supports timezones. Everything in the SDK ended up as the thinnest possible implementation, just like all the other standard library calls, so this feels like the correct plan. I think you have everything worked out on the RIA side. Did you want to finish the coding or should I? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[rumbledethumps]

Any work on timezones is predicated on investigating ntp and wifi. I'm not interested in looking at partial work. The whole stack needs to be explored. In other words, prove to me your work won't create more work for me in the future, including config data migration work. That's the bar for anything going into main.

[rumbledethumps]

In case you haven't seen this:
https://github.com/raspberrypi/pico-examples/tree/master/pico_w/wifi/ntp_client
It's just not worth doing timezones until everything up to ntp is working. Apps can do it themselves if they really must.

[brentward]

Here is the latest version. I think it is ready for a PR but I thought I should create another version for comment before:
https://github.com/brentward/rp6502/releases/tag/v0.3%2BRTC

[brentward]

Thanks, I’ll fix all of these and create a PR this evening. I have an example I’ve been using to test so I’ll clean it up and post it too.

[brentward]

I've created a PR. I have a fork of examples with test examples but I thought I should make sure it is what you are expecting before I open a PR on examples.
It is: https://github.com/brentward/examples/tree/rtc-test
It has a program that runs through all the kernel calls and shows the output and also calls the c library time function. It also contains a program to set the clock with user input and a program to display the time.

[rumbledethumps]

Regarding PRs... it's fine to open one if the work is done and you don't expect more than a couple commits to address a code review. It's also fine to open one then close it and make a new one later. It's only leaving one open and pushing a bunch of commits will flood the maintainer's inbox.

Hopefully I sound reasonable. Just need to set boundaries on my time or I'll run out and we'll never get to 1.0. I'll look at this tomorrow.

[brentward]

Perfectly reasonable. I definitely appreciate all the hard work you've done and I want to respect your time.

[rumbledethumps]

That was a hint to push the examples.
Only one example and test please. Unless you have a really good reason for more.
gets is not safe. use fgets instead.

[rumbledethumps]

I need another PR with BSD-3 licenses on the new files. Copypasta a header comment, add your name as a new line if you like.

[rumbledethumps]

I kicked it around a bit and settled on calling it ria/api/clk.c. I did a bunch of housekeeping on my own work in the same commit.
picocomputer/rp6502@288880c

[rumbledethumps]

I wrote the docs and did some other housekeeping: picocomputer/picocomputer.github.io@3b31b86