-
Notifications
You must be signed in to change notification settings - Fork 7.4k
include/drivers: Add RTC support #52618
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
include/drivers: Add RTC support #52618
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok to me
Should these be in the doxygen Io group? |
include/zephyr/drivers/rtcc.h
Outdated
* @param mask Parameters of the date and time to match | ||
* @param handler Handler invoked once the alarm is triggered | ||
* @param user_data Optional user data which passed to the handler when invoked | ||
* @note Handler and user data will be lost of device resets before alarm is triggered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing documentation end, does it compile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not :) It is a draft to discuss which functions should be in which API
include/zephyr/drivers/rtcc.h
Outdated
rtcc_time_get_t time_get; | ||
rtcc_alarm_start_t alarm_start; | ||
rtcc_alarm_cancel_t alarm_cancel; | ||
rtcc_timestamp_get_t timestamp_get; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I am trying to develop an individual RTC driver for MC146818B RTC, available on many x86 boards.
I have checked these header files and rtcc.h will be very useful. But this could have two more APIs which will support READ/WRITE of CMOS battery backed SRAM (256 bytes in my case).
APIs similar to
uint8_t (*read_offset)(const struct device *dev, struct rtc_address *addr)
&
int (*write_offset)(const struct device *dev, struct rtc_address *addr, uint8_t value)
and supporting data types,
struct rtc_address {
/* RAM bank */
enum rtc_ram_bank rtc_bank;
/* offset */
uint8_t offset;
};
enum rtc_ram_bank {
RTC_STD_RAM,
RTC_EXT_RAM,
};
This enum helps to identify between 2 blocks of SRAM.
This memory could be used by BIOS or for any other services.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retention registers are not specific to an RTC module, some I2C attached sensors also allow for retained registers, and some MCUs have internal retention registers and retained SRAM.
We need a tailored API for reading/writing to SRAM, retention registers, and other forms of retained memory, like external EEPROMs.
This functionality is then added to the external RTC node in the devicetree as such:
&i2c2 {
pinctrl-0 = <&i2c2_scl_ph4 &i2c2_sda_ph5>;
pinctrl-names = "default";
status = "okay";
clock-frequency = <I2C_BITRATE_FAST>;
/* Implements RTCC API */
pca8565@2a {
compatible = "pca8565";
reg = <0x2a>;
/* Implements said reg read/write API */
backup_regs {
compatible = "pca8565_backup_regs";
};
};
};
The RTCC API should be limited to setting and getting time. Retained memory needs its own API that can then be implemented by RTC modules that support the feature. Some RTC modules also support watchdog, and we don't want to add that functionality to the rtc or rtcc APIs either, it already has a dedicated API.
This issue #51298 has some more discussion around this. We still don't have an API for performing this, because it is actually quite complex to create a solution that can be used effectively in the devicetree and as a driver API :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's quite possible that another API and device tree binding already exists that would address this. We don't need to have separate API just for reading and writing at an offset.
E.g. flash.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you suggestion. I found that "bbram.h" will serve the purpose for reading/writing to battery backed RAM.
Regarding the APIs in rtcc.h, could there be a possibility to add an API to get current alarm time like rtcc_get_alarm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you suggestion. I found that "bbram.h" will serve the purpose for reading/writing to battery backed RAM.
Regarding the APIs in rtcc.h, could there be a possibility to add an API to get current alarm time like rtcc_get_alarm.
We can definitely add that. Can you think of a use case for getting the timer configuration? The Raspberry Pi Pico SDK doesn't provide an alarm_get
https://raspberrypi.github.io/pico-sdk-doxygen/group__hardware__rtc.html And what should happen if the timer is not configured, should we return an error code maybe?
Architecture WG:
|
This following idea makes it possible to access rtc and rtcc alarms and timestamps, from the devicetree, making it possible to create a time and wakeup subsystem, which will utilize the specified hardware to keep time and possibly configure alarms for wakeup.
From the applications perspective, the alarms and timestamps address is used to specify the id of alarm to use, similar to how an I2C slave address is specified. This uses the id field in the APIs. @galak What do you think of this idea? |
5ef59cc
to
4a33239
Compare
Dev-Review (Dec 13, 2022): discussed devicetree bits, decided to drop for now as there isn't a need and can bring back if needed in future. |
|
659a515
to
05e6fd4
Compare
UpdateThe three APIs have been updated based on the feedback from the dev-review (Dec 13 2022). They are now in a finalized state, ready for thorough review. If and once the APIs have been accepted, creating tests and drivers can begin. |
include/zephyr/drivers/rtc.h
Outdated
(const struct rtc_driver_api *)dev->api; | ||
|
||
if (api->counter_set == NULL) { | ||
return -ENOSYS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like all of the API calls are optional. Is that intentional?
If any API calls are optional, it should be explicitly called out (but there should be a very good reason for that).
Otherwise, we should remove all of the NULL
checks. Attempting to execute NULL will naturally cause exceptions to be thrown / crash code.
This commit adds the rtc.h header file which contains API functions for real-time-clocks, which are low power devices which track and represent broken-down time. It also changes one line of doxygen documentation in the maxim_ds3132.h file to place it in its own group. The handlers for use of the API from userspace is also added with this commit. The API is split into one mandatory section, setting and getting time, and three optional sections, alarms, update event callback, and clock calibration. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
This test suite adds tests for the following: - Setting and getting time - Validating time is incrementing correctly - Validating behavior of alarms with callback disabled - Validating behavior of alarms with callback enabled - Validating update callback The test suite uses the devicetree alias rtc to find the device to test. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
The emulated RTC device driver is used to emulate a real RTC device. Note that it is not a replacement for the native_rtc module, which is used to control simulated time, get time from the host system, etc. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
Added initial maintainers entry for RTC. Will be expanded with collaborators before merged. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
This test suite validates the RTC API helper functions, like currently, and exclusively, the rtc_time_to_tm() helper function. It also validates that the struct rtc_time is member to member compatible with the struct tm from the tm_sec to and including the tm_isdts member. Signed-off-by: Bjarki Arge Andreasen <[email protected]>
2696e1f
to
221558c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments/fixes
between broken-down time and the unix timestamp within the RTC | ||
drivers, which internally used the broken-down time representation. | ||
|
||
The disadvantages of this approach where that hardware counters can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disadvantages of this approach where that hardware counters can | |
The disadvantages of this approach were that hardware counters could |
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
menuconfig RTC | ||
bool "RTC driver support" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would put "real time clock" in a help field here, just so that it is obvious to someone browsing Kconfig options without having to dive into the documentation
*/ | ||
typedef int (*rtc_api_get_calibration)(const struct device *dev, int32_t *calibration); | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this is missing here:
/**
* @cond INTERNAL_HIDDEN
*
* For internal driver use only, skip these in public documentation.
*/
rtc_api_get_calibration get_calibration; | ||
#endif /* CONFIG_RTC_CALIBRATION */ | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And from here:
/** @endcond */
properties: | ||
alarms-count: | ||
type: int | ||
required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really required? Is a better solution not to have it default to 0?
console to be manually compared. The user must review the set and | ||
gotten values to ensure they are valid. | ||
|
||
By default, only the mandatory Setting and getting time is enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, only the mandatory Setting and getting time is enabled | |
By default, only the mandatory setting and getting of time is enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this is showing outdated? I opened all the commits just as I started reviewing this, github bug? If so/comment no longer applies, ignore
``CONFIG_RTC_ALARM``, ``CONFIG_RTC_UPDATE`` and | ||
``CONFIG_RTC_CALIBRATION``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should use correct references
:kconfig:option:`<name>`
``CONFIG_RTC_ALARM``, ``CONFIG_RTC_UPDATE`` and | ||
``CONFIG_RTC_CALIBRATION``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As before, use
:kconfig:option:`<name>`
syntax
contains the device tree alias ``rtc``, the following command can be used | ||
for reference: | ||
|
||
:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like these parts should be using .. code-block:: bash
?
tests: | ||
drivers.rtc.rtc_api: | ||
tags: drivers rtc api helpers | ||
platform_allow: native_posix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is probably going to need to also run on at least variations of all architectures, e.g. mips, arm, etc. and if they have different endian options.
Also needs release notes update |
config RTC_ALARM | ||
bool "RTC driver alarm support" | ||
help | ||
This is an option which enables driver support for RTC alarms. | ||
|
||
config RTC_UPDATE | ||
bool "RTC driver update event callback support" | ||
help | ||
This is an option which enables driver support for the RTC | ||
update event callback. | ||
|
||
config RTC_CALIBRATION | ||
bool "RTC driver clock calibration support" | ||
help | ||
This is an option which enables driver support for RTC clock | ||
calibration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these really necessary? At some point, configuration options get too fine-grained.
🎉 |
Introduction
This PR adds an API for real-time clocks.
A real time clock (RTC) contains a gregorian or persian calendar, which keeps track of date and time. These contain alarms which compare with fields of the RTC date and time, like weekday, hour and minute.
Emulated device driver
This PR introduces and emulated RTC device and device driver. It can be added to the devicetree like any other device, and will act just like a real RTC device, except it is unable to retain the clock or calendar between resets, and is most likely less accurate since it uses the system clock to track time.
The devices can be configured to support any amount of alarms since they are entirely software, which is useful for emulating the capabilities of any real hardware.
The emulated RTC device has been added to the native_posix board, simply enable
CONFIG_RTC
to include the drivers for them.Real device driver
The first real device driver has been created and tested. It supports the ATSAM family of chips. It has been dropped from this PR to avoid delaying the merge of this PR, since it is blocked by another PR unrelated to the RTC API.
RTC API tests
Tests have been added which test the behavior of RTC devices. The tests have been validated against the emulated RTC device driver implemented on the native_posix board, and the ATSAM device driver using a proprietary board.
The API tests will run on any board which has an alias
rtc
in the boards yaml file, and includes the aliasrtc
in the boards devicetree.